Skip to content

Commit 84c1395

Browse files
jderussejderusse
committed
Fix circular loop with EntityManager
1 parent c6b79fd commit 84c1395

File tree

4 files changed

+175
-31
lines changed

4 files changed

+175
-31
lines changed

‎ContainerAwareEventManager.php‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
use Psr\Container\ContainerInterface;
1717

1818
/**
19-
* Allows lazy loading of listener services.
19+
* Allows lazy loading of listener and subscriber services.
2020
*
2121
* @author Johannes M. Schmitt <[email protected]>
2222
*/
@@ -28,13 +28,16 @@ class ContainerAwareEventManager extends EventManager
2828
* <event> => <listeners>
2929
*/
3030
private $listeners = [];
31+
private $subscribers;
3132
private $initialized = [];
33+
private $initializedSubscribers = false;
3234
private $methods = [];
3335
private $container;
3436

35-
public function __construct(ContainerInterface $container)
37+
public function __construct(ContainerInterface $container, array $subscriberIds = [])
3638
{
3739
$this->container = $container;
40+
$this->subscribers = $subscriberIds;
3841
}
3942

4043
/**
@@ -44,6 +47,9 @@ public function __construct(ContainerInterface $container)
4447
*/
4548
public function dispatchEvent($eventName, EventArgs $eventArgs = null)
4649
{
50+
if (!$this->initializedSubscribers) {
51+
$this->initializeSubscribers();
52+
}
4753
if (!isset($this->listeners[$eventName])) {
4854
return;
4955
}
@@ -66,6 +72,9 @@ public function dispatchEvent($eventName, EventArgs $eventArgs = null)
6672
*/
6773
public function getListeners($event = null)
6874
{
75+
if (!$this->initializedSubscribers) {
76+
$this->initializeSubscribers();
77+
}
6978
if (null !== $event) {
7079
if (!isset($this->initialized[$event])) {
7180
$this->initializeListeners($event);
@@ -90,6 +99,10 @@ public function getListeners($event = null)
9099
*/
91100
public function hasListeners($event)
92101
{
102+
if (!$this->initializedSubscribers) {
103+
$this->initializeSubscribers();
104+
}
105+
93106
return isset($this->listeners[$event]) && $this->listeners[$event];
94107
}
95108

@@ -138,14 +151,24 @@ public function removeEventListener($events, $listener)
138151

139152
private function initializeListeners(string $eventName)
140153
{
154+
$this->initialized[$eventName] = true;
141155
foreach ($this->listeners[$eventName] as $hash => $listener) {
142156
if (\is_string($listener)) {
143157
$this->listeners[$eventName][$hash] = $listener = $this->container->get($listener);
144158

145159
$this->methods[$eventName][$hash] = $this->getMethod($listener, $eventName);
146160
}
147161
}
148-
$this->initialized[$eventName] = true;
162+
}
163+
164+
private function initializeSubscribers()
165+
{
166+
$this->initializedSubscribers = true;
167+
foreach ($this->subscribers as $id => $subscriber) {
168+
if (\is_string($subscriber)) {
169+
parent::addEventSubscriber($this->subscribers[$id] = $this->container->get($subscriber));
170+
}
171+
}
149172
}
150173

151174
/**

‎DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass;
1313

14+
use Symfony\Bridge\Doctrine\ContainerAwareEventManager;
15+
use Symfony\Component\DependencyInjection\ChildDefinition;
1416
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
1517
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
1618
use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -55,15 +57,24 @@ public function process(ContainerBuilder $container)
5557
}
5658

5759
$this->connections = $container->getParameter($this->connections);
58-
$this->addTaggedSubscribers($container);
59-
$this->addTaggedListeners($container);
60+
$listenerRefs = [];
61+
$this->addTaggedSubscribers($container, $listenerRefs);
62+
$this->addTaggedListeners($container, $listenerRefs);
63+
64+
// replace service container argument of event managers with smaller service locator
65+
// so services can even remain private
66+
foreach ($listenerRefs as $connection => $refs) {
67+
$this->getEventManagerDef($container, $connection)
68+
->replaceArgument(0, ServiceLocatorTagPass::register($container, $refs));
69+
}
6070
}
6171

62-
private function addTaggedSubscribers(ContainerBuilder $container)
72+
private function addTaggedSubscribers(ContainerBuilder $container, array &$listenerRefs)
6373
{
6474
$subscriberTag = $this->tagPrefix.'.event_subscriber';
6575
$taggedSubscribers = $this->findAndSortTags($subscriberTag, $container);
6676

77+
$managerDefs = [];
6778
foreach ($taggedSubscribers as $taggedSubscriber) {
6879
[$id, $tag] = $taggedSubscriber;
6980
$connections = isset($tag['connection']) ? [$tag['connection']] : array_keys($this->connections);
@@ -72,16 +83,33 @@ private function addTaggedSubscribers(ContainerBuilder $container)
7283
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))));
7384
}
7485

75-
$this->getEventManagerDef($container, $con)->addMethodCall('addEventSubscriber', [new Reference($id)]);
86+
if (!isset($managerDefs[$con])) {
87+
$managerDef = $parentDef = $this->getEventManagerDef($container, $con);
88+
while ($parentDef instanceof ChildDefinition) {
89+
$parentDef = $container->findDefinition($parentDef->getParent());
90+
}
91+
$managerClass = $container->getParameterBag()->resolveValue($parentDef->getClass());
92+
$managerDefs[$con] = [$managerDef, $managerClass];
93+
} else {
94+
[$managerDef, $managerClass] = $managerDefs[$con];
95+
}
96+
97+
if (ContainerAwareEventManager::class === $managerClass) {
98+
$listenerRefs[$con][$id] = new Reference($id);
99+
$refs = $managerDef->getArguments()[1] ?? [];
100+
$refs[] = $id;
101+
$managerDef->setArgument(1, $refs);
102+
} else {
103+
$managerDef->addMethodCall('addEventSubscriber', [new Reference($id)]);
104+
}
76105
}
77106
}
78107
}
79108

80-
private function addTaggedListeners(ContainerBuilder $container)
109+
private function addTaggedListeners(ContainerBuilder $container, array &$listenerRefs)
81110
{
82111
$listenerTag = $this->tagPrefix.'.event_listener';
83112
$taggedListeners = $this->findAndSortTags($listenerTag, $container);
84-
$listenerRefs = [];
85113

86114
foreach ($taggedListeners as $taggedListener) {
87115
[$id, $tag] = $taggedListener;
@@ -100,13 +128,6 @@ private function addTaggedListeners(ContainerBuilder $container)
100128
$this->getEventManagerDef($container, $con)->addMethodCall('addEventListener', [[$tag['event']], $id]);
101129
}
102130
}
103-
104-
// replace service container argument of event managers with smaller service locator
105-
// so services can even remain private
106-
foreach ($listenerRefs as $connection => $refs) {
107-
$this->getEventManagerDef($container, $connection)
108-
->replaceArgument(0, ServiceLocatorTagPass::register($container, $refs));
109-
}
110131
}
111132

112133
private function getEventManagerDef(ContainerBuilder $container, string $name)

‎Tests/ContainerAwareEventManagerTest.php‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Bridge\Doctrine\Tests;
1313

14+
use Doctrine\Common\EventSubscriber;
1415
use PHPUnit\Framework\TestCase;
1516
use Symfony\Bridge\Doctrine\ContainerAwareEventManager;
1617
use Symfony\Component\DependencyInjection\Container;
@@ -28,6 +29,8 @@ protected function setUp(): void
2829

2930
public function testDispatchEvent()
3031
{
32+
$this->evm = new ContainerAwareEventManager($this->container, ['lazy4']);
33+
3134
$this->container->set('lazy1', $listener1 = new MyListener());
3235
$this->evm->addEventListener('foo', 'lazy1');
3336
$this->evm->addEventListener('foo', $listener2 = new MyListener());
@@ -37,10 +40,18 @@ public function testDispatchEvent()
3740
$this->container->set('lazy3', $listener5 = new MyListener());
3841
$this->evm->addEventListener('foo', $listener5 = new MyListener());
3942
$this->evm->addEventListener('bar', $listener5);
43+
$this->container->set('lazy4', $subscriber1 = new MySubscriber(['foo']));
44+
$this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));
45+
46+
$this->assertSame(0, $subscriber1->calledSubscribedEventsCount);
47+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
4048

4149
$this->evm->dispatchEvent('foo');
4250
$this->evm->dispatchEvent('bar');
4351

52+
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
53+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
54+
4455
$this->assertSame(0, $listener1->calledByInvokeCount);
4556
$this->assertSame(1, $listener1->calledByEventNameCount);
4657
$this->assertSame(0, $listener2->calledByInvokeCount);
@@ -51,10 +62,16 @@ public function testDispatchEvent()
5162
$this->assertSame(0, $listener4->calledByEventNameCount);
5263
$this->assertSame(1, $listener5->calledByInvokeCount);
5364
$this->assertSame(1, $listener5->calledByEventNameCount);
65+
$this->assertSame(0, $subscriber1->calledByInvokeCount);
66+
$this->assertSame(1, $subscriber1->calledByEventNameCount);
67+
$this->assertSame(1, $subscriber2->calledByInvokeCount);
68+
$this->assertSame(0, $subscriber2->calledByEventNameCount);
5469
}
5570

56-
public function testAddEventListenerAfterDispatchEvent()
71+
public function testAddEventListenerAndSubscriberAfterDispatchEvent()
5772
{
73+
$this->evm = new ContainerAwareEventManager($this->container, ['lazy7']);
74+
5875
$this->container->set('lazy1', $listener1 = new MyListener());
5976
$this->evm->addEventListener('foo', 'lazy1');
6077
$this->evm->addEventListener('foo', $listener2 = new MyListener());
@@ -64,10 +81,18 @@ public function testAddEventListenerAfterDispatchEvent()
6481
$this->container->set('lazy3', $listener5 = new MyListener());
6582
$this->evm->addEventListener('foo', $listener5 = new MyListener());
6683
$this->evm->addEventListener('bar', $listener5);
84+
$this->container->set('lazy7', $subscriber1 = new MySubscriber(['foo']));
85+
$this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));
86+
87+
$this->assertSame(0, $subscriber1->calledSubscribedEventsCount);
88+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
6789

6890
$this->evm->dispatchEvent('foo');
6991
$this->evm->dispatchEvent('bar');
7092

93+
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
94+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
95+
7196
$this->container->set('lazy4', $listener6 = new MyListener());
7297
$this->evm->addEventListener('foo', 'lazy4');
7398
$this->evm->addEventListener('foo', $listener7 = new MyListener());
@@ -77,10 +102,19 @@ public function testAddEventListenerAfterDispatchEvent()
77102
$this->container->set('lazy6', $listener10 = new MyListener());
78103
$this->evm->addEventListener('foo', $listener10 = new MyListener());
79104
$this->evm->addEventListener('bar', $listener10);
105+
$this->evm->addEventSubscriber($subscriber3 = new MySubscriber(['bar']));
106+
107+
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
108+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
109+
$this->assertSame(1, $subscriber3->calledSubscribedEventsCount);
80110

81111
$this->evm->dispatchEvent('foo');
82112
$this->evm->dispatchEvent('bar');
83113

114+
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);
115+
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);
116+
$this->assertSame(1, $subscriber3->calledSubscribedEventsCount);
117+
84118
$this->assertSame(0, $listener1->calledByInvokeCount);
85119
$this->assertSame(2, $listener1->calledByEventNameCount);
86120
$this->assertSame(0, $listener2->calledByInvokeCount);
@@ -91,6 +125,10 @@ public function testAddEventListenerAfterDispatchEvent()
91125
$this->assertSame(0, $listener4->calledByEventNameCount);
92126
$this->assertSame(2, $listener5->calledByInvokeCount);
93127
$this->assertSame(2, $listener5->calledByEventNameCount);
128+
$this->assertSame(0, $subscriber1->calledByInvokeCount);
129+
$this->assertSame(2, $subscriber1->calledByEventNameCount);
130+
$this->assertSame(2, $subscriber2->calledByInvokeCount);
131+
$this->assertSame(0, $subscriber2->calledByEventNameCount);
94132

95133
$this->assertSame(0, $listener6->calledByInvokeCount);
96134
$this->assertSame(1, $listener6->calledByEventNameCount);
@@ -102,6 +140,8 @@ public function testAddEventListenerAfterDispatchEvent()
102140
$this->assertSame(0, $listener9->calledByEventNameCount);
103141
$this->assertSame(1, $listener10->calledByInvokeCount);
104142
$this->assertSame(1, $listener10->calledByEventNameCount);
143+
$this->assertSame(1, $subscriber3->calledByInvokeCount);
144+
$this->assertSame(0, $subscriber3->calledByEventNameCount);
105145
}
106146

107147
public function testGetListenersForEvent()
@@ -166,3 +206,21 @@ public function foo()
166206
++$this->calledByEventNameCount;
167207
}
168208
}
209+
210+
class MySubscriber extends MyListener implements EventSubscriber
211+
{
212+
public $calledSubscribedEventsCount = 0;
213+
private $listenedEvents;
214+
215+
public function __construct(array $listenedEvents)
216+
{
217+
$this->listenedEvents = $listenedEvents;
218+
}
219+
220+
public function getSubscribedEvents(): array
221+
{
222+
++$this->calledSubscribedEventsCount;
223+
224+
return $this->listenedEvents;
225+
}
226+
}

0 commit comments

Comments
 (0)