From 79090928916350a1da48c540ec3e275855420fe7 Mon Sep 17 00:00:00 2001 From: Konstantin Myakshin Date: Thu, 30 May 2019 00:00:39 +0300 Subject: [PATCH] [Messenger] Improve error message when routing to an invalid transport (closes #31613) --- .../FrameworkExtension.php | 19 ++++++++++------ .../Fixtures/php/messenger.php | 5 +++++ .../Fixtures/php/messenger_routing.php | 3 ++- .../messenger_routing_invalid_transport.php | 16 ++++++++++++++ .../Fixtures/xml/messenger.xml | 3 +++ .../Fixtures/xml/messenger_routing.xml | 3 ++- .../messenger_routing_invalid_transport.xml | 22 +++++++++++++++++++ .../Fixtures/yml/messenger.yml | 4 ++++ .../Fixtures/yml/messenger_routing.yml | 3 ++- .../messenger_routing_invalid_transport.yml | 9 ++++++++ .../FrameworkExtensionTest.php | 20 ++++++++++------- 11 files changed, 89 insertions(+), 18 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing_invalid_transport.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing_invalid_transport.xml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing_invalid_transport.yml diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 38b552716d..1d83148ff7 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -1774,6 +1774,16 @@ class FrameworkExtension extends Extension } } + $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 @@ class FrameworkExtension extends Extension // 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/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger.php index 1160dfc573..adb8239d04 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger.php @@ -9,5 +9,10 @@ $container->loadFromExtension('framework', [ FooMessage::class => ['sender.bar', 'sender.biz'], BarMessage::class => 'sender.foo', ], + 'transports' => [ + 'sender.biz' => 'null://', + 'sender.bar' => 'null://', + 'sender.foo' => 'null://', + ], ], ]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing.php index 0cecbd71a5..eb45950901 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing.php @@ -7,7 +7,7 @@ $container->loadFromExtension('framework', [ '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'], ], @@ -15,6 +15,7 @@ $container->loadFromExtension('framework', [ ], 'transports' => [ 'amqp' => 'amqp://localhost/%2f/messages', + 'audit' => 'null://', ], ], ]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing_invalid_transport.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing_invalid_transport.php new file mode 100644 index 0000000000..ee77e3a3f2 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing_invalid_transport.php @@ -0,0 +1,16 @@ +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/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger.xml index e0dc11360a..bacd772dcb 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger.xml @@ -14,6 +14,9 @@ + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml index 62028169ea..0b022e78a0 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml @@ -11,7 +11,7 @@ - + @@ -21,6 +21,7 @@ + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing_invalid_transport.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing_invalid_transport.xml new file mode 100644 index 0000000000..98c487fbf8 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing_invalid_transport.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger.yml index 7f038af11f..82fea3b27a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger.yml +++ b/src/Symfony/Bundle/FrameworkBundle/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/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml index 540d0d8ad0..0e493eb882 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml +++ b/src/Symfony/Bundle/FrameworkBundle/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/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing_invalid_transport.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing_invalid_transport.yml new file mode 100644 index 0000000000..3bf0f2ddf9 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/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/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 8bea0514ac..60fefc659f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -668,8 +668,8 @@ abstract class FrameworkExtensionTest extends TestCase $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 @@ abstract class FrameworkExtensionTest extends TestCase $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 @@ abstract class FrameworkExtensionTest extends TestCase $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');