Bus Dispatcher is not a singleton, preventing any extensibility

[clarkwinkelmann]

Bug Report

Current Behavior The reason to use the bus dispatcher is to make commands extensible. However it's currently not possible to extend them in any way.

Related flarum/issue-archive#345

Steps to Reproduce

1. Register a bus pipe from an extension:

return [
    function (\Illuminate\Contracts\Bus\Dispatcher $bus) {
        $bus->pipeThrough([
            function($command, $next)
            {
                die('command stopped by pipe');
            },
        ]);
    },
];

2. Try creating a discussion or replying
3. No error happens, even though the pipe should stop any command from executing

Expected Behavior It should be possible to register pipes and custom mappings on the bus dispatcher.

Environment

• Flarum version: beta 13, beta 14

Possible Solution This can be fixed by using a singleton in the BusServiceProvider

https://github.com/flarum/core/blob/967cd0e3ca793be8d83f8ba569fb0beb85452e85/src/Bus/BusServiceProvider.php#L22

That's what Laravel is using in its bundled BusServiceProvider

Additional Context Looking at the code it must be broken since at least beta 8.

Evidence from that 2016 post suggests it was working at one point in the past https://discuss.flarum.org/d/3384-hooking-into-bus-dispatch-commands/2

[franzliedke]

Non-singletons can be extended, with Container::extend() or Container::resolving(). :grin: :wave:

[clarkwinkelmann]

Oh right, that should work from a service provider.

Maybe we should have an extender for this as well, so developers don't need to worry about it.

Now I'm wondering which extension I needed this for :joy:

[askvortsov1]

Maybe we should have an extender for this as well, so developers don't need to worry about it.

If we end up keeping it, we could just make it a singleton? Now that routes can be overriden, this is much less urgent.

[clarkwinkelmann]

I have successfully implemented bus pipes using resolving in a few extensions now, so that seems a perfectly reasonable solution.

I don't think making it a singleton improves it significantly, since you would still need a service provider and a boot method to extend it. resolving seems a reasonable solution.

I suggest we close this, unless we want to make an actual extender for it.

[askvortsov1]

unless we want to make an actual extender for it.

That might not be a bad idea. Let's revisit it around 0.2 though.