Permalink
Browse files

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
  • Loading branch information...
fabpot committed Jun 28, 2018
2 parents d3a43b1 + 130ec05 commit 83232f85ff2d38dd4db6d84f4f964fa2254ab615
@@ -52,3 +52,9 @@ SecurityBundle
`security.authentication.trust_resolver.rememberme_class` parameters to define
the token classes is deprecated. To use
custom tokens extend the existing AnonymousToken and RememberMeToken.
DoctrineBridge
--------------
* The `lazy` attribute on `doctrine.event_listener` tags was removed.
Listeners are now lazy by default. So any `lazy` attributes can safely be removed from those tags.
@@ -12,6 +12,7 @@
namespace Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
@@ -80,10 +81,10 @@ private function addTaggedListeners(ContainerBuilder $container)
{
$listenerTag = $this->tagPrefix.'.event_listener';
$taggedListeners = $this->findAndSortTags($listenerTag, $container);
$listenerRefs = array();
foreach ($taggedListeners as $taggedListener) {
list($id, $tag) = $taggedListener;
$taggedListenerDef = $container->getDefinition($id);
if (!isset($tag['event'])) {
throw new InvalidArgumentException(sprintf('Doctrine event listener "%s" must specify the "event" attribute.', $id));
}
@@ -93,15 +94,19 @@ private function addTaggedListeners(ContainerBuilder $container)
if (!isset($this->connections[$con])) {
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))));
}
if ($lazy = !empty($tag['lazy'])) {
$taggedListenerDef->setPublic(true);
}
$listenerRefs[$con][$id] = new Reference($id);
// we add one call per event per service so we have the correct order
$this->getEventManagerDef($container, $con)->addMethodCall('addEventListener', array(array($tag['event']), $lazy ? $id : new Reference($id)));
$this->getEventManagerDef($container, $con)->addMethodCall('addEventListener', array(array($tag['event']), $id));
}
}
// replace service container argument of event managers with smaller service locator
// so services can even remain private
foreach ($listenerRefs as $connection => $refs) {
$this->getEventManagerDef($container, $connection)
->replaceArgument(0, ServiceLocatorTagPass::register($container, $refs));
}
}
private function getEventManagerDef(ContainerBuilder $container, $name)
@@ -13,9 +13,11 @@
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass\RegisterEventListenersAndSubscribersPass;
use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\ServiceLocator;
class RegisterEventListenersAndSubscribersPassTest extends TestCase
{
@@ -68,7 +70,6 @@ public function testProcessEventListenersWithPriorities()
->addTag('doctrine.event_listener', array(
'event' => 'foo_bar',
'priority' => 3,
'lazy' => true,
))
;
$container
@@ -86,25 +87,30 @@ public function testProcessEventListenersWithPriorities()
;
$this->process($container);
$methodCalls = $container->getDefinition('doctrine.dbal.default_connection.event_manager')->getMethodCalls();
$eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager');
$methodCalls = $eventManagerDef->getMethodCalls();
$this->assertEquals(
array(
array('addEventListener', array(array('foo_bar'), new Reference('c'))),
array('addEventListener', array(array('foo_bar'), new Reference('a'))),
array('addEventListener', array(array('bar'), new Reference('a'))),
array('addEventListener', array(array('foo'), new Reference('b'))),
array('addEventListener', array(array('foo'), new Reference('a'))),
array('addEventListener', array(array('foo_bar'), 'c')),
array('addEventListener', array(array('foo_bar'), 'a')),
array('addEventListener', array(array('bar'), 'a')),
array('addEventListener', array(array('foo'), 'b')),
array('addEventListener', array(array('foo'), 'a')),
),
$methodCalls
);
// not lazy so must be reference
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Reference', $methodCalls[0][1][1]);
// lazy so id instead of reference and must mark service public
$this->assertSame('a', $methodCalls[1][1][1]);
$this->assertTrue($container->getDefinition('a')->isPublic());
$serviceLocatorDef = $container->getDefinition((string) $eventManagerDef->getArgument(0));
$this->assertSame(ServiceLocator::class, $serviceLocatorDef->getClass());
$this->assertEquals(
array(
'c' => new ServiceClosureArgument(new Reference('c')),
'a' => new ServiceClosureArgument(new Reference('a')),
'b' => new ServiceClosureArgument(new Reference('b')),
),
$serviceLocatorDef->getArgument(0)
);
}
public function testProcessEventListenersWithMultipleConnections()
@@ -136,20 +142,45 @@ public function testProcessEventListenersWithMultipleConnections()
$this->process($container);
$eventManagerDef = $container->getDefinition('doctrine.dbal.default_connection.event_manager');
// first connection
$this->assertEquals(
array(
array('addEventListener', array(array('onFlush'), new Reference('a'))),
array('addEventListener', array(array('onFlush'), new Reference('b'))),
array('addEventListener', array(array('onFlush'), 'a')),
array('addEventListener', array(array('onFlush'), 'b')),
),
$container->getDefinition('doctrine.dbal.default_connection.event_manager')->getMethodCalls()
$eventManagerDef->getMethodCalls()
);
$serviceLocatorDef = $container->getDefinition((string) $eventManagerDef->getArgument(0));
$this->assertSame(ServiceLocator::class, $serviceLocatorDef->getClass());
$this->assertEquals(
array(
array('addEventListener', array(array('onFlush'), new Reference('a'))),
array('addEventListener', array(array('onFlush'), new Reference('c'))),
'a' => new ServiceClosureArgument(new Reference('a')),
'b' => new ServiceClosureArgument(new Reference('b')),
),
$container->getDefinition('doctrine.dbal.second_connection.event_manager')->getMethodCalls()
$serviceLocatorDef->getArgument(0)
);
// second connection
$secondEventManagerDef = $container->getDefinition('doctrine.dbal.second_connection.event_manager');
$this->assertEquals(
array(
array('addEventListener', array(array('onFlush'), 'a')),
array('addEventListener', array(array('onFlush'), 'c')),
),
$secondEventManagerDef->getMethodCalls()
);
$serviceLocatorDef = $container->getDefinition((string) $secondEventManagerDef->getArgument(0));
$this->assertSame(ServiceLocator::class, $serviceLocatorDef->getClass());
$this->assertEquals(
array(
'a' => new ServiceClosureArgument(new Reference('a')),
'c' => new ServiceClosureArgument(new Reference('c')),
),
$serviceLocatorDef->getArgument(0)
);
}
@@ -269,11 +300,13 @@ private function createBuilder($multipleConnections = false)
$connections = array('default' => 'doctrine.dbal.default_connection');
$container->register('doctrine.dbal.default_connection.event_manager', 'stdClass');
$container->register('doctrine.dbal.default_connection.event_manager', 'stdClass')
->addArgument(new Reference('service_container'));
$container->register('doctrine.dbal.default_connection', 'stdClass');
if ($multipleConnections) {
$container->register('doctrine.dbal.second_connection.event_manager', 'stdClass');
$container->register('doctrine.dbal.second_connection.event_manager', 'stdClass')
->addArgument(new Reference('service_container'));
$container->register('doctrine.dbal.second_connection', 'stdClass');
$connections['second'] = 'doctrine.dbal.second_connection';
}

0 comments on commit 83232f8

Please sign in to comment.