You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Original Issue: https://api.github.com/repos/zendframework/zendframework/issues/7384
User: @larsnystrom
Created On: 2015-03-30T14:29:30Z
Updated At: 2015-11-06T23:54:06Z
Body
This PR fixes #7381 without any BC breaks. It allows a session validator to be instantiated with dependencies and adds some default functionality to the SessionManagerFactory to get() session validators from the ServiceManager.
It introduces the new interface Zend\Session\Validator\ValidatorServiceInterface which extends Zend\Session\Validator\ValidatorInterface. It modifies the Zend\Session\Service\SessionManagerFactory to also inject validator services into the session manager. Lastly it modifies Zend\Session\SessionManager and Zend\Session\ValidatorChain to attach the validator services to the ValidatorChain and inject the reference value from the current session into the validator service using the setData() method defined in the ValidatorServiceInterface.
A validator which implements the ValidatorServiceInterface can be registered under the key services in the configuration array for session validators, like this:
The example above illustrates that the old way of registering validators still works, and that new validator services goes under the services key of the validators array.
All validators under the services key must:
implement the ValidatorServiceInterface, and
be registered with the ServiceManager.
I'm open for discussion, but I think this is the best way to implement this feature without a BC break.
Caveats:
In the old way of doing things we only registered session validators when creating the session. On subsequent requests the session validators would be instantiated automatically. The new validator services work differently in that they must be injected in the SessionManager on each request. If a service validator isn't injected it will be improperly created by the ValidatorChain, which may cause unknowing developers some trouble.
Things I haven't thought of yet: (hey, that's a paradox!)
How to insert the validators when creating the session. I'm not sure how to do this without removing them all, re-getting them from the service manager, injecting the reference value and then attaching them again. There are probably other ways, but it's still overly complicated.
Comment
User: @Martin-P
Created On: 2015-03-30T14:44:32Z
Updated At: 2015-03-30T14:44:32Z
Body
I like the way you fixed this without a BC break, but it also feels a bit like a workaround for the somewhat messy implementation of the SessionValidators. Personally I think Zend\Session can use a bit of a cleanup, so SessionValidators are created at one point in the process. Yes, that would mean a BC break, but looking at the current implementation I think that it can only be an improvement of the code.
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.
Comment
User: @larsnystrom
Created On: 2015-03-30T15:54:19Z
Updated At: 2015-03-30T15:54:19Z
Body
I'm all for refactoring of Zend\Session, but since it would cause a BC break it will probably have to wait until ZF3 (or until somebody splits ZF into it's separate components). There are some serious issues with the current implementation, so I'm looking forward to that, but without word from higher up the chain of command here, it's just not possible.
I also agree with you that the ValidatorChain should never create it's own objects. However, that's another thing that can't be changed without a BC break. To be honest I think we should scrap the ValidatorChain altogether and just use an array in the SessionManager. I don't see the point in using a priority queue when we can't even specify priorities.
I also think it was a bad idea to force validation of the session in SessionManager::start(), especially when you can't specify which values to validate before running it.
One thing that could be done is adding an argument to ValidatorInterface::isValid(), to provide the session storage to the validator. That way you could validate pretty much anything you'd like, even certain combinations of values. It would also be a more intuitive way of doing the validation, following the same pattern used in Zend\Validator where isValid() always takes the value to be validated as the argument.
I guess this component was put together in a hurry. Splitting ZF into separate components would allow us to start fixing the real problems here. In the meantime we can stuff like I did here.
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.htmlOriginal Issue: https://api.github.com/repos/zendframework/zendframework/issues/7384
User: @larsnystrom
Created On: 2015-03-30T14:29:30Z
Updated At: 2015-11-06T23:54:06Z
Body
This PR fixes #7381 without any BC breaks. It allows a session validator to be instantiated with dependencies and adds some default functionality to the
SessionManagerFactory
toget()
session validators from theServiceManager
.It introduces the new interface
Zend\Session\Validator\ValidatorServiceInterface
which extendsZend\Session\Validator\ValidatorInterface
. It modifies theZend\Session\Service\SessionManagerFactory
to also inject validator services into the session manager. Lastly it modifiesZend\Session\SessionManager
andZend\Session\ValidatorChain
to attach the validator services to theValidatorChain
and inject the reference value from the current session into the validator service using thesetData()
method defined in theValidatorServiceInterface
.A validator which implements the
ValidatorServiceInterface
can be registered under the keyservices
in the configuration array for session validators, like this:The example above illustrates that the old way of registering validators still works, and that new validator services goes under the
services
key of thevalidators
array.All validators under the
services
key must:ValidatorServiceInterface
, andServiceManager
.I'm open for discussion, but I think this is the best way to implement this feature without a BC break.
Caveats:
SessionManager
on each request. If a service validator isn't injected it will be improperly created by theValidatorChain
, which may cause unknowing developers some trouble.Things I haven't thought of yet: (hey, that's a paradox!)
Comment
User: @Martin-P
Created On: 2015-03-30T14:44:32Z
Updated At: 2015-03-30T14:44:32Z
Body
I like the way you fixed this without a BC break, but it also feels a bit like a workaround for the somewhat messy implementation of the
SessionValidators
. Personally I thinkZend\Session
can use a bit of a cleanup, soSessionValidators
are created at one point in the process. Yes, that would mean a BC break, but looking at the current implementation I think that it can only be an improvement of the code.See also my other comment (zendframework/zendframework#7381 (comment)) regarding the
SessionValidators
:Comment
User: @larsnystrom
Created On: 2015-03-30T15:54:19Z
Updated At: 2015-03-30T15:54:19Z
Body
I'm all for refactoring of
Zend\Session
, but since it would cause a BC break it will probably have to wait until ZF3 (or until somebody splits ZF into it's separate components). There are some serious issues with the current implementation, so I'm looking forward to that, but without word from higher up the chain of command here, it's just not possible.I also agree with you that the
ValidatorChain
should never create it's own objects. However, that's another thing that can't be changed without a BC break. To be honest I think we should scrap theValidatorChain
altogether and just use an array in theSessionManager
. I don't see the point in using a priority queue when we can't even specify priorities.I also think it was a bad idea to force validation of the session in
SessionManager::start()
, especially when you can't specify which values to validate before running it.One thing that could be done is adding an argument to
ValidatorInterface::isValid()
, to provide the session storage to the validator. That way you could validate pretty much anything you'd like, even certain combinations of values. It would also be a more intuitive way of doing the validation, following the same pattern used inZend\Validator
whereisValid()
always takes the value to be validated as the argument.I guess this component was put together in a hurry. Splitting ZF into separate components would allow us to start fixing the real problems here. In the meantime we can stuff like I did here.
Originally posted by @GeeH at zendframework/zend-session#48
The text was updated successfully, but these errors were encountered: