Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DoctrineBridge] Deprecate passing doctrine subscribers to ContainerAwareEventManager #49918

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions UPGRADE-6.3.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bridge/Doctrine/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
24 changes: 13 additions & 11 deletions src/Symfony/Bridge/Doctrine/ContainerAwareEventManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
149 changes: 110 additions & 39 deletions src/Symfony/Bridge/Doctrine/Tests/ContainerAwareEventManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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);
Expand All @@ -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
nicolas-grekas marked this conversation as resolved.
Show resolved Hide resolved
*/
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());
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -22,6 +23,8 @@

class RegisterEventListenersAndSubscribersPassTest extends TestCase
{
use ExpectDeprecationTrait;

public function testExceptionOnAbstractTaggedSubscriber()
{
$this->expectException(\InvalidArgumentException::class);
Expand Down Expand Up @@ -195,6 +198,9 @@ public function testProcessEventListenersWithMultipleConnections()
);
}

/**
* @group legacy
*/
public function testProcessEventSubscribersWithMultipleConnections()
{
$container = $this->createBuilder(true);
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -279,6 +286,9 @@ public function testProcessEventSubscribersWithMultipleConnections()
);
}

/**
* @group legacy
*/
public function testProcessEventSubscribersWithPriorities()
{
$container = $this->createBuilder();
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -341,6 +352,9 @@ public function testProcessEventSubscribersWithPriorities()
);
}

/**
* @group legacy
*/
public function testProcessEventSubscribersAndListenersWithPriorities()
{
$container = $this->createBuilder();
Expand Down Expand Up @@ -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');
Expand Down