Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

[Feature] Allow Session validators to be created using the service manager #49

Open
GeeH opened this issue Jun 28, 2016 · 1 comment
Open

Comments

@GeeH
Copy link
Contributor

GeeH commented Jun 28, 2016

This issue has been moved from the zendframework repository as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html


Original Issue: https://api.github.com/repos/zendframework/zendframework/issues/7381
User: @larsnystrom
Created On: 2015-03-28T13:55:05Z
Updated At: 2015-03-30T13:26:15Z
Body
At the moment it is not possible to inject any dependencies into a session validator. There is simply no way to do that, since all session validators are created inside the SessionManager or the ValidatorChain.

In a perfect world, the word new would never have made it into any of the Session classes, but here we are.

I think I can hack a way around this by extending ValidatorInterface and add a method called setData(), which could be used to inject the session value to validate, instead of using the constructor to do that. This would allow a factory to create the validator, and the ValidatorChain to inject the session value to validate.

By extending the ValidatorInterface there will be no BC breaks, but the code will get pretty ugly. I still think that would be better than the current situation.

Do you have any thoughts on this?


Comment

User: @Martin-P
Created On: 2015-03-28T14:22:01Z
Updated At: 2015-03-28T14:22:01Z
Body

I think I can hack a way around this by extending ValidatorInterface and add a method called setData(), which could be used to inject the session value to validate

When the reference value exists in the current session it is automatically injected into the validator in Zend\Session\SessionManager::initializeValidatorChain(). You don't have to do this yourself.

You can also add validators to the ValidatorChain directly by using SessionManager::getValidatorChain().


Comment

User: @larsnystrom
Created On: 2015-03-28T14:33:53Z
Updated At: 2015-03-28T14:33:53Z
Body
Inserting the reference value is not the problem I'm trying to address. It's inserting dependencies that is a problem.

Let's just look at the RemoteAddr validator. The only reason $useProxy is static is because we can't inject the value. Usually that kind of configuration would be injected when the object is created.

I'm also working on an application where I need to inject some configuration and objects into the validator. I could go the static road, and create a static member with a static setter and configure the class during bootstrap. But really, shouldn't that be part of a factory? Isn't that the whole point of a factory?


Comment

User: @larsnystrom
Created On: 2015-03-28T14:42:57Z
Updated At: 2015-03-28T14:45:08Z
Body

When the reference value exists in the current session it is automatically injected into the validator in Zend\Session\SessionManager::initializeValidatorChain().

This is not correct. I just submitted PR #7380 which removes that misconception. Reference values are only injected in the ValidatorChain constructor.

But that is a separate issue.


Comment

User: @Martin-P
Created On: 2015-03-28T14:59:17Z
Updated At: 2015-03-28T14:59:17Z
Body
I see my PR which introduced that code had some redundant code, nice catch 👍 However, this does not make it a misconception. Reference values are still injected in the ValidatorChain.

Reference values are only injected in the ValidatorChain constructor.

That is correct. To start a session you have to call SessionManager::start() which always creates a ValidatorChain (line 127) which creates validators based on key/value pairs in $_SESSION['__VALID'].

Perhaps you can post some code for your use case to make the issue more clear? Thanks.


Comment

User: @larsnystrom
Created On: 2015-03-28T15:34:50Z
Updated At: 2015-03-28T15:34:50Z
Body
Here's a crude example:

public function isValid()
{
    $config = require APPLICATION_ROOT . '/module/Application/config/module.config.php';

    $appVersion = $config['app']['version'];

    return ($appVersion == $this->getData());
}

Obviously, requiring the entire module configuration in the session validator is a very bad idea. This is my problem right now, but in other validators I want objects which are created using factories when i validate the session. Basically I want to inject configuation values and services.

Do you know of a way to inject the application version number in the above session validator without using public static anywhere? (Of course, public static would still be better than the above)


Comment

User: @Martin-P
Created On: 2015-03-28T17:05:19Z
Updated At: 2015-03-28T17:05:19Z
Body
I think you should be able to add those validators like this:

// This validator can be created using a factory
$customValidator = $serviceManager->get('My\Custom\Session\Validator');
$sessionManager  = $serviceManager->get('Zend\Session\ServiceManagerInterface');

$event    = 'session.validate';
$callback = array($customValidator, 'customMethod');
$priority = 100;
$sessionManager->getValidatorChain()->attach($event, $callback, $priority);

$sessionManager->start();

Comment

User: @larsnystrom
Created On: 2015-03-28T17:44:19Z
Updated At: 2015-03-28T17:44:49Z
Body
The problem is that the validators are stored in the session storage, so when the session is started the next time, the validator will get created inside the ValidatorChain constructor where it'll only get the reference value injected. The ValidatorChain doesn't have a service manager to create the validator with.


Comment

User: @Martin-P
Created On: 2015-03-30T07:44:16Z
Updated At: 2015-03-30T07:44:16Z
Body
Okay, I see what you mean, that is indeed a problem.

Validators can be created at multiple points in the process: inside SessionManager and inside ValidatorChain. IMO the ValidatorChain should never create its own validators and should only be a dumb object containing the callback functions for validating the session. The validators itself should be injected at one point in the process and the session values can be injected there. Also that would make it easier to implement the feature with the ServiceManager.


@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-session; a new issue has been opened at laminas/laminas-session#19.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants