diff --git a/src/Symfony/Bridge/Doctrine/ContainerAwareEventManager.php b/src/Symfony/Bridge/Doctrine/ContainerAwareEventManager.php index 9b3c1595a41f0..386d8c62703c6 100644 --- a/src/Symfony/Bridge/Doctrine/ContainerAwareEventManager.php +++ b/src/Symfony/Bridge/Doctrine/ContainerAwareEventManager.php @@ -13,6 +13,7 @@ use Doctrine\Common\EventArgs; use Doctrine\Common\EventManager; +use Doctrine\Common\EventSubscriber; use Psr\Container\ContainerInterface; /** @@ -34,6 +35,9 @@ class ContainerAwareEventManager extends EventManager private $methods = []; private $container; + /** + * @param list $subscriberIds List of subscribers, subscriber ids, or [events, listener] tuples + */ public function __construct(ContainerInterface $container, array $subscriberIds = []) { $this->container = $container; @@ -113,6 +117,10 @@ public function hasListeners($event) */ public function addEventListener($events, $listener) { + if (!$this->initializedSubscribers) { + $this->initializeSubscribers(); + } + $hash = $this->getHash($listener); foreach ((array) $events as $event) { @@ -135,6 +143,10 @@ public function addEventListener($events, $listener) */ public function removeEventListener($events, $listener) { + if (!$this->initializedSubscribers) { + $this->initializeSubscribers(); + } + $hash = $this->getHash($listener); foreach ((array) $events as $event) { @@ -149,6 +161,24 @@ public function removeEventListener($events, $listener) } } + public function addEventSubscriber(EventSubscriber $subscriber): void + { + if (!$this->initializedSubscribers) { + $this->initializeSubscribers(); + } + + parent::addEventSubscriber($subscriber); + } + + public function removeEventSubscriber(EventSubscriber $subscriber): void + { + if (!$this->initializedSubscribers) { + $this->initializeSubscribers(); + } + + parent::removeEventSubscriber($subscriber); + } + private function initializeListeners(string $eventName) { $this->initialized[$eventName] = true; @@ -164,20 +194,15 @@ private function initializeListeners(string $eventName) private function initializeSubscribers() { $this->initializedSubscribers = true; - - $eventListeners = $this->listeners; - // reset eventListener to respect priority: EventSubscribers have a higher priority - $this->listeners = []; - foreach ($this->subscribers as $id => $subscriber) { - if (\is_string($subscriber)) { - parent::addEventSubscriber($this->subscribers[$id] = $this->container->get($subscriber)); + foreach ($this->subscribers as $subscriber) { + if (\is_array($subscriber)) { + $this->addEventListener(...$subscriber); + continue; } - } - foreach ($eventListeners as $event => $listeners) { - if (!isset($this->listeners[$event])) { - $this->listeners[$event] = []; + if (\is_string($subscriber)) { + $subscriber = $this->container->get($subscriber); } - $this->listeners[$event] += $listeners; + parent::addEventSubscriber($subscriber); } $this->subscribers = []; } diff --git a/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php b/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php index 61046c28a5098..a6853fb4809b4 100644 --- a/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php +++ b/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php @@ -57,9 +57,7 @@ public function process(ContainerBuilder $container) } $this->connections = $container->getParameter($this->connections); - $listenerRefs = []; - $this->addTaggedSubscribers($container, $listenerRefs); - $this->addTaggedListeners($container, $listenerRefs); + $listenerRefs = $this->addTaggedServices($container); // replace service container argument of event managers with smaller service locator // so services can even remain private @@ -69,15 +67,20 @@ public function process(ContainerBuilder $container) } } - private function addTaggedSubscribers(ContainerBuilder $container, array &$listenerRefs) + private function addTaggedServices(ContainerBuilder $container): array { + $listenerTag = $this->tagPrefix.'.event_listener'; $subscriberTag = $this->tagPrefix.'.event_subscriber'; - $taggedSubscribers = $this->findAndSortTags($subscriberTag, $container); + $listenerRefs = []; + $taggedServices = $this->findAndSortTags([$subscriberTag, $listenerTag], $container); $managerDefs = []; - foreach ($taggedSubscribers as $taggedSubscriber) { - [$id, $tag] = $taggedSubscriber; + foreach ($taggedServices as $taggedSubscriber) { + [$tagName, $id, $tag] = $taggedSubscriber; $connections = isset($tag['connection']) ? [$tag['connection']] : array_keys($this->connections); + if ($listenerTag === $tagName && !isset($tag['event'])) { + throw new InvalidArgumentException(sprintf('Doctrine event listener "%s" must specify the "event" attribute.', $id)); + } foreach ($connections as $con) { if (!isset($this->connections[$con])) { throw new RuntimeException(sprintf('The Doctrine connection "%s" referenced in service "%s" does not exist. Available connections names: "%s".', $con, $id, implode('", "', array_keys($this->connections)))); @@ -95,39 +98,25 @@ private function addTaggedSubscribers(ContainerBuilder $container, array &$liste } if (ContainerAwareEventManager::class === $managerClass) { - $listenerRefs[$con][$id] = new Reference($id); $refs = $managerDef->getArguments()[1] ?? []; - $refs[] = $id; + $listenerRefs[$con][$id] = new Reference($id); + if ($subscriberTag === $tagName) { + $refs[] = $id; + } else { + $refs[] = [[$tag['event']], $id]; + } $managerDef->setArgument(1, $refs); } else { - $managerDef->addMethodCall('addEventSubscriber', [new Reference($id)]); + if ($subscriberTag === $tagName) { + $managerDef->addMethodCall('addEventSubscriber', [new Reference($id)]); + } else { + $managerDef->addMethodCall('addEventListener', [[$tag['event']], new Reference($id)]); + } } } } - } - - private function addTaggedListeners(ContainerBuilder $container, array &$listenerRefs) - { - $listenerTag = $this->tagPrefix.'.event_listener'; - $taggedListeners = $this->findAndSortTags($listenerTag, $container); - foreach ($taggedListeners as $taggedListener) { - [$id, $tag] = $taggedListener; - if (!isset($tag['event'])) { - throw new InvalidArgumentException(sprintf('Doctrine event listener "%s" must specify the "event" attribute.', $id)); - } - - $connections = isset($tag['connection']) ? [$tag['connection']] : array_keys($this->connections); - foreach ($connections as $con) { - if (!isset($this->connections[$con])) { - throw new RuntimeException(sprintf('The Doctrine connection "%s" referenced in service "%s" does not exist. Available connections names: "%s".', $con, $id, implode('", "', array_keys($this->connections)))); - } - $listenerRefs[$con][$id] = new Reference($id); - - // we add one call per event per service so we have the correct order - $this->getEventManagerDef($container, $con)->addMethodCall('addEventListener', [[$tag['event']], $id]); - } - } + return $listenerRefs; } private function getEventManagerDef(ContainerBuilder $container, string $name) @@ -149,14 +138,16 @@ private function getEventManagerDef(ContainerBuilder $container, string $name) * @see https://bugs.php.net/53710 * @see https://bugs.php.net/60926 */ - private function findAndSortTags(string $tagName, ContainerBuilder $container): array + private function findAndSortTags(array $tagNames, ContainerBuilder $container): array { $sortedTags = []; - foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $tags) { - foreach ($tags as $attributes) { - $priority = $attributes['priority'] ?? 0; - $sortedTags[$priority][] = [$serviceId, $attributes]; + foreach ($tagNames as $tagName) { + foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $tags) { + foreach ($tags as $attributes) { + $priority = $attributes['priority'] ?? 0; + $sortedTags[$priority][] = [$tagName, $serviceId, $attributes]; + } } } diff --git a/src/Symfony/Bridge/Doctrine/Tests/ContainerAwareEventManagerTest.php b/src/Symfony/Bridge/Doctrine/Tests/ContainerAwareEventManagerTest.php index c77c13e59fecb..1631fa8ae37e7 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/ContainerAwareEventManagerTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/ContainerAwareEventManagerTest.php @@ -27,10 +27,24 @@ protected function setUp(): void $this->evm = new ContainerAwareEventManager($this->container); } + public function testDispatchEventRespectOrder() + { + $this->evm = new ContainerAwareEventManager($this->container, ['sub1', [['foo'], 'list1'], 'sub2']); + + $this->container->set('list1', $listener1 = new MyListener()); + $this->container->set('sub1', $subscriber1 = new MySubscriber(['foo'])); + $this->container->set('sub2', $subscriber2 = new MySubscriber(['foo'])); + + $this->assertSame([$subscriber1, $listener1, $subscriber2], array_values($this->evm->getListeners('foo'))); + } + public function testDispatchEvent() { $this->evm = new ContainerAwareEventManager($this->container, ['lazy4']); + $this->container->set('lazy4', $subscriber1 = new MySubscriber(['foo'])); + $this->assertSame(0, $subscriber1->calledSubscribedEventsCount); + $this->container->set('lazy1', $listener1 = new MyListener()); $this->evm->addEventListener('foo', 'lazy1'); $this->evm->addEventListener('foo', $listener2 = new MyListener()); @@ -40,10 +54,8 @@ public function testDispatchEvent() $this->container->set('lazy3', $listener5 = new MyListener()); $this->evm->addEventListener('foo', $listener5 = new MyListener()); $this->evm->addEventListener('bar', $listener5); - $this->container->set('lazy4', $subscriber1 = new MySubscriber(['foo'])); $this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar'])); - $this->assertSame(0, $subscriber1->calledSubscribedEventsCount); $this->assertSame(1, $subscriber2->calledSubscribedEventsCount); $this->evm->dispatchEvent('foo'); @@ -72,8 +84,13 @@ public function testAddEventListenerAndSubscriberAfterDispatchEvent() { $this->evm = new ContainerAwareEventManager($this->container, ['lazy7']); + $this->container->set('lazy7', $subscriber1 = new MySubscriber(['foo'])); + $this->assertSame(0, $subscriber1->calledSubscribedEventsCount); + $this->container->set('lazy1', $listener1 = new MyListener()); $this->evm->addEventListener('foo', 'lazy1'); + $this->assertSame(1, $subscriber1->calledSubscribedEventsCount); + $this->evm->addEventListener('foo', $listener2 = new MyListener()); $this->container->set('lazy2', $listener3 = new MyListener()); $this->evm->addEventListener('bar', 'lazy2'); @@ -81,10 +98,8 @@ public function testAddEventListenerAndSubscriberAfterDispatchEvent() $this->container->set('lazy3', $listener5 = new MyListener()); $this->evm->addEventListener('foo', $listener5 = new MyListener()); $this->evm->addEventListener('bar', $listener5); - $this->container->set('lazy7', $subscriber1 = new MySubscriber(['foo'])); $this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar'])); - $this->assertSame(0, $subscriber1->calledSubscribedEventsCount); $this->assertSame(1, $subscriber2->calledSubscribedEventsCount); $this->evm->dispatchEvent('foo'); diff --git a/src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php b/src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php index 28b983324e55d..358f6693cca92 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php @@ -85,18 +85,18 @@ public function testProcessEventListenersWithPriorities() $this->process($container); $eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager'); - $methodCalls = $eventManagerDef->getMethodCalls(); $this->assertEquals( [ - ['addEventListener', [['foo_bar'], 'c']], - ['addEventListener', [['foo_bar'], 'a']], - ['addEventListener', [['bar'], 'a']], - ['addEventListener', [['foo'], 'b']], - ['addEventListener', [['foo'], 'a']], + [['foo_bar'], 'c'], + [['foo_bar'], 'a'], + [['bar'], 'a'], + [['foo'], 'b'], + [['foo'], 'a'], ], - $methodCalls + $eventManagerDef->getArgument(1) ); + $this->assertEquals([], $eventManagerDef->getMethodCalls()); $serviceLocatorDef = $container->getDefinition((string) $eventManagerDef->getArgument(0)); $this->assertSame(ServiceLocator::class, $serviceLocatorDef->getClass()); @@ -144,11 +144,12 @@ public function testProcessEventListenersWithMultipleConnections() // first connection $this->assertEquals( [ - ['addEventListener', [['onFlush'], 'a']], - ['addEventListener', [['onFlush'], 'b']], + [['onFlush'], 'a'], + [['onFlush'], 'b'], ], - $eventManagerDef->getMethodCalls() + $eventManagerDef->getArgument(1) ); + $this->assertEquals([], $eventManagerDef->getMethodCalls()); $serviceLocatorDef = $container->getDefinition((string) $eventManagerDef->getArgument(0)); $this->assertSame(ServiceLocator::class, $serviceLocatorDef->getClass()); @@ -164,11 +165,12 @@ public function testProcessEventListenersWithMultipleConnections() $secondEventManagerDef = $container->getDefinition('doctrine.dbal.second_connection.event_manager'); $this->assertEquals( [ - ['addEventListener', [['onFlush'], 'a']], - ['addEventListener', [['onFlush'], 'c']], + [['onFlush'], 'a'], + [['onFlush'], 'c'], ], - $secondEventManagerDef->getMethodCalls() + $secondEventManagerDef->getArgument(1) ); + $this->assertEquals([], $secondEventManagerDef->getMethodCalls()); $serviceLocatorDef = $container->getDefinition((string) $secondEventManagerDef->getArgument(0)); $this->assertSame(ServiceLocator::class, $serviceLocatorDef->getClass()); @@ -315,6 +317,104 @@ public function testProcessEventSubscribersWithPriorities() ); } + public function testProcessEventSubscribersAndListenersWithPriorities() + { + $container = $this->createBuilder(); + + $container + ->register('a', 'stdClass') + ->addTag('doctrine.event_subscriber') + ; + $container + ->register('b', 'stdClass') + ->addTag('doctrine.event_subscriber', [ + 'priority' => 5, + ]) + ; + $container + ->register('c', 'stdClass') + ->addTag('doctrine.event_subscriber', [ + 'priority' => 10, + ]) + ; + $container + ->register('d', 'stdClass') + ->addTag('doctrine.event_subscriber', [ + 'priority' => 10, + ]) + ; + $container + ->register('e', 'stdClass') + ->addTag('doctrine.event_subscriber', [ + 'priority' => 10, + ]) + ; + $container + ->register('f', 'stdClass') + ->setPublic(false) + ->addTag('doctrine.event_listener', [ + 'event' => 'bar', + ]) + ->addTag('doctrine.event_listener', [ + 'event' => 'foo', + 'priority' => -5, + ]) + ->addTag('doctrine.event_listener', [ + 'event' => 'foo_bar', + 'priority' => 3, + ]) + ; + $container + ->register('g', 'stdClass') + ->addTag('doctrine.event_listener', [ + 'event' => 'foo', + ]) + ; + $container + ->register('h', 'stdClass') + ->addTag('doctrine.event_listener', [ + 'event' => 'foo_bar', + 'priority' => 4, + ]) + ; + + $this->process($container); + + $eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager'); + + $this->assertEquals( + [ + 'c', + 'd', + 'e', + 'b', + [['foo_bar'], 'h'], + [['foo_bar'], 'f'], + 'a', + [['bar'], 'f'], + [['foo'], 'g'], + [['foo'], 'f'], + ], + $eventManagerDef->getArgument(1) + ); + + $serviceLocatorDef = $container->getDefinition((string) $eventManagerDef->getArgument(0)); + $this->assertSame(ServiceLocator::class, $serviceLocatorDef->getClass()); + $this->assertEquals( + [ + 'a' => new ServiceClosureArgument(new Reference('a')), + 'b' => new ServiceClosureArgument(new Reference('b')), + 'c' => new ServiceClosureArgument(new Reference('c')), + 'd' => new ServiceClosureArgument(new Reference('d')), + 'e' => new ServiceClosureArgument(new Reference('e')), + 'f' => new ServiceClosureArgument(new Reference('f')), + 'g' => new ServiceClosureArgument(new Reference('g')), + 'h' => new ServiceClosureArgument(new Reference('h')), + ], + $serviceLocatorDef->getArgument(0) + ); + } + public function testProcessNoTaggedServices() { $container = $this->createBuilder(true);