symfony / symfony

bzikarsky wants someone to pull 3 commits into symfony:master from bzikarsky:master

Showing 1 comment with 3 commits and 1 commit comment

#20 Closed

Fixed an error in EventDispatcher::disconnect, Added support for "global" event listeners with $name = null

bzikarsky — Tue Oct 12 18:10:50 -0700 2010

Fixed an error in EventDispatcher::disconnect
The behaviour of disconnect() didn't match its description: If you pass NULL as the listener, no listener instead of all an event's listeners is disconnected. I added the missing check for null and a test for it.
Added support (like already described in DocCode) for listeners, which are notified on all events. They have to be connected to the event with the name NULL.
I added some first tests. Probably some more tests to verify notify/filter/notifyAll/... should be added as well to guarantee correct behavior if those methods stop using getListeners().

bzikarsky added some commits

Tue Oct 12 17:56:04 -0700 2010

`1af0e36`		`Fixed bug in EventDispatcher::disconnect if the second argument is nu...`
`da8ed51`		`Added a test to verify that only the event's listeners get disconnected.`
`5cd5655`		`Added support for "global" event listeners with NULL as a name`

fabpot commented on 5cd5655

Tue Oct 12 22:37:43 -0700 2010

I don't see a need for global event listeners; and I don't see how you can write one. The behavior of a listener depends on the type (notify, filter, ...). The arguments are different for all events. So, except if you have a useful usage and a concrete example of such a need for that, I won't add this feature.

bzikarsky commented

Wed Oct 13 02:10:04 -0700 2010

Maybe I was too unclear about "global" in this case, What I refer to is a listener which fires on all events on a dispatcher. I was quite sure, I read in somewhere in the doc code, but can't find it right now. Anyway what I would use it for:

Logging: I want to be able to log all events fired on my event dispatcher.
Dynamic events: I have a case, where a generic event_listener is injected by a "third" class, which does not know yet which events the event_listener will listen to. This is solvable by implementing a method setupEventListeners(EventDispatcher $ed) on the object, instead of calling
```
 $subject->getEventDispatcher()->connect(
     null, /* pass every event to us */
     array($this, "handleWorkerEvent")
 );
```

But there are downsides if there are lots of event-listeners and you don't know whether they have been added yet. So you got to check the existence each time. It'd be much easier when just can throw in an event listener, listening to all events.

Maybe I can write an even fine-grained control by interpreting "" as wildcard in a name. Then "" would be global, but you could also add listeners to specific categories of events, like "client.*".

Comment on this pull request

You must be logged in to comment on pull requests.

Showing 3 unique commits by 1 author.

`1af0e369`	bzikarsky	`Fixed bug in EventDispatcher::disconnect if the second argument i...`	Tue Oct 12 17:56:04 -0700 2010
`da8ed51a`	bzikarsky	`Added a test to verify that only the event's listeners get discon...`	Tue Oct 12 18:05:25 -0700 2010
`5cd5655c`	bzikarsky	`Added support for "global" event listeners with NULL as a name`	Tue Oct 12 18:53:58 -0700 2010

Showing 2 changed files with 58 additions and 16 deletions.

M	src/Symfony/Component/EventDispatcher/EventDispatcher.php	54 ••••
M	tests/Symfony/Tests/Component/EventDispatcher/EventDispatcherTest.php	20 •••••

src/Symfony/Component/EventDispatcher/EventDispatcher.php

