8000 feature #27675 [DoctrineBridge] always load event listeners lazy via … · symfony/symfony@83232f8 · GitHub
[go: up one dir, main page]
Skip to content

Commit 83232f8

Browse files
fabpotfabpot
committed
feature #27675 [DoctrineBridge] always load event listeners lazy via ServiceLocator (dmaicher)
This PR was squashed before being merged into the 4.2-dev branch (closes #27675). Discussion ---------- [DoctrineBridge] always load event listeners lazy via ServiceLocator | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes/no | Tests pass? | yes | Fixed tickets | #27661 | License | MIT | Doc PR | symfony/symfony-docs#9973 As described in #27661 this PR suggests to always load doctrine event listeners lazily from a service locator instead of the full service container. If we agree to move forward I could tackle the remaining todos: - [x] update UPGRADE.md - [x] documentation PR - [x] tested on real app Commits ------- 130ec05 [DoctrineBridge] always load event listeners lazy via ServiceLocator
2 parents d3a43b1 + 130ec05 commit 83232f8

File tree

3 files changed

+71
-27
lines changed

3 files changed

+71
-27
lines changed

‎UPGRADE-4.2.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,9 @@ SecurityBundle
5252
`security.authentication.trust_resolver.rememberme_class` parameters to define
5353
the token classes is deprecated. To use
5454
custom tokens extend the existing AnonymousToken and RememberMeToken.
55+
56+
DoctrineBridge
57+
--------------
58+
59+
* The `lazy` attribute on `doctrine.event_listener` tags was removed.
60+
Listeners are now lazy by default. So any `lazy` attributes can safely be removed from those tags.

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass;
1313

1414
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
15+
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
1516
use Symfony\Component\DependencyInjection\ContainerBuilder;
1617
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
1718
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
@@ -80,10 +81,10 @@ private function addTaggedListeners(ContainerBuilder $container)
8081
{
8182
$listenerTag = $this->tagPrefix.'.event_listener';
8283
$taggedListeners = $this->findAndSortTags($listenerTag, $container);
84+
$listenerRefs = array();
8385

8486
foreach ($taggedListeners as $taggedListener) {
8587
list($id, $tag) = $taggedListener;
86-
$taggedListenerDef = $container->getDefinition($id);
8788
if (!isset($tag['event'])) {
8889
throw new InvalidArgumentException(sprintf('Doctrine event listener "%s" must specify the "event" attribute.', $id));
8990
}
@@ -93,15 +94,19 @@ private function addTaggedListeners(ContainerBuilder $container)
9394
if (!isset($this->connections[$con])) {
9495
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))));
9596
}
96-
97-
if ($lazy = !empty($tag['lazy'])) {
98-
$taggedListenerDef->setPublic(true);
99-
}
97+
$listenerRefs[$con][$id] = new Reference($id);
10098

10199
// we add one call per event per service so we have the correct order
102-
$this->getEventManagerDef($container, $con)->addMethodCall('addEventListener', array(array($tag['event']), $lazy ? $id : new Reference($id)));
100+
$this->getEventManagerDef($container, $con)->addMethodCall('addEventListener', array(array($tag['event']), $id));
103101
}
104102
}
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+
}
105110
}
106111

107112
private function getEventManagerDef(ContainerBuilder $container, $name)

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

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass\RegisterEventListenersAndSubscribersPass;
16+
use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
1617
use Symfony\Component\DependencyInjection\ContainerBuilder;
1718
use Symfony\Component\DependencyInjection\Definition;
1819
use Symfony\Component\DependencyInjection\Reference;
20+
use Symfony\Component\DependencyInjection\ServiceLocator;
1921

2022
class RegisterEventListenersAndSubscribersPassTest extends TestCase
2123
{
@@ -68,7 +70,6 @@ public function testProcessEventListenersWithPriorities()
6870
->addTag('doctrine.event_listener', array(
6971
'event' => 'foo_bar',
7072
'priority' => 3,
71-
'lazy' => true,
7273
))
7374
;
7475
$container
@@ -86,25 +87,30 @@ public function testProcessEventListenersWithPriorities()
8687
;
8788

8889
$this->process($container);
89-
$methodCalls = $container->getDefinition('doctrine.dbal.default_connection.event_manager')->getMethodCalls();
90+
$eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager');
91+
$methodCalls = $eventManagerDef->getMethodCalls();
9092

9193
$this->assertEquals(
9294
array(
93-
array('addEventListener', array(array('foo_bar'), new Reference('c'))),
94-
array('addEventListener', array(array('foo_bar'), new Reference('a'))),
95-
array('addEventListener', array(array('bar'), new Reference('a'))),
96-
array('addEventListener', array(array('foo'), new Reference('b'))),
97-
array('addEventListener', array(array('foo'), new Reference('a'))),
95+
array('addEventListener', array(array('foo_bar'), 'c')),
96+
array('addEventListener', array(array('foo_bar'), 'a')),
97+
array('addEventListener', array(array('bar'), 'a')),
98+
array('addEventListener', array(array('foo'), 'b')),
99+
array('addEventListener', array(array('foo'), 'a')),
98100
),
99101
$methodCalls
100102
);
101103

102-
// not lazy so must be reference
103-
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Reference', $methodCalls[0][1][1]);
104-
105-
// lazy so id instead of reference and must mark service public
106-
$this->assertSame('a', $methodCalls[1][1][1]);
107-
$this->assertTrue($container->getDefinition('a')->isPublic());
104+
$serviceLocatorDef = $container->getDefinition((string) $eventManagerDef->getArgument(0));
105+
$this->assertSame(ServiceLocator::class, $serviceLocatorDef->getClass());
106+
$this->assertEquals(
107+
array(
108+
'c' => new ServiceClosureArgument(new Reference('c')),
109+
'a' => new ServiceClosureArgument(new Reference('a')),
110+
'b' => new ServiceClosureArgument(new Reference('b')),
111+
),
112+
$serviceLocatorDef->getArgument(0)
113+
);
108114
}
109115

110116
public function testProcessEventListenersWithMultipleConnections()
@@ -136,20 +142,45 @@ public function testProcessEventListenersWithMultipleConnections()
136142

137143
$this->process($container);
138144

145+
$eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager');
146+
147+
// first connection
139148
$this->assertEquals(
140149
array(
141-
array('addEventListener', array(array('onFlush'), new Reference('a'))),
142-
array('addEventListener', array(array('onFlush'), new Reference('b'))),
150+
array('addEventListener', array(array('onFlush'), 'a')),
151+
array('addEventListener', array(array('onFlush'), 'b')),
143152
),
144-
$container->getDefinition('doctrine.dbal.default_connection.event_manager')->getMethodCalls()
153+
$eventManagerDef->getMethodCalls()
145154
);
146155

156+
$serviceLocatorDef = $container->getDefinition((string) $eventManagerDef->getArgument(0));
157+
$this->assertSame(ServiceLocator::class, $serviceLocatorDef->getClass());
147158
$this->assertEquals(
148159
array(
149-
array('addEventListener', array(array('onFlush'), new Reference('a'))),
150-
array('addEventListener', array(array('onFlush'), new Reference('c'))),
160+
'a' => new ServiceClosureArgument(new Reference('a')),
161+
'b' => new ServiceClosureArgument(new Reference('b')),
151162
),
152-
$container->getDefinition('doctrine.dbal.second_connection.event_manager')->getMethodCalls()
163+
$serviceLocatorDef->getArgument(0)
164+
);
165+
166+
// second connection
167+
$secondEventManagerDef = $container->getDefinition('doctrine.dbal.second_connection.event_manager');
168+
$this->assertEquals(
169+
array(
170+
array('addEventListener', array(array('onFlush'), 'a')),
171+
array('addEventListener', array(array('onFlush'), 'c')),
172+
),
173+
$secondEventManagerDef->getMethodCalls()
174+
);
175+
176+
$serviceLocatorDef = $container->getDefinition((string) $secondEventManagerDef->getArgument(0));
177+
$this->assertSame(ServiceLocator::class, $serviceLocatorDef->getClass());
178+
$this->assertEquals(
179+
array(
180+
'a' => new ServiceClosureArgument(new Reference('a')),
181+
'c' => new ServiceClosureArgument(new Reference('c')),
182+
),
183+
$serviceLocatorDef->getArgument(0)
153184
);
154185
}
155186

@@ -269,11 +300,13 @@ private function createBuilder($multipleConnections = false)
269300

270301
$connections = array('default' => 'doctrine.dbal.default_connection');
271302

272-
$container->register('doctrine.dbal.default_connection.event_manager', 'stdClass');
303+
$container->register('doctrine.dbal.default_connection.event_manager', 'stdClass')
304+
->addArgument(new Reference('service_container'));
273305
$container->register('doctrine.dbal.default_connection', 'stdClass');
274306

275307
if ($multipleConnections) {
276-
$container->register('doctrine.dbal.second_connection.event_manager', 'stdClass');
308+
$container->register('doctrine.dbal.second_connection.event_manager', 'stdClass')
309+
->addArgument(new Reference('service_container'));
277310
$container->register('doctrine.dbal.second_connection', 'stdClass');
278311
$connections['second'] = 'doctrine.dbal.second_connection';
279312
}

0 commit comments

Comments
 (0)
0