8000 feature #49918 [DoctrineBridge] Deprecate passing doctrine subscriber… · symfony/symfony@b42394b · GitHub
[go: up one dir, main page]
Skip to content

Commit b42394b

Browse files
fabpotfabpot
committed
feature #49918 [DoctrineBridge] Deprecate passing doctrine subscribers to ContainerAwareEventManager (alli83)
This PR was merged into the 6.3 branch. Discussion ---------- [DoctrineBridge] Deprecate passing doctrine subscribers to ContainerAwareEventManager | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | Fix #49586 | License | MIT | Doc PR | Following issue #49586, this PR aims to deprecate passing doctrine subscribers to ContainerAwareEventManager. As mentioned, "[#[AsDoctrineListener]]... is a way better alternative anyway." Following #49387 (comment), in PR #49610 DoctrineSchemaSubscribers have already been deprecated in favor of listeners. Commits ------- c08780e [DoctrineBridge] Deprecate passing Doctrine subscribers to ContainerAwareEventManager, use listeners instead
2 parents 8a36deb + c08780e commit b42394b

File tree

6 files changed

+141
-50
lines changed

6 files changed

+141
-50
lines changed

‎UPGRADE-6.3.md

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ DependencyInjection
1818
DoctrineBridge
1919
--------------
2020

21+
* Deprecate passing Doctrine subscribers to `ContainerAwareEventManager` class, use listeners instead
2122
* Deprecate `DoctrineDbalCacheAdapterSchemaSubscriber` in favor of `DoctrineDbalCacheAdapterSchemaListener`
2223
* Deprecate `MessengerTransportDoctrineSchemaSubscriber` in favor of `MessengerTransportDoctrineSchemaListener`
2324
* Deprecate `RememberMeTokenProviderDoctrineSchemaSubscriber` in favor of `RememberMeTokenProviderDoctrineSchemaListener`

‎src/Symfony/Bridge/Doctrine/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ CHANGELOG
44
6.3
55
---
66

7+
* Deprecate passing Doctrine subscribers to `ContainerAwareEventManager` class, use listeners instead
78
* Add `AbstractSchemaListener`, `LockStoreSchemaListener` and `PdoSessionHandlerSchemaListener`
89
* Deprecate `DoctrineDbalCacheAdapterSchemaSubscriber` in favor of `DoctrineDbalCacheAdapterSchemaListener`
910
* Deprecate `MessengerTransportDoctrineSchemaSubscriber` in favor of `MessengerTransportDoctrineSchemaListener`

‎src/Symfony/Bridge/Doctrine/ContainerAwareEventManager.php

+13-11
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,18 @@ class ContainerAwareEventManager extends EventManager
2929
* <event> => <listeners>
3030
*/
3131
private array $listeners = [];
32-
private array $subscribers;
3332
private array $initialized = [];
3433
private bool $initializedSubscribers = false;
3534
private array $methods = [];
3635
private ContainerInterface $container;
3736

3837
/**
39-
* @param list<string|EventSubscriber|array{string[], string|object}> $subscriberIds List of subscribers, subscriber ids, or [events, listener] tuples
38+
* @param list<array{string[], string|object}> $listeners List of [events, listener] tuples
4039
*/
41-
public function __construct(ContainerInterface $container, array $subscriberIds = [])
40+
public function __construct(ContainerInterface $container, array $listeners = [])
4241
{
4342
$this->container = $container;
44-
$this->subscribers = $subscriberIds;
43+
$this->listeners = $listeners;
4544
}
4645

