[Messenger] Define multiple buses from the `framework.messenger.buses` configuration #26864

sroze · 2018-04-08T14:41:49Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#26652
License	MIT
Doc PR	symfony/symfony-docs#9617

Not everybody will benefit from having only one bus, especially with the CQRS-like usages. While keeping the extremely use of use of the single bus, this PR has the following:

Create multiple buses from the YAML configuration
Tag middleware only a specific buses
Register middlewares from the YAML configuration

Even if it's visible in the PR's tests, here's how it will look like, for a completely full-customised version:

framework:
    messenger:
        default_bus: commands
        buses:
            commands: ~
            events:
                middlewares:
                    - validation
                    - route_messages
                    - "Your\\Middleware\\Service"
                    - call_message_handler

A few things to note:

The YAML configuration creates messenger.bus.[name] services for the buses.
The YAML configuration for middleware just adds tags to the corresponding middlewares.
If the middleware definition does not exists, it creates it. (without any magic on the arguments though, if it isn't auto-wirable, well... "your problem")
In the PR, there is this "TolerateNoHandler" middleware that is a great example for event buses

ogizanagi · 2018-04-08T15:01:33Z

IMHO, the framework.messenger.buses.[bus_name].middlewares node should be kept simple, with no priority handling (only the order matters). Otherwise, even if we'll have the debug command to help, it just brings confusion between services registered through the config and the ones through DI tags. It's far easier to read the list of middlewares applied on a bus on a single place.

I wonder if we shouldn't even remove the DI tag or just keep it for the default bus if framework.messenger.buses.[default_bus_name].middlewares is not set.

ro0NL · 2018-04-08T15:23:36Z

I tend to prefer DI config over semantic middleware config, thus keep the tag.

Concerning attributes i'd say bus: string|null which either applies to the given or default bus. No big fan of the current explode trick compared to multiple tags.

Last but not least, i think 1 (default) bus should be configured minimum.

Alternatively it could wire pre-defined middlewares on a per-case basis, e.g.

framework:
    messenger:
        buses:
            events:
                tolerate_no_handler: true

sroze · 2018-04-08T15:30:30Z

IMHO, the framework.messenger.buses.[bus_name].middlewares node should be kept simple, with no priority handling (only the order matters).

That's how I started it. The problem with this approach is how to place a middleware between/before/after the ones that are configured with the tags (and need priorities). I don't see any harm in being able to configure the priority, especially that when you don't, the configuration syntax is super easy. (i.e. by default, it's all middlewares priority 0, in the order in which you've put them in the configuration).

Concerning attributes i'd say bus: string|null which either applies to the given or default bus. No big fan of the current explode trick compared to multiple tags.

It's actually convenient to do so within the code base to simplify the handling of either a middleware is only applied to a given set of buses or all of them, hence me doing this way here. I don't mind changing it but I don't have a strong argument in doing so tbh.

Last but not least, i think st least 1 (default) bus should be configured.

That's the case if you don't configure your owns. If you do, then I'd argue it's now up to you :)

ro0NL · 2018-04-08T15:36:07Z

That's the case if you don't configure your owns.

Right, my bad. I missed the default bus entry in config :)

I don't have a strong argument in doing so tbh

IMHO it's more pure :) but no strong opinion either.

About framework.messenger.buses.[bus_name].middlewares

You have to configure a service anyway, having to specify it here again is double bookkeeping, also this is not the place to infer new service definitions or so. IMHO :)

Having a single way of doing things is nice, and given you need to define the service i'd say tag it as a middleware as well, if needed.

SO we should either favor the tag or [bus_name].middlewares: [list of service ids].

ogizanagi · 2018-04-08T15:42:59Z

The problem with this approach is how to place a middleware between/before/after the ones that are configured with the tags (and need priorities).

That's why I'd suggest removing the tag 😄
The harm is not about having to handle a priority attribute in the config, but about allowing to configure things from 2 different places. I wouldn't allow both DI and semantic config for configuring middlewares.
I don't see much benefits to the DI tag for middlewares, while the semantic config is expressive and already allows to handle the stack order. That's how work the tactician-bundle. Never had to complain about it.

ro0NL · 2018-04-08T15:45:01Z

A tag seems more flexible though :) considering third parties who want to register a middleware. Or put different what about the default middlewares currently?

