[Messenger] Add Transport attribute to configure message to transport mapping #41179

maxim-dovydenok · 2021-05-11T19:55:44Z

Q	A
Branch?	5.x
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #41106
License	MIT
Doc PR	symfony/symfony-docs#15332

This PR adds Transport attribute to configure Message->Transport mapping. This attribute is read in the SenderLocator class.
Using this attribute it will be possible to configure transport for the message in the message class file instead of messenger.yaml.

carsonbot · 2021-05-12T15:59:42Z

Hey!

I think @tyx has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

src/Symfony/Component/Messenger/Attribute/Senders.php

src/Symfony/Component/Messenger/Transport/Sender/SendersLocator.php

maxim-dovydenok · 2021-05-12T19:20:59Z

Added some changes:

renamed Senders to Transport mapping and made it repeatable
made it overwrite file config
added reading from parent classes and interfaces
and added tests for the cases above

ro0NL · 2021-05-13T07:25:45Z

made it overwrite file config

sorry, i meant file config should overwrite attributes. So we have a way to opt-out from whatever the code is telling us.

without attributes AND file config, we could maybe inherit the parent attributes (as we inherit file config). Meaning we also have a way to opt out from the parent attr.

src/Symfony/Component/Messenger/CHANGELOG.md

maxim-dovydenok · 2021-05-13T10:51:15Z

sorry, i meant file config should overwrite attributes. So we have a way to opt-out from whatever the code is telling us.

So lets say we have:

#[Transport('from_attribute')]
class Message implements MessageInterface
// and
routing:
  App/Message/MessageInterface: 'from_interface_config'
  App/Message/Message: 'from_config'

Do you expect it to be sent to from_interface_config, from_config? - attributes are ignored since config is present.

without attributes AND file config, we could maybe inherit the parent attributes (as we inherit file config). Meaning we also have a way to opt out from the parent attr.

Why would we want to opt-out from parent attributes? I mean we don't have an option to opt-out from MessageInterface routing (see example above) currently, at least that I'm aware of, do we?

ro0NL · 2021-05-13T11:03:43Z

Do you expect it to be sent to from_interface_config, from_config? - attributes are ignored since config is present

Yes.

I see i made a thinking error. Each type either derives from file config, or if unavailable, attributes. Meaning we can opt-out from the parent by also specifying file config for that type.

nicolas-grekas · 2021-05-13T11:07:49Z

What's the way to configure a transport right now using a DI tag?
The implementation should reuse that tag instead of harcoding the attribute deep in SenderLocator.

maxim-dovydenok · 2021-05-13T11:21:44Z

What's the way to configure a transport right now using a DI tag?
The implementation should reuse that tag instead of harcoding the attribute deep in SenderLocator.

At the moment we have this code in FrameworkExtension. It reads routing from the config and passes the message-to-senders map to the SendersLocator (where I did changes).

        $messageToSendersMapping = [];
        foreach ($config['routing'] as $message => $messageConfiguration) {
            if ('*' !== $message && !class_exists($message) && !interface_exists($message, false)) {
                throw new LogicException(sprintf('Invalid Messenger routing configuration: class or interface "%s" not found.', $message));
            }

            foreach ($messageConfiguration['senders'] as $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'];
        }

        $sendersServiceLocator = ServiceLocatorTagPass::register($container, $senderReferences);

        $container->getDefinition('messenger.senders_locator')
            ->replaceArgument(0, $messageToSendersMapping)
            ->replaceArgument(1, $sendersServiceLocator)
        ;

There's also code, that uses $config['transports'] to add transports to the container. Each transport has a messenger.receiver tag, but I'm not sure how it can help us to configure routing using attributes.

ro0NL

@nicolas-grekas this is indeed about routing-messages-to-a-transport

my main motivation here is, to have a single list of things; the messages in code. Or reduce config management if you like :)

src/Symfony/Component/Messenger/Tests/Transport/Sender/SendersLocatorTest.php