4746
public function dispatchEvent($eventName, EventArgs $eventArgs = null): void
@@ -182,17 +181,20 @@ private function initializeListeners(string $eventName): void
182181
private function initializeSubscribers(): void
183182
{
184183
$this->initializedSubscribers = true;
185-
foreach ($this->subscribers as $subscriber) {
186-
if (\is_array($subscriber)) {
187-
$this->addEventListener(...$subscriber);
184+
$listeners = $this->listeners;
185+
$this->listeners = [];
186+
foreach ($listeners as $listener) {
187+
if (\is_array($listener)) {
188+
$this->addEventListener(...$listener);
188189
continue;
189190
}
190-
if (\is_string($subscriber)) {
191-
$subscriber = $this->container->get($subscriber);
191+
if (\is_string($listener)) {
192+
$listener = $this->container->get($listener);
192193
}
193-
parent::addEventSubscriber($subscriber);
194+
// throw new \InvalidArgumentException(sprintf('Using Doctrine subscriber "%s" is not allowed, declare it as a listener instead.', \is_object($listener) ? $listener::class : $listener));
195+
trigger_deprecation('symfony/doctrine-bridge', '6.3', 'Using Doctrine subscribers as services is deprecated, declare listeners instead');
196+
parent::addEventSubscriber($listener);
194197
}
195-
$this->subscribers = [];
196198
}
197199

198200
private function getHash(string|object $listener): string

‎src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php

+1
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ private function addTaggedServices(ContainerBuilder $container): array
106106
$refs = $managerDef->getArguments()[1] ?? [];
107107
$listenerRefs[$con][$id] = new Reference($id);
108108
if ($subscriberTag === $tagName) {
109+
trigger_deprecation('symfony/doctrine-bridge', '6.3', 'Using Doctrine subscribers as services is deprecated, declare listeners instead');
109110
$refs[] = $id;
110111
} else {
111112
$refs[] = [[$tag['event']], $id];

‎src/Symfony/Bridge/Doctrine/Tests/ContainerAwareEventManagerTest.php

+110-39
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,30 @@ protected function setUp(): void
3232

3333
public function testDispatchEventRespectOrder()
3434
{
35-
$this->evm = new ContainerAwareEventManager($this->container, ['sub1', [['foo'], 'list1'], 'sub2']);
35+
$this->evm = new ContainerAwareEventManager($this->container, [[['foo'], 'list1'], [['foo'], 'list2']]);
3636

3737
$this->container->set('list1', $listener1 = new MyListener());
38+
$this->container->set('list2', $listener2 = new MyListener());
39+
40+
$this->assertSame([$listener1, $listener2], array_values($this->evm->getListeners('foo')));
41+
}
42+
43+
/**
44+
* @group legacy
45+
*/
46+
public function testDispatchEventRespectOrderWithSubscribers()
47+
{
48+
$this->evm = new ContainerAwareEventManager($this->container, ['sub1', 'sub2']);
49+
3850
$this->container->set('sub1', $subscriber1 = new MySubscriber(['foo']));
3951
$this->container->set('sub2', $subscriber2 = new MySubscriber(['foo']));
4052

41-
$this->assertSame([$subscriber1, $listener1, $subscriber2], array_values($this->evm->getListeners('foo')));
53+
$this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
54+
$this->assertSame([$subscriber1, $subscriber2], array_values($this->evm->getListeners('foo')));
4255
}
4356

4457
public function testDispatchEvent()
4558
{
46-
$this->evm = new ContainerAwareEventManager($this->container, ['lazy4']);
47-
48-
$this->container->set('lazy4', $subscriber1 = new MySubscriber(['foo']));
49-
$this->assertSame(0, $subscriber1->calledSubscribedEventsCount);
50-
5159
$this->container->set('lazy1', $listener1 = new MyListener());
5260
$this->evm->addEventListener('foo', 'lazy1');
5361
$this->evm->addEventListener('foo', $listener2 = new MyListener());
@@ -57,16 +65,10 @@ public function testDispatchEvent()
5765
$this->container->set('lazy3', $listener5 = new MyListener());
5866
$this->evm->addEventListener('foo', $listener5 = new MyListener());
5967
$this->evm->addEventListener('bar', $listener5);
60-
$this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));
61-
62-
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
6368

6469
$this->evm->dispatchEvent('foo');
6570
$this->evm->dispatchEvent('bar');
6671

67-
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
68-
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
69-
7072
$this->assertSame(0, $listener1->calledByInvokeCount);
7173
$this->assertSame(1, $listener1->calledByEventNameCount);
7274
$this->assertSame(0, $listener2->calledByInvokeCount);
@@ -77,40 +79,57 @@ public function testDispatchEvent()
7779
$this->assertSame(0, $listener4->calledByEventNameCount);
7880
$this->assertSame(1, $listener5->calledByInvokeCount);
7981
$this->assertSame(1, $listener5->calledByEventNameCount);
80-
$this->assertSame(0, $subscriber1->calledByInvokeCount);
81-
$this->assertSame(1, $subscriber1->calledByEventNameCount);
82-
$this->assertSame(1, $subscriber2->calledByInvokeCount);
83-
$this->assertSame(0, $subscriber2->calledByEventNameCount);
8482
}
8583

86-
public function testAddEventListenerAndSubscriberAfterDispatchEvent()
84+
/**
85+
* @group legacy
86+
*/
87+
public function testDispatchEventWithSubscribers()
8788
{
88-
$this->evm = new ContainerAwareEventManager($this->container, ['lazy7']);
89+
$this->evm = new ContainerAwareEventManager($this->container, ['lazy4']);
8990

90-
$this->container->set('lazy7', $subscriber1 = new MySubscriber(['foo']));
91+
$this->container->set('lazy4', $subscriber1 = new MySubscriber(['foo']));
9192
$this->assertSame(0, $subscriber1->calledSubscribedEventsCount);
9293

9394
$this->container->set('lazy1', $listener1 = new MyListener());
95+
$this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
9496
$this->evm->addEventListener('foo', 'lazy1');
97+
$this->evm->addEventListener('foo', $listener2 = new MyListener());
98+
$this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));
99+
100+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
101+
102+
$this->evm->dispatchEvent('foo');
103+
$this->evm->dispatchEvent('bar');
104+
95105
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
106+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
107+
108+
$this->assertSame(0, $listener1->calledByInvokeCount);
109+
$this->assertSame(1, $listener1->calledByEventNameCount);
110+
$this->assertSame(0, $listener2->calledByInvokeCount);
111+
$this->assertSame(1, $listener2->calledByEventNameCount);
112+
$this->assertSame(0, $subscriber1->calledByInvokeCount);
113+
$this->assertSame(1, $subscriber1->calledByEventNameCount);
114+
$this->assertSame(1, $subscriber2->calledByInvokeCount);
115+
$this->assertSame(0, $subscriber2->calledByEventNameCount);
116+
}
96117

