ecotoneframework / ecotone-dev

Ecotone Framework Development - This is Monorepo which contains all official public modules
https://docs.ecotone.tech
Other
37 stars 16 forks source link

Initialize ES aggregates without side-effects #364

Closed unixslayer closed 1 month ago

unixslayer commented 2 months ago

Don't know if this is more of a bug rather than a feature request.

Initializing ES aggregate generates side-effects which may be actual tested functionality. Consider following example:

<?php

use Ecotone\Lite\EcotoneLite;
use Ecotone\Modelling\Attribute\CommandHandler;
use Ecotone\Modelling\Attribute\EventHandler;
use Ecotone\Modelling\Attribute\QueryHandler;
use Ecotone\Modelling\Attribute\EventSourcingAggregate;
use Ecotone\Modelling\Attribute\EventSourcingHandler;
use Ecotone\Modelling\CommandBus;
use Ecotone\Modelling\QueryBus;
use PHPUnit\Framework\TestCase;

#[EventSourcingAggregate]
class Aggregate {

    private array $orders = [];

    #[CommandHandler]
    public function placeOrder(PlaceOrder $command): array
    {
        // some heavy business process

        return [new OrderWasPlaced($command->aggregateId, $command->orderId)];
    }

    #[EventSourcingHandler]
    public function applyOrderWasPlaced(OrderWasPlaced $event): void
    {
        $this->orders[] = $event->order();
    }

    #[QueryHandler('orders')]
    public function orders(): array
    {
        return $this->orders;
    }
}

#[EventSourcingAggregate]
class AnotherAggregate {

    private bool $done = false;

    #[CommandHandler]
    public function placeOrder(DoStuff $command): array
    {
        if ($this->done) {
            return [];
        }

        return [new StuffWasDone($command->orderId)];
    }

    #[EventSourcingHandler]
    public function applyStuffWasDone(StuffWasDone $event): void
    {
        $this->done = true;
    }
}

class Service {

    #[EventHandler]
    public function whenOrderWasPlaced(OrderWasPlaced $event, QueryBus $queryBus, CommandBus $commandBus): void
    {
        $orders = $queryBus->sendWithRouting('orders', metadata: ['aggregate.id' => $event->aggregateId]);
        foreach ($orders as $order) {
            $commandBus->send(new DoStuff($order->orderId));
        }
    }
}

readonly class OrderWasPlaced {
    public function __construct(
        public string $aggregateId,
        public string $orderId
    ) {
    }
}

readonly class DoStuff {
    public function __construct(public string $orderId) {}
}

readonly class StuffWasDone {
    public function __construct(public string $orderId) {}
}

class ServiceTest extends TestCase
{

    public function test_service(): void
    {
        $ecotone = EcotoneLite::bootstrapFlowTestingWithEventStore(
            classesToResolve: [Aggregate::class, Service::class],
            containerOrAvailableServices: [new Service()]
        );

        $aggregateId = 'aggregate-1';
        $orderId = 'order-1';

        $ecotone->withEventsFor($aggregateId, Aggregate::class, [new OrderWasPlaced($aggregateId, $orderId)]);

        // at this point OrderWasPlaced was already published triggering Service::whenOrderWasPlaced, but we don't know that due FlowTestSupport::discardRecordedMessages was also called

        $ecotone->publishEvent(new OrderWasPlaced($aggregateId, $orderId));

        self::assertEquals([new DoStuff($orderId)], $ecotone->getRecordedCommands()); // this actually happened twice
        self::assertEquals([new StuffWasDone($orderId)], $ecotone->getRecordedEvents()); // this will fail
    }
}

Currently there is no way to properly check what actually happened because side-effect we would like to test against already happened when Aggregate was initialized. The only way to verify business behavior would be to expose value of AnotherAggregate::$done which seems a bit odd only for test purpose.

Being able to initialize aggregate without additional side-effect will increase tests readability. It may also introduce testing concurrency in scenario when Aggregate is initialized with multiple events, publishing other than the last one may have different outcome based on business rules.

jlabedo commented 1 month ago

Hello @unixslayer , why would you need to initialize the aggregate if you want to test Service ? The behaviour seems natural as Service has been registered in EcotoneLite.

unixslayer commented 1 month ago

Hi @jlabedo Aggregate as a unit of cohesion and single source of truth may be directly queried from the service. Testing Service behavior won't be deterministic if it is a side-effect of aggregate initialization. Also if Service does a part in larger workflow, I would rather set-up my aggregate and test integration in isolation without going through whole process. Imagine the size of the test here.

Service may also be triggered in any point in time with aggregate being already persisted with specific state.

Currently Ecotone\Lite\Test\FlowTestSupport::withEventsFor calls discardRecordedMessages which only hides possible side-effects.