alsar · 2021-10-11T12:34:51Z

@shiftby Is this still planed for 5.4?

kbond

I really like this. I think a common pattern is for apps to create Sync/Async marker interfaces to simplify routing rules.

src/Symfony/Component/Messenger/Attribute/Transport.php

src/Symfony/Component/Messenger/Transport/Sender/SendersLocator.php

src/Symfony/Component/Messenger/Tests/Transport/Sender/SendersLocatorTest.php

changes made

kbond · 2022-03-17T18:58:08Z

@shiftby, I took the liberty of rebasing and cleaning this up - hope we can get this in 6.1.

@ro0NL, @nicolas-grekas, could we get a fresh review?

weaverryan

I would totally use this :)

src/Symfony/Component/Messenger/Transport/Sender/SendersLocator.php

weaverryan · 2022-03-18T14:27:11Z

src/Symfony/Component/Messenger/Transport/Sender/SendersLocator.php

+                $typeSenderAliases = $this->getSendersFromAttributes($type);
+            }
+
+            $senderAliases = array_merge($senderAliases, $typeSenderAliases);


If I understand it correctly, IF even one real route exists for the message (something in $this->sendersMap[$type]), then we do NOT also add the #[Transport] senders? So basically any config would override the attribute?

I think that is... probably the correct behavior. But what about the * catch-all route in messenger.yaml (or an interface)? I'm wondering if someone will have the use-case where they generally use the attribute, but then they decide to, in addition to the transport in each class, route all messages (or all messages for an interface) to a 2nd transport via the routing config in messenger.yaml.

So, I'm conflicted on this point.

weaverryan · 2022-03-18T14:27:55Z

src/Symfony/Component/Messenger/Transport/Sender/SendersLocator.php

+            $attributeInstance = $attribute->newInstance();
+
+            return $attributeInstance->name;
+        }, $attributes);


It's unfortunate to need to do this at runtime (and not compile time). But as messages are not services... or tagged in any way, there is no way for us to collect the attribute information at container build time. So 👍 for this approach.

This is indeed unfortunate. Any clever ideas? I understand it depends, but is there a non-negligible performance impact to processing the attributes at runtime?

kbond · 2022-03-18T15:28:12Z

I made a little change to allow sub-classes of the Transport attribute. This allows you to create custom, app-specific attributes to attach to your messages:

#[\Attribute(\Attribute::TARGET_CLASS)]
class HighPriority extends Transport
{
    public function __construct()
    {
        parent::__construct('high_priority');
    }
}

#[HighPriority]
class MyMessage {}

pounard · 2024-06-24T09:01:14Z

I opened #57506 which is kind of a duplicate of this issue (sorry for the noise). It seems that there wasn't any activity here for a year or so, what's the status here?

pounard · 2024-06-24T19:09:30Z

I'm sorry if I hijacked this, but #57507 is ready. This PR is stalled for approximatly a year, and it will cause conflict if merged over the most recent Symfony version, this would mean rewrite pretty much everything.

Should we close this PR?

…sage routing (pounard) This PR was merged into the 7.2 branch. Discussion ---------- [Messenger] Introduce `#[AsMessage]` attribute for message routing | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Fix #57506 | License | MIT Basic implementation of #57506. * Adds the `Symfony\Component\Messenger\Attribute\AsMessage` attribute, with the `$transport` parameter which can be `string` or `array`. * Implement runtime routing in `Symfony\Component\Messenger\Transport\Sender\SendersLocator`. Rationale: * Messages are not services, it cannot be computed during container compilation. * Reflection is very fast, it shouldn't be significant for performances, yet I don't have measured it yet. * YAML configuration and `Symfony\Component\Messenger\Stamp\TransportNamesStamp` will always override the attribute values, allowing users to change hardcoded routing on a per-environment basis. * This is the simplest implementation I could think of for discussion. Links and references: * #33912 where the discussion started, 5 years ago. * #49143 closed PR that was doing the same, but at compile time, rejected because the actual doctrine is that messages should never be services. * #41179 is stilled opened, and awaiting for modifications, but it was written for an earlier version of Symfony and is inactive for a year or so, yet messenger code has evolved since, and this PR will never merge as-is, it requires to be fully rewrote, reason why I opened this new one. Commits ------- d652842 feature #57506 [Messenger] introduce AsMessage attribute for message routing