118+
public function testAddEventListenerAfterDispatchEvent()
119+
{
120+
$this->container->set('lazy1', $listener1 = new MyListener());
121+
$this->evm->addEventListener('foo', 'lazy1');
97122
$this->evm->addEventListener('foo', $listener2 = new MyListener());
98123
$this->container->set('lazy2', $listener3 = new MyListener());
99124
$this->evm->addEventListener('bar', 'lazy2');
100125
$this->evm->addEventListener('bar', $listener4 = new MyListener());
101126
$this->container->set('lazy3', $listener5 = new MyListener());
102127
$this->evm->addEventListener('foo', $listener5 = new MyListener());
103128
$this->evm->addEventListener('bar', $listener5);
104-
$this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));
105-
106-
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
107129

108130
$this->evm->dispatchEvent('foo');
109131
$this->evm->dispatchEvent('bar');
110132

111-
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
112-
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
113-
114133
$this->container->set('lazy4', $listener6 = new MyListener());
115134
$this->evm->addEventListener('foo', 'lazy4');
116135
$this->evm->addEventListener('foo', $listener7 = new MyListener());
@@ -120,19 +139,10 @@ public function testAddEventListenerAndSubscriberAfterDispatchEvent()
120139
$this->container->set('lazy6', $listener10 = new MyListener());
121140
$this->evm->addEventListener('foo', $listener10 = new MyListener());
122141
$this->evm->addEventListener('bar', $listener10);
123-
$this->evm->addEventSubscriber($subscriber3 = new MySubscriber(['bar']));
124-
125-
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
126-
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
127-
$this->assertSame(1, $subscriber3->calledSubscribedEventsCount);
128142

