[DoctrineBridge] Deprecate passing Doctrine subscribers to ContainerA…

…wareEventManager, use listeners instead
symfony · Apr 7, 2023 · c08780e · c08780e
1 parent 967a351
commit c08780e
Show file tree

Hide file tree

Showing 6 changed files with 141 additions and 50 deletions.
diff --git a/UPGRADE-6.3.md b/UPGRADE-6.3.md
@@ -17,6 +17,7 @@ DependencyInjection
 DoctrineBridge
 --------------
 
+ * Deprecate passing Doctrine subscribers to `ContainerAwareEventManager` class, use listeners instead
  * Deprecate `DoctrineDbalCacheAdapterSchemaSubscriber` in favor of `DoctrineDbalCacheAdapterSchemaListener`
  * Deprecate `MessengerTransportDoctrineSchemaSubscriber` in favor of `MessengerTransportDoctrineSchemaListener`
  * Deprecate `RememberMeTokenProviderDoctrineSchemaSubscriber` in favor of `RememberMeTokenProviderDoctrineSchemaListener`

diff --git a/src/Symfony/Bridge/Doctrine/CHANGELOG.md b/src/Symfony/Bridge/Doctrine/CHANGELOG.md
@@ -4,6 +4,7 @@ CHANGELOG
 6.3
 ---
 
+ * Deprecate passing Doctrine subscribers to `ContainerAwareEventManager` class, use listeners instead
  * Add `AbstractSchemaListener`, `LockStoreSchemaListener` and `PdoSessionHandlerSchemaListener`
  * Deprecate `DoctrineDbalCacheAdapterSchemaSubscriber` in favor of `DoctrineDbalCacheAdapterSchemaListener`
  * Deprecate `MessengerTransportDoctrineSchemaSubscriber` in favor of `MessengerTransportDoctrineSchemaListener`

diff --git a/src/Symfony/Bridge/Doctrine/ContainerAwareEventManager.php b/src/Symfony/Bridge/Doctrine/ContainerAwareEventManager.php
@@ -29,19 +29,18 @@ class ContainerAwareEventManager extends EventManager
      * <event> => <listeners>
      */
     private array $listeners = [];
-    private array $subscribers;
     private array $initialized = [];
     private bool $initializedSubscribers = false;
     private array $methods = [];
     private ContainerInterface $container;
 
     /**
-     * @param list<string|EventSubscriber|array{string[], string|object}> $subscriberIds List of subscribers, subscriber ids, or [events, listener] tuples
+     * @param list<array{string[], string|object}> $listeners List of [events, listener] tuples
      */
-    public function __construct(ContainerInterface $container, array $subscriberIds = [])
+    public function __construct(ContainerInterface $container, array $listeners = [])
     {
         $this->container = $container;
-        $this->subscribers = $subscriberIds;
+        $this->listeners = $listeners;
     }
 
     public function dispatchEvent($eventName, EventArgs $eventArgs = null): void
@@ -182,17 +181,20 @@ private function initializeListeners(string $eventName): void
     private function initializeSubscribers(): void
     {
         $this->initializedSubscribers = true;
-        foreach ($this->subscribers as $subscriber) {
-            if (\is_array($subscriber)) {
-                $this->addEventListener(...$subscriber);
+        $listeners = $this->listeners;
+        $this->listeners = [];
+        foreach ($listeners as $listener) {
+            if (\is_array($listener)) {
+                $this->addEventListener(...$listener);
                 continue;
             }
-            if (\is_string($subscriber)) {
-                $subscriber = $this->container->get($subscriber);
+            if (\is_string($listener)) {
+                $listener = $this->container->get($listener);
             }
-            parent::addEventSubscriber($subscriber);
+            // throw new \InvalidArgumentException(sprintf('Using Doctrine subscriber "%s" is not allowed, declare it as a listener instead.', \is_object($listener) ? $listener::class : $listener));
+            trigger_deprecation('symfony/doctrine-bridge', '6.3', 'Using Doctrine subscribers as services is deprecated, declare listeners instead');
+            parent::addEventSubscriber($listener);
         }
-        $this->subscribers = [];
     }
 
     private function getHash(string|object $listener): string

diff --git a/...ge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php b/...ge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php
@@ -106,6 +106,7 @@ private function addTaggedServices(ContainerBuilder $container): array
                     $refs = $managerDef->getArguments()[1] ?? [];
                     $listenerRefs[$con][$id] = new Reference($id);
                     if ($subscriberTag === $tagName) {
+                        trigger_deprecation('symfony/doctrine-bridge', '6.3', 'Using Doctrine subscribers as services is deprecated, declare listeners instead');
                         $refs[] = $id;
                     } else {
                         $refs[] = [[$tag['event']], $id];

diff --git a/src/Symfony/Bridge/Doctrine/Tests/ContainerAwareEventManagerTest.php b/src/Symfony/Bridge/Doctrine/Tests/ContainerAwareEventManagerTest.php
@@ -32,22 +32,30 @@ protected function setUp(): void
 
     public function testDispatchEventRespectOrder()
     {
-        $this->evm = new ContainerAwareEventManager($this->container, ['sub1', [['foo'], 'list1'], 'sub2']);
+        $this->evm = new ContainerAwareEventManager($this->container, [[['foo'], 'list1'], [['foo'], 'list2']]);
 
         $this->container->set('list1', $listener1 = new MyListener());
+        $this->container->set('list2', $listener2 = new MyListener());
+
+        $this->assertSame([$listener1, $listener2], array_values($this->evm->getListeners('foo')));
+    }
+
+    /**
+     * @group legacy
+     */
+    public function testDispatchEventRespectOrderWithSubscribers()
+    {
+        $this->evm = new ContainerAwareEventManager($this->container, ['sub1', 'sub2']);
+
         $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')));
+        $this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
+        $this->assertSame([$subscriber1,  $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());
@@ -57,16 +65,10 @@ public function testDispatchEvent()
         $this->container->set('lazy3', $listener5 = new MyListener());
         $this->evm->addEventListener('foo', $listener5 = new MyListener());
         $this->evm->addEventListener('bar', $listener5);
-        $this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));
-
-        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
 
         $this->evm->dispatchEvent('foo');
         $this->evm->dispatchEvent('bar');
 
-        $this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
-        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
-
         $this->assertSame(0, $listener1->calledByInvokeCount);
         $this->assertSame(1, $listener1->calledByEventNameCount);
         $this->assertSame(0, $listener2->calledByInvokeCount);
@@ -77,40 +79,57 @@ public function testDispatchEvent()
         $this->assertSame(0, $listener4->calledByEventNameCount);
         $this->assertSame(1, $listener5->calledByInvokeCount);
         $this->assertSame(1, $listener5->calledByEventNameCount);
-        $this->assertSame(0, $subscriber1->calledByInvokeCount);
-        $this->assertSame(1, $subscriber1->calledByEventNameCount);
-        $this->assertSame(1, $subscriber2->calledByInvokeCount);
-        $this->assertSame(0, $subscriber2->calledByEventNameCount);
     }
 
-    public function testAddEventListenerAndSubscriberAfterDispatchEvent()
+    /**
+     * @group legacy
+     */
+    public function testDispatchEventWithSubscribers()
     {
-        $this->evm = new ContainerAwareEventManager($this->container, ['lazy7']);
+        $this->evm = new ContainerAwareEventManager($this->container, ['lazy4']);
 
-        $this->container->set('lazy7', $subscriber1 = new MySubscriber(['foo']));
+        $this->container->set('lazy4', $subscriber1 = new MySubscriber(['foo']));
         $this->assertSame(0, $subscriber1->calledSubscribedEventsCount);
 
         $this->container->set('lazy1', $listener1 = new MyListener());
+        $this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
         $this->evm->addEventListener('foo', 'lazy1');
+        $this->evm->addEventListener('foo', $listener2 = new MyListener());
+        $this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));
+
+        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
+
+        $this->evm->dispatchEvent('foo');
+        $this->evm->dispatchEvent('bar');
+
         $this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
+        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
+
+        $this->assertSame(0, $listener1->calledByInvokeCount);
+        $this->assertSame(1, $listener1->calledByEventNameCount);
+        $this->assertSame(0, $listener2->calledByInvokeCount);
+        $this->assertSame(1, $listener2->calledByEventNameCount);
+        $this->assertSame(0, $subscriber1->calledByInvokeCount);
+        $this->assertSame(1, $subscriber1->calledByEventNameCount);
+        $this->assertSame(1, $subscriber2->calledByInvokeCount);
+        $this->assertSame(0, $subscriber2->calledByEventNameCount);
+    }
 
+    public function testAddEventListenerAfterDispatchEvent()
+    {
+        $this->container->set('lazy1', $listener1 = new MyListener());
+        $this->evm->addEventListener('foo', 'lazy1');
         $this->evm->addEventListener('foo', $listener2 = new MyListener());
         $this->container->set('lazy2', $listener3 = new MyListener());
         $this->evm->addEventListener('bar', 'lazy2');
         $this->evm->addEventListener('bar', $listener4 = new MyListener());
         $this->container->set('lazy3', $listener5 = new MyListener());
         $this->evm->addEventListener('foo', $listener5 = new MyListener());
         $this->evm->addEventListener('bar', $listener5);
-        $this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));
-
-        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
 
         $this->evm->dispatchEvent('foo');
         $this->evm->dispatchEvent('bar');
 
-        $this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
-        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
-
         $this->container->set('lazy4', $listener6 = new MyListener());
         $this->evm->addEventListener('foo', 'lazy4');
         $this->evm->addEventListener('foo', $listener7 = new MyListener());
@@ -120,19 +139,10 @@ public function testAddEventListenerAndSubscriberAfterDispatchEvent()
         $this->container->set('lazy6', $listener10 = new MyListener());
         $this->evm->addEventListener('foo', $listener10 = new MyListener());
         $this->evm->addEventListener('bar', $listener10);
-        $this->evm->addEventSubscriber($subscriber3 = new MySubscriber(['bar']));
-
-        $this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
-        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
-        $this->assertSame(1, $subscriber3->calledSubscribedEventsCount);
 
         $this->evm->dispatchEvent('foo');
         $this->evm->dispatchEvent('bar');
 
-        $this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
-        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
-        $this->assertSame(1, $subscriber3->calledSubscribedEventsCount);
-
         $this->assertSame(0, $listener1->calledByInvokeCount);
         $this->assertSame(2, $listener1->calledByEventNameCount);
         $this->assertSame(0, $listener2->calledByInvokeCount);
@@ -143,10 +153,6 @@ public function testAddEventListenerAndSubscriberAfterDispatchEvent()
         $this->assertSame(0, $listener4->calledByEventNameCount);
         $this->assertSame(2, $listener5->calledByInvokeCount);
         $this->assertSame(2, $listener5->calledByEventNameCount);
-        $this->assertSame(0, $subscriber1->calledByInvokeCount);
-        $this->assertSame(2, $subscriber1->calledByEventNameCount);
-        $this->assertSame(2, $subscriber2->calledByInvokeCount);
-        $this->assertSame(0, $subscriber2->calledByEventNameCount);
 
         $this->assertSame(0, $listener6->calledByInvokeCount);
         $this->assertSame(1, $listener6->calledByEventNameCount);
@@ -158,16 +164,81 @@ public function testAddEventListenerAndSubscriberAfterDispatchEvent()
         $this->assertSame(0, $listener9->calledByEventNameCount);
         $this->assertSame(1, $listener10->calledByInvokeCount);
         $this->assertSame(1, $listener10->calledByEventNameCount);
+    }
+
+    /**
+     * @group legacy
+     */
+    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->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
+        $this->evm->addEventListener('foo', 'lazy1');
+        $this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
+
+        $this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));
+
+        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
+
+        $this->evm->dispatchEvent('foo');
+        $this->evm->dispatchEvent('bar');
+
+        $this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
+        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
+
+        $this->container->set('lazy6', $listener2 = new MyListener());
+        $this->evm->addEventListener('foo', $listener2 = new MyListener());
+        $this->evm->addEventListener('bar', $listener2);
+        $this->evm->addEventSubscriber($subscriber3 = new MySubscriber(['bar']));
+
+        $this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
+        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
+        $this->assertSame(1, $subscriber3->calledSubscribedEventsCount);
+
+        $this->evm->dispatchEvent('foo');
+        $this->evm->dispatchEvent('bar');
+
+        $this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
+        $this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
+        $this->assertSame(1, $subscriber3->calledSubscribedEventsCount);
+
+        $this->assertSame(0, $listener1->calledByInvokeCount);
+        $this->assertSame(2, $listener1->calledByEventNameCount);
+        $this->assertSame(0, $subscriber1->calledByInvokeCount);
+        $this->assertSame(2, $subscriber1->calledByEventNameCount);
+        $this->assertSame(2, $subscriber2->calledByInvokeCount);
+        $this->assertSame(0, $subscriber2->calledByEventNameCount);
+
+        $this->assertSame(1, $listener2->calledByInvokeCount);
+        $this->assertSame(1, $listener2->calledByEventNameCount);
         $this->assertSame(1, $subscriber3->calledByInvokeCount);
         $this->assertSame(0, $subscriber3->calledByEventNameCount);
     }
 
     public function testGetListenersForEvent()
+    {
+        $this->container->set('lazy', $listener1 = new MyListener());
+        $this->evm->addEventListener('foo', 'lazy');
+        $this->evm->addEventListener('foo', $listener2 = new MyListener());
+
+        $this->assertSame([$listener1, $listener2], array_values($this->evm->getListeners('foo')));
+    }
+
+    /**
+     * @group legacy
+     */
+    public function testGetListenersForEventWhenSubscribersArePresent()
     {
         $this->evm = new ContainerAwareEventManager($this->container, ['lazy2']);
 
         $this->container->set('lazy', $listener1 = new MyListener());
         $this->container->set('lazy2', $subscriber1 = new MySubscriber(['foo']));
+        $this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
         $this->evm->addEventListener('foo', 'lazy');
         $this->evm->addEventListener('foo', $listener2 = new MyListener());
 

diff --git a/...e/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php b/...e/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php
@@ -14,6 +14,7 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\Bridge\Doctrine\ContainerAwareEventManager;
 use Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass\RegisterEventListenersAndSubscribersPass;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
@@ -22,6 +23,8 @@
 
 class RegisterEventListenersAndSubscribersPassTest extends TestCase
 {
+    use ExpectDeprecationTrait;
+
     public function testExceptionOnAbstractTaggedSubscriber()
     {
         $this->expectException(\InvalidArgumentException::class);
@@ -195,6 +198,9 @@ public function testProcessEventListenersWithMultipleConnections()
         );
     }
 
+    /**
+     * @group legacy
+     */
     public function testProcessEventSubscribersWithMultipleConnections()
     {
         $container = $this->createBuilder(true);
@@ -232,6 +238,7 @@ public function testProcessEventSubscribersWithMultipleConnections()
             ])
         ;
 
+        $this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
         $this->process($container);
 
         $eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager');
@@ -279,6 +286,9 @@ public function testProcessEventSubscribersWithMultipleConnections()
         );
     }
 
+    /**
+     * @group legacy
+     */
     public function testProcessEventSubscribersWithPriorities()
     {
         $container = $this->createBuilder();
@@ -312,6 +322,7 @@ public function testProcessEventSubscribersWithPriorities()
             ])
         ;
 
+        $this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
         $this->process($container);
 
         $eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager');
@@ -341,6 +352,9 @@ public function testProcessEventSubscribersWithPriorities()
         );
     }
 
+    /**
+     * @group legacy
+     */
     public function testProcessEventSubscribersAndListenersWithPriorities()
     {
         $container = $this->createBuilder();
@@ -402,6 +416,7 @@ public function testProcessEventSubscribersAndListenersWithPriorities()
             ])
         ;
 
+        $this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
         $this->process($container);
 
         $eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager');