Skip to content

Commit bf01d35

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#52693 [Messenger] Fix message handlers with multiple from_transports (valtzu)
This PR was merged into the 6.3 branch. Discussion ---------- [Messenger] Fix message handlers with multiple `from_transports` | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#48529 | License | MIT When you define multiple `AsMessageHandler` attributes or `messenger.message_handler` tags with otherwise same definitions but with different transports, the last definition overwrites all previous ones, despite `debug:messenger` showing it correctly: ``` The following messages can be dispatched: -------------------------------------------------------------------------------------------------------------------- App\Message\TriggeringMessage handled by App\MessageHandler\TriggeringMessageHandler (when from_transport=a) handled by App\MessageHandler\TriggeringMessageHandler (when from_transport=b) handled by App\MessageHandler\TriggeringMessageHandler (when method=handleTriggeringMessage, from_transport=a) handled by App\MessageHandler\TriggeringMessageHandler (when method=handleTriggeringMessage, from_transport=b) ``` Example code: ```php #[AsMessageHandler(fromTransport: 'a', handles: TriggeringMessage::class)] #[AsMessageHandler(fromTransport: 'b', handles: TriggeringMessage::class)] class TriggeringMessageHandler { public function __invoke(): void { echo __FUNCTION__."\n"; } #[AsMessageHandler(fromTransport: 'a', handles: TriggeringMessage::class)] #[AsMessageHandler(fromTransport: 'b', handles: TriggeringMessage::class)] public function handleTriggeringMessage(): void { echo __FUNCTION__."\n"; } } ``` When sending a `TriggeringMessage` to both `a` & `b` transports, I get following `bin/console messenger:consume a b` output on **6.3**: ``` [critical] Error thrown while handling message App\Message\TriggeringMessage. Removing from transport after 0 retries. Error: "No handler for message "App\Message\TriggeringMessage"." __invoke handleTriggeringMessage ``` And with this fix: ``` __invoke handleTriggeringMessage __invoke handleTriggeringMessage ``` Commits ------- bff3cc1 Fix message handlers with multiple from_transports
2 parents ff3aa75 + bff3cc1 commit bf01d35

File tree

4 files changed

+33
-5
lines changed

4 files changed

+33
-5
lines changed

‎src/Symfony/Component/Messenger/DependencyInjection/MessengerPass.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ private function registerHandlers(ContainerBuilder $container, array $busIds): v
106106
unset($options['handles']);
107107
$priority = $options['priority'] ?? 0;
108108
$method = $options['method'] ?? '__invoke';
109+
$fromTransport = $options['from_transport'] ?? '';
109110