129143
$this->evm->dispatchEvent('foo');
130144
$this->evm->dispatchEvent('bar');
131145

132-
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
133-
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
134-
$this->assertSame(1, $subscriber3->calledSubscribedEventsCount);
135-
136146
$this->assertSame(0, $listener1->calledByInvokeCount);
137147
$this->assertSame(2, $listener1->calledByEventNameCount);
138148
$this->assertSame(0, $listener2->calledByInvokeCount);
@@ -143,10 +153,6 @@ public function testAddEventListenerAndSubscriberAfterDispatchEvent()
143153
$this->assertSame(0, $listener4->calledByEventNameCount);
144154
$this->assertSame(2, $listener5->calledByInvokeCount);
145155
$this->assertSame(2, $listener5->calledByEventNameCount);
146-
$this->assertSame(0, $subscriber1->calledByInvokeCount);
147-
$this->assertSame(2, $subscriber1->calledByEventNameCount);
148-
$this->assertSame(2, $subscriber2->calledByInvokeCount);
149-
$this->assertSame(0, $subscriber2->calledByEventNameCount);
150156

151157
$this->assertSame(0, $listener6->calledByInvokeCount);
152158
$this->assertSame(1, $listener6->calledByEventNameCount);
@@ -158,16 +164,81 @@ public function testAddEventListenerAndSubscriberAfterDispatchEvent()
158164
$this->assertSame(0, $listener9->calledByEventNameCount);
159165
$this->assertSame(1, $listener10->calledByInvokeCount);
160166
$this->assertSame(1, $listener10->calledByEventNameCount);
167+
}
168+
169+
/**
170+
* @group legacy
171+
*/
172+
public function testAddEventListenerAndSubscriberAfterDispatchEvent()
173+
{
174+
$this->evm = new ContainerAwareEventManager($this->container, ['lazy7']);
175+
176+
$this->container->set('lazy7', $subscriber1 = new MySubscriber(['foo']));
177+
$this->assertSame(0, $subscriber1->calledSubscribedEventsCount);
178+
179+
$this->container->set('lazy1', $listener1 = new MyListener());
180+
$this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
181+
$this->evm->addEventListener('foo', 'lazy1');
182+
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
183+
184+
$this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));
185+
186+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
187+
188+
$this->evm->dispatchEvent('foo');
189+
$this->evm->dispatchEvent('bar');
190+
191+
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
192+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
193+
194+
$this->container->set('lazy6', $listener2 = new MyListener());
195+
$this->evm->addEventListener('foo', $listener2 = new MyListener());
196+
$this->evm->addEventListener('bar', $listener2);
197+
$this->evm->addEventSubscriber($subscriber3 = new MySubscriber(['bar']));
198+
199+
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
200+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
201+
$this->assertSame(1, $subscriber3->calledSubscribedEventsCount);
202+
203+
$this->evm->dispatchEvent('foo');
204+
$this->evm->dispatchEvent('bar');
205+
206+
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
207+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
208+
$this->assertSame(1, $subscriber3->calledSubscribedEventsCount);
209+
210+
$this->assertSame(0, $listener1->calledByInvokeCount);
< 10000 code>211+
$this->assertSame(2, $listener1->calledByEventNameCount);
212+
$this->assertSame(0, $subscriber1->calledByInvokeCount);
213+
$this->assertSame(2, $subscriber1->calledByEventNameCount);
214+
$this->assertSame(2, $subscriber2->calledByInvokeCount);
215+
$this->assertSame(0, $subscriber2->calledByEventNameCount);
216+
217+
$this->assertSame(1, $listener2->calledByInvokeCount);
218+
$this->assertSame(1, $listener2->calledByEventNameCount);
161219
$this->assertSame(1, $subscriber3->calledByInvokeCount);
162220
$this->assertSame(0, $subscriber3->calledByEventNameCount);
163221
}
164222

