[Messenger] Introduce handle method on MessageBusInterface instead of Trait #29428

ghost · 2018-12-02T14:37:25Z

Description
This issue is related to the PR #29167 and #28983, which I believe does not solve the recent changes in the Messenger component.

There should be a handle method on the MessageBusInterface itself, which I believe is a better solution than introducing a Trait. Using said Trait comes with a set of limitations that I believe can be solved by adding previous functionality back to the MessageBusInterface.

Using the Trait forces you to use $messageBus as the property name, before you were able to name it to your liking, such as $commandBus or $queryBus, which makes more sense from a semantical point of view.
The Symfony Profiler component does no longer show the count of messages handled by a bus, due to binding now being impossible, thanks to the enforced property naming in the handle() method of the Trait.
It seems odd to inject a MessageBusInterface, for instance in a Controller, which then also needs to use HandleTrait. It should be as simple as calling $this->queryBus->handle(new Query());
(Edit: Alternatively I have to create my own QueryBus that uses the HandleTrait, which should not be needed since my Framework already offers a working Bus and also causes confusion where that Trait should actually be used.)
Introducing a Trait still does not resolve the issue of having no return type. The same issue still remains, but now more problems have resulted due to the change.

I understand the issue with return types, as discussed in the PR mentioned above, however the current implementation seems to be too focused on solving a specific issue, which might not need to be solved at all.
Regarding the explanation of implementing your own QueryBus from this PR, again, I believe this is going out of the way of how things should work. I think sacrificing not having a return type on the handle method is an acceptable trade-off here.

Example
I propose to (re)introduce a handle() method on the MessageBusInterface, which will solve the issues mentioned above, as well as make it simpler to use. The HandleTrait can be removed again, and the logic from within the handle method would be moved back into the MessageBus implementation.

interface MessageBusInterface
{
    /**
     * Dispatches the given message.
     *
     * @param object|Envelope $message The message or the message pre-wrapped in an envelope
     */
    public function dispatch($message): Envelope;

    /**
     * Handles the given message.
     *
     * @param object|Envelope $message The message or the message pre-wrapped in an envelope
     *
     * @return mixed The value returned by the handler
     */
    public function handle($message);
}

The text was updated successfully, but these errors were encountered:

luzivenom · 2018-12-02T15:02:23Z

I have been struggling with the same problem. I thought the MessageBus was a great way to implement queries. I just found out today that I need to use a Trait now.

ghost · 2018-12-02T15:07:48Z

@luzivenom You're not forced to use the Trait, you could implement the logic yourself. For instance, you could do something like this (Which is similar to how it is implemented at the moment)

$envelope = $this->queryBus->dispatch($message);

// Needs some additional checking of course
// https://github.com/symfony/messenger/blob/master/HandleTrait.php
$result = $envelope->last(HandledStamp::class)->getResult();

I just think that it's a step backwards from previously being able to do

$result = $this->queryBus->dispatch($message);

which could now simply become

$result = $this->queryBus->handle($message);

jvasseur · 2018-12-02T16:09:20Z

Regarding points 2 and 3 (and maybe 1 to) if think your are not supposed to use the trait inside controllers, instead you should create your own QueryBus class by using the trait and then inject this class into your controllers.

ghost · 2018-12-02T16:28:41Z

@jvasseur Agreed, I do not want to use Traits in my Controllers (and you should not, it was just an example if I don't want to implement my own Bus).
I just don't see the need to create my own QueryBus, if I have a working Bus that my Framework already provides me with. I can still inject a MessageBusInterface for dispatching commands for instance, but I need to implement it myself to dispatch queries? Seems odd to me.

TomWotaWun · 2018-12-02T17:06:34Z

@jvasseur Idk about creating my own Bus. I agree with @hacked that it seems odd to create my own bus which uses another bus to do what my framework should be doing in the first place. I saw that @ogizanagi mentioned in #29167 that he wants to add a base class. Would that help with this?

jvasseur · 2018-12-02T17:21:52Z

I don't disagree that it makes things a bit more complex.

Personally I'm in favor of always decorating the MessageBus with my own class (even for command busses). It allow adding parameters (and return types in case of query buses) type, allow decorating the messages with envelopes instead of having to do it in controllers and allow using auto-wiring when working with multiple bus instances.

The problem with letting the framework do everything is that Message buses can be used in a lot of cases and the framework can't really know how you expect it to work so encouraging people to decorate it looks like a good trade-of.

nicolas-grekas · 2018-12-02T18:38:17Z

@jvasseur got it 100% right. The job of a framework is to encourage maintainable practices. Having a public method that takes any object and that returns an unspecified value with a pluggable implementation doesn't belong to that description. What the trait provides is an implementation that may help implement a query/command bus - you can copy/paste it if the exact details of its implementation don't fit your style. And in any way, in you code, you should really declare both a more specific argument type AND return type. The component cannot help here, only you know what to declare.

nicolas-grekas · 2018-12-11T13:06:47Z

Closing as explained.

chalasr added Messenger RFC RFC = Request For Comments (proposals about features that you want to be discussed) labels Dec 2, 2018

nicolas-grekas closed this as completed Dec 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Messenger] Introduce handle method on MessageBusInterface instead of Trait #29428

[Messenger] Introduce handle method on MessageBusInterface instead of Trait #29428