[Messenger] Automatically route messages when `from_transport` tag is set #35111

ruudk · 2019-12-26T11:25:53Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
Deprecations?	no
License	MIT
Doc PR	todo

When using multiple transports with Symfony Messenger, you need to add routing for every message to the designated transport(s).

This looks something like this:

# config/packages/messenger.yaml
framework:
    messenger:
        routing:
            'App\Command\CreateProjectCommand': sync
            'App\Command\DeleteProjectCommand': async

As your project grows, this list can become very long. And it's yet another step that a developer needs to take to let the message be handled properly.

Messenger supports a way to explicitly bind handlers to a given transport. That can now also be enabled via tag attributes:

# config/services.yaml
App\Handler\Synchronous\:
    resource: '%kernel.project_dir%/src/Handler/Synchronous'
    tags: [{ name: 'messenger.message_handler', bus: 'command.bus', from_transport: 'sync' }]

App\Handler\Asynchronous\:
    resource: '%kernel.project_dir%/src/Handler/Asynchronous'
    tags: [{ name: 'messenger.message_handler', bus: 'command.bus', from_transport: 'async' }]

# src/Handler/Synchronous/CreateProjectHandler.php
namespace App\Handler\Synchronous;

class CreateProjectHandler // No interfaces needed
{
    public function __invoke(CreateProjectCommand $command)
    {
        // Create the project
    }
}

# src/Handler/Asynchronous/DeleteProjectHandler.php
namespace App\Handler\Asynchronous;

class DeleteProjectHandler // No interfaces needed
{
    public function __invoke(DeleteProjectCommand $command)
    {
        // Delete the project
    }
}

As you can see the DeleteProjectHandler is executed asynchronously because it's located in the src/Handler/Asynchronous directory and tagged with from_transport: async attribute.

If a developer wants to execute it synchronously instead of asynchronously, it needs to move the handler to the Synchronous directory + update the symfony.messenger.routing configuration.

This PR introduces a new way of doing this: auto routing. When enabled, you no longer need to create any routing:

 # config/packages/messenger.yaml
 framework:
     messenger:
+        auto_routing: true 
-        routing:
-           'App\Command\CreateProjectCommand': sync
-            'App\Command\DeleteProjectCommand': async

The MessengerPass will automatically detect that CreateProjectHandler should be invoked with a CreateProjectCommand and uses from_transport: sync. Therefore, it knows that CreateProjectCommand should be routed to the sync transport. For DeleteProjectCommand it sees from_transport: async and creates the routing accordingly.

This functionality also works when using MessageSubscriberInterface:

# src/SomeHandler.php
namespace App;

use Symfony\Component\Messenger\Handler\MessageSubscriberInterface;

class SomeHandler implements MessageSubscriberInterface
{
    public function __invoke(SomeCommand $command)
    {
        // Do something
    }

    public static function getHandledMessages(): iterable
    {
        yield SomeCommand::class => [
            'from_transport' => 'async',
        ];
    }
}

When auto routing is enabled you can still manually create routes. The MessengerPass will simply append anything it detects.

When you have multiple transports, it can become quite cumbersome to map every message to all the transports that it needs. Instead, you can tag each handler with `from_transport` to explicitly say on which transport it should handle. Having both the transport and the message that the handler will support, we can automatically add the routing too.

Koc · 2019-12-30T15:43:34Z

framework:
    messenger:
        transports:
            events:
                dsn: amqp://guest:guest@127.0.0.1/%2f/events
                options:
                    queues:
                        3rdparty:
                            binding_keys: [ 3rdparty ]
                        projection:
                            binding_keys: [ projection ]

            events_3rdparty: amqp://guest:guest@127.0.0.1/%2f/events?queues[3rdparty]
            events_projection: amqp://guest:guest@127.0.0.1/%2f/events?queues[projection]

        routing:
            'App\Message\RegisterBet': events

Above config allows us publish messages to the events transport and replicate message to 2 queues. After that message will be consumed by the events_3rdparty and events_projection transports. So we have 1 transport for publishing and 2 transports for reading.

Am I right that since your changes message will be automatically published to all of this transports?

cc @sroze

ruudk · 2019-12-30T15:52:34Z

@Koc clever trick of separating the write and read. Didn't know this was possible.

My change will only affect handlers that are tagged with from_transport like this:

   Domain\Subscriber\Asynchronous\:
        resource: '%kernel.project_dir%/src/Domain/Subscriber/Asynchronous'
        tags:
            - { name: 'messenger.message_handler', bus: 'event.bus', from_transport: 'async' }

    Domain\Subscriber\Synchronous\:
        resource: '%kernel.project_dir%/src/Domain/Subscriber/Synchronous'
        tags:
            - { name: 'messenger.message_handler', bus: 'event.bus', from_transport: 'sync' }

Before this change, you would need to add all the messages (events in this case) to the routing and map them to the correct transport(s).

Now, the routing will be automatically discovered based on from_transport + handler + message it supports on __invoke.

Example:

Domain\Subscriber\Asynchronous\UserCreated\SendWelcomeEmail
Domain\Subscriber\Asynchronous\UserCreated\TrackMetrics
Domain\Subscriber\Synchronous\UserCreated\ApplyDiscount

With the config above, UserCreated will be automatically routed to async and sync transport because 2 handlers are registrered that are async and support UserCreated and 1 handler that is sync.

If you remove the Domain\Subscriber\Synchronous\UserCreated\ApplyDiscount handler later, it will only route UserCreated to async transport as there is no sync handler anymore.

ruudk · 2020-01-05T10:25:08Z

@sroze I would like to know how you feel about this and if you considered this but didn't do for a good reason.

sroze · 2020-01-08T22:35:57Z

@ruudk this is a very good idea. However, we need to make sure that just adding one handler with a from_transport doesn't cause the other (sync) handlers not to be called, as you described in your examples.

Should this be default behaviour? Or should it be done with a configuration like routing: auto?

I think that this would be a BC-break if this was turned on by default configuration because it might be that an application developer decided to configure a from_transport but did (or does) not want the message to be sent to this transport when dispatching it. (Basically @Koc's example).

We have two ways going forward with this:

routing: auto-type of configuration in the framework.messenger: it could be enabled by default by the recipe. The downside is that it's only applicable globally and therefore you can't mix the auto-routing & @Koc's example.
Add a new option (via_transport?) which auto-configures the routing. The obvious downside is that it would be super confusing...

I'd go for option 1, given that this behaviour would be optional, like the auto-wiring. Any PoV @symfony/mergers?

ruudk · 2020-01-09T17:34:36Z

@sroze thanks for your reply. What if we just make it possible to enable this behavior by tagging the handler with auto_route: true attribute? Downside is that this will not work out of the box with auto wire. More advanced users can enable it by manually tagging their handlers with attributes.

ruudk · 2020-01-09T19:08:11Z

@sroze Implemented this in e06568c.

Nyholm · 2020-01-19T20:04:30Z

Sorry for asking. But isnt this the exact feature of a MessageSubscriberInterface?

ruudk · 2020-01-21T13:15:07Z

@Nyholm No, AFAIK not. It will register the handler to the correct messages and transports but it will not add entries to framework.messenger.routing. You still have to either say MyCommand: ['sync', 'async'] or *: ['sync', 'async'] (which I don't like).

Nyholm · 2020-01-21T14:01:11Z

I see, so using from_transport on the messenger.message_handler tag OR using MessageSubscriberInterface will be the same as using routing.

I like that =)

I dont believe we need an opt-in. If you currently not using any routing (and using from_transport) then no message will be consumed.

Could you update your PR description with some examples?

Nyholm · 2020-01-21T14:03:23Z

Hm.. I see that Samuel thinks it should be opt-in... Since some people may have turned of handlers in the wrong way.. fair.

I suggest adding a new option: framework.messenger.auto_routing. This is boolan false by default.

ruudk · 2020-01-22T12:16:51Z

@Nyholm I have implemented what we discussed and I have updated the opening post with a lot of examples. Please let me know if you have any suggestions to make the opening post more clear for a future blog post by @javiereguiluz 😊

Nyholm

Thank you.

I think you made a typo in the tests, otherwise Im happy with this.

Nyholm · 2020-01-22T12:19:12Z

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

@@ -1046,6 +1046,10 @@ private function addMessengerSection(ArrayNodeDefinition $rootNode)
                        ->ifTrue(static function ($v): bool { return isset($v['buses']) && null !== $v['default_bus'] && !isset($v['buses'][$v['default_bus']]); })
                        ->then(static function (array $v): void { throw new InvalidConfigurationException(sprintf('The specified default bus "%s" is not configured. Available buses are "%s".', $v['default_bus'], implode('", "', array_keys($v['buses'])))); })
                    ->end()
+                    ->booleanNode('auto_routing')
+                        ->info('Automatically create routing for handlers that specify "from_transport" via tag attribute or MessageSubscriberInterface.')
+                        ->defaultFalse()


FYI, I think this should be defaultTrue() in Symfony 6. But that is a separate discussion.

How and where do you keep track of this?

I am not sure. I wanted to write a note here to help other reviewers to pitch in. They will either say to open second PR/issue after this get merged so we can discuss if this is a good default or not, OR they will ask you to do something clever and add deprecation messages somewhere.

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

Tobion

The use-case for the from_transport option is when you have multiple handlers for the same message and want to only execute certain handlers based on the transport.
This use-case totally conflicts with auto routing as you introduced here.
Say you have Message A, Handler B (from transport X) and Handler C (from transport Y). Auto routing would send the message A to both transports always? Then there is no point in having the from_transport option configured in the first place.
So those concepts conflict and should not be mixed. So I'm -1 on this PR.

I agree that making the routing easier would be nice. But not by misusing from_transport option.
What we need is a way to configure the routing, which is for sending, on the message (e.g. by implementing a certain interface). The handler configuration is for the receiving and not the right place.

Nyholm · 2020-01-26T16:00:06Z

Say you have Message A, Handler B (from transport X) and Handler C (from transport Y). Auto routing would send the message A to both transports always?

Not as I have understood it. If A comes from transport X till till go to B, if it comes from Y it will go to C. If A comes from Z, no handler will be called.

Tobion · 2020-01-26T16:48:10Z

Not as I have understood it. If A comes from transport X till till go to B, if it comes from Y it will go to C. If A comes from Z, no handler will be called.

That's not the question. The question is about the routing to the sender which this PR is about.

weaverryan · 2020-01-31T19:36:19Z

Messenger is built around sending/routing "messages" not "handlers". This makes the use-case of HandlerA "goes to" the sync transport and HandlerB "goes to" the async transport tricky. The from_transport value of your handlers needs to be perfectly configured with your routing else:

(A) a handler might not be called (because you forgot to route the class to an additional transport)
or
(B) a handler might be called twice (because you forgot to add from_transport to one of your handlers).

For example, if a message has 1 handler today (with no from_transport) and we add a 2nd handler WITH a from_transport and then change the routing for the message to be [sync, async], the first handler (with no from_transport) would be called twice. It's not a technical problem... it's just easy to misconfigure from_transport because it's so "far away" from the routing config that it must work perfectly with. I show, but recommend not to use, from_transport in our screencasts... which is a bummer :/.

What I like about this PR is that it aims to solve that: by (in theory) relying only on from_transport if you have this situation. What I don't like is that it feels like it's making a hectic system more complex - it's now difficult to understand which messages are being routed to which transports and why. And if you're doing more advanced routing - like in the earlier example by @Koc #35111 (comment) - things get fuzzier.

For example, if you have from_transport set to async_priority_high, auto-routing would send that message to async_priority_high. But, what if I am using special routing in my app where I actually want this to be sent to async... and it's due to my advanced routing config that Rabbit ultimately sends it to a queue that is read by async_priority_high? That's an advanced feature, but that's the kind of trouble I see here.

What would I propose instead? I'm not sure... but something that keeps the routing in one place. Maybe something like this?

framework:
    messenger:
        routing:
            # internally, this would send to async & sync + set the from_transport on the handlers
            # more advanced users could still route by class and set the `from_transport` specifically
            # on the handlers if they have more advanced config
            'App\Command\CreateProjectCommand':
                App\CommandHandler\CreateProjectHandler1: async
                App\CommandHandler\CreateProjectHandler2: sync

            'App\Command\DeleteProjectCommand': async

To me, that is the clearest. However, it's also the most verbose - it's more long classes in YAML :). We could, thus, perhaps have an "auto" feature on a class-by-class basis:

framework:
    messenger:
        routing:
            'App\Command\CreateProjectCommand':
                # would do the behavior of this PR: route based on `from_transport` of handlers
                # I guess every handler should be required to have `from_transport`?
                auto_route: true

            'App\Command\DeleteProjectCommand': async

This isn't bad... but it still requires every class to be routed... or to use * or to use some interface that all messages implements... all of which are an ugly workaround.

In the end, I don't really like any of this. I get the feeling that the best solution might be something quite different... that I can't imagine right now. Ultimately, sending and receiving are two separate (but often linked) things. Maybe this PR is the right direction... I'm just not sure...

ruudk requested a review from sroze as a code owner December 26, 2019 11:25

carsonbot added Status: Needs Review Feature labels Dec 26, 2019

ruudk force-pushed the auto-routing branch from 856e52f to 616171b Compare December 26, 2019 11:26

ruudk force-pushed the auto-routing branch from 616171b to a46690b Compare December 27, 2019 10:25

ruudk changed the title ~~Automatically route messages when from_transport tag is set~~ [Messenger] Automatically route messages when from_transport tag is set Dec 27, 2019

nicolas-grekas added this to the next milestone Dec 28, 2019

sroze added the Messenger label Jan 8, 2020

Only enable routing when auto_route: true

e06568c

ruudk added 3 commits January 22, 2020 12:41

Apply feedback

34e1654

Remove backticks

58cd1a4

Check if parameter exists

6f88ab8

Nyholm approved these changes Jan 22, 2020

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jan 22, 2020

ruudk added 3 commits January 22, 2020 13:24

Fix test

3e2f6ed

Place booleanNode under children

c6ae541

Fix config test

9536000

Tobion suggested changes Jan 26, 2020

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Jan 26, 2020

ruudk closed this May 14, 2020

ruudk deleted the auto-routing branch May 14, 2020 12:37

nicolas-grekas modified the milestones: next, 5.1 May 16, 2020

ruudk mentioned this pull request Sep 21, 2020

The future of SimpleBus SimpleBus/SimpleBus#37

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Messenger] Automatically route messages when `from_transport` tag is set #35111

[Messenger] Automatically route messages when `from_transport` tag is set #35111

ruudk commented Dec 26, 2019 •

edited

Loading

Koc commented Dec 30, 2019 •

edited

Loading

ruudk commented Dec 30, 2019 •

edited

Loading

ruudk commented Jan 5, 2020

sroze commented Jan 8, 2020

ruudk commented Jan 9, 2020

ruudk commented Jan 9, 2020

Nyholm commented Jan 19, 2020

ruudk commented Jan 21, 2020

Nyholm commented Jan 21, 2020

Nyholm commented Jan 21, 2020

ruudk commented Jan 22, 2020

Nyholm left a comment

Nyholm Jan 22, 2020 •

edited

Loading

ruudk Jan 22, 2020

Nyholm Jan 22, 2020

Tobion left a comment •

edited

Loading

Nyholm commented Jan 26, 2020

Tobion commented Jan 26, 2020

weaverryan commented Jan 31, 2020

[Messenger] Automatically route messages when from_transport tag is set #35111

[Messenger] Automatically route messages when from_transport tag is set #35111

Conversation

ruudk commented Dec 26, 2019 • edited Loading

Koc commented Dec 30, 2019 • edited Loading

ruudk commented Dec 30, 2019 • edited Loading

ruudk commented Jan 5, 2020

sroze commented Jan 8, 2020

ruudk commented Jan 9, 2020

ruudk commented Jan 9, 2020

Nyholm commented Jan 19, 2020

ruudk commented Jan 21, 2020

Nyholm commented Jan 21, 2020

Nyholm commented Jan 21, 2020

ruudk commented Jan 22, 2020

Nyholm left a comment

Choose a reason for hiding this comment

Nyholm Jan 22, 2020 • edited Loading

Choose a reason for hiding this comment

ruudk Jan 22, 2020

Choose a reason for hiding this comment

Nyholm Jan 22, 2020

Choose a reason for hiding this comment

Tobion left a comment • edited Loading

Choose a reason for hiding this comment

Nyholm commented Jan 26, 2020

Tobion commented Jan 26, 2020

weaverryan commented Jan 31, 2020

[Messenger] Automatically route messages when `from_transport` tag is set #35111

[Messenger] Automatically route messages when `from_transport` tag is set #35111

ruudk commented Dec 26, 2019 •

edited

Loading

Koc commented Dec 30, 2019 •

edited

Loading

ruudk commented Dec 30, 2019 •

edited

Loading

Nyholm Jan 22, 2020 •

edited

Loading

Tobion left a comment •

edited

Loading