Add lazy loading for handlers

[kpicaza]

In a bus with 32 handler, memory usage is reduced by 2-thirds

with LaravelLocator

./vendor/bin/phpunit --filter=testCreateObservationComment
PHPUnit 6.3.0 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 6.67 seconds, Memory: 64.00MB

with LaravelLazyLocator

./vendor/bin/phpunit --filter=testCreateObservationComment
PHPUnit 6.3.0 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 638 ms, Memory: 24.00MB

[joselfonseca]

Hello there!, it seems that this brakes some of the tests, can you please check that and also write a test for the code you want to commit.

https://travis-ci.org/joselfonseca/laravel-tactician/builds/262188742?utm_source=github_status&utm_medium=notification

Thanks!

[kpicaza]

Thanks for response, i will work around and update pull request again, most probably this weekend.

[kpicaza]

Hello,

There was different things:

First is your comment; i need to test my code(of course), although, my code does not brake tests, even so, i added similar test than LaravelLocator overriding general config and all is ok. I need a little more time to make better tests.

Second the real test brake was in generatorTest file, because file path does not match with real file paths, i fix it.

and for last, Travis build break on HHVM because :

HHVM is no longer supported on Ubuntu Precise. Please consider using Trusty with dist: trusty.

Thanks again for that great package, we are using in production on a medium/hi traffic site and it works like a charm.

Cheers.

[joselfonseca]

Let me look into this tonight. Thanks!

[kpicaza]

Thanks again,

After a little workaround, i fix tests and give a nice coverage for new class. But our use case is some different to the described in the Readme file. And i dont know if this commits has sense for described usage.

Here is an example:

<?php
// config/laravel-tactician.php

return [
    'locator' => Joselfonseca\LaravelTactician\Tests\Locator\LaravelLazyLocator::class,
    'inflector' => League\Tactician\Handler\MethodNameInflector\HandleInflector::class,
    'extractor' => League\Tactician\Handler\CommandNameExtractor\ClassNameExtractor::class,
    'bus' => \League\Tactician\CommandBus::class,
    'handle-map' => [
        SomeCommand::class => SomeHandler::class
    ]

Then we have custom provider for bus

<?php

namespace App\Providers;

use Doctrine\ORM\EntityManager;
use Illuminate\Container\Container;
use Illuminate\Support\ServiceProvider;
use League\Tactician\CommandBus;
use League\Tactician\Handler\CommandHandlerMiddleware;
use League\Tactician\Handler\CommandNameExtractor\ClassNameExtractor;
use League\Tactician\Plugins\LockingMiddleware;

class CommandBusProvider extends ServiceProvider
{
    /**
     * Register the application services.
     *
     * @return void
     */
    public function register()
    {
        $this->app->bind(CommandBus::class, function (Container $container) {
            $config = config('laravel-tactician');
            $inflector = $container->make($config['inflector']);
            $locator = $container->make($config['locator']);
            $nameExtractor = $container->make($config['extractor']);

            collect($config['handle-map'])->each(function ($handler, $command) use ($locator) {
                $locator->addHandler($handler, $command);
            });

            $commandHandlerMiddleware = new CommandHandlerMiddleware($nameExtractor, $locator, $inflector);
            return new CommandBus([
                new LockingMiddleware(),
                $commandHandlerMiddleware,
            ]);
        });
    }
}

[joselfonseca]

I see you are using tactician.

'bus' => \League\Tactician\CommandBus::class,

Will have to see if this works for the default command bus proposed in the package. Thanks for pointing this out though, i will try to work on some tests to see if that performance issue is presented with the other bus which I think it may be the case.

[johannesschobel]

any news on this?

[joselfonseca]

Merged on #14