ogizanagi · 2018-04-08T15:47:40Z

considering third parties who want to register a middleware.

That's the point to me: it's only a userland config. No third party implied. A third party could just help registering a default service to use, or provide a factory.

Or put different what about the default middlewares currently?

They can just be configured as default values for the middlewares node.
If the user want to add a middleware before or after, they'll have to be explicit about it.

ro0NL · 2018-04-08T15:50:29Z

Well.. i was considering exactly that, register/tag a middleware to capture any message dispatched and pass it to a service.

sroze · 2018-04-08T18:49:40Z

I think that your conversation is a good example of why we need to keep both configurations.

@ogizanagi As far as I understand, your concern is that it can introduce some complexity about knowing where/how the middlewares are configuration: I appreciate that but I don't believe this will be the case as it is basically a question of how you, as a developer, use these configuration mechanisms. Though, if we see a large proportion of issues coming that are related to that, we can later decide which one to deprecate I guess.

romaricdrigon · 2018-04-09T07:37:00Z

Hello all,
I would also be in favor of removing the tag. The duplicated config will be hard to trace, and can lead to side effect.
Imagine I have an application 2 buses. Now I install a bundle that provides a middleware, as a tagged service, which I want to add to only one of the bus. How to deal with such situation? How to easily unregister the middleware from one bus?

ro0NL · 2018-04-09T07:47:42Z

At this point im 👍 for keeping both configs (DI+semantic).

Please dont remove tagging middlewares, it makes it real hard to hook in and basically defeats the feature as decorating the bus at this point is probably easier 😂

Now I install a bundle that provides a middleware, as a tagged service, which I want to add to only one of the bus. How to deal with such situation? How to easily unregister the middleware from one bus?

It's probably the bundle's intend to enable the middleware on all buses. Otherwise it should allow you to configure the bus service to use. I.e. How to easily unregister the middleware from one bus? seems highly hypothetical.

Also we never had this debate with bundles tagging e.g. kernel.event_listener etc. You would simply remove the definition if it doesnt fit then too.

sroze · 2018-04-09T08:23:57Z

I've rebased the pull-request so it contains the new middleware tag.

sroze · 2018-04-09T14:46:02Z

@ogizanagi What if you could enable/disable tags from the framework.messenger.tags or something? 🤔

sroze · 2018-04-09T14:54:14Z

If we drop tags, "bundles" could use the prependConfiguration to have some sort of automated configuration.

ro0NL · 2018-04-09T15:32:03Z

Im really curious what others think about @ogizanagi's statement:

it's only a userland config. No third party implied

I tend to disagree with that. Why is a bus middleware "userland-config" and a kernel event listener not?

ogizanagi · 2018-04-09T17:25:13Z

So, we've talked a bit on Slack with @sroze today. Here's mostly what I shared with him about my point of view:

Here is how I see a typical configuration would look like with semantic config only and removing the DI tag:

framework:
  messenger:
    buses:
      commands:
        middlewares:
          - logging # fwb alias
          - validation # fwb alias
          # DoctrineBundle transaction middleware.
          # This demonstrates support of factories we could provide, i.e the doctrine_transaction_middleware
          # is an abstract service the FrameworkBundle would decline with following arguments
          # (here "default" being the manager name to use. Only simple scalar arguments, i.e no services references)
          - doctrine_transaction_middleware: [default]
          # Third-parties would just have to provide a service id (or factories for most custom needs) and document usage.
          # No automatic middleware registering, the user opt-in explicitly for each bus.
          - msg_php.console_message_subscriber_middleware
          # userland middleware service id
          - app.messenger.my_custom_middleware
          - message_handler # alias
      events:
        middlewares:
          - logging
          - tolerate_no_handler # fwb alias
          - message_handler

where logging, validation, tolerate_no_handler & message_handler are aliases handled by the FrameworkBundle for convenience, which also removes by the same occasion the need for the messenger.bus.middlewares.validation key from #26648.

Everything is configured in one place and explicit. Nothing behind the scene. No need for priority. No need for third-parties to register middlewares themselves (just to provide and document middleware services ids/factories).