110111
if (isset($options['bus'])) {
111112
if (!\in_array($options['bus'], $busIds)) {
@@ -131,10 +132,10 @@ private function registerHandlers(ContainerBuilder $container, array $busIds): v
131132
throw new RuntimeException(sprintf('Invalid handler service "%s": method "%s::%s()" does not exist.', $serviceId, $r->getName(), $method));
132133
}
133134

134-
if ('__invoke' !== $method) {
135+
if ('__invoke' !== $method || '' !== $fromTransport) {
135136
$wrapperDefinition = (new Definition('Closure'))->addArgument([new Reference($serviceId), $method])->setFactory('Closure::fromCallable');
136137

137-
$definitions[$definitionId = '.messenger.method_on_object_wrapper.'.ContainerBuilder::hash($message.':'.$priority.':'.$serviceId.':'.$method)] = $wrapperDefinition;
138+
$definitions[$definitionId = '.messenger.method_on_object_wrapper.'.ContainerBuilder::hash($message.':'.$priority.':'.$serviceId.':'.$method.':'.$fromTransport)] = $wrapperDefinition;
138139
} else {
139140
$definitionId = $serviceId;
140141
}

‎src/Symfony/Component/Messenger/Tests/DependencyInjection/MessengerPassTest.php

+17-3
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
use Symfony\Component\Messenger\Tests\Fixtures\SecondMessage;
5353
use Symfony\Component\Messenger\Tests\Fixtures\TaggedDummyHandler;
5454
use Symfony\Component\Messenger\Tests\Fixtures\TaggedDummyHandlerWithUnionTypes;
55+
use Symfony\Component\Messenger\Tests\Fixtures\ThirdMessage;
5556
use Symfony\Component\Messenger\Tests\Fixtures\UnionBuiltinTypeArgumentHandler;
5657
use Symfony\Component\Messenger\Tests\Fixtures\UnionTypeArgumentHandler;
5758
use Symfony\Component\Messenger\Tests\Fixtures\UnionTypeOneMessage;
@@ -102,7 +103,7 @@ public function testFromTransportViaTagAttribute()
102103
$container = $this->getContainerBuilder($busId = 'message_bus');
103104
$container
104105
->register(DummyHandler::class, DummyHandler::class)
105-
->addTag('messenger.message_handler', ['from_transport' => 'async'])
106+
->addTag('messenger.message_handler', ['from_transport' => 'async', 'method' => '__invoke'])
106107
;
107108

108109
(new MessengerPass())->process($container);
@@ -113,7 +114,7 @@ public function testFromTransportViaTagAttribute()
113114
$handlerDescriptionMapping = $handlersLocatorDefinition->getArgument(0);
114115
$this->assertCount(1, $handlerDescriptionMapping);
115116

116-
$this->assertHandlerDescriptor($container, $handlerDescriptionMapping, DummyMessage::class, [DummyHandler::class], [['from_transport' => 'async']]);
117+
$this->assertHandlerDescriptor($container, $handlerDescriptionMapping, DummyMessage::class, [[DummyHandler::class, '__invoke']], [['from_transport' => 'async']]);
117118
}
118119

119120
public function testHandledMessageTypeResolvedWithMethodAndNoHandlesViaTagAttributes()
@@ -178,7 +179,7 @@ public function testTaggedMessageHandler()
178179
$this->assertSame(HandlersLocator::class, $handlersLocatorDefinition->getClass());
179180

180181
$handlerDescriptionMapping = $handlersLocatorDefinition->getArgument(0);
181-
$this->assertCount(2, $handlerDescriptionMapping);
182+
$this->assertCount(3, $handlerDescriptionMapping);
182183

183184
$this->assertHandlerDescriptor($container, $handlerDescriptionMapping, DummyMessage::class, [TaggedDummyHandler::class], [[]]);
184185
$this->assertHandlerDescriptor(
@@ -187,6 +188,19 @@ public function testTaggedMessageHandler()
187188
SecondMessage::class,
188189
[[TaggedDummyHandler::class, 'handleSecondMessage']]
189190
);
191+
$this->assertHandlerDescriptor(
192+
$container,
193+
$handlerDescriptionMapping,
194+
ThirdMessage::class,
195+
[
196+
[TaggedDummyHandler::class, 'handleThirdMessage'],
197+
[TaggedDummyHandler::class, 'handleThirdMessage'],
198+
],
199+
[
200+
['from_transport' => 'a'],
201+
['from_transport' => 'b'],
202+
],
203+
);
190204
}
191205

192206
public function testTaggedMessageHandlerWithUnionTypes()

‎src/Symfony/Component/Messenger/Tests/Fixtures/TaggedDummyHandler.php

+6
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,10 @@ public function __invoke(DummyMessage $message)
1515
public function handleSecondMessage(SecondMessage $message)
1616
{
1717
}
18+
19+
#[AsMessageHandler(fromTransport: 'a')]
20+
#[AsMessageHandler(fromTransport: 'b')]
21+
public function handleThirdMessage(ThirdMessage $message): void
22+
{
23+
}
1824
}

‎src/Symfony/Component/Messenger/Tests/Fixtures/ThirdMessage.php

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace Symfony\Component\Messenger\Tests\Fixtures;
4+
5+
class ThirdMessage
6+
{
7+
}

0 commit comments

Comments
 (0)