[Messenger] Allow to typehint multiple buses #27054

sroze · 2018-04-25T20:14:29Z

Q	A
Branch?	master
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	ø
License	MIT
Doc PR	ø

When defining multiple buses, it becomes impossible to type-hint the MessageBusInterface. This PR adds the ability to wrap the bus within a decorator that you will be able to typehint.

As a developer, you will create your classes:

namespace App\Bus;

use Symfony\Component\Messenger\DecoratedMessageBus;

final class CommandBus extends DecoratedMessageBus
{
}

Configure your bus to use it:

framework:
    messenger:
        buses:
            commands:
                decorator_class: 'App\Bus\CommandBus'

You can now typehint your CommandBus 🥁 💃 ⭐️

chalasr

mainly wording suggestions

chalasr · 2018-04-25T21:08:36Z

src/Symfony/Component/Messenger/DecoratedMessageBus.php

+ *
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+abstract class DecoratedMessageBus implements MessageBusInterface


If concrete children are decorators then this is a base decorator, isn't it? Also core abstract classes are prefixed by Abstract so, AbstractMessageBusDecorator?

chalasr · 2018-04-26T10:45:07Z

src/Symfony/Component/Messenger/DecoratedMessageBus.php

+/**
+ * A decorated message bus.
+ *
+ * Use this abstract class to created your message bus decorator to specialise your


typo to create

chalasr · 2018-04-26T10:51:49Z

src/Symfony/Component/Messenger/DecoratedMessageBus.php

+ * A decorated message bus.
+ *
+ * Use this abstract class to created your message bus decorator to specialise your
+ * bus instances and type-hint them.


I suggest type-hint against them, dependents type-hint one of their dependencies against the type they need

chalasr · 2018-04-26T10:52:44Z

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

@@ -1481,8 +1481,13 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
                throw new LogicException('The Validation middleware is only available when the Validator component is installed and enabled. Try running "composer require symfony/validator".');
            }

+            $tagAttributes = array('name' => $name);
+            if (isset($bus['decorator_class'])) {
+                $tagAttributes['decorator_class'] = $bus['decorator_class'];


should it fail if the class does not implement MessageBusInterface at least? Perhaps give a complete hint about what is needed for using this option

chalasr · 2018-04-26T10:57:59Z

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

@@ -413,3 +425,7 @@ public function handle($message, callable $next)
        return $next($message);
    }
 }
+
+final class MyTypeHintedBus extends DecoratedMessageBus


TypeHinted is misleading here :) a class is a type, method arguments are type-hinted

ro0NL · 2018-04-26T16:01:36Z

it becomes impossible to type-hint the MessageBusInterface

you'll get the default bus AFAIK...

You can now typehint your CommandBus

What if the same decorated class is used multiple times across buses?

isn't this just like saying My\CommandBus: '@messenger.bus.command', using regular DI wiring. Put different; do we really want/need this feature?

sroze · 2018-04-27T07:00:24Z

What if the same decorated class is used multiple times across buses?

Well, you know what is going to happen: the auto-wiring won't be happy :)

isn't this just like saying My\CommandBus: '@messenger.bus.command', using regular DI wiring. Put different; do we really want/need this feature?

Ish. It still means you'd have to create the My\CommandBus class, but it's a good point. Should we go with just the class and no FWB configuration?

ro0NL · 2018-04-27T08:17:28Z

Or just a trait maybe =/ i would indeed avoid the FWB config.

sroze · 2018-04-27T08:59:25Z

It works well with the following configuration 👍

services:
    'App\Bus\CommandBus':
        decorates: "messenger.bus.commands"

sroze · 2018-04-27T09:26:22Z

To be completed by a lovely documentation :)

kbond · 2018-04-27T12:40:18Z

In development, with the following configuration:

services:
    'App\Bus\CommandBus':
        decorates: "messenger.bus.commands"

The decorated bus is an instance of Symfony\Component\Messenger\MessageBus not Symfony\Component\Messenger\TraceableMessageBus

sroze · 2018-04-27T12:52:20Z

@kbond you need to typehint ither MessageBusInterface or your own App\Bus\CommandBus not Symfony\Component\Messenger\MessageBus 😉

kbond · 2018-04-27T13:03:03Z

@sroze I type-hinted my own bus which extends from AbstractMessageBusDecorator. The $decoratedBus property is not and instance of Symfony\Component\Messenger\TraceableMessageBus

It is the correct bus it just doesn't show any activity in the profiler.

sroze · 2018-04-27T13:08:18Z

I'm sorry but I don't understand the problem... "it works well on my machine" 😅 Can you give me the exact error you face and the classes & configuration you have? :)

kbond · 2018-04-27T13:15:31Z

There is no error.

messenger config:

framework:
    messenger:
        default_bus: event
        buses:
            command:
                default_middlewares: false
                middlewares:
                    - logging
                    - Symfony\Bridge\Doctrine\Messenger\DoctrineTransactionMiddleware
                    - call_message_handler
            query:
                default_middlewares: false
                middlewares:
                    - logging
                    - call_message_handler
            event:
                middleware:
                    - tolerate_no_handler

service config:

services:
    App\Messenger\CommandBus:
        decorates: messenger.bus.command

my command bus class:

namespace App\Messenger;

use Symfony\Component\Messenger\AbstractMessageBusDecorator;

final class CommandBus extends AbstractMessageBusDecorator
{
}

controller injecting command bus:

public function myAction(CommandBus $commandBus)
{
    dump($commandBus);

    // ...
}

Screenshot of dumped $commandBus:

Notice the decorated bus is just MessageBus not TraceableMessageBus.

kbond · 2018-04-27T13:46:24Z

Note, if I change my service config to:

services:
    App\Messenger\CommandBus: ['@messenger.bus.command']

The TraceableMessageBus is the decoratedBus.

sroze · 2018-04-27T16:10:31Z

Oh, excellent. Thank you for checking that. That's what will be in the documentation then :)

sroze · 2018-04-28T09:10:23Z

Status: Ready

ro0NL

not sure core needs it.. (i'd write this out anyway :)) but 👍 for the intend allthough i'd use a trait to avoid inheritance and own the private decorated bus in my own class.

weaverryan

Hmm, it seems like a pretty small feature in its final form. So, I think it's ok. @sroze can you open a Docs PR before this gets merged with a quick dump of the final "docs" needed to make this work? I'm not 100% clear which service configuration was finally the correct one from the comments.

sroze · 2018-05-03T14:07:06Z

@weaverryan symfony/symfony-docs#9727

nicolas-grekas · 2018-05-04T03:02:27Z

I'm very skeptical about this PR and the related § in symfony/symfony-docs#9727.
This encourages type-hinting against a concrete implementation and totally breaks SOLID, isn't it?
I acknowledge there is a DX issue and that this proposal tries to solve it. Coincidentally, I was talking about the exact same issue with @weaverryan today, telling him about a solution we might want to push forward in 4.2 (it's too early to elaborate here for now).

sroze · 2018-05-04T07:07:01Z

As you know, I agree with you... What if we change the documentation to show an example with creating the interface? For 4.1, this is very likely to be the only option we can have in IMHO :)

…

On Fri, 4 May 2018 at 04:02, Nicolas Grekas ***@***.***> wrote: I'm very skeptical about this PR and the related § in symfony/symfony-docs#9727 <symfony/symfony-docs#9727>. This encourages type-hinting against a concrete implementation and totally breaks SOLID, isn't it? I acknowledge there is a DX issue and that this proposal tries to solve it. Coincidentally, I was talking about the exact same issue with @weaverryan <https://github.com/weaverryan> today, tell him about a solution we might want to push forward in 4.2. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27054 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAxHEXK2IaTUiHVjw76nM1eR29U5okjfks5tu8TJgaJpZM4TkFYV> .

nicolas-grekas · 2018-05-04T12:07:48Z

No need for any dedicated interfaces, using bindings solves the issue:
$commandBus: @messenger.bus.command

sroze · 2018-05-04T12:10:38Z

Outch. Not sure how I feel about that one. I'll give a try.

nicolas-grekas · 2018-05-05T16:58:43Z

As explained, I'm:-1: as this would put users in a dead end. We should tell about using bindings instead, and seek for a better experience in 4.2 that doesn't break SOLID.

sroze · 2018-05-06T13:56:38Z

Closing, as I'll update the documentation to recommend the binding, that makes sense actually.

sroze added 2 commits April 25, 2018 20:07

WIP

e4a6a06

Configure the class used to decorate your bus

78c15c1

carsonbot added the Status: Needs Review label Apr 25, 2018

sroze added the Messenger label Apr 25, 2018

sroze added this to the 4.1 milestone Apr 25, 2018

chalasr reviewed Apr 26, 2018

View reviewed changes

Remove the FWB configuration and rename the abstract decorator

2b601ad

sroze force-pushed the uses-custom-class branch 2 times, most recently from 99669ba to 2b601ad Compare April 27, 2018 09:10

carsonbot removed the Status: Needs Review label Apr 28, 2018

ro0NL approved these changes Apr 28, 2018

View reviewed changes

carsonbot added the Status: Reviewed label Apr 28, 2018

weaverryan approved these changes May 3, 2018

View reviewed changes

sroze mentioned this pull request May 3, 2018

[Messenger] Update the messenger documentation symfony/symfony-docs#9727

Merged

6 tasks

ogizanagi mentioned this pull request May 5, 2018

[DI] Allow binding by type+name #27165

Merged

sroze closed this May 6, 2018

sroze deleted the uses-custom-class branch May 6, 2018 13:56

nicolas-grekas mentioned this pull request May 7, 2018

[DI] Extend semantic of @param to describe intent and use it when autowiring services and parameters #27172

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Messenger] Allow to typehint multiple buses #27054

[Messenger] Allow to typehint multiple buses #27054

sroze commented Apr 25, 2018

chalasr left a comment

chalasr Apr 25, 2018

chalasr Apr 26, 2018

chalasr Apr 26, 2018 •

edited

Loading

chalasr Apr 26, 2018

chalasr Apr 26, 2018

ro0NL commented Apr 26, 2018 •

edited

Loading

sroze commented Apr 27, 2018

ro0NL commented Apr 27, 2018

sroze commented Apr 27, 2018 •

edited

Loading

sroze commented Apr 27, 2018

kbond commented Apr 27, 2018

sroze commented Apr 27, 2018

kbond commented Apr 27, 2018 •

edited

Loading

sroze commented Apr 27, 2018

kbond commented Apr 27, 2018

kbond commented Apr 27, 2018

sroze commented Apr 27, 2018

sroze commented Apr 28, 2018

ro0NL left a comment

weaverryan left a comment

sroze commented May 3, 2018

nicolas-grekas commented May 4, 2018 •

edited

Loading

sroze commented May 4, 2018 via email

nicolas-grekas commented May 4, 2018 •

edited

Loading

sroze commented May 4, 2018 •

edited

Loading

nicolas-grekas commented May 5, 2018 •

edited

Loading

sroze commented May 6, 2018

[Messenger] Allow to typehint multiple buses #27054

[Messenger] Allow to typehint multiple buses #27054

Conversation

sroze commented Apr 25, 2018

chalasr left a comment

Choose a reason for hiding this comment

chalasr Apr 25, 2018

Choose a reason for hiding this comment

chalasr Apr 26, 2018

Choose a reason for hiding this comment

chalasr Apr 26, 2018 • edited Loading

Choose a reason for hiding this comment

chalasr Apr 26, 2018

Choose a reason for hiding this comment

chalasr Apr 26, 2018

Choose a reason for hiding this comment

ro0NL commented Apr 26, 2018 • edited Loading

sroze commented Apr 27, 2018

ro0NL commented Apr 27, 2018

sroze commented Apr 27, 2018 • edited Loading

sroze commented Apr 27, 2018

kbond commented Apr 27, 2018

sroze commented Apr 27, 2018

kbond commented Apr 27, 2018 • edited Loading

sroze commented Apr 27, 2018

kbond commented Apr 27, 2018

kbond commented Apr 27, 2018

sroze commented Apr 27, 2018

sroze commented Apr 28, 2018

ro0NL left a comment

Choose a reason for hiding this comment

weaverryan left a comment

Choose a reason for hiding this comment

sroze commented May 3, 2018

nicolas-grekas commented May 4, 2018 • edited Loading

sroze commented May 4, 2018 via email

nicolas-grekas commented May 4, 2018 • edited Loading

sroze commented May 4, 2018 • edited Loading

nicolas-grekas commented May 5, 2018 • edited Loading

sroze commented May 6, 2018

chalasr Apr 26, 2018 •

edited

Loading

ro0NL commented Apr 26, 2018 •

edited

Loading

sroze commented Apr 27, 2018 •

edited

Loading

kbond commented Apr 27, 2018 •

edited

Loading

nicolas-grekas commented May 4, 2018 •

edited

Loading

nicolas-grekas commented May 4, 2018 •

edited

Loading

sroze commented May 4, 2018 •

edited

Loading

nicolas-grekas commented May 5, 2018 •

edited

Loading