[EventDispatcher] Allow registration of arbitrary callbacks

This in effect removes the direct link between event name and the method name on the handler.
Any callback can be given as a handler and the event name becomes an arbitrary string. Allowing for easier namespacing (see next commit)

src/Symfony/Bundle/FrameworkBundle/ContainerAwareEventDispatcher.php

@@ -55,22 +55,20 @@ class ContainerAwareEventDispatcher extends EventDispatcher
     /**
      * Adds a service as event listener
      *
-     * @param string|array $eventNames One or more events for which the listener is added
-     * @param string       $serviceId  The ID of the listener service
-     * @param integer      $priority   The higher this value, the earlier an event listener
-     *                                 will be triggered in the chain.
-     *                                 Defaults to 0.
+     * @param string   $eventName Event for which the listener is added
+     * @param array    $callback  The service ID of the listener service & the method
+     *                            name that has to be called
+     * @param integer  $priority  The higher this value, the earlier an event listener
+     *                            will be triggered in the chain.
+     *                            Defaults to 0.
      */
-    public function addListenerService($eventNames, $serviceId, $priority = 0)
+    public function addListenerService($eventName, $callback, $priority = 0)
     {
-        if (!is_string($serviceId)) {
-            throw new \InvalidArgumentException('Expected a string argument');
+        if (!is_array($callback) || 2 !== count($callback)) {
+            throw new \InvalidArgumentException('Expected an array("service", "method") argument');
         }
 
-        foreach ((array) $eventNames as $eventName) {
-            // Prevent duplicate entries
-            $this->listenerIds[$eventName][$serviceId] = $priority;
-        }
+        $this->listenerIds[$eventName][$callback[0].'::'.$callback[1]] = array($callback[0], $callback[1], $priority);
     }
 
     /**
@@ -84,14 +82,15 @@ class ContainerAwareEventDispatcher extends EventDispatcher
     public function dispatch($eventName, Event $event = null)
     {
         if (isset($this->listenerIds[$eventName])) {
-            foreach ($this->listenerIds[$eventName] as $serviceId => $priority) {
+            foreach ($this->listenerIds[$eventName] as $args) {
+                $serviceId = $args[0];
                 $listener = $this->container->get($serviceId);
 
                 if (!isset($this->listeners[$eventName][$serviceId])) {
-                    $this->addListener($eventName, $listener, $priority);
+                    $this->addListener($eventName, array($listener, $args[1]), $args[2]);
                 } elseif ($listener !== $this->listeners[$eventName][$serviceId]) {
-                    $this->removeListener($eventName, $this->listeners[$eventName][$serviceId]);
-                    $this->addListener($eventName, $listener, $priority);
+                    $this->removeListener($eventName, array($listener, $args[1]));
+                    $this->addListener($eventName, array($listener, $args[1]), $args[2]);
                 }
 
                 $this->listeners[$eventName][$serviceId] = $listener;

src/Symfony/Bundle/FrameworkBundle/Debug/TraceableEventDispatcher.php

@@ -48,21 +48,26 @@ class TraceableEventDispatcher extends ContainerAwareEventDispatcher implements
      *
      * @throws \RuntimeException if the listener method is not callable
      */
-    public function addListener($eventNames, $listener, $priority = 0)
+    public function addListener($eventName, $listener, $priority = 0)
     {
-        if (!$listener instanceof \Closure) {
-            foreach ((array) $eventNames as $method) {
-                if (!is_callable(array($listener, $method))) {
-                    $msg = sprintf('The event method "%s()" is not callable on the class "%s".', $method, get_class($listener));
-                    if (null !== $this->logger) {
-                        $this->logger->err($msg);
-                    }
-                    throw new \RuntimeException($msg);
-                }
+        if (!is_callable($listener)) {
+            if (is_string($listener)) {
+                $type = '[string] '.$listener;
+            } elseif (is_array($listener)) {
+                $type = '[array] '.$listener[0].', '.$listener[1];
+            } elseif (is_object($listener)) {
+                $type = '[object] '.get_class($listener);
+            } else {
+                $type = '[?] '.var_export($listener, true);
             }
+            $msg = sprintf('The given callback (%s) for event "%s" is not callable.', $type, $eventName);
+            if (null !== $this->logger) {
+                $this->logger->err($msg);
+            }
+            throw new \RuntimeException($msg);
         }
 
-        parent::addListener($eventNames, $listener, $priority);
+        parent::addListener($eventName, $listener, $priority);
     }
 
     /**
@@ -71,28 +76,40 @@ class TraceableEventDispatcher extends ContainerAwareEventDispatcher implements
     protected function doDispatch($listeners, $eventName, Event $event)
     {
         foreach ($listeners as $listener) {
-            if ($listener instanceof \Closure) {
-                $listener->__invoke($event);
-            } else {
+            // TODO: remove this before final release, temporary transitional code
+            if (is_object($listener) && method_exists($listener, $eventName)) {
                 $listener->$eventName($event);
+                trigger_error('Event listeners should now be registered using a complete callback as the listener instead of just an instance. Adjust your code ASAP.', E_USER_DEPRECATED);
+            } else {
+                // only this call should remain
+                call_user_func($listener, $event);
             }
 
+            $info = $this->getListenerInfo($listener, $eventName);
+
             if (null !== $this->logger) {
-                $this->logger->debug(sprintf('Notified event "%s" to listener "%s".', $eventName, get_class($listener)));
+                $this->logger->debug(sprintf('Notified event "%s" to listener "%s".', $eventName, $info['pretty']));
             }
 
-            $this->called[$eventName.'.'.get_class($listener)] = $this->getListenerInfo($listener, $eventName);
+            $this->called[$eventName.'.'.$info['pretty']] = $info;
 
             if ($event->isPropagationStopped()) {
                 if (null !== $this->logger) {
-                    $this->logger->debug(sprintf('Listener "%s" stopped propagation of the event "%s".', get_class($listener), $eventName));
+                    $this->logger->debug(sprintf('Listener "%s" stopped propagation of the event "%s".', $info['pretty'], $eventName));
 
                     $skippedListeners = $this->getListeners($eventName);
                     $skipped = false;
 
                     foreach ($skippedListeners as $skippedListener) {
                         if ($skipped) {
-                            $this->logger->debug(sprintf('Listener "%s" was not called for event "%s".', get_class($skippedListener), $eventName));
+                            $typeDef = is_object($skippedListener)
+                                ? get_class($skippedListener)
+                                : is_array($skippedListener)
+                                    ? is_object($skippedListener[0])
+                                        ? get_class($skippedListener[0])
+                                        : implode('::', $skippedListener)
+                                    : implode('::', $skippedListener);
+                            $this->logger->debug(sprintf('Listener "%s" was not called for event "%s".', $typeDef, $eventName));
                         }
 
                         if ($skippedListener === $listener) {
@@ -121,10 +138,11 @@ class TraceableEventDispatcher extends ContainerAwareEventDispatcher implements
     {
         $notCalled = array();
 
-        foreach (array_keys($this->getListeners()) as $name) {
-            foreach ($this->getListeners($name) as $listener) {
-                if (!isset($this->called[$name.'.'.get_class($listener)])) {
-                    $notCalled[$name.'.'.get_class($listener)] = $this->getListenerInfo($listener, $name);
+        foreach ($this->getListeners() as $name => $listeners) {
+            foreach ($listeners as $listener) {
+                $info = $this->getListenerInfo($listener, $name);
+                if (!isset($this->called[$name.'.'.$info['pretty']])) {
+                    $notCalled[$name.'.'.$info['pretty']] = $info;
                 }
             }
         }
@@ -144,11 +162,33 @@ class TraceableEventDispatcher extends ContainerAwareEventDispatcher implements
     {
         $info = array('event' => $eventName);
         if ($listener instanceof \Closure) {
-            $info += array('type' => 'Closure');
-        } else {
-            $class = get_class($listener);
+            $info += array(
+                'type' => 'Closure',
+                'pretty' => 'closure'
+            );
+        } elseif (is_string($listener)) {
+            try {
+                $r = new \ReflectionFunction($listener);
+                $file = $r->getFileName();
+                $line = $r->getStartLine();
+            } catch (\ReflectionException $e) {
+                $file = null;
+                $line = null;
+            }
+            $info += array(
+                'type'  => 'Function',
+                'function' => $listener,
+                'file'  => $file,
+                'line'  => $line,
+                'pretty' => $listener,
+            );
+        } elseif (is_array($listener) || (is_object($listener) && is_callable($listener))) {
+            if (!is_array($listener)) {
+                $listener = array($listener, '__invoke');
+            }
+            $class = get_class($listener[0]);
             try {
-                $r = new \ReflectionMethod($class, $eventName);
+                $r = new \ReflectionMethod($class, $listener[1]);
                 $file = $r->getFileName();
                 $line = $r->getStartLine();
             } catch (\ReflectionException $e) {
@@ -158,8 +198,10 @@ class TraceableEventDispatcher extends ContainerAwareEventDispatcher implements
             $info += array(
                 'type'  => 'Method',
                 'class' => $class,
+                'method' => $listener[1],
                 'file'  => $file,
-                'line'  => $line
+                'line'  => $line,
+                'pretty' => $class.'::'.$listener[1],
             );
         }
 

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/RegisterKernelListenersPass.php

@@ -33,7 +33,11 @@ class RegisterKernelListenersPass implements CompilerPassInterface
                     throw new \InvalidArgumentException(sprintf('Service "%s" must define the "event" attribute on "kernel.listener" tags.', $id));
                 }
 
-                $definition->addMethodCall('addListenerService', array($event['event'], $id, $priority));
+                if (!isset($event['method'])) {
+                    throw new \InvalidArgumentException(sprintf('Service "%s" must define the "method" attribute on "kernel.listener" tags.', $id));
+                }
+
+                $definition->addMethodCall('addListenerService', array($event['event'], array($id, $event['method']), $priority));
             }
         }
     }

src/Symfony/Bundle/FrameworkBundle/Tests/ContainerAwareEventDispatcherTest.php

@@ -25,7 +25,7 @@ class ContainerAwareEventDispatcherTest extends \PHPUnit_Framework_TestCase
         $container->set('service.listener', $service);
 
         $dispatcher = new ContainerAwareEventDispatcher($container);
-        $dispatcher->addListenerService('onEvent', 'service.listener');
+        $dispatcher->addListenerService('onEvent', array('service.listener', 'onEvent'));
 
         $dispatcher->dispatch('onEvent', $event);
     }
@@ -46,8 +46,8 @@ class ContainerAwareEventDispatcherTest extends \PHPUnit_Framework_TestCase
         $container->set('service.listener', $service);
 
         $dispatcher = new ContainerAwareEventDispatcher($container);
-        $dispatcher->addListenerService('onEvent', 'service.listener', 5);
-        $dispatcher->addListenerService('onEvent', 'service.listener', 10);
+        $dispatcher->addListenerService('onEvent', array('service.listener', 'onEvent'), 5);
+        $dispatcher->addListenerService('onEvent', array('service.listener', 'onEvent'), 10);
 
         $dispatcher->dispatch('onEvent', $event);
     }
@@ -67,7 +67,7 @@ class ContainerAwareEventDispatcherTest extends \PHPUnit_Framework_TestCase
         $container->set('service.listener', $service, 'scope');
 
         $dispatcher = new ContainerAwareEventDispatcher($container);
-        $dispatcher->addListenerService('onEvent', 'service.listener');
+        $dispatcher->addListenerService('onEvent', array('service.listener', 'onEvent'));
 
         $container->leaveScope('scope');
         $dispatcher->dispatch('onEvent');
@@ -93,7 +93,7 @@ class ContainerAwareEventDispatcherTest extends \PHPUnit_Framework_TestCase
         $container->set('service.listener', $service1, 'scope');
 
         $dispatcher = new ContainerAwareEventDispatcher($container);
-        $dispatcher->addListenerService('onEvent', 'service.listener');        
+        $dispatcher->addListenerService('onEvent', array('service.listener', 'onEvent'));
         $dispatcher->dispatch('onEvent', $event);
 
         $service2 = $this->getMock('Symfony\Bundle\FrameworkBundle\Tests\Service');
@@ -118,6 +118,6 @@ class ContainerAwareEventDispatcherTest extends \PHPUnit_Framework_TestCase
 class Service
 {
     function onEvent(Event $e)
-    {        
+    {
     }
 }

src/Symfony/Bundle/MonologBundle/DependencyInjection/MonologExtension.php

@@ -125,7 +125,7 @@ class MonologExtension extends Extension
                 $handler['level'],
                 $handler['bubble'],
             ));
-            $definition->addTag('kernel.listener', array('event' => 'onCoreResponse'));
+            $definition->addTag('kernel.listener', array('event' => 'core.response', 'method' => 'onCoreResponse'));
             break;
 
         case 'rotating_file':

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/events.html.twig

@@ -47,8 +47,11 @@
 {% macro display_listener(listener) %}
     {% if listener.type == "Closure" %}
         Closure
-    {% else %}
+    {% elseif listener.type == "Function" %}
         {% set link = listener.file|file_link(listener.line) %}
-        {{ listener.class|abbr_class }}::{% if link %}<a href="{{ link }}">{{ listener.event }}</a>{% else %}{{ listener.event }}{% endif %}
+        {% if link %}<a href="{{ link }}">{{ listener.function }}</a>{% else %}{{ listener.function }}{% endif %}
+    {% elseif listener.type == "Method" %}
+        {% set link = listener.file|file_link(listener.line) %}
+        {{ listener.class|abbr_class }}::{% if link %}<a href="{{ link }}">{{ listener.method }}</a>{% else %}{{ listener.method }}{% endif %}
     {% endif %}
 {% endmacro %}

src/Symfony/Component/EventDispatcher/EventDispatcher.php

@@ -102,32 +102,24 @@ class EventDispatcher implements EventDispatcherInterface
      *
      * @api
      */
-    public function addListener($eventNames, $listener, $priority = 0)
+    public function addListener($eventName, $listener, $priority = 0)
     {
-        foreach ((array) $eventNames as $eventName) {
-            if (!isset($this->listeners[$eventName][$priority])) {
-                $this->listeners[$eventName][$priority] = new \SplObjectStorage();
-            }
-
-            $this->listeners[$eventName][$priority]->attach($listener);
-            unset($this->sorted[$eventName]);
-        }
+        $this->listeners[$eventName][$priority][] = $listener;
+        unset($this->sorted[$eventName]);
     }
 
     /**
      * @see EventDispatcherInterface::removeListener
      */
-    public function removeListener($eventNames, $listener)
+    public function removeListener($eventName, $listener)
     {
-        foreach ((array) $eventNames as $eventName) {
-            if (!isset($this->listeners[$eventName])) {
-                continue;
-            }
+        if (!isset($this->listeners[$eventName])) {
+            return;
+        }
 
-            foreach (array_keys($this->listeners[$eventName]) as $priority) {
-                if (isset($this->listeners[$eventName][$priority][$listener])) {
-                    unset($this->listeners[$eventName][$priority][$listener], $this->sorted[$eventName]);
-                }
+        foreach ($this->listeners[$eventName] as $priority => $listeners) {
+            if (false !== ($key = array_search($listener, $listeners))) {
+                unset($this->listeners[$eventName][$priority][$key], $this->sorted[$eventName]);
             }
         }
     }
@@ -137,7 +129,12 @@ class EventDispatcher implements EventDispatcherInterface
      */
     public function addSubscriber(EventSubscriberInterface $subscriber, $priority = 0)
     {
-        $this->addListener($subscriber->getSubscribedEvents(), $subscriber, $priority);
+        foreach ((array) $subscriber->getSubscribedEvents() as $eventName => $method) {
+            if (is_numeric($eventName)) {
+                $eventName = $method;
+            }
+            $this->addListener($eventName, array($subscriber, $method), $priority);
+        }
     }
 
     /**
@@ -145,7 +142,12 @@ class EventDispatcher implements EventDispatcherInterface
      */
     public function removeSubscriber(EventSubscriberInterface $subscriber)
     {
-        $this->removeListener($subscriber->getSubscribedEvents(), $subscriber);
+        foreach ((array) $subscriber->getSubscribedEvents() as $eventName => $method) {
+            if (is_numeric($eventName)) {
+                $eventName = $method;
+            }
+            $this->removeListener($eventName, array($subscriber, $method));
+        }
     }
 
     /**
@@ -161,10 +163,13 @@ class EventDispatcher implements EventDispatcherInterface
     protected function doDispatch($listeners, $eventName, Event $event)
     {
         foreach ($listeners as $listener) {
-            if ($listener instanceof \Closure) {
-                $listener->__invoke($event);
-            } else {
+            // TODO: remove this before final release, temporary transitional code
+            if (is_object($listener) && method_exists($listener, $eventName)) {
                 $listener->$eventName($event);
+                //trigger_error('Event listeners should now be registered using a complete callback as the listener instead of just an instance. Adjust your code ASAP.', E_USER_DEPRECATED);
+            } else {
+                // only this call should remain
+                call_user_func($listener, $event);
             }
 
             if ($event->isPropagationStopped()) {

src/Symfony/Component/Form/FormBuilder.php

@@ -273,9 +273,9 @@ class FormBuilder
      *
      * @return FormBuilder The current builder
      */
-    public function addEventListener($eventNames, $listener, $priority = 0)
+    public function addEventListener($eventName, $listener, $priority = 0)
     {
-        $this->dispatcher->addListener($eventNames, $listener, $priority);
+        $this->dispatcher->addListener($eventName, $listener, $priority);
 
         return $this;
     }

src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php

@@ -59,7 +59,7 @@ class ExceptionListener
      */
     public function register(EventDispatcherInterface $dispatcher)
     {
-        $dispatcher->addListener(Events::onCoreException, $this);
+        $dispatcher->addListener(CoreEvents::exception, array($this, 'core.exception'));
     }
 
     /**

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

@@ -42,7 +42,8 @@ class EventDispatcherTest extends \PHPUnit_Framework_TestCase
 
     public function testAddListener()
     {
-        $this->dispatcher->addListener(array('preFoo', 'postFoo'), $this->listener);
+        $this->dispatcher->addListener('preFoo', array($this->listener, 'preFoo'));
+        $this->dispatcher->addListener('postFoo', array($this->listener, 'postFoo'));
         $this->assertTrue($this->dispatcher->hasListeners(self::preFoo));
         $this->assertTrue($this->dispatcher->hasListeners(self::postFoo));
         $this->assertEquals(1, count($this->dispatcher->getListeners(self::preFoo)));
@@ -55,12 +56,19 @@ class EventDispatcherTest extends \PHPUnit_Framework_TestCase
         $listener1 = new TestEventListener();
         $listener2 = new TestEventListener();
         $listener3 = new TestEventListener();
+        $listener1->name = '1';
+        $listener2->name = '2';
+        $listener3->name = '3';
 
-        $this->dispatcher->addListener('preFoo', $listener1, -10);
-        $this->dispatcher->addListener('preFoo', $listener2);
-        $this->dispatcher->addListener('preFoo', $listener3, 10);
+        $this->dispatcher->addListener('preFoo', array($listener1, 'preFoo'), -10);
+        $this->dispatcher->addListener('preFoo', array($listener2, 'preFoo'), 10);
+        $this->dispatcher->addListener('preFoo', array($listener3, 'preFoo'));
 
-        $expected = array($listener3, $listener2, $listener1);
+        $expected = array(
+            array($listener2, 'preFoo'),
+            array($listener3, 'preFoo'),
+            array($listener1, 'preFoo'),
+        );
 
         $this->assertSame($expected, $this->dispatcher->getListeners('preFoo'));
     }
@@ -91,7 +99,8 @@ class EventDispatcherTest extends \PHPUnit_Framework_TestCase
 
     public function testDispatch()
     {
-        $this->dispatcher->addListener(array('preFoo', 'postFoo'), $this->listener);
+        $this->dispatcher->addListener('preFoo', array($this->listener, 'preFoo'));
+        $this->dispatcher->addListener('postFoo', array($this->listener, 'postFoo'));
         $this->dispatcher->dispatch(self::preFoo);
         $this->assertTrue($this->listener->preFooInvoked);
         $this->assertFalse($this->listener->postFooInvoked);
@@ -103,7 +112,8 @@ class EventDispatcherTest extends \PHPUnit_Framework_TestCase
         $listener = function () use (&$invoked) {
             $invoked++;
         };
-        $this->dispatcher->addListener(array('preFoo', 'postFoo'), $listener);
+        $this->dispatcher->addListener('preFoo', $listener);
+        $this->dispatcher->addListener('postFoo', $listener);
         $this->dispatcher->dispatch(self::preFoo);
         $this->assertEquals(1, $invoked);
     }
@@ -143,9 +153,9 @@ class EventDispatcherTest extends \PHPUnit_Framework_TestCase
 
     public function testRemoveListener()
     {
-        $this->dispatcher->addListener(array('preBar'), $this->listener);
+        $this->dispatcher->addListener('preBar', $this->listener);
         $this->assertTrue($this->dispatcher->hasListeners(self::preBar));
-        $this->dispatcher->removeListener(array('preBar'), $this->listener);
+        $this->dispatcher->removeListener('preBar', $this->listener);
         $this->assertFalse($this->dispatcher->hasListeners(self::preBar));
         $this->dispatcher->removeListener(array('notExists'), $this->listener);
     }
@@ -160,7 +170,7 @@ class EventDispatcherTest extends \PHPUnit_Framework_TestCase
 
     public function testRemoveSubscriber()
     {
-        $eventSubscriber = new TestEventSubscriber();
+        $eventSubscriber = new TestEventSubscriber2();
         $this->dispatcher->addSubscriber($eventSubscriber);
         $this->assertTrue($this->dispatcher->hasListeners(self::preFoo));
         $this->assertTrue($this->dispatcher->hasListeners(self::postFoo));
@@ -191,6 +201,14 @@ class TestEventListener
 }
 
 class TestEventSubscriber implements EventSubscriberInterface
+{
+    public static function getSubscribedEvents()
+    {
+        return array('preFoo' => 'preFoo', 'postFoo' => 'postFoo');
+    }
+}
+
+class TestEventSubscriber2 implements EventSubscriberInterface
 {
     public static function getSubscribedEvents()
     {