View file @ 5cd5655

          
                  @@ -24,9 +24,9 @@ class EventDispatcher

                       /**

                        * Connects a listener to a given event name.

                        *

                  -     * @param string  $name      An event name

                  -     * @param mixed   $listener  A PHP callable

                  -     * @param integer $priority  The priority (between -10 and 10 -- defaults to 0)

                  +     * @param string|null $name      An event name or null to make the listener listen to all events

                  +     * @param mixed       $listener  A PHP callable

                  +     * @param integer     $priority  The priority (between -10 and 10 -- defaults to 0)

                        */

                       public function connect($name, $listener, $priority = 0)

                       {

                  @@ -40,17 +40,22 @@ class EventDispatcher

                       /**

                        * Disconnects a listener for a given event name.

                        *

                  -     * @param string     $name      An event name

                  -     * @param mixed|null $listener  A PHP callable or null to disconnect all listeners

                  +     * @param string|null     $name      An event name or null to disconnect a global listener

                  +     * @param mixed|null      $listener  A PHP callable or null to disconnect all listeners

                        *

                        * @return mixed false if listener does not exist, null otherwise

                        */

                  -    public function disconnect($name, $listener = null)

                  +    public function disconnect($name=null, $listener = null)

                       {

                           if (!isset($this->listeners[$name])) {

                               return false;

                           }

                  +        if (null === $listener) {

                  +          unset($this->listeners[$name]);

                  +          return;

                  +        }

                  +

                           foreach ($this->listeners[$name] as $priority => $callables) {

                               foreach ($callables as $i => $callable) {

                                   if ($listener === $callable) {

                  @@ -69,7 +74,7 @@ class EventDispatcher

                        */

                       public function notify(Event $event)

                       {

                  -        foreach ($this->getListeners($event->getName()) as $listener) {

                  +        foreach ($this->getListeners($event->getName(), true) as $listener) {

                               call_user_func($listener, $event);

                           }

                  @@ -85,7 +90,7 @@ class EventDispatcher

                        */

                       public function notifyUntil(Event $event)

                       {

                  -        foreach ($this->getListeners($event->getName()) as $listener) {

                  +        foreach ($this->getListeners($event->getName(), true) as $listener) {

                               if (call_user_func($listener, $event)) {

                                   $event->setProcessed(true);

                                   break;

                  @@ -105,7 +110,7 @@ class EventDispatcher

                        */

                       public function filter(Event $event, $value)

                       {

                  -        foreach ($this->getListeners($event->getName()) as $listener) {

                  +        foreach ($this->getListeners($event->getName(), true) as $listener) {

                               $value = call_user_func($listener, $event, $value);

                           }

                  @@ -117,30 +122,47 @@ class EventDispatcher

                       /**

                        * Returns true if the given event name has some listeners.

                        *

                  -     * @param  string   $name    The event name

                  +     * @param  string   $name             The event name

                  +     * @param  boolean  $includeGlobals   Flag whether the global listeners 

                  +     *                                    (name=null)  should be included

                        *

                        * @return Boolean true if some listeners are connected, false otherwise

                        */

                  -    public function hasListeners($name)

                  +    public function hasListeners($name, $includeGlobals=false)

                       {

                  -        return (Boolean) count($this->getListeners($name));

                  +        return (Boolean) count($this->getListeners($name, $includeGlobals));

                       }

                       /**

                        * Returns all listeners associated with a given event name.

                        *

                  -     * @param  string   $name    The event name

                  -     *

                  +     * @param  string   $name             The event name

                  +     * @param  boolean  $includeGlobals   Flag whether the global listeners 

                  +     *                                    (name=null)  should be included

                        * @return array  An array of listeners

                        */

                  -    public function getListeners($name)

                  +    public function getListeners($name, $includeGlobals=false)

                       {

                           if (!isset($this->listeners[$name])) {

                               return array();

                           }

                  -

                  +        

                           $listeners = array();

                           $all = $this->listeners[$name];

                  +        

                  +        // add global listeners if required and existing, but only if the

                  +        // it's not global collection which is requested

                  +        if (isset($this->listeners[null]) && $includeGlobals && $name != null) {

                  +      foreach ($this->listeners[null] as $prio => $globalListeners) {

                  +        if (isset($all[$prio])) {

                  +          $all[$prio] = array_merge($all[$prio], $globalListeners);

                  +        } else {

                  +          $all[$prio] = $globalListeners;

                  +        }

                  +      }

                  +        }

                  +        

                  +        // sort with respect to priority and flatten array

                           ksort($all);

                           foreach ($all as $l) {

                               $listeners = array_merge($listeners, $l);

tests/Symfony/Tests/Component/EventDispatcher/EventDispatcherTest.php

View file @ 5cd5655

...	...	@@ -30,6 +30,10 @@ class EventDispatcherTest extends \PHPUnit_Framework_TestCase
30	30	$this->assertEquals(array('listenToBarBar'), $dispatcher->getListeners('barbar'), '->disconnect() disconnects a listener for an event name');
31	31
32	32	$this->assertFalse($dispatcher->disconnect('foobar', 'listen'), '->disconnect() returns false if the listener does not exist');
	33	+
	34	+ $dispatcher->disconnect('bar');
	35	+ $this->assertEquals(array(), $dispatcher->getListeners('bar'), '->disconnect() without a listener disconnects all listeners of for an event name');
	36	+ $this->assertEquals(array('listenToBarBar'), $dispatcher->getListeners('barbar'), '->disconnect() without a listener disconnects all listeners of for an event name');
33	37	}
34	38
35	39	public function testGetHasListeners()
...	...	@@ -100,6 +104,22 @@ class EventDispatcherTest extends \PHPUnit_Framework_TestCase
100	104	$e = $dispatcher->filter($event = new Event(new \stdClass(), 'foo'), 'foo');
101	105	$this->assertEquals('-foo-', $e->getReturnValue(), '->filter() filters a value');
102	106	}
	107	+
	108	+ public function testGlobalConnectAndDisconnect()
	109	+ {
	110	+ $dispatcher = new EventDispatcher();
	111	+ $dispatcher->connect('bar', 'listenToBar');
	112	+ $dispatcher->connect('barbar', 'listenToBar');
	113	+ $dispatcher->connect(null, 'listenToBarBar');
	114	+ $this->assertEquals(array('listenToBar', 'listenToBarBar'), $dispatcher->getListeners('bar', true), '->connect() with event name null connects a global listener (1)');
	115	+ $this->assertEquals(array('listenToBar', 'listenToBarBar'), $dispatcher->getListeners('barbar', true), '->connect() with event name null connects a global listener (2)');
	116	+ $this->assertEquals(array('listenToBar'), $dispatcher->getListeners('bar', false), '->connect() with event name null connects a global listener (3)');
	117	+ $this->assertEquals(array('listenToBar'), $dispatcher->getListeners('barbar', false), '->connect() with event name null connects a global listener (4)');
	118	+
	119	+ $dispatcher->disconnect(null, 'listenToBarBar');
	120	+ $this->assertEquals(array('listenToBar'), $dispatcher->getListeners('bar', true), '->disconnect() with event name null disconnects a global listener');
	121	+ }
	122	+
103	123	}
104	124
105	125	class Listener

symfony / symfony

Fixed an error in EventDispatcher::disconnect, Added support for "global" event listeners with $name = null

Comment on this pull request

Active Participants

Your current locale selection: English. Choose another?

Keyboard Shortcuts

Site wide shortcuts

Commit list

Pull request list

Issues

Network Graph

...	...	@@ -24,9 +24,9 @@ class EventDispatcher
24	24	/**
25	25	* Connects a listener to a given event name.
26	26	*
27		- * @param string $name An event name
28		- * @param mixed $listener A PHP callable
29		- * @param integer $priority The priority (between -10 and 10 -- defaults to 0)
	27	+ * @param string\|null $name An event name or null to make the listener listen to all events
	28	+ * @param mixed $listener A PHP callable
	29	+ * @param integer $priority The priority (between -10 and 10 -- defaults to 0)
30	30	*/
31	31	public function connect($name, $listener, $priority = 0)
32	32	{
...	...	@@ -40,17 +40,22 @@ class EventDispatcher
40	40	/**
41	41	* Disconnects a listener for a given event name.
42	42	*
43		- * @param string $name An event name
44		- * @param mixed\|null $listener A PHP callable or null to disconnect all listeners
	43	+ * @param string\|null $name An event name or null to disconnect a global listener
	44	+ * @param mixed\|null $listener A PHP callable or null to disconnect all listeners
45	45	*
46	46	* @return mixed false if listener does not exist, null otherwise
47	47	*/
48		- public function disconnect($name, $listener = null)
	48	+ public function disconnect($name=null, $listener = null)
49	49	{
50	50	if (!isset($this->listeners[$name])) {
51	51	return false;
52	52	}
53	53
	54	+ if (null === $listener) {
	55	+ unset($this->listeners[$name]);
	56	+ return;
	57	+ }
	58	+
54	59	foreach ($this->listeners[$name] as $priority => $callables) {
55	60	foreach ($callables as $i => $callable) {
56	61	if ($listener === $callable) {
...	...	@@ -69,7 +74,7 @@ class EventDispatcher
69	74	*/
70	75	public function notify(Event $event)
71	76	{
72		- foreach ($this->getListeners($event->getName()) as $listener) {
	77	+ foreach ($this->getListeners($event->getName(), true) as $listener) {
73	78	call_user_func($listener, $event);
74	79	}
75	80
...	...	@@ -85,7 +90,7 @@ class EventDispatcher
85	90	*/
86	91	public function notifyUntil(Event $event)
87	92	{
88		- foreach ($this->getListeners($event->getName()) as $listener) {
	93	+ foreach ($this->getListeners($event->getName(), true) as $listener) {
89	94	if (call_user_func($listener, $event)) {
90	95	$event->setProcessed(true);
91	96	break;
...	...	@@ -105,7 +110,7 @@ class EventDispatcher
105	110	*/
106	111	public function filter(Event $event, $value)
107	112	{
108		- foreach ($this->getListeners($event->getName()) as $listener) {
	113	+ foreach ($this->getListeners($event->getName(), true) as $listener) {
109	114	$value = call_user_func($listener, $event, $value);
110	115	}
111	116
...	...	@@ -117,30 +122,47 @@ class EventDispatcher
117	122	/**
118	123	* Returns true if the given event name has some listeners.
119	124	*
120		- * @param string $name The event name
	125	+ * @param string $name The event name
	126	+ * @param boolean $includeGlobals Flag whether the global listeners
	127	+ * (name=null) should be included
121	128	*
122	129	* @return Boolean true if some listeners are connected, false otherwise
123	130	*/
124		- public function hasListeners($name)
	131	+ public function hasListeners($name, $includeGlobals=false)
125	132	{
126		- return (Boolean) count($this->getListeners($name));
	133	+ return (Boolean) count($this->getListeners($name, $includeGlobals));
127	134	}
128	135
129	136	/**
130	137	* Returns all listeners associated with a given event name.
131	138	*
132		- * @param string $name The event name
133		- *
	139	+ * @param string $name The event name
	140	+ * @param boolean $includeGlobals Flag whether the global listeners
	141	+ * (name=null) should be included
134	142	* @return array An array of listeners
135	143	*/
136		- public function getListeners($name)
	144	+ public function getListeners($name, $includeGlobals=false)
137	145	{
138	146	if (!isset($this->listeners[$name])) {
139	147	return array();
140	148	}
141		-
	149	+
142	150	$listeners = array();
143	151	$all = $this->listeners[$name];
	152	+
	153	+ // add global listeners if required and existing, but only if the
	154	+ // it's not global collection which is requested
	155	+ if (isset($this->listeners[null]) && $includeGlobals && $name != null) {
	156	+ foreach ($this->listeners[null] as $prio => $globalListeners) {
	157	+ if (isset($all[$prio])) {
	158	+ $all[$prio] = array_merge($all[$prio], $globalListeners);
	159	+ } else {
	160	+ $all[$prio] = $globalListeners;
	161	+ }
	162	+ }
	163	+ }
	164	+
	165	+ // sort with respect to priority and flatten array
144	166	ksort($all);
145	167	foreach ($all as $l) {
146	168	$listeners = array_merge($listeners, $l);