[DoctrineBridge] always load event listeners lazy via ServiceLocator

UPGRADE-4.2.md

@@ -27,3 +27,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.

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

@@ -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)

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

@@ -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';
         }