Supporting both config & DI tag means decentralizing the middleware stack, struggling with priorities (even if the debug command would help) and allowing third-parties to register middleware automagically (which I really don't think is useful).
A parallel (limited though) could be made with Monolog's handlers stack: no DI tag is provided AFAIK to register handlers (And thank goodness, that's fortunate. That'll only bring more confusion 😄).

Even the one legit use-case I see within the framework can be solved differently: the MessengerDataCollector has to be registered automatically on debug mode. The solution without tag is simple: just decorate the HandleMessageMiddleware just like we do with validator & event dispatcher collectors for instance.

Finally, I used Tactician for more than 2 years. This is how the middlewares are registered by the bundle (but simpler, no factory support). Never had to complain and I'm not aware of
any issue asking for a DI tag. So, what about starting with this?

Anyway, if a majority still prefers keeping the DI tag, then I'd be in favor of no semantic config at all. Supporting both just seems confusing and pointless to me. :)

ro0NL · 2018-04-09T18:20:44Z

allowing third-parties to register middleware automagically (which I really don't think is useful).

ok, more thinking about it and decorating instead sounds reasonable, if not better :) in my case it's also about capturing dispatch messages like data collector does.

Config looks good 👍 perhaps denote built-in aliases (non service ids i guess) with :logging, etc. Or put different who wins / what happens if a logging service is defined.

ogizanagi · 2018-04-09T20:17:04Z

To me, fwb known aliases would always win and are unlikely to collide with actual middlewares' ids, but yes, I guess a way to denote built-in aliases would be nice :)

chalasr

+1 for dropping the tag, not worth the confusion it could cause IMHO.

chalasr · 2018-04-10T08:05:06Z

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

+            return;
+        }
+
+        $busesNames = $container->getParameter('messenger.buses_names');


the parameter should be removed once used, no need to expose user config

chalasr · 2018-04-10T08:56:09Z

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

@@ -1448,6 +1449,26 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder

        $loader->load('messenger.xml');

