[Messenger] Improve error message when routing to an invalid transport (closes #31613)

Koc · Koc · commit fbb209386318 · 2019-09-28T01:24:27.000+03:00
diff --git a/DependencyInjection/FrameworkExtension.php b/DependencyInjection/FrameworkExtension.php
@@ -1774,6 +1774,16 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
             }
         }
 
+        $senderReferences = [];
+        // alias => service_id
+        foreach ($senderAliases as $alias => $serviceId) {
+            $senderReferences[$alias] = new Reference($serviceId);
+        }
+        // service_id => service_id
+        foreach ($senderAliases as $serviceId) {
+            $senderReferences[$serviceId] = new Reference($serviceId);
+        }
+
         $messageToSendersMapping = [];
         foreach ($config['routing'] as $message => $messageConfiguration) {
             if ('*' !== $message && !class_exists($message) && !interface_exists($message, false)) {
@@ -1782,19 +1792,14 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
 
             // make sure senderAliases contains all senders
             foreach ($messageConfiguration['senders'] as $sender) {
-                if (!isset($senderAliases[$sender])) {
-                    $senderAliases[$sender] = $sender;
+                if (!isset($senderReferences[$sender])) {
+                    throw new LogicException(sprintf('Invalid Messenger routing configuration: the "%s" class is being routed to a sender called "%s". This is not a valid transport or service id.', $message, $sender));
                 }
             }
 
             $messageToSendersMapping[$message] = $messageConfiguration['senders'];
         }
 
-        $senderReferences = [];
-        foreach ($senderAliases as $alias => $serviceId) {
-            $senderReferences[$alias] = new Reference($serviceId);
-        }
-
         $container->getDefinition('messenger.senders_locator')
             ->replaceArgument(0, $messageToSendersMapping)
             ->replaceArgument(1, ServiceLocatorTagPass::register($container, $senderReferences))
diff --git a/Tests/DependencyInjection/Fixtures/php/messenger.php b/Tests/DependencyInjection/Fixtures/php/messenger.php
@@ -9,5 +9,10 @@
             FooMessage::class => ['sender.bar', 'sender.biz'],
             BarMessage::class => 'sender.foo',
         ],
+        'transports' => [
+            'sender.biz' => 'null://',
+            'sender.bar' => 'null://',
+            'sender.foo' => 'null://',
+        ],
     ],
 ]);
diff --git a/Tests/DependencyInjection/Fixtures/php/messenger_routing.php b/Tests/DependencyInjection/Fixtures/php/messenger_routing.php
@@ -7,14 +7,15 @@
             'default_serializer' => 'messenger.transport.symfony_serializer',
         ],
         'routing' => [
-            'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage' => ['amqp', 'audit'],
+            'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage' => ['amqp', 'messenger.transport.audit'],
             'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\SecondMessage' => [
                 'senders' => ['amqp', 'audit'],
             ],
             '*' => 'amqp',
         ],
         'transports' => [
             'amqp' => 'amqp://localhost/%2f/messages',
+            'audit' => 'null://',
         ],
     ],
 ]);
diff --git a/Tests/DependencyInjection/Fixtures/php/messenger_routing_invalid_transport.php b/Tests/DependencyInjection/Fixtures/php/messenger_routing_invalid_transport.php
@@ -0,0 +1,16 @@
+<?php
+
+$container->loadFromExtension('framework', [
+    'serializer' => true,
+    'messenger' => [
+        'serializer' => [
+            'default_serializer' => 'messenger.transport.symfony_serializer',
+        ],
+        'routing' => [
+            'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage' => 'invalid',
+        ],
+        'transports' => [
+            'amqp' => 'amqp://localhost/%2f/messages',
+        ],
+    ],
+]);
diff --git a/Tests/DependencyInjection/Fixtures/xml/messenger.xml b/Tests/DependencyInjection/Fixtures/xml/messenger.xml
@@ -14,6 +14,9 @@
             <framework:routing message-class="Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\BarMessage">
                 <framework:sender service="sender.foo" />
             </framework:routing>
+            <framework:transport name="sender.bar" dsn="null://" />
+            <framework:transport name="sender.biz" dsn="null://" />
+            <framework:transport name="sender.foo" dsn="null://" />
         </framework:messenger>
     </framework:config>
 </container>
diff --git a/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml b/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml
@@ -11,7 +11,7 @@
             <framework:serializer default-serializer="messenger.transport.symfony_serializer" />
             <framework:routing message-class="Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage">
                 <framework:sender service="amqp" />
-                <framework:sender service="audit" />
+                <framework:sender service="messenger.transport.audit" />
             </framework:routing>
             <framework:routing message-class="Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\SecondMessage">
                 <framework:sender service="amqp" />
@@ -21,6 +21,7 @@
                 <framework:sender service="amqp" />
             </framework:routing>
             <framework:transport name="amqp" dsn="amqp://localhost/%2f/messages" />
+            <framework:transport name="audit" dsn="null://" />
         </framework:messenger>
     </framework:config>
 </container>
diff --git a/Tests/DependencyInjection/Fixtures/xml/messenger_routing_invalid_transport.xml b/Tests/DependencyInjection/Fixtures/xml/messenger_routing_invalid_transport.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xmlns:framework="http://symfony.com/schema/dic/symfony"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd
+        http://symfony.com/schema/dic/symfony https://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
+
+    <framework:config>
+        <framework:serializer enabled="true" />
+        <framework:messenger>
+            <framework:serializer default-serializer="messenger.transport.symfony_serializer" />
+            <framework:routing message-class="Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage">
+                <framework:sender service="invalid" />
+            </framework:routing>
+            <framework:routing message-class="Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\SecondMessage">
+                <framework:sender service="amqp" />
+                <framework:sender service="audit" />
+            </framework:routing>
+            <framework:transport name="amqp" dsn="amqp://localhost/%2f/messages" />
+        </framework:messenger>
+    </framework:config>
+</container>
diff --git a/Tests/DependencyInjection/Fixtures/yml/messenger.yml b/Tests/DependencyInjection/Fixtures/yml/messenger.yml
@@ -3,3 +3,7 @@ framework:
         routing:
             'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\FooMessage': ['sender.bar', 'sender.biz']
             'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\BarMessage': 'sender.foo'
+        transports:
+            sender.biz: 'null://'
+            sender.bar: 'null://'
+            sender.foo: 'null://'
diff --git a/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml b/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml
@@ -4,9 +4,10 @@ framework:
         serializer:
             default_serializer: messenger.transport.symfony_serializer
         routing:
-            'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage': [amqp, audit]
+            'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage': [amqp, messenger.transport.audit]
             'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\SecondMessage':
                 senders: [amqp, audit]
             '*': amqp
         transports:
             amqp: 'amqp://localhost/%2f/messages'
+            audit: 'null://'
diff --git a/Tests/DependencyInjection/Fixtures/yml/messenger_routing_invalid_transport.yml b/Tests/DependencyInjection/Fixtures/yml/messenger_routing_invalid_transport.yml
@@ -0,0 +1,9 @@
+framework:
+    serializer: true
+    messenger:
+        serializer:
+            default_serializer: messenger.transport.symfony_serializer
+        routing:
+            'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage': invalid
+        transports:
+            amqp: 'amqp://localhost/%2f/messages'
diff --git a/Tests/DependencyInjection/FrameworkExtensionTest.php b/Tests/DependencyInjection/FrameworkExtensionTest.php
@@ -668,8 +668,8 @@ public function testMessenger()
         $this->assertTrue($container->getAlias('message_bus')->isPublic());
         $this->assertTrue($container->hasAlias('messenger.default_bus'));
         $this->assertTrue($container->getAlias('messenger.default_bus')->isPublic());
-        $this->assertFalse($container->hasDefinition('messenger.transport.amqp.factory'));
-        $this->assertFalse($container->hasDefinition('messenger.transport.redis.factory'));
+        $this->assertTrue($container->hasDefinition('messenger.transport.amqp.factory'));
+        $this->assertTrue($container->hasDefinition('messenger.transport.redis.factory'));
         $this->assertTrue($container->hasDefinition('messenger.transport_factory'));
         $this->assertSame(TransportFactory::class, $container->getDefinition('messenger.transport_factory')->getClass());
     }
@@ -712,14 +712,11 @@ public function testMessengerRouting()
         $senderLocatorDefinition = $container->getDefinition('messenger.senders_locator');
 
         $sendersMapping = $senderLocatorDefinition->getArgument(0);
-        $this->assertEquals([
-            'amqp',
-            'audit',
-        ], $sendersMapping[DummyMessage::class]);
+        $this->assertEquals(['amqp', 'messenger.transport.audit'], $sendersMapping[DummyMessage::class]);
         $sendersLocator = $container->getDefinition((string) $senderLocatorDefinition->getArgument(1));
-        $this->assertSame(['amqp', 'audit'], array_keys($sendersLocator->getArgument(0)));
+        $this->assertSame(['amqp', 'audit', 'messenger.transport.amqp', 'messenger.transport.audit'], array_keys($sendersLocator->getArgument(0)));
         $this->assertEquals(new Reference('messenger.transport.amqp'), $sendersLocator->getArgument(0)['amqp']->getValues()[0]);
-        $this->assertEquals(new Reference('audit'), $sendersLocator->getArgument(0)['audit']->getValues()[0]);
+        $this->assertEquals(new Reference('messenger.transport.audit'), $sendersLocator->getArgument(0)['messenger.transport.audit']->getValues()[0]);
     }
 
     public function testMessengerTransportConfiguration()
@@ -776,6 +773,13 @@ public function testMessengerMiddlewareFactoryErroneousFormat()
         $this->createContainerFromFile('messenger_middleware_factory_erroneous_format');
     }
 
+    public function testMessengerInvalidTransportRouting()
+    {
+        $this->expectException('LogicException');
+        $this->expectExceptionMessage('Invalid Messenger routing configuration: the "Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage" class is being routed to a sender called "invalid". This is not a valid transport or service id.');
+        $this->createContainerFromFile('messenger_routing_invalid_transport');
+    }
+
     public function testTranslator()
     {
         $container = $this->createContainerFromFile('full');
