Drop support for app()->call() when calling event handlers

[brendt]

HandlesEvents::handle calls handlers like so:

app()->call([$this, $handlerClassOrMethod], $parameters);

The reasoning being that you could inject extra dependencies in event handlers. I think we don't need that functionality for two reasons:

• app()->call()'s implementation assumes named arguments. That means that every handler method should use $event as the variable name. When not doing so an obscure error is thrown:

Illuminate\Contracts\Container\BindingResolutionException : Unresolvable dependency resolving [Parameter #0 [ <required> string $cartUuid ]] in class Spatie\Shop\Cart\Events\CartItemAdded

 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/Container.php:1053
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/Container.php:964
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/Container.php:899
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/Container.php:861
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/Container.php:712
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Foundation/Application.php:796
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/Container.php:651
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Foundation/Application.php:781
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:173
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:124
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:36
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/Util.php:40
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:93
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:37
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Container/Container.php:610
 /Users/brent/dev/spatie/laravel-event-sourcing/src/EventHandlers/HandlesEvents.php:47
 /Users/brent/dev/spatie/laravel-event-sourcing/src/Projectionist.php:242
 /Users/brent/dev/spatie/laravel-event-sourcing/src/Projectionist.php:226
 /Users/brent/dev/spatie/laravel-event-sourcing/src/Projectionist.php:207
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php:261
 /Users/brent/dev/spatie/laravel-event-sourcing/src/StoredEvents/StoredEvent.php:72
 /Users/brent/dev/spatie/laravel-event-sourcing/src/StoredEvents/StoredEvent.php:60
 /Users/brent/dev/spatie/laravel-event-sourcing/src/AggregateRoots/AggregateRoot.php:68
 /Users/brent/dev/spatie/es-cart/vendor/laravel/framework/src/Illuminate/Collections/Traits/EnumeratesValues.php:234
 /Users/brent/dev/spatie/laravel-event-sourcing/src/AggregateRoots/AggregateRoot.php:68
 /Users/brent/dev/spatie/es-cart/src/Cart/Actions/AddCartItem.php:25
 /Users/brent/dev/spatie/es-cart/tests/Cart/Actions/AddCartItemTest.php:40

Note the obscurity of this error. It happened because a projector had an implementation like so:

class CartProjector extends Projector
{
    public function onCartInitialized(CartInitialized $cartInitialized): void { /* … */ }
}

The error doesn't mention this projector though. Instead the container fails because it tries to autowire CartInitialized, since the actual event can only be injected if the argument name is $event.

• Second, it's overkill to allow dependency injection in handler methods, since projectors, reactors and aggregate roots already support autowiring in their constructor. If you really need external dependencies, they can be injected over there.

So I propose to replace app()->call() with $this->$handlerClassOrMethod($storedEvent->event).

There's one caveat: right now both the aggregate root UUID and the stored event are also (optionally) passed to the handler, and the implementation should actually be:

$this->$handlerClassOrMethod($storedEvent->event, $storedEvent, $storedEvent->aggregate_uuid)

I don't think this is necessary though: the AR UUID is already exposed on the event itself via its meta data, and we could easily keep a reference to its stored event ID as well in its meta data:

$event->storedEvent();
$event->aggregateRootUuid(); // This one is already possible

[freekmurze]

So I propose to replace app()->call() with $this->$handlerClassOrMethod($storedEvent->event).

Agreed!

I don't think this is necessary though: the AR UUID is already exposed

Indeed.

Let's just implement your proposal! 👍

[brendt]

Awesome, I'll merge it in v5 next week

[brendt]

Merged in v5: 99e083d

[brendt]

• Let's add aggreate_root_uuid and stored_event_id on the stored event table itself instead of via meta data
• Add ID with UUID on stored events and use UUID in the event repository