+        $container->setParameter('messenger.buses_names', array_keys($config['buses']));
+        foreach ($config['buses'] as $name => $bus) {
+            $container->setDefinition($busId = 'messenger.bus.'.$name, (new Definition(MessageBus::class, array(array())))->addTag('messenger.bus')->setPublic(true));


what about keeping them private? IMHO there is no reason to not use proper dependency injection for controllers as of 3.4

I'd put an abstract messenger.bus definition in xml to be extended here also, helps for discoverability (e.g. with phpstorm plugin that links to definitions)

chalasr · 2018-04-10T08:57:22Z

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

+                }
+
+                $container->getDefinition($middleware['service'])->addTag('messenger.bus_middleware', array(
+                    'bus' => $name,


should be on one line

chalasr · 2018-04-10T09:16:12Z

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

+        foreach ($config['buses'] as $name => $bus) {
+            $container->setDefinition($busId = 'messenger.bus.'.$name, (new Definition(MessageBus::class, array(array())))->addTag('messenger.bus')->setPublic(true));
+
+            if ($name == $config['default_bus']) {


better use strict comparison

chalasr · 2018-04-10T09:16:25Z

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

+        foreach ($container->findTaggedServiceIds('messenger.bus_middleware') as $serviceId => $tags) {
+            foreach ($tags as $tag) {
+                $priority = $tag['priority'] ?? 0;
+                $buses = isset($tag['bus']) ? explode(',', $tag['bus']) : $busesNames;


Not fond of the special syntax, AFAIK we are used to require adding as much tags as needed e.g.

App\SomeMiddleware: tags: - { name: "messenger.bus_middleware", bus: "query" } - { name: "messenger.bus_middleware", bus: "command" }

ro0NL · 2018-04-12T15:25:29Z

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

-                if (!$container->has($middleware['service'])) {
-                    $container->setDefinition($middleware['service'], new Definition($middleware['service']));
-                }
+            $container->setDefinition(


so in user-land you need a compiler pass, find all buses by tag (messenger.bus), and decorate each one.

or is it better to decorate call_message_handler once?

I'd say it depends on your use-case. Both approaches would work.

Nyholm · 2018-04-20T07:24:30Z

I've just started to test this PR on a project. It works well. It is not easy to configure but it is flexible. With "easy" I mean: It would be nice to have:

  busses: 
      acme:
          type: command_bus

Im not sure that is a realistic because whatever a "command_bus" is for me it might not be the same for you. So Im +1 with the config.

I get an error though:

    messenger:
        default_bus: events
        buses:
            events:
                middlewares:
                    - route_messages
                    - 'Symfony\Component\Messenger\Middleware\TolerateNoHandler'
                    - call_message_handler

In ResolveChildDefinitionsPass.php line 71:
                                                                                                                          
  Service "messenger.bus.events.middleware.Symfony\Component\Messenger\Middleware\TolerateNoHandler": Parent definition   
  "Symfony\Component\Messenger\Middleware\TolerateNoHandler" does not exist.

If I change my config to the following, it will work:

    messenger:
        default_bus: events
        buses:
            events:
                middlewares:
                    - route_messages
                    - 'messenger.middleware.tolerate_no_handler'
                    - call_message_handler

sroze · 2018-04-20T07:48:30Z

You are right, my example in the original pull-request description wasn't accurate anymore (updated now). The reason you had this exception is because it tried to create an instance of the middleware for this bus based on the parent definition that you define.

In the example you mentioned, you could even:

- 'messenger.middleware.tolerate_no_handler'
+ tolerate_no_handler

Regarding the buses.[name].type configuration, I like the idea but is obviously trickier to settle on what should be in which. Therefore I'd argue this shouldn't be in this PR at least 🙃 (could be a good 3rd party bundle as well).

weaverryan · 2018-04-21T01:02:41Z

Hey guys!

Sorry to come late after so much work.

With the way the PR is right now, if you define you own middlewares, you override the default "logging", "call_message_handler" and "route_messages" handlers, so you need to define them. It feels like it could be a source of troubles for the users... What's your point of view on what we could/should do around that?

I actually think this is a huge problem. Imagine a user wants to add a new middleware: they setup their config, set middleware to [my_new_middleware] and suddenly things that worked before don't - my message isn't being handled or routed.

I'm not sure of the best solution - I think the argument around tags is valid, and the fact that experience with TacticianBundle has validated their approach a bit. Actually, one solution would be to be more like TacticianBundle: have zero default middleware out-of-the-box. Then, in the recipe, include the 3 middleware in messenger.yaml with comments. When a user creates a second bus, they'll copy and paste.

weaverryan · 2018-04-21T01:09:05Z

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

@@ -1052,6 +1053,21 @@ function ($a) {
                                ->end()
                            ->end()
                        ->end()
+                        ->scalarNode('default_bus')->defaultValue('default')->end()


Could we default this to null, then in the extension:

If there is 1 extension, just use it

If there are multiple, throw a clear exception that the default_bus key needs to be defined?

I could see a situation where someone sees:

The default bus named "default" is not defined. Define it or change the default bus name.

And thinking, where did default come from?

Yep, I like that idea 👍

Changed it this way.

sroze · 2018-04-21T14:39:08Z

Actually, one solution would be to be more like TacticianBundle: have zero default middleware out-of-the-box.

This means we can't auto-activate the messenger when we know it's installed (i.e. the MessageBus class exists) and we would only rely on the YAML configuration. It's pretty good to be able to do this (i.e. doing some configuration via the recipe) but it's more boilerplate (and much harder to maintain, once the recipe is installed, you're done...)

What about having the followingmiddlewares per bus:

logging
validation
all the middlewares in the buses.[name].middlewares configuration
route_messages
call_message_handler

As a developer, you can disable the "default middlewares" with:

buses:
    [name]:
        default_middlewares: false

sroze · 2018-04-21T15:53:51Z

I've pushed 3 commits:

0d78a0e that allows middlewares to be services (no requirement for them to be abstract)
d39604e forces to have a matching default_bus only when using more than one bus. Otherwise, well, it take the name of that only bus.
8d16cda my favourite alternative to replace all the configured middlewares: we have a set of hard-coded default middlewares and you can squeeze yours in between. If you really need to (I believe this will be rare) you can disable the default middlewares with default_middlewares: false.

ro0NL · 2018-04-21T18:01:29Z

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


+        $defaultMiddlewares = array('before' => array('logging'), 'after' => array('route_messages', 'call_message_handler'));
        if ($config['middlewares']['validation']['enabled']) {


at this point..shouldnt we let users this configure as a regular middleware, per bus, if needed? Extra configs feels a bit too much config IMHO. Perhaps try to prepend either way like 'logging'.

Or put different, it's a confusing config as it relies on default_middlewares also.

This one special middleware handling is indeed a little bit weird. Is there any special reason when you would not want this middleware? Could it ever cause issues? My instinct is to always include it when default_middlewares is enabled. If you really don't want it for some reason, you can set that config to false (unless there's a somewhat common use-case for not wanting this middleware).

I tend to opt-in to a validation middleware.. im not sure validating messages by default makes sense.

Also it's not like adding - validation to your middlewares (per bus) is "bad config" or so IMHO :)

Is there any special reason when you would not want this middleware?

Yes, when the component is not included.

I tend to opt-in to a validation middleware.. im not sure validating messages by default makes sense.

Let's do this. So it is explicit and removes some extra configuration in there.

weaverryan

Other than one small comment, yea, I like this approach. I can't think of any other part of the framework that has this exact problem, and so, the solution is also a bit unique. But, it's both practical (works immediately for the 90%) and extensible.

weaverryan · 2018-04-21T18:23:42Z

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


+        $defaultMiddlewares = array('before' => array('logging'), 'after' => array('route_messages', 'call_message_handler'));
        if ($config['middlewares']['validation']['enabled']) {


This one special middleware handling is indeed a little bit weird. Is there any special reason when you would not want this middleware? Could it ever cause issues? My instinct is to always include it when default_middlewares is enabled. If you really don't want it for some reason, you can set that config to false (unless there's a somewhat common use-case for not wanting this middleware).

weaverryan · 2018-04-22T00:15:38Z

Ping @symfony/deciders!

I'm 👍 for this approach, and it's a big PR, so we need to get it merged in soon. The most important aspect is how middleware are added. The final decision about that is explained very well here: #26864 (comment)

ro0NL

now we have the messenger.bus tag instead of tagging middlewares.. from a vendor perspective im leaning to wire my own bus, thus tag it. Given that i think it should work like any other bus that comes from config.

There's no framework-way of asking for an event or command bus, and i want to avoid configuration for that on my side. So i'll autowire the default bus to be used for commands, but then i still need one for events. Alternatively i could wire a one-size-fits-all bus by injecting all tagged buses, and break on first successful bus handling a message.

ro0NL · 2018-04-22T12:30:01Z

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

+            }
+
+            if ('messenger.middleware.validation' === $messengerMiddlewareId && !$container->has('validator')) {
+                throw new RuntimeException('The Validation middleware is only available when the Validator component is installed and enabled. Try running "composer require symfony/validator".');


would be nice if we can improve such error messages at the DI service level

I think it's a wider question within core. That's the best way I found to keep the great DX while using this middleware without the Validator component. I would keep it here.

IMHO, this still belongs to the Fwb extension where we have all the keys to detect if validation is installed and enabled. The messenger.middleware.validation is really specific to the Fwb wiring, so the pass alone doesn't have to deal with this unless strictly necessary.

I see the point that this has little to do in here. Moved to the FrameworkBundle 👍

ro0NL · 2018-04-22T12:33:54Z

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

+            }
+
+            if ($container->hasDefinition('messenger.data_collector')) {
+                $this->registerBusToCollector($container, $busId, $tags[0]);


should we iterate $tags as well or throw in case of >1?

I don't believe there is a use-case for a bus to be tagged multiple times 🤔

ro0NL · 2018-04-22T12:44:14Z

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

+                $container->setAlias(MessageBusInterface::class, $busId);
+            }
+
+            $container->setDefinition(


should/can we process this at the compile pass level?

Yes, very good idea! So we add it only when we need it 💃

ogizanagi

👍 With only minor comments. Thanks for considering what we discussed on Slack.
I'm not found of the default middlewares vs always using explicit configuration but I guess that's a good tradeoff.

ogizanagi · 2018-04-23T11:24:02Z

...y/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_multiple_buses.php

+                'default_middlewares' => false,
+                'middlewares' => array(
+                    'route_messages',
+                    'Symfony\\Component\\Messenger\\Middleware\\TolerateNoHandler',


It's a bit misleading reading the tests and seing this FQCN while the actual middleware exists and has a service id that should be used (or the shortcut), not the FQCN. Is it really relevant to test a FQCN here?

Good point; changed.

ogizanagi · 2018-04-23T11:26:19Z

src/Symfony/Component/Messenger/DataCollector/MessengerDataCollector.php

 {
+    private $traceableBuses = array();
+
+    public function registerBus(string $name, TraceableMessageBus $bus)


Why not injected in the constructor?

This is to match with what already exists like the CacheDataCollector for example.

ogizanagi · 2018-04-23T11:29:02Z

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

+                $container->getParameterBag()->remove($busMiddlewaresParameter);
+            }
+
+            if ($container->getParameter('kernel.debug') && $container->hasDefinition('messenger.data_collector')) {


Is the $container->getParameter('kernel.debug') check useful, or $container->hasDefinition('messenger.data_collector') would be enough?

True, we can remove the $container->getParameter('kernel.debug') now.

ogizanagi · 2018-04-23T11:34:15Z

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

+            }
+
+            if ('messenger.middleware.validation' === $messengerMiddlewareId && !$container->has('validator')) {
+                throw new RuntimeException('The Validation middleware is only available when the Validator component is installed and enabled. Try running "composer require symfony/validator".');


IMHO, this still belongs to the Fwb extension where we have all the keys to detect if validation is installed and enabled. The messenger.middleware.validation is really specific to the Fwb wiring, so the pass alone doesn't have to deal with this unless strictly necessary.

ogizanagi · 2018-04-23T11:36:13Z

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

+            $config['default_bus'] = array_keys($config['buses'])[0];
+        }
+
+        $defaultMiddlewares = array('before' => array('logging'), 'after' => array('route_messages', 'call_message_handler'));


Isn't validation supposed to be in defaultMiddlewares['before'] if the component is enabled, as described in #26864 (comment) ?

See #26864 (comment)

Alright, let's stick with it :)

sroze · 2018-04-25T12:53:00Z

Repushed the last commit to make AppVeyor green again 💚

sroze · 2018-04-25T16:25:07Z

Let's merge! Thank you all for your feedback 🎉

…` configuration

…messenger.buses` configuration (sroze) This PR was squashed before being merged into the 4.1-dev branch (closes #26864). Discussion ---------- [Messenger] Define multiple buses from the `framework.messenger.buses` configuration | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #26652 | License | MIT | Doc PR | symfony/symfony-docs#9617 Not everybody will benefit from having only one bus, especially with the CQRS-like usages. While keeping the extremely use of use of the single bus, this PR has the following: - Create multiple buses from the YAML configuration - Tag middleware only a specific buses - Register middlewares from the YAML configuration Even if it's visible in the PR's tests, here's how it will look like, for a completely full-customised version: ```yaml framework: messenger: default_bus: commands buses: commands: ~ events: middlewares: - validation - route_messages - "Your\\Middleware\\Service" - call_message_handler ``` A few things to note: 1. The YAML configuration creates `messenger.bus.[name]` services for the buses. 2. The YAML configuration for middleware just adds tags to the corresponding middlewares. 3. If the middleware definition does not exists, it creates it. (without any magic on the arguments though, if it isn't auto-wirable, well... "your problem") 4. In the PR, there is this "TolerateNoHandler" middleware that is a great example for event buses Commits ------- e5deb84 [Messenger] Define multiple buses from the `framework.messenger.buses` configuration

…izanagi) This PR was squashed before being merged into the 4.1 branch (closes #27128). Discussion ---------- [Messenger] Middleware factories support in config | Q | A | ------------- | --- | Branch? | master  | Bug fix? | no | New feature? | yes  | BC breaks? | no  | Deprecations? | no  | Tests pass? | yes  | Fixed tickets | N/A  | License | MIT | Doc PR | todo Following #26864, this would allow to configure easily the middlewares by using an abstract factory definition to which are provided simple arguments (just scalars, no services references). For instance, here is how the DoctrineBundle would benefit from such a feature (also solving the wiring of the `DoctrineTransactionMiddleware` reverted in #26684): ```yaml framework: messenger: buses: default: middleware: - logger - doctrine_transaction_middleware: ['entity_manager_name'] ``` where `doctrine_transaction_middleware` would be an abstract factory definition provided by the doctrine bundle: ```yml services: doctrine.orm.messenger.middleware_factory.transaction: class: Symfony\Bridge\Doctrine\Messenger\DoctrineTransactionMiddlewareFactory arguments: ['@doctrine'] doctrine_transaction_middleware: class: Symfony\Bridge\Doctrine\Messenger\DoctrineTransactionMiddleware factory: ['@doctrine.orm.messenger.middleware_factory.transaction', 'createMiddleware'] abstract: true # the default arguments to use when none provided from config. # i.e: # middlewares: # - doctrine_transaction_middleware: ~ arguments: ['default'] ``` and is interpreted as: ```yml buses: default: middleware: - id: logger arguments: { } - id: doctrine_transaction_middleware arguments: - entity_manager_name default_middleware: true ``` --- <details> <summary>Here is the whole config reference with these changes: </summary> ```yaml # Messenger configuration messenger: enabled: true routing: # Prototype message_class: senders: [] serializer: enabled: true format: json context: # Prototype name: ~ encoder: messenger.transport.serializer decoder: messenger.transport.serializer adapters: # Prototype name: dsn: ~ options: [] default_bus: null buses: # Prototype name: default_middleware: true middleware: # Prototype - id: ~ # Required arguments: [] ``` </details> Commits ------- f5ef421 [Messenger] Middleware factories support in config

…izanagi) This PR was squashed before being merged into the 4.1 branch (closes #27128). Discussion ---------- [Messenger] Middleware factories support in config | Q | A | ------------- | --- | Branch? | master  | Bug fix? | no | New feature? | yes  | BC breaks? | no  | Deprecations? | no  | Tests pass? | yes  | Fixed tickets | N/A  | License | MIT | Doc PR | todo Following symfony/symfony#26864, this would allow to configure easily the middlewares by using an abstract factory definition to which are provided simple arguments (just scalars, no services references). For instance, here is how the DoctrineBundle would benefit from such a feature (also solving the wiring of the `DoctrineTransactionMiddleware` reverted in symfony/symfony#26684): ```yaml framework: messenger: buses: default: middleware: - logger - doctrine_transaction_middleware: ['entity_manager_name'] ``` where `doctrine_transaction_middleware` would be an abstract factory definition provided by the doctrine bundle: ```yml services: doctrine.orm.messenger.middleware_factory.transaction: class: Symfony\Bridge\Doctrine\Messenger\DoctrineTransactionMiddlewareFactory arguments: ['@doctrine'] doctrine_transaction_middleware: class: Symfony\Bridge\Doctrine\Messenger\DoctrineTransactionMiddleware factory: ['@doctrine.orm.messenger.middleware_factory.transaction', 'createMiddleware'] abstract: true # the default arguments to use when none provided from config. # i.e: # middlewares: # - doctrine_transaction_middleware: ~ arguments: ['default'] ``` and is interpreted as: ```yml buses: default: middleware: - id: logger arguments: { } - id: doctrine_transaction_middleware arguments: - entity_manager_name default_middleware: true ``` --- <details> <summary>Here is the whole config reference with these changes: </summary> ```yaml # Messenger configuration messenger: enabled: true routing: # Prototype message_class: senders: [] serializer: enabled: true format: json context: # Prototype name: ~ encoder: messenger.transport.serializer decoder: messenger.transport.serializer adapters: # Prototype name: dsn: ~ options: [] default_bus: null buses: # Prototype name: default_middleware: true middleware: # Prototype - id: ~ # Required arguments: [] ``` </details> Commits ------- f5ef421474 [Messenger] Middleware factories support in config

carsonbot added Status: Needs Review Feature labels Apr 8, 2018

sroze force-pushed the multiple-buses branch from aa48465 to 1517d1b Compare April 8, 2018 14:42

sroze added the Messenger label Apr 8, 2018

sroze added this to the 4.1 milestone Apr 8, 2018

sroze mentioned this pull request Apr 8, 2018

[Messenger] Allow different types of busses #26652

Closed

ro0NL mentioned this pull request Apr 9, 2018

Support Symfony Messenger msgphp/msgphp#134

Merged

sroze force-pushed the multiple-buses branch from 1517d1b to 5c1a762 Compare April 9, 2018 08:23

chalasr reviewed Apr 10, 2018

View reviewed changes

sroze force-pushed the multiple-buses branch 2 times, most recently from 62b4115 to 35e4323 Compare April 12, 2018 15:06

ro0NL reviewed Apr 12, 2018

View reviewed changes

sroze mentioned this pull request Apr 20, 2018

[Messenger] Remove the Doctrine middleware configuration from the FrameworkBundle symfony/symfony-docs#9638

Closed

weaverryan reviewed Apr 21, 2018

View reviewed changes

sroze force-pushed the multiple-buses branch from 4ee0060 to 39389f0 Compare April 21, 2018 16:25

ro0NL reviewed Apr 21, 2018

View reviewed changes

weaverryan reviewed Apr 21, 2018

View reviewed changes

sroze force-pushed the multiple-buses branch from c6e3fbf to e782f39 Compare April 22, 2018 10:04

ro0NL reviewed Apr 22, 2018

View reviewed changes

sroze force-pushed the multiple-buses branch from b1123bb to 955a2af Compare April 22, 2018 13:45

ogizanagi approved these changes Apr 23, 2018

View reviewed changes

sroze force-pushed the multiple-buses branch from 5e858fd to a01a75b Compare April 25, 2018 12:52

[Messenger] Define multiple buses from the `framework.messenger.buses…

e5deb84

…` configuration

sroze force-pushed the multiple-buses branch from a01a75b to e5deb84 Compare April 25, 2018 16:40

sroze merged commit e5deb84 into symfony:master Apr 25, 2018

sroze mentioned this pull request Apr 25, 2018

[Messenger] Add debug:messenger CLI command #26803

Merged

ogizanagi mentioned this pull request May 2, 2018

[Messenger] Middleware factories support in config #27128

Merged

sroze deleted the multiple-buses branch May 3, 2018 13:43

fabpot mentioned this pull request May 7, 2018

Release v4.1.0-BETA1 #27181

Merged


		$defaultMiddlewares = array('before' => array('logging'), 'after' => array('route_messages', 'call_message_handler'));
		if ($config['middlewares']['validation']['enabled']) {

[Messenger] Define multiple buses from the framework.messenger.buses configuration #26864

[Messenger] Define multiple buses from the framework.messenger.buses configuration #26864

Conversation

sroze commented Apr 8, 2018 • edited Loading

ogizanagi commented Apr 8, 2018

ro0NL commented Apr 8, 2018 • edited Loading

sroze commented Apr 8, 2018

ro0NL commented Apr 8, 2018 • edited Loading

ogizanagi commented Apr 8, 2018

ro0NL commented Apr 8, 2018

ogizanagi commented Apr 8, 2018

ro0NL commented Apr 8, 2018 • edited Loading

sroze commented Apr 8, 2018

romaricdrigon commented Apr 9, 2018

ro0NL commented Apr 9, 2018 • edited Loading

sroze commented Apr 9, 2018

sroze commented Apr 9, 2018

sroze commented Apr 9, 2018

ro0NL commented Apr 9, 2018

ogizanagi commented Apr 9, 2018 • edited Loading

ro0NL commented Apr 9, 2018 • edited Loading

ogizanagi commented Apr 9, 2018

chalasr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr Apr 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nyholm commented Apr 20, 2018

sroze commented Apr 20, 2018

weaverryan commented Apr 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sroze commented Apr 21, 2018 • edited Loading

sroze commented Apr 21, 2018

ro0NL Apr 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL Apr 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weaverryan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weaverryan commented Apr 22, 2018

ro0NL left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogizanagi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sroze commented Apr 25, 2018

sroze commented Apr 25, 2018

[Messenger] Define multiple buses from the `framework.messenger.buses` configuration #26864

[Messenger] Define multiple buses from the `framework.messenger.buses` configuration #26864

sroze commented Apr 8, 2018 •

edited

Loading

ro0NL commented Apr 8, 2018 •

edited

Loading

ro0NL commented Apr 8, 2018 •

edited

Loading

ro0NL commented Apr 8, 2018 •

edited

Loading

ro0NL commented Apr 9, 2018 •

edited

Loading

ogizanagi commented Apr 9, 2018 •

edited

Loading

ro0NL commented Apr 9, 2018 •

edited

Loading

chalasr Apr 10, 2018 •

edited

Loading

sroze commented Apr 21, 2018 •

edited

Loading

ro0NL Apr 21, 2018 •

edited

Loading

ro0NL Apr 21, 2018 •

edited

Loading

ro0NL left a comment •

edited

Loading