165223
public function testGetListenersForEvent()
224+
{
225+
$this->container->set('lazy', $listener1 = new MyListener());
226+
$this->evm->addEventListener('foo', 'lazy');
227+
$this->evm->addEventListener('foo', $listener2 = new MyListener());
228+
229+
$this->assertSame([$listener1, $listener2], array_values($this->evm->getListeners('foo')));
230+
}
231+
232+
/**
233+
* @group legacy
234+
*/
235+
public function testGetListenersForEventWhenSubscribersArePresent()
166236
{
167237
$this->evm = new ContainerAwareEventManager($this->container, ['lazy2']);
168238

169239
$this->container->set('lazy', $listener1 = new MyListener());
170240
$this->container->set('lazy2', $subscriber1 = new MySubscriber(['foo']));
241+
$this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
171242
$this->evm->addEventListener('foo', 'lazy');
172243
$this->evm->addEventListener('foo', $listener2 = new MyListener());
173244

‎src/Symfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPassTest.php

+15
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Bridge\Doctrine\ContainerAwareEventManager;
1616
use Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass\RegisterEventListenersAndSubscribersPass;
17+
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
1718
use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
1819
use Symfony\Component\DependencyInjection\ContainerBuilder;
1920
use Symfony\Component\DependencyInjection\Definition;
@@ -22,6 +23,8 @@
2223

2324
class RegisterEventListenersAndSubscribersPassTest extends TestCase
2425
{
26+
use ExpectDeprecationTrait;
27+
2528
public function testExceptionOnAbstractTaggedSubscriber()
2629
{
2730
$this->expectException(\InvalidArgumentException::class);
@@ -195,6 +198,9 @@ public function testProcessEventListenersWithMultipleConnections()
195198
);
196199
}
197200

201+
/**
202+
* @group legacy
203+
*/
198204
public function testProcessEventSubscribersWithMultipleConnections()
199205
{
200206
$container = $this->createBuilder(true);
@@ -232,6 +238,7 @@ public function testProcessEventSubscribersWithMultipleConnections()
232238
])
233239
;
234240

241+
$this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
235242
$this->process($container);
236243

237244
$eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager');
@@ -279,6 +286,9 @@ public function testProcessEventSubscribersWithMultipleConnections()
279286
);
280287
}
281288

289+
/**
290+
* @group legacy
291+
*/
282292
public function testProcessEventSubscribersWithPriorities()
283293
{
284294
$container = $this->createBuilder();
@@ -312,6 +322,7 @@ public function testProcessEventSubscribersWithPriorities()
312322
])
313323
;
314324

325+
$this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
315326
$this->process($container);
316327

317328
$eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager');
@@ -341,6 +352,9 @@ public function testProcessEventSubscribersWithPriorities()
341352
);
342353
}
343354

355+
/**
356+
* @group legacy
357+
*/
344358
public function testProcessEventSubscribersAndListenersWithPriorities()
345359
{
346360
$container = $this->createBuilder();
@@ -402,6 +416,7 @@ public function testProcessEventSubscribersAndListenersWithPriorities()
402416
])
403417
;
404418

419+
$this->expectDeprecation('Since symfony/doctrine-bridge 6.3: Using Doctrine subscribers as services is deprecated, declare listeners instead');
405420
$this->process($container);
406421

407422
$eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager');

0 commit comments

Comments
 (0)
0