zendframework / zend-session

Manage and preserve session data, a logical complement of cookie data, across multiple page requests by the same client.
BSD 3-Clause "New" or "Revised" License
42 stars 62 forks source link

Remove default session manager as it introduces hidden dependencies #58

Open olegkrivtsov opened 8 years ago

olegkrivtsov commented 8 years ago

Currently the Zend\Session\AbstractContainer class has the setDefaultManager() static method which completely breaks the concept which forces developers to explicitly specify the dependencies for a class, model, controller etc. As a result, many ZF3 components implicitly use the default session manager.

If I configure 'not default' session manager, I have to pass it everywhere including controllers, services, validators (like FlashMessenger) etc., but I think this possiblity is not developed enough in ZF3, because all components use default session manager implicitly.

I suggest to remove the static default session manager and force users to configure and provide the session manager explicitly.

jcaillot commented 8 years ago

my two pennies worth on this one: I do not personally see yet why there should not be a default session manager. A shared configured session sounds like a logical thing to me. However I totally agree that we could use a more elegant plumbing on this

For instance, when the session manager is configured using config options array, if the session manager has not been explicitely instanciated, the Zend\Authentication\AuthenticationService storage container will not use it thus ignoring the configured session. Here 's some pseudo code to make myself clear :

let's suppose we have this config :

'session_manager' => array(
        // enables the session manager as the default manager for container (default to true)
        'enable_default_container_manager' => true,
        'validators' => array(
            'Zend\Session\Validator\HttpUserAgent',
            'Zend\Session\Validator\RemoteAddr',
        ),
    ),
'session_config' => array(
        'php_save_handler'  => 'redis', 
        'cookie_secure' => true,
        'cookie_httponly' => true,
      ...
    ),

The following WON'T use the configuration:

$authService = new \Zend\Authentication\AuthenticationService();
authService->getStorage()->write( $anything );

BUT this will:

$uselessVar = $services->get('Zend\Session\SessionManager'); 
$authService = new \Zend\Authentication\AuthenticationService();
authService->getStorage()->write( $anything );

as during instanciation the sessionManager factory will call the static Container::setDefaultManager @see Service/SessionManagerFactory.php line 137

olegkrivtsov commented 8 years ago

I agree there should be the "default" session manager service in the service manager (and it exists already - it is named Zend\Session\SessionManager. I suggest to remove the "static" default session manager, because its usage is confusing and conflicts with the concept of services.

adamlundrigan commented 8 years ago

TL;DR: I agree

In the vast majority of cases developers need only reconfigure rather than replace the default session manager. There are a number of provided ways to influence the configuration of the session manager:

The default factories for Zend\Session\SessionManager (found in the Zend\Session\Service namespace) will read a number of keys from the application configuration:

Documentation for all of these options can be found here: http://zendframework.github.io/zend-session/config/

You could also attach a delegator factory to Zend\Session\SessionManager and then manipulate the SessionManager instance any way you need.

That said, and as @jcaillot shows, the "magic" injection of the default session scope isn't without it's problems. Personally I do prefer to inject the session manager or session container into the places I need it, and ZF doesn't preclude you from doing that -- the Container "default manager" stuff is a convenience layer but in no way mandatory.

In it's current state the only real benefit you get from the Container "default manager" setup is you can do new Container('name') instead of needing a factory to build the Container and inject the SessionManager. It's a "benefit" that's questionable at best for the reason you stated (hidden dependency).

adamlundrigan commented 8 years ago

@jcaillot this is due to the fact that service manager factories aren't executed until the key is requested. AuthenticationService relies on a session Container instance, and that Container can't fall back to the default SessionManager until the default is injected into it. That default isn't injected until SessionManagerFactory is executed for the first time. In the absence of an injected default session manager, Container will instantiate one without config (here).

You've also identified the only feasible workaround at this point: if you have customized session manager configurations you need to pull the session manager somewhere early in the bootstrap process to ensure that your configured session manager becomes the system-wide default.

weierophinney commented 4 years ago

This repository has been closed and moved to laminas/laminas-session; a new issue has been opened at https://github.com/laminas/laminas-session/issues/15.