…sage routing (pounard) This PR was merged into the 7.2 branch. Discussion ---------- [Messenger] Introduce `#[AsMessage]` attribute for message routing | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Fix #57506 | License | MIT Basic implementation of #57506. * Adds the `Symfony\Component\Messenger\Attribute\AsMessage` attribute, with the `$transport` parameter which can be `string` or `array`. * Implement runtime routing in `Symfony\Component\Messenger\Transport\Sender\SendersLocator`. Rationale: * Messages are not services, it cannot be computed during container compilation. * Reflection is very fast, it shouldn't be significant for performances, yet I don't have measured it yet. * YAML configuration and `Symfony\Component\Messenger\Stamp\TransportNamesStamp` will always override the attribute values, allowing users to change hardcoded routing on a per-environment basis. * This is the simplest implementation I could think of for discussion. Links and references: * symfony/symfony#33912 where the discussion started, 5 years ago. * symfony/symfony#49143 closed PR that was doing the same, but at compile time, rejected because the actual doctrine is that messages should never be services. * symfony/symfony#41179 is stilled opened, and awaiting for modifications, but it was written for an earlier version of Symfony and is inactive for a year or so, yet messenger code has evolved since, and this PR will never merge as-is, it requires to be fully rewrote, reason why I opened this new one. Commits ------- d65284239f feature #57506 [Messenger] introduce AsMessage attribute for message routing

nicolas-grekas · 2024-10-10T06:12:29Z

Let us know if #57507 isn't enough. Thanks @maxim-dovydenok for pushing this forward.

maxim-dovydenok requested a review from sroze as a code owner May 11, 2021 19:55

carsonbot added Status: Needs Review Feature Messenger labels May 11, 2021

maxim-dovydenok force-pushed the ticket_41106 branch from 467ab2c to 0c31db6 Compare May 11, 2021 19:57

maxim-dovydenok mentioned this pull request May 11, 2021

[Messenger] [WCM] Add example of how to use attribute Senders symfony/symfony-docs#15332

Closed

nicolas-grekas added this to the 5.x milestone May 11, 2021

maxim-dovydenok force-pushed the ticket_41106 branch from b45b3aa to 7651b34 Compare May 12, 2021 11:25

ro0NL reviewed May 12, 2021

View reviewed changes

src/Symfony/Component/Messenger/Attribute/Senders.php Outdated Show resolved Hide resolved

src/Symfony/Component/Messenger/Transport/Sender/SendersLocator.php Outdated Show resolved Hide resolved

ro0NL reviewed May 12, 2021

View reviewed changes

src/Symfony/Component/Messenger/Transport/Sender/SendersLocator.php Outdated Show resolved Hide resolved

maxim-dovydenok force-pushed the ticket_41106 branch from a1503a0 to 91b3e8f Compare May 12, 2021 19:19

maxim-dovydenok changed the title ~~[Messenger] Add Senders attribute to configure message to sender mapping~~ [Messenger] Add Transport attribute to configure message to transport mapping May 12, 2021

maxim-dovydenok force-pushed the ticket_41106 branch from 77baba0 to 9b09951 Compare May 12, 2021 19:31

OskarStark reviewed May 13, 2021

View reviewed changes

src/Symfony/Component/Messenger/CHANGELOG.md Outdated Show resolved Hide resolved

ro0NL reviewed May 13, 2021

View reviewed changes

src/Symfony/Component/Messenger/Tests/Transport/Sender/SendersLocatorTest.php Outdated Show resolved Hide resolved

