-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Added ability to configure MvcEvent listeners. #3931
Added ability to configure MvcEvent listeners. #3931
Conversation
Change-Id: I568ec78485724be3ec68c0b754d9af95b3ea1b03
@Stephanoff I think you should have an array "$defaultListeners" with 'RouteListener', 'DispatchListener', 'ViewManager', 'SendResponseListener' and do a merge with the "$listeners" in bootstrap() before attach. In this case, you can override only one listener, you should not re-write all the listeners. |
@Stephanoff can you add unit tests with override listeners ? |
@blanchonvincent I thought about list of default listeners, but I guess that setting hardcoded list of listeners will prevent developers to build application with custom set of listeners. |
@blanchonvincent but from other side, developers can use custom implementation of Zend\Mvc\Application with overriden Application::bootstrap() method for more flexible configuration. Ok, I will add default set of listeners. |
Change-Id: I382bc52191f3202b47ecd7910a2ffb4108c7e78e
@Stephanoff i would say : $defaultListeners = array('RouteListener', 'DispatchListener', 'ViewManager', 'SendReponseListener');
$listeners = array_merge($defaultListeners, $listeners); "array_merge" keep second value for two keys, with that each developer can override all or only one listener. |
@Stephanoff you must add unit tests with the PR |
@Stephanoff you PR is really cool, i like and need that :) |
Thank you for assistance. I'm familiar with phpunit, but restricted with time. Give me one hour. |
Change-Id: Ib47b1604675e5beef8945b57df9dfb28ddef3ffd
Change-Id: Ifb6241ff0d3799dbefdd9bd160e7f14c3ab68bf8
@blanchonvincent Added test for custom listener. I'm not sure that this is best case, may be you can suggest more better approcah. |
@Stephanoff you should array_merge to merge listeners : https://github.com/blanchonvincent/zf2/blob/develop-application/library/Zend/Mvc/Application.php#L133 |
But using array_merge we cannot be ensure uniquess of listeners. |
@Stephanoff oops => array_unique(array_merge($defaultListeners, $listeners)) |
Change-Id: Ib79a1e12bf93f53f44765ac2add526c3ccecfa6f
Change-Id: I32e556441e7ba46367b1a1a7485ab00e0cdcd6df
@blanchonvincent Updated Application and TestCase classes. |
$events->attach($serviceManager->get('DispatchListener')); | ||
$events->attach($serviceManager->get('ViewManager')); | ||
$events->attach($serviceManager->get('SendResponseListener')); | ||
$defaultListeners = array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd actually make this a class property. That would make overriding them simpler, as the developer wouldn't need to override the bootstrap()
method, only the property. (I can do this during merge.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
Added ability to configure MvcEvent listeners.
Added ability to configure initial list of MvcEvent listeners with config file: