Skip to content

Commit 9027376

Browse files
committed
minor #34155 Revert SyncTransport simplification and fix properly (weaverryan)
This PR was squashed before being merged into the 4.4 branch (closes #34155). Discussion ---------- Revert SyncTransport simplification and fix properly | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #34115 (and also related to #34066) | License | MIT | Doc PR | Not needed In #34069, I made `SyncTransport` simpler by removing that transport class and making the whole things a config trick. I felt GREAT about that solution... until i realized two big problems: 1) It kills using env vars for `sync://` because we read the config values at build time - #34115 - that could probably be fixed by adding a factory, but then there is also the next problem 2) If someone routed a message to `[async, sync]` (weird, but allowed), my #34069 config solution basically maps this internally to `[async]`, which actually causes the message to *not* be handled immediately. Basically, my solution only worked if you route a message ONLY to one sync transport, but fails if you route to multiple transports. So... this fixes things in a less-cool, but sensible way: A) The first commit reverts #34069 exactly B) The second commit solves the issue that we need to know if a message is being handled in a "worker" context or not, so middleware can decide if they should reset things before/after handling things. Previously we were using `ReceivedStamp` to know this. But because `SyncTransport` also "receives" the message and adds this stamp, it's not enough. To fix this, I added a new `ConsumedByWorkerStamp` that clearly means: "This message is being handled by a worker" (and so, you might want to "reset" some things before/after handling). Thanks! Commits ------- 01a9fefe77 Adding ConsumedByWorkerStamp as way to mark a message in a "worker context" 38f19a960c Revert "[Messenger] Removing "sync" transport and replacing it with much nicer config trick"
2 parents fd4b79b + dfadbfa commit 9027376

File tree

6 files changed

+26
-87
lines changed

6 files changed

+26
-87
lines changed

DependencyInjection/FrameworkExtension.php

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1755,27 +1755,18 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
17551755
$container->setAlias('messenger.default_serializer', $config['serializer']['default_serializer']);
17561756
}
17571757

1758-
$senderReferences = [];
1759-
$syncTransports = [];
1758+
$senderAliases = [];
17601759
$transportRetryReferences = [];
17611760
foreach ($config['transports'] as $name => $transport) {
17621761
$serializerId = $transport['serializer'] ?? 'messenger.default_serializer';
17631762

1764-
if (0 === strpos($transport['dsn'], 'sync://')) {
1765-
$syncTransports[] = $name;
1766-
} else {
1767-
$transportDefinition = (new Definition(TransportInterface::class))
1768-
->setFactory([new Reference('messenger.transport_factory'), 'createTransport'])
1769-
->setArguments([$transport['dsn'], $transport['options'] + ['transport_name' => $name], new Reference($serializerId)])
1770-
->addTag('messenger.receiver', ['alias' => $name])
1771-
;
1772-
$container->setDefinition($transportId = 'messenger.transport.'.$name, $transportDefinition);
1773-
1774-
// alias => service_id
1775-
$senderReferences[$name] = new Reference($transportId);
1776-
// service_id => service_id
1777-
$senderReferences[$transportId] = new Reference($transportId);
1778-
}
1763+
$transportDefinition = (new Definition(TransportInterface::class))
1764+
->setFactory([new Reference('messenger.transport_factory'), 'createTransport'])
1765+
->setArguments([$transport['dsn'], $transport['options'] + ['transport_name' => $name], new Reference($serializerId)])
1766+
->addTag('messenger.receiver', ['alias' => $name])
1767+
;
1768+
$container->setDefinition($transportId = 'messenger.transport.'.$name, $transportDefinition);
1769+
$senderAliases[$name] = $transportId;
17791770

17801771
if (null !== $transport['retry_strategy']['service']) {
17811772
$transportRetryReferences[$name] = new Reference($transport['retry_strategy']['service']);
@@ -1793,25 +1784,30 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
17931784
}
17941785
}
17951786

1787+
$senderReferences = [];
1788+
// alias => service_id
1789+
foreach ($senderAliases as $alias => $serviceId) {
1790+
$senderReferences[$alias] = new Reference($serviceId);
1791+
}
1792+
// service_id => service_id
1793+
foreach ($senderAliases as $serviceId) {
1794+
$senderReferences[$serviceId] = new Reference($serviceId);
1795+
}
1796+
17961797
$messageToSendersMapping = [];
17971798
foreach ($config['routing'] as $message => $messageConfiguration) {
17981799
if ('*' !== $message && !class_exists($message) && !interface_exists($message, false)) {
17991800
throw new LogicException(sprintf('Invalid Messenger routing configuration: class or interface "%s" not found.', $message));
18001801
}
18011802

1802-
// filter out "sync" senders
1803-
$realSenders = [];
1803+
// make sure senderAliases contains all senders
18041804
foreach ($messageConfiguration['senders'] as $sender) {
1805-
if (isset($senderReferences[$sender])) {
1806-
$realSenders[] = $sender;
1807-
} elseif (!\in_array($sender, $syncTransports, true)) {
1805+
if (!isset($senderReferences[$sender])) {
18081806
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));
18091807
}
18101808
}
18111809

1812-
if ($realSenders) {
1813-
$messageToSendersMapping[$message] = $realSenders;
1814-
}
1810+
$messageToSendersMapping[$message] = $messageConfiguration['senders'];
18151811
}
18161812

18171813
$container->getDefinition('messenger.senders_locator')

Resources/config/messenger.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@
7575
<tag name="messenger.transport_factory" />
7676
</service>
7777

78+
<service id="messenger.transport.sync.factory" class="Symfony\Component\Messenger\Transport\Sync\SyncTransportFactory">
79+
<tag name="messenger.transport_factory" />
80+
<argument type="service" id="messenger.routable_message_bus" />
81+
</service>
82+
7883
<service id="messenger.transport.in_memory.factory" class="Symfony\Component\Messenger\Transport\InMemoryTransportFactory">
7984
<tag name="messenger.transport_factory" />
8085
<tag name="kernel.reset" method="reset" />

Tests/DependencyInjection/Fixtures/php/messenger_sync_transport.php

Lines changed: 0 additions & 15 deletions
This file was deleted.

Tests/DependencyInjection/Fixtures/xml/messenger_sync_transport.xml

Lines changed: 0 additions & 24 deletions
This file was deleted.

Tests/DependencyInjection/Fixtures/yml/messenger_sync_transport.yml

Lines changed: 0 additions & 10 deletions
This file was deleted.

Tests/DependencyInjection/FrameworkExtensionTest.php

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddAnnotationsCachedReaderPass;
1717
use Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension;
1818
use Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage;
19-
use Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\FooMessage;
20-
use Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\SecondMessage;
2119
use Symfony\Bundle\FrameworkBundle\Tests\TestCase;
2220
use Symfony\Bundle\FullStack;
2321
use Symfony\Component\Cache\Adapter\AdapterInterface;
@@ -784,17 +782,6 @@ public function testMessengerInvalidTransportRouting()
784782
$this->createContainerFromFile('messenger_routing_invalid_transport');
785783
}
786784

787-
public function testMessengerSyncTransport()
788-
{
789-
$container = $this->createContainerFromFile('messenger_sync_transport');
790-
$senderLocatorDefinition = $container->getDefinition('messenger.senders_locator');
791-
792-
$sendersMapping = $senderLocatorDefinition->getArgument(0);
793-
$this->assertEquals(['amqp'], $sendersMapping[DummyMessage::class]);
794-
$this->assertArrayNotHasKey(SecondMessage::class, $sendersMapping);
795-
$this->assertEquals(['amqp'], $sendersMapping[FooMessage::class]);
796-
}
797-
798785
public function testTranslator()
799786
{
800787
$container = $this->createContainerFromFile('full');

0 commit comments

Comments
 (0)