Prooph\ServiceBus\PluginServiceLocatorPlugin is not compatible with the EventBus`

[Ocramius]

Prooph\ServiceBus\Plugin\ServiceLocatorPlugin relies onProoph\ServiceBus\MessageBus::EVENT_PARAM_MESSAGE_HANDLER, but the Prooph\ServiceBus\EventBus has its own EVENT_PARAM_EVENT_LISTENERS

This means that registering a ServiceLocatorPlugin against an EventBus is not causing any effect, as no events are intercepted.

[codeliner]

Oh, that would be a really bad bug. I guess this line claryfies the issue?
The event bus is just special in allowing multiple handlers for one message (aka listeners for an event)
Should we make it more explicit?

Update: It is also explained in the docs ;)

[Ocramius]

@codeliner I am aware of the fact that they behave differently, but since the type system allows me to register any plugin against an EventBus, this looks kinda weird.

[codeliner]

@Ocramius The idea is is the following (which is also explained in the docs and in the code):
CommandBus, QueryBus and EventBus are all some kind of message bus. That is the reason why they share basic logic (by extending Prooph\ServiceBus\MessageBus).
That said, a plugin can be a "message bus plugin" and be attached to every bus type.
The ServiceLocatorPlugin is the best example. Why should it be different for the EventBus?
The event bus uses the same Prooph\ServiceBus\MessageBus::EVENT_PARAM_MESSAGE_HANDLER, just for every listener. For the ServiceLocatorPlugin it does not matter if a command handler should be lazy loaded or multiple event listeners. The plugin just asks the container if it knows the message handler. That's it.

What would you change so that it looks less weird?

[Ocramius]

@codeliner I managed to simply write my own service locator plugin for now, but here's what is quick and simple improvement to be applied in order to make this less confusing to debug/use: make the listener registration process throw exceptions if registration of unknown event names is used.

Basically, a CommandBus and an EventBus have different internal events and event dispatcher, but the event names are all pre-defined upfront (we know which events will be used): attaching to an event that is unknown may throw an exception when running in a "stricter" mode.

This can all be hidden in the private API of the package: as long as the event system is not shared across two buses, this should be fine.

[codeliner]

@Ocramius Very good idea. We'll do that. Thank you.

But to clarify things: The ServiceLocatorPlugin shipped with prooph/service-bus works with the event bus. See my first comment and the linked line in the event bus.
Do I understand your use case wrong? Can you provide me a gist of your ServiceLocatorPlugin?

[Ocramius]

Here are the two factories that I use to setup the event store and event bus in my project:

<?php

declare(strict_types=1);

namespace MyProject\Factory\Infrastructure\Messaging;

use Doctrine\DBAL\Connection;
use Interop\Container\ContainerInterface;
use Prooph\Common\Event\ActionEvent;
use Prooph\Common\Event\ActionEventEmitter;
use Prooph\Common\Event\ActionEventListenerAggregate;
use Prooph\Common\Event\ProophActionEventEmitter;
use Prooph\Common\Messaging\FQCNMessageFactory;
use Prooph\Common\Messaging\NoOpMessageConverter;
use Prooph\EventStore\Adapter\Doctrine\DoctrineEventStoreAdapter;
use Prooph\EventStore\Adapter\PayloadSerializer\JsonPayloadSerializer;
use Prooph\EventStore\EventStore;
use Prooph\EventStoreBusBridge\EventPublisher;
use Prooph\ServiceBus\EventBus;
use Prooph\ServiceBus\MessageBus;

final class EventStoreFactory
{
    public function __invoke(ContainerInterface $container) : EventStore
    {
        /* @var $connection Connection */
        $connection = $container->get(Connection::class);

        $eventStore = new EventStore(
            new DoctrineEventStoreAdapter(
                $connection,
                new FQCNMessageFactory(),
                new NoOpMessageConverter(),
                new JsonPayloadSerializer()
            ),
            new ProophActionEventEmitter()
        );

        $eventBus = new EventBus();

        $eventBus->utilize($this->buildProjectorRouter($container));

        (new EventPublisher($eventBus))->setUp($eventStore);

        return $eventStore;
    }

    // this is where I replaced the base plugin to make it work with the event store
    // each service inside this container is a `callable[]`
    private function buildProjectorRouter(ContainerInterface $container) : ActionEventListenerAggregate
    {
        return new class ($container) implements ActionEventListenerAggregate
        {
            /**
             * @var ContainerInterface
             */
            private $projectors;

            public function __construct(ContainerInterface $projectors)
            {
                $this->projectors = $projectors;
            }

            public function attach(ActionEventEmitter $dispatcher)
            {
                $dispatcher->attachListener(MessageBus::EVENT_ROUTE, [$this, 'onRoute']);
            }

            public function detach(ActionEventEmitter $dispatcher)
            {
                throw new \BadMethodCallException('Not implemented');
            }

            public function onRoute(ActionEvent $actionEvent)
            {
                $messageName = (string) $actionEvent->getParam(MessageBus::EVENT_PARAM_MESSAGE_NAME);

                if ($this->projectors->has($messageName)) {
                    $actionEvent->setParam(
                        EventBus::EVENT_PARAM_EVENT_LISTENERS,
                        $this->projectors->get($messageName)
                    );
                }
            }
        };
    }
}

<?php

declare(strict_types=1);

namespace MyProject\Factory\Infrastructure\Messaging;

use Interop\Container\ContainerInterface;
use Prooph\Common\Event\ActionEvent;
use Prooph\Common\Event\ActionEventEmitter;
use Prooph\Common\Event\ActionEventListenerAggregate;
use Prooph\EventStore\EventStore;
use Prooph\EventStoreBusBridge\TransactionManager;
use Prooph\ServiceBus\CommandBus;
use Prooph\ServiceBus\MessageBus;
use Prooph\ServiceBus\Plugin\ServiceLocatorPlugin;

final class CommandBusFactory
{
    public function __invoke(ContainerInterface $container) : callable
    {
        $commandBus = new CommandBus();

        // just using a `ServiceLocatorPlugin`
        $commandBus->utilize(new ServiceLocatorPlugin($container));
        // this one simply takes the command name and makes it the handler name
        $commandBus->utilize($this->buildCommandRouter());

        $transactionManager = new TransactionManager();

        $transactionManager->setUp($container->get(EventStore::class));

        $commandBus->utilize($transactionManager);

        return function ($command) use ($commandBus) {
            $commandBus->dispatch($command);
        };
    }

    private function buildCommandRouter() : ActionEventListenerAggregate
    {
        return new class implements ActionEventListenerAggregate {
            public function attach(ActionEventEmitter $dispatcher)
            {
                $dispatcher->attachListener(MessageBus::EVENT_ROUTE, [$this, "onRoute"]);
            }

            public function detach(ActionEventEmitter $dispatcher)
            {
                throw new \BadMethodCallException('Not implemented');
            }

            public function onRoute(ActionEvent $actionEvent)
            {
                $actionEvent->setParam(
                    MessageBus::EVENT_PARAM_MESSAGE_HANDLER,
                    (string) $actionEvent->getParam(MessageBus::EVENT_PARAM_MESSAGE_NAME)
                );
            }
        };
    }
}

[codeliner]

@Ocramius The ProjectorRouter looks good. Could be a new core plugin. @prolic What do you think?

Question:

if ($this->projectors->has($messageName)) {
                    $actionEvent->setParam(
                        EventBus::EVENT_PARAM_EVENT_LISTENERS,
                        $this->projectors->get($messageName)
                    );
                }

Your container (projectors) returns an array of projectors, right?

Now look at this loop from the event bus:

foreach ($actionEvent->getParam(self::EVENT_PARAM_EVENT_LISTENERS, []) as $eventListener) {
                $actionEvent->setParam(self::EVENT_PARAM_MESSAGE_HANDLER, $eventListener);
                if (is_string($eventListener) && ! is_callable($eventListener)) {
                    $actionEvent->setName(self::EVENT_LOCATE_HANDLER);
                    $this->trigger($actionEvent);
                }
                $actionEvent->setName(self::EVENT_INVOKE_HANDLER);
                $this->trigger($actionEvent);
            }

As you can see self::EVENT_LOCATE_HANDLER is triggered for each event listener, if it is a string! The event listener is set as self::EVENT_PARAM_MESSAGE_HANDLER in the action event before triggering the action event.

So the default ServiceLocatorPlugin works the same as for command handlers. No exception.

[Ocramius]

Your container (projectors) returns an array of projectors, right?

Correct

As you can see self::EVENT_LOCATE_HANDLER is triggered for each event listener, if it is a string!
So the default ServiceLocatorPlugin works the same as for command handlers. No exception.

Ah yes, there is some confusion here, sorry (my bad, I opened this issue after working around it, so I didn't really recall it). The problem is that the event parameters are indeed different here.

[codeliner]

Ok, so we have no bug but a list of suggestions and new ideas. Well done. Thank you @Ocramius :)

[prolic]

👍

[codeliner]

I've created two new issue. I think we can close this one now.

[Ocramius]

Linking #106 and #105