bug #29558 [Messenger] Restore message handlers laziness (chalasr)

nicolas-grekas · nicolas-grekas · commit 0b99a8e28ca0 · 2018-12-13T09:58:21.000+01:00
This PR was merged into the 4.2 branch. Discussion ---------- [Messenger] Restore message handlers laziness | Q | A | ------------- | --- | Branch? | 4.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a The `HandlersLocator` introduced in 4.2 currently needs to instantiate all message handlers of the bus in order to be wired, while they were lazily loaded in 4.1 via ContainerHandlerLocator (removed). Example: ```yaml framework: messenger: buses: messenger.bus.command: ~ services: App\Messenger\: resource: '../src/Messenger/*Handler.php' tags: - { name: messenger.message_handler, bus: messenger.bus.command } ``` ```php namespace App\Messenger; class BarCommand {} class BarCommandHandler { public function __invoke(BarCommand $command): void {} } class FooCommand {} class FooCommandHandler { public function __invoke(FooCommand $command): void {} } ``` (Dumped `HandleMessageMiddleware` factory) Before: ```php return $this->privates['messenger.bus.command.middleware.handle_message'] = new \Symfony\Component\Messenger\Middleware\HandleMessageMiddleware(new \Symfony\Component\Messenger\Handler\HandlersLocator(array( 'App\\Messenger\\BarCommand' => \current(array(0 => array(0 => new \App\Messenger\BarCommandHandler()))), 'App\\Messenger\\FooCommand' => \current(array(0 => array(0 => new \App\Messenger\FooCommandHandler()))) ))); ``` After: ```php return $this->privates['messenger.bus.command.middleware.handle_message'] = new \Symfony\Component\Messenger\Middleware\HandleMessageMiddleware(new \Symfony\Component\Messenger\Handler\HandlersLocator(array( 'App\\Messenger\\BarCommand' => new RewindableGenerator(function () { yield 0 => ($this->privates['App\Messenger\BarCommandHandler'] ?? ($this->privates['App\Messenger\BarCommandHandler'] = new \App\Messenger\BarCommandHandler())); }, 1), 'App\\Messenger\\FooCommand' => new RewindableGenerator(function () { yield 0 => ($this->privates['App\Messenger\FooCommandHandler'] ?? ($this->privates['App\Messenger\FooCommandHandler'] = new \App\Messenger\FooCommandHandler())); }, 1) ))); ``` Commits ------- c608e73 [Messenger] Restore message handlers laziness
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -37,7 +37,6 @@
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\DependencyInjection\Alias;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
-use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
 use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -1600,12 +1599,7 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
                 $senders[$sender] = new Reference($senderAliases[$sender] ?? $sender);
             }
 
-            $sendersId = 'messenger.senders.'.$message;
-            $container->register($sendersId, RewindableGenerator::class)
-                ->setFactory('current')
-                ->addArgument(array(new IteratorArgument($senders)));
-            $messageToSendersMapping[$message] = new Reference($sendersId);
-
+            $messageToSendersMapping[$message] = new IteratorArgument($senders);
             $messagesToSendAndHandle[$message] = $messageConfiguration['send_and_handle'];
         }
 
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
@@ -651,10 +651,11 @@ public function testMessengerRouting()
         );
 
         $this->assertSame($messageToSendAndHandleMapping, $senderLocatorDefinition->getArgument(1));
+        $sendersMapping = $senderLocatorDefinition->getArgument(0);
         $this->assertEquals(array(
             'amqp' => new Reference('messenger.transport.amqp'),
             'audit' => new Reference('audit'),
-        ), $container->getDefinition('messenger.senders.'.DummyMessage::class)->getArgument(0)[0]->getValues());
+        ), $sendersMapping[DummyMessage::class]->getValues());
     }
 
     /**
diff --git a/src/Symfony/Component/Messenger/DependencyInjection/MessengerPass.php b/src/Symfony/Component/Messenger/DependencyInjection/MessengerPass.php
@@ -12,7 +12,6 @@
 namespace Symfony\Component\Messenger\DependencyInjection;
 
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
-use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
 use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
@@ -163,11 +162,7 @@ private function registerHandlers(ContainerBuilder $container, array $busIds)
         foreach ($handlersByBusAndMessage as $bus => $handlersByMessage) {
             foreach ($handlersByMessage as $message => $handlerIds) {
                 $handlers = array_map(function (string $handlerId) { return new Reference($handlerId); }, $handlerIds);
-                $handlersId = "messenger.handlers.$bus.$message";
-                $definitions[$handlersId] = (new Definition(RewindableGenerator::class))
-                    ->setFactory('current')
-                    ->addArgument(array($handlers));
-                $handlersLocatorMappingByBus[$bus][$message] = new Reference($handlersId);
+                $handlersLocatorMappingByBus[$bus][$message] = new IteratorArgument($handlers);
             }
         }
         $container->addDefinitions($definitions);
diff --git a/src/Symfony/Component/Messenger/Tests/DependencyInjection/MessengerPassTest.php b/src/Symfony/Component/Messenger/Tests/DependencyInjection/MessengerPassTest.php
@@ -12,7 +12,7 @@
 namespace Symfony\Component\Messenger\Tests\DependencyInjection;
 
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
+use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
 use Symfony\Component\DependencyInjection\Compiler\ResolveChildDefinitionsPass;
 use Symfony\Component\DependencyInjection\Compiler\ResolveClassPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -67,8 +67,8 @@ public function testProcess()
         $this->assertSame(HandlersLocator::class, $handlersLocatorDefinition->getClass());
         $this->assertEquals(
             array(
-                DummyMessage::class => new Reference("messenger.handlers.$busId.".DummyMessage::class),
-                SecondMessage::class => new Reference("messenger.handlers.$busId.".SecondMessage::class),
+                DummyMessage::class => new IteratorArgument(array(new Reference(DummyHandler::class))),
+                SecondMessage::class => new IteratorArgument(array(new Reference(MissingArgumentTypeHandler::class))),
             ),
             $handlersLocatorDefinition->getArgument(0)
         );
@@ -107,8 +107,8 @@ public function testProcessHandlersByBus()
         $this->assertSame(HandlersLocator::class, $commandBusHandlersLocatorDefinition->getClass());
         $this->assertEquals(
             array(
-                MultipleBusesMessage::class => new Reference("messenger.handlers.$commandBusId.".MultipleBusesMessage::class),
-                DummyCommand::class => new Reference("messenger.handlers.$commandBusId.".DummyCommand::class),
+                MultipleBusesMessage::class => new IteratorArgument(array(new Reference(MultipleBusesMessageHandler::class))),
+                DummyCommand::class => new IteratorArgument(array(new Reference(DummyCommandHandler::class))),
             ),
             $commandBusHandlersLocatorDefinition->getArgument(0)
         );
@@ -117,8 +117,8 @@ public function testProcessHandlersByBus()
         $this->assertSame(HandlersLocator::class, $queryBusHandlersLocatorDefinition->getClass());
         $this->assertEquals(
             array(
-                DummyQuery::class => new Reference("messenger.handlers.$queryBusId.".DummyQuery::class),
-                MultipleBusesMessage::class => new Reference("messenger.handlers.$queryBusId.".MultipleBusesMessage::class),
+                DummyQuery::class => new IteratorArgument(array(new Reference(DummyQueryHandler::class))),
+                MultipleBusesMessage::class => new IteratorArgument(array(new Reference(MultipleBusesMessageHandler::class))),
             ),
             $queryBusHandlersLocatorDefinition->getArgument(0)
         );
@@ -155,13 +155,10 @@ public function testGetClassesFromTheHandlerSubscriberInterface()
         $handlersMapping = $container->getDefinition($busId.'.messenger.handlers_locator')->getArgument(0);
 
         $this->assertArrayHasKey(DummyMessage::class, $handlersMapping);
-        $this->assertEquals(new Reference("messenger.handlers.$busId.".DummyMessage::class), $handlersMapping[DummyMessage::class]);
+        $this->assertEquals(new IteratorArgument(array(new Reference(HandlerWithMultipleMessages::class))), $handlersMapping[DummyMessage::class]);
 
         $this->assertArrayHasKey(SecondMessage::class, $handlersMapping);
-        $handlersDefinition = $container->getDefinition($handlersMapping[SecondMessage::class]);
-
-        $this->assertSame(RewindableGenerator::class, $handlersDefinition->getClass());
-        $this->assertEquals(array(new Reference(PrioritizedHandler::class), new Reference(HandlerWithMultipleMessages::class)), $handlersDefinition->getArgument(0)[0]);
+        $this->assertEquals(new IteratorArgument(array(new Reference(PrioritizedHandler::class), new Reference(HandlerWithMultipleMessages::class))), $handlersMapping[SecondMessage::class]);
     }
 
     public function testGetClassesAndMethodsAndPrioritiesFromTheSubscriber()
@@ -183,13 +180,13 @@ public function testGetClassesAndMethodsAndPrioritiesFromTheSubscriber()
         $this->assertArrayHasKey(DummyMessage::class, $handlersMapping);
         $this->assertArrayHasKey(SecondMessage::class, $handlersMapping);
 
-        $dummyHandlerReference = $container->getDefinition($handlersMapping[DummyMessage::class])->getArgument(0)[0][0];
+        $dummyHandlerReference = $handlersMapping[DummyMessage::class]->getValues()[0];
         $dummyHandlerDefinition = $container->getDefinition($dummyHandlerReference);
         $this->assertSame('callable', $dummyHandlerDefinition->getClass());
         $this->assertEquals(array(new Reference(HandlerMappingMethods::class), 'dummyMethod'), $dummyHandlerDefinition->getArgument(0));
         $this->assertSame(array('Closure', 'fromCallable'), $dummyHandlerDefinition->getFactory());
 
-        $secondHandlerReference = $container->getDefinition($handlersMapping[SecondMessage::class])->getArgument(0)[0][1];
+        $secondHandlerReference = $handlersMapping[SecondMessage::class]->getValues()[1];
         $secondHandlerDefinition = $container->getDefinition($secondHandlerReference);
         $this->assertSame(PrioritizedHandler::class, $secondHandlerDefinition->getClass());
     }
@@ -281,11 +278,11 @@ public function testItShouldNotThrowIfGeneratorIsReturnedInsteadOfArray()
         $handlersMapping = $container->getDefinition($busId.'.messenger.handlers_locator')->getArgument(0);
 
         $this->assertArrayHasKey(DummyMessage::class, $handlersMapping);
-        $firstReference = $container->getDefinition($handlersMapping[DummyMessage::class])->getArgument(0)[0][0];
+        $firstReference = $handlersMapping[DummyMessage::class]->getValues()[0];
         $this->assertEquals(array(new Reference(HandlerWithGenerators::class), 'dummyMethod'), $container->getDefinition($firstReference)->getArgument(0));
 
         $this->assertArrayHasKey(SecondMessage::class, $handlersMapping);
-        $secondReference = $container->getDefinition($handlersMapping[SecondMessage::class])->getArgument(0)[0][0];
+        $secondReference = $handlersMapping[SecondMessage::class]->getValues()[0];
         $this->assertEquals(array(new Reference(HandlerWithGenerators::class), 'secondMessage'), $container->getDefinition($secondReference)->getArgument(0));
     }
 
@@ -304,13 +301,13 @@ public function testItRegistersHandlersOnDifferentBuses()
         $eventsHandlerMapping = $container->getDefinition($eventsBusId.'.messenger.handlers_locator')->getArgument(0);
 
         $this->assertEquals(array(DummyMessage::class), array_keys($eventsHandlerMapping));
-        $firstReference = $container->getDefinition($eventsHandlerMapping[DummyMessage::class])->getArgument(0)[0][0];
+        $firstReference = $eventsHandlerMapping[DummyMessage::class]->getValues()[0];
         $this->assertEquals(array(new Reference(HandlerOnSpecificBuses::class), 'dummyMethodForEvents'), $container->getDefinition($firstReference)->getArgument(0));
 
         $commandsHandlerMapping = $container->getDefinition($commandsBusId.'.messenger.handlers_locator')->getArgument(0);
 
         $this->assertEquals(array(DummyMessage::class), array_keys($commandsHandlerMapping));
-        $firstReference = $container->getDefinition($commandsHandlerMapping[DummyMessage::class])->getArgument(0)[0][0];
+        $firstReference = $commandsHandlerMapping[DummyMessage::class]->getValues()[0];
         $this->assertEquals(array(new Reference(HandlerOnSpecificBuses::class), 'dummyMethodForCommands'), $container->getDefinition($firstReference)->getArgument(0));
     }
 
