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

Added ability to configure MvcEvent listeners. #3931

Merged
merged 7 commits into from Mar 11, 2013
Merged

Added ability to configure MvcEvent listeners. #3931

merged 7 commits into from Mar 11, 2013

Conversation

andrei-stsiapanau
Copy link
Contributor

Added ability to configure initial list of MvcEvent listeners with config file:

array(
    'listeners' => array('RouteListener', 'DispatchListener', 'ViewManager', 'SendReponseListener'),
)

weierophinney and others added 2 commits March 1, 2013 09:35
Change-Id: I568ec78485724be3ec68c0b754d9af95b3ea1b03
@blanchonvincent
Copy link
Contributor

@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.

@blanchonvincent
Copy link
Contributor

@Stephanoff can you add unit tests with override listeners ?

@andrei-stsiapanau
Copy link
Contributor Author

@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.

@andrei-stsiapanau
Copy link
Contributor Author

@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
@blanchonvincent
Copy link
Contributor

@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.

@blanchonvincent
Copy link
Contributor

@Stephanoff you must add unit tests with the PR

@blanchonvincent
Copy link
Contributor

@Stephanoff you PR is really cool, i like and need that :)
If you are not familiar with unit tests, i will help you.

@andrei-stsiapanau
Copy link
Contributor Author

Thank you for assistance. I'm familiar with phpunit, but restricted with time. Give me one hour.

Change-Id: Ib47b1604675e5beef8945b57df9dfb28ddef3ffd
Change-Id: Ifb6241ff0d3799dbefdd9bd160e7f14c3ab68bf8
@andrei-stsiapanau
Copy link
Contributor Author

@blanchonvincent Added test for custom listener. I'm not sure that this is best case, may be you can suggest more better approcah.

@blanchonvincent
Copy link
Contributor

@andrei-stsiapanau
Copy link
Contributor Author

But using array_merge we cannot be ensure uniquess of listeners.
Regarding unit tests - I will replace my test with your.

@blanchonvincent
Copy link
Contributor

@Stephanoff oops => array_unique(array_merge($defaultListeners, $listeners))

Change-Id: Ib79a1e12bf93f53f44765ac2add526c3ccecfa6f
Change-Id: I32e556441e7ba46367b1a1a7485ab00e0cdcd6df
@andrei-stsiapanau
Copy link
Contributor Author

@blanchonvincent Updated Application and TestCase classes.

@ghost ghost assigned weierophinney Mar 8, 2013
$events->attach($serviceManager->get('DispatchListener'));
$events->attach($serviceManager->get('ViewManager'));
$events->attach($serviceManager->get('SendResponseListener'));
$defaultListeners = array(
Copy link
Member

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.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

weierophinney added a commit that referenced this pull request Mar 11, 2013
Added ability to configure MvcEvent listeners.
weierophinney added a commit that referenced this pull request Mar 11, 2013
- trailing whitespace
weierophinney added a commit that referenced this pull request Mar 11, 2013
@weierophinney weierophinney merged commit ae18b80 into zendframework:develop Mar 11, 2013
@andrei-stsiapanau andrei-stsiapanau deleted the develop-application branch March 12, 2013 06:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants