[DoctrineBridge] feat: deprecate passing doctrine subscribers to Cont…

…ainerAwareEventManager
symfony · Apr 5, 2023 · eee47c7 · eee47c7
1 parent 847416f
commit eee47c7
Show file tree

Hide file tree

Showing 5 changed files with 120 additions and 42 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
@@ -190,6 +190,9 @@ private function initializeSubscribers(): void
             if (\is_string($subscriber)) {
                 $subscriber = $this->container->get($subscriber);
             }
+
+            trigger_deprecation('symfony/doctrine-bridge', '6.3', 'Using Doctrine subscribers as services is deprecated, declare listeners instead');
+
             parent::addEventSubscriber($subscriber);
         }
         $this->subscribers = [];

diff --git a/...ge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php b/...ge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php
@@ -106,6 +106,8 @@ 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,82 @@ 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());
 
@@ -191,18 +263,18 @@ public function testGetListeners()
     public function testGetAllListeners()
     {
         $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->getAllListeners()['foo']));
     }
 
     public function testRemoveEventListener()
     {
         $this->container->set('lazy', $listener1 = new MyListener());
+
         $this->evm->addEventListener('foo', 'lazy');
         $this->evm->addEventListener('foo', $listener2 = new MyListener());
-
         $this->evm->removeEventListener('foo', $listener2);
         $this->assertSame([$listener1], array_values($this->evm->getListeners('foo')));
 
@@ -217,7 +289,6 @@ public function testRemoveEventListenerAfterDispatchEvent()
         $this->evm->addEventListener('foo', $listener2 = new MyListener());
 
         $this->evm->dispatchEvent('foo');
-
         $this->evm->removeEventListener('foo', $listener2);
         $this->assertSame([$listener1], array_values($this->evm->getListeners('foo')));