thephpleague / tactician

A small, flexible command bus
http://tactician.thephpleague.com
MIT License
858 stars 88 forks source link

CommandBus Builder #58

Closed ghost closed 9 years ago

ghost commented 9 years ago

What do you think about adding a simple builder?

For example:

<?php

use League\Tactician\CommandBus;
use League\Tactician\Handler\CommandHandlerMiddleware;
use League\Tactician\Handler\CommandNameExtractor\ClassNameExtractor;
use League\Tactician\Handler\CommandNameExtractor\CommandNameExtractor;
use League\Tactician\Handler\Locator\HandlerLocator;
use League\Tactician\Handler\MethodNameInflector\HandleInflector;
use League\Tactician\Handler\MethodNameInflector\MethodNameInflector;
use League\Tactician\Middleware;

class Builder
{
    private $middlewares = [];
    private $handlerLocator;
    private $commandNameExtractor;
    private $inflector;

    public function __construct(
        HandlerLocator $handlerLocator,
        CommandNameExtractor $commandNameExtractor = null,
        MethodNameInflector $inflector = null
    ) {
        $this->handlerLocator = $handlerLocator;
        $this->commandNameExtractor = $commandNameExtractor ?: new ClassNameExtractor();
        $this->inflector = $inflector ?: new HandleInflector();
    }

    public function attachMiddleware(Middleware $middleware, $priority = null)
    {
        if ($priority !== null) {
            $this->middlewares[$priority] = $middleware;
        } else {
            $this->middlewares[] = $middleware;
        }
    }

    public function build()
    {
        ksort($this->middlewares);
        $this->middlewares[] = new CommandHandlerMiddleware(
            $this->commandNameExtractor,
            $this->handlerLocator,
            $this->inflector
        );

        return new CommandBus($this->middlewares);
    }
}

And then, we can configure CommandBus using a configuration file, for example:

<?php

$builder = new Builder(new ContainerBasedLocator($container));
$middlewares = require './middlewares.php';
foreach ($middlewares as $middleware) {
    if (is_array($middleware)) {
        if (sizeof($middleware) != 2) {
            throw new \LogicException('Invalid middleware definition.');
        }
        list($middleware, $priority) = $middleware;
    } else {
        $priority = null;
    }
    $builder->attachMiddleware($container->get($middleware), $priority);
}
$bus = $builder->build();
<?php

// middlewares.php

return [
    [LockingMiddleware::class, -255],
    [LoggingMiddleware::class, 255],
    [TransactionMiddleware::class, 0],
];
rosstuck commented 9 years ago

Hi @Kilte,

First off, sorry for the delay answering you, I was busy most of the weekend. My apologies! :sweat:

I like the general idea though I'm not sure how common the usecase is for it. If we were to include something similar in core, perhaps it could be a refactored version of the Quickstart setup object? Would that be interesting to you?

I would probably want to remove the priority numbers though, since I've tried to avoid these and discourage them whenever I can. The order of middleware is very important to how the app runs so I want folks to make explicit decisions on that whenever possible.

ghost commented 9 years ago

Hi @rosstuck.

Yes, QuickStart is similar and can be replaced by Builder.

About priorities, maybe you're right. In my case they are necessary, since the bus is configured from multiple places. I can implement priorities manually, it does not matter.

Finally, we can replace QuickStart by simple builder (without middleware priorities). In my opinion this will be more useful.

sagikazarmark commented 9 years ago

Hey @Kilte @rosstuck

I am thinking about exactly this for the last few days. I like the idea, I have a few use cases. Actually this might be a big help in DI.

:+1: :100:

rosstuck commented 9 years ago

If you two are both on board for this, perhaps it'd be good to layout some rough API notes and square this off, then we can merge it to core. :)

ghost commented 9 years ago

@rosstuck, @sagikazarmark, I've created a pull request. Waiting for suggestions =).