Skip to content

Commit c53541f

Browse files
fabpotfabpot
committed
bug #27084 [Messenger] Relax messenger config and fix some bugs (yceruto)
This PR was merged into the 4.1-dev branch. Discussion ---------- [Messenger] Relax messenger config and fix some bugs | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - After playing a little with the config of this component I found some bugs around. Reproducer: 1. Install a fresh Symfony flex project with `^4.0@dev` dependencies 2. Add the `symfony/messenger` requirement 3. Add the following configuration separately: Note that `symfony/serializer` has not been installed yet. ATM it's not required. ---------------------- Configuring my custom transport (not amqp): ```yaml framework: messenger: transports: custom: 'my_transport://localhost/msgs' routing: '*': custom ``` **Before** (Current behaviour): Threw a logic exception, IMO unrelated at this point: > Using the default encoder/decoder, Symfony Messenger requires the Serializer. Enable it or install it by running "composer require symfony/serializer-pack". **After** (Proposal): Pass! The Messenger's serializer config is disabled by definition because the Serializer component is not installed yet, then the related (default) encoder/decoder aliases are invalid, so the amqp transport factory service is removed altogether. ---------------------- According to the previous exception I configured my custom encoder/decoder services: ```yaml framework: messenger: encoder: App\Serializer\Serializer decoder: App\Serializer\Serializer transports: custom: 'my_transport://localhost/msgs' routing: '*': custom ``` **Before**: The same exception has been thrown, now a bit vague according to the config: > Using the default encoder/decoder, Symfony Messenger requires the Serializer. Enable it or install it by running "composer require symfony/serializer-pack". **After**: Pass! the serializer is disabled by definition but there is custom encoder/decoder services, so let's keep the amqp transport factory with their custom encoder/decoder deps. ---------------------- Just enabling the serializer option: ```yaml framework: messenger: serializer: true ``` **Before**: Pass! Unexpected, as there is no transport configuration the exception wasn't thrown and still keeps the amqp transport factory service with invalid encoder/decoder (Serializer) dependencies. **After**: The Serializer config & support is verified if it's enabled regardless of the transport configuration and this exception is thrown for this case: > The default Messenger serializer cannot be enabled as the Serializer support is not enabled. I've removed the "install" part because at this point only we're checking whether the `framework.serializer` is enabled or not, so the next step after that should be enable the Serializer support in `framework.serializer`, which already verify whether the Serializer component is installed or not. IMO "composer require symfony/serializer-pack" should be there and not here. Also because `symfony/serializer` is not a hard dependency of this component. ---------------------- By last, I disabled the serializer option manually: ```yaml framework: messenger: serializer: false transports: custom: 'my_transport://localhost/msgs' routing: '*': custom ``` **Before**: I received this DI exception: > The service "messenger.transport.amqp.factory" has a dependency on a non-existent service "messenger.transport.serializer". **After**: Pass! (same behaviour that the first example) ---------------------- As I explained earlier, this PR enables or disables the Messenger's serializer config based on the current Symfony platform and whether the Serializer component is installed or not, like other config with similar behaviour. Tests included :) Cheers! Commits ------- a05e2e2 Relax Messenger config and fix some bugs
2 parents 8123bb1 + a05e2e2 commit c53541f

19 files changed

+73
-6
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,7 @@ function ($a) {
10101010
->end()
10111011
->end()
10121012
->arrayNode('serializer')
1013-
->canBeDisabled()
1013+
->{!class_exists(FullStack::class) && class_exists(Serializer::class) ? 'canBeDisabled' : 'canBeEnabled'}()
10141014
->addDefaultsIfNotSet()
10151015
->children()
10161016
->scalarNode('format')->defaultValue('json')->end()

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+9-2
Original file line numberDiff line numberDiff line change
@@ -1444,15 +1444,18 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
14441444
$loader->load('messenger.xml');
14451445

14461446
if ($this->isConfigEnabled($container, $config['serializer'])) {
1447-
if (\count($config['transports']) > 0 && !$this->isConfigEnabled($container, $serializerConfig)) {
1448-
throw new LogicException('Using the default encoder/decoder, Symfony Messenger requires the Serializer. Enable it or install it by running "composer require symfony/serializer-pack".');
1447+
if (!$this->isConfigEnabled($container, $serializerConfig)) {
1448+
throw new LogicException('The default Messenger serializer cannot be enabled as the Serializer support is not available. Try enable it or install it by running "composer require symfony/serializer-pack".');
14491449
}
14501450

14511451
$container->getDefinition('messenger.transport.serializer')
14521452
->replaceArgument(1, $config['serializer']['format'])
14531453
->replaceArgument(2, $config['serializer']['context']);
14541454
} else {
14551455
$container->removeDefinition('messenger.transport.serializer');
1456+
if ('messenger.transport.serializer' === $config['encoder'] || 'messenger.transport.serializer' === $config['decoder']) {
1457+
$container->removeDefinition('messenger.transport.amqp.factory');
1458+
}
14561459
}
14571460

14581461
$container->setAlias('messenger.transport.encoder', $config['encoder']);
@@ -1501,6 +1504,10 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
15011504
$container->getDefinition('messenger.asynchronous.routing.sender_locator')->replaceArgument(1, $messageToSenderIdsMapping);
15021505

15031506
foreach ($config['transports'] as $name => $transport) {
1507+
if (0 === strpos($transport['dsn'], 'amqp://') && !$container->hasDefinition('messenger.transport.amqp.factory')) {
1508+
throw new LogicException('The default AMQP transport is not available. Make sure you have installed and enabled the Serializer component. Try enable it or install it by running "composer require symfony/serializer-pack".');
1509+
}
1510+
15041511
$senderDefinition = (new Definition(SenderInterface::class))
15051512
->setFactory(array(new Reference('messenger.transport_factory'), 'createSender'))
15061513
->setArguments(array($transport['dsn'], $transport['options']))

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor
256256
'routing' => array(),
257257
'transports' => array(),
258258
'serializer' => array(
259-
'enabled' => true,
259+
'enabled' => !class_exists(FullStack::class),
260260
'format' => 'json',
261261
'context' => array(),
262262
),

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger.php

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
$container->loadFromExtension('framework', array(
77
'messenger' => array(
8+
'serializer' => false,
89
'routing' => array(
910
FooMessage::class => array('sender.bar', 'sender.biz'),
1011
BarMessage::class => 'sender.foo',

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_amqp_transport_no_serializer.php

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
$container->loadFromExtension('framework', array(
4+
'messenger' => array(
5+
'serializer' => array(
6+
'enabled' => false,
7+
),
8+
'transports' => array(
9+
'default' => 'amqp://localhost/%2f/messages',
10+
),
11+
),
12+
));

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_transport.php

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22

33
$container->loadFromExtension('framework', array(
4+
'serializer' => true,
45
'messenger' => array(
56
'serializer' => array(
67
'format' => 'csv',

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_transports.php

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
$container->loadFromExtension('framework', array(
44
'serializer' => true,
55
'messenger' => array(
6+
'serializer' => true,
67
'transports' => array(
78
'default' => 'amqp://localhost/%2f/messages',
89
'customised' => array(

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/serializer_disabled.php

+3
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,7 @@
44
'serializer' => array(
55
'enabled' => false,
66
),
7+
'messenger' => array(
8+
'serializer' => false,
9+
),
710
));

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger.xml

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
<framework:config>
99
<framework:messenger>
10+
<framework:serializer enabled="false" />
1011
<framework:routing message-class="Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\FooMessage">
1112
<framework:sender service="sender.bar" />
1213
<framework:sender service="sender.biz" />

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_amqp_transport_no_serializer.xml

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<container xmlns="http://symfony.com/schema/dic/services"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xmlns:framework="http://symfony.com/schema/dic/symfony"
5+
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
6+
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
7+
8+
<framework:config>
9+
<framework:messenger>
10+
<framework:serializer enabled="false" />
11+
<framework:transport name="default" dsn="amqp://localhost/%2f/messages" />
12+
</framework:messenger>
13+
</framework:config>
14+
</container>

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_transport.xml

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
77

88
<framework:config>
9+
<framework:serializer enabled="true" />
910
<framework:messenger>
1011
<framework:serializer format="csv">
1112
<framework:context>

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_transports.xml

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
<framework:config>
99
<framework:serializer enabled="true" />
1010
<framework:messenger>
11+
<framework:serializer enabled="true" />
1112
<framework:transport name="default" dsn="amqp://localhost/%2f/messages" />
1213
<framework:transport name="customised" dsn="amqp://localhost/%2f/messages?exchange_name=exchange_name">
1314
<framework:options>

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/serializer_disabled.xml

+3
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,8 @@
77

88
<framework:config>
99
<framework:serializer enabled="false" />
10+
<framework:messenger>
11+
<framework:serializer enabled="false" />
12+
</framework:messenger>
1013
</framework:config>
1114
</container>

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger.yml

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
framework:
22
messenger:
3+
serializer: false
34
routing:
45
'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\FooMessage': ['sender.bar', 'sender.biz']
56
'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\BarMessage': 'sender.foo'

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_amqp_transport_no_serializer.yml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
framework:
2+
messenger:
3+
serializer: false
4+
transports:
5+
default: 'amqp://localhost/%2f/messages'

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_transport.yml

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
framework:
2+
serializer: true
23
messenger:
34
serializer:
45
format: csv

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_transports.yml

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
framework:
22
serializer: true
33
messenger:
4+
serializer: true
45
transports:
56
default: 'amqp://localhost/%2f/messages'
67
customised:

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/serializer_disabled.yml

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
framework:
22
serializer:
33
enabled: false
4+
messenger:
5+
serializer: false

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

+14-2
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,8 @@ public function testWebLink()
526526
public function testMessenger()
527527
{
528528
$container = $this->createContainerFromFile('messenger');
529-
$this->assertTrue($container->has('message_bus'));
529+
$this->assertTrue($container->hasAlias('message_bus'));
530+
$this->assertFalse($container->hasDefinition('messenger.transport.amqp.factory'));
530531
}
531532

532533
public function testMessengerTransports()
@@ -556,6 +557,8 @@ public function testMessengerTransports()
556557
$this->assertCount(2, $receiverArguments);
557558
$this->assertSame('amqp://localhost/%2f/messages?exchange_name=exchange_name', $receiverArguments[0]);
558559
$this->assertSame(array('queue' => array('name' => 'Queue')), $receiverArguments[1]);
560+
561+
$this->assertTrue($container->hasDefinition('messenger.transport.amqp.factory'));
559562
}
560563

561564
public function testMessengerRouting()
@@ -574,13 +577,22 @@ public function testMessengerRouting()
574577

575578
/**
576579
* @expectedException \Symfony\Component\DependencyInjection\Exception\LogicException
577-
* @expectedExceptionMessage Using the default encoder/decoder, Symfony Messenger requires the Serializer. Enable it or install it by running "composer require symfony/serializer-pack".
580+
* @expectedExceptionMessage The default Messenger serializer cannot be enabled as the Serializer support is not available. Try enable it or install it by running "composer require symfony/serializer-pack".
578581
*/
579582
public function testMessengerTransportConfigurationWithoutSerializer()
580583
{
581584
$this->createContainerFromFile('messenger_transport_no_serializer');
582585
}
583586

587+
/**
588+
* @expectedException \Symfony\Component\DependencyInjection\Exception\LogicException
589+
* @expectedExceptionMessage The default AMQP transport is not available. Make sure you have installed and enabled the Serializer component. Try enable it or install it by running "composer require symfony/serializer-pack".
590+
*/
591+
public function testMessengerAMQPTransportConfigurationWithoutSerializer()
592+
{
593+
$this->createContainerFromFile('messenger_amqp_transport_no_serializer');
594+
}
595+
584596
public function testMessengerTransportConfiguration()
585597
{
586598
$container = $this->createContainerFromFile('messenger_transport');

0 commit comments

Comments
 (0)