maxim-dovydenok force-pushed the ticket_41106 branch from 7f82bba to 3c36432 Compare May 13, 2021 12:41

fabpot modified the milestones: 5.4, 6.1 Nov 16, 2021

kbond previously requested changes Mar 15, 2022

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Mar 15, 2022

[Messenger] Add Senders attribute to configure message to sender mapping

8b86e7d

maxim-dovydenok and others added 2 commits March 17, 2022 14:49

[Messenger] Fix tests on 7.2

af5646c

cleanup

016db38

kbond force-pushed the ticket_41106 branch from 3c36432 to 016db38 Compare March 17, 2022 18:56

weaverryan reviewed Mar 18, 2022

View reviewed changes

kbond added 2 commits March 18, 2022 11:20

fix per review

3638688

allow subclasses of Transport

9068719

fabpot modified the milestones: 6.1, 6.2 May 20, 2022

nicolas-grekas modified the milestones: 6.2, 6.3 Nov 5, 2022

alexandre-daubois mentioned this pull request Jan 28, 2023

[FrameworkBundle][Messenger] Add AsRoutedMessage attribute #49143

Closed

nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023

ro0NL approved these changes Sep 30, 2023

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Sep 30, 2023

nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023

xabbuh modified the milestones: 7.1, 7.2 May 15, 2024

pounard mentioned this pull request Jun 24, 2024

[Messenger] Introduce #[AsMessage] attribute for message routing #57507

Merged

nicolas-grekas closed this Oct 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Messenger] Add Transport attribute to configure message to transport mapping #41179

[Messenger] Add Transport attribute to configure message to transport mapping #41179

maxim-dovydenok commented May 11, 2021 •

edited

Loading

carsonbot commented May 12, 2021

maxim-dovydenok commented May 12, 2021

ro0NL commented May 13, 2021

maxim-dovydenok commented May 13, 2021 •

edited

Loading

ro0NL commented May 13, 2021

nicolas-grekas commented May 13, 2021

maxim-dovydenok commented May 13, 2021

ro0NL left a comment

alsar commented Oct 11, 2021

kbond left a comment

kbond commented Mar 17, 2022 •

edited

Loading

weaverryan left a comment

weaverryan Mar 18, 2022

weaverryan Mar 18, 2022

kbond Mar 18, 2022

kbond commented Mar 18, 2022

pounard commented Jun 24, 2024 •

edited

Loading

pounard commented Jun 24, 2024

nicolas-grekas commented Oct 10, 2024

[Messenger] Add Transport attribute to configure message to transport mapping #41179

[Messenger] Add Transport attribute to configure message to transport mapping #41179

Conversation

maxim-dovydenok commented May 11, 2021 • edited Loading

carsonbot commented May 12, 2021

maxim-dovydenok commented May 12, 2021

ro0NL commented May 13, 2021

maxim-dovydenok commented May 13, 2021 • edited Loading

ro0NL commented May 13, 2021

nicolas-grekas commented May 13, 2021

maxim-dovydenok commented May 13, 2021

ro0NL left a comment

Choose a reason for hiding this comment

alsar commented Oct 11, 2021

kbond left a comment

Choose a reason for hiding this comment

kbond commented Mar 17, 2022 • edited Loading

weaverryan left a comment

Choose a reason for hiding this comment

weaverryan Mar 18, 2022

Choose a reason for hiding this comment

weaverryan Mar 18, 2022

Choose a reason for hiding this comment

kbond Mar 18, 2022

Choose a reason for hiding this comment

kbond commented Mar 18, 2022

pounard commented Jun 24, 2024 • edited Loading

pounard commented Jun 24, 2024

nicolas-grekas commented Oct 10, 2024

maxim-dovydenok commented May 11, 2021 •

edited

Loading

maxim-dovydenok commented May 13, 2021 •

edited

Loading

kbond commented Mar 17, 2022 •

edited

Loading

pounard commented Jun 24, 2024 •

edited

Loading