An overengineered 'created' event that improved my entire design

TL;DR:

My Billit import saves a purchase-invoice bill and then hands the slow work to two separate async commands: one that fetches the associated PDF and one that guesses its category. I overengineered the triggering 'created' event and wanted to write about it. Then I realised something that improved my design.

Nothing told the rest of the app a bill was created at all or to do anything with it. I forgot firing the CreatedEvent. While fixing that bug and adding the event, I tricked myself into worrying about firing it too early and placed it in the PDF retrieval handler. Writing that reasoning down for this blog post showed me that my worry was unfounded because of my queue setup.

It also made me realise a much more important thing: "created" and "complete" are two different claims and deserve two different events, allowing me to improve the design dramatically.

I forgot the CreatedEvent

When a purchase invoice (i.e.: a bill) comes in from Billit, the import handler saves it to the database and then dispatches two commands. One to retrieve the associated PDF file, because only the file ID is sent in the import data, and another to guess the category which is a field entirely missing from Billit's API.

$bill = new Bill()
    ->setAmount((float)$amount)
    ->setDate($date)
    ->...;

$duplicates = $this->entityManager->getRepository(Bill::class)->findDuplicates($bill);
if (empty($duplicates)) {
    $this->entityManager->persist($bill);
    $this->entityManager->flush();
    
    // After bill is saved, retrieve PDF and any other attached files
    if (isset($item->OrderPDF->FileID)) {
        $this->commandBus->dispatch(new RetrieveBillitFileCommand($item->OrderPDF->FileID, Bill::class, $bill->getId()));
    }
    $this->commandBus->dispatch(new GuessBillCategoryCommand($bill->getId()));
    
    $created++;
}

This is how the whole handler looked.

A little bit of data extraction is omitted, but all the handler did was create the bill and tell the app to get the PDF and try finding a suitable category (using AI, I've written a separate post about that).
Nothing that signaled to the rest of the app "a new bill was added, go do something with it."

The CreatedEvent was not dispatched which meant nothing downstream ever fired automatically. The accountant never got notified about a new bill, for example.

The fix

Fixing the mistake is easy and obvious, right? We put it right there, next to the two commands, right after the moment the entity is flushed:

...
$this->entityManager->persist($bill);
$this->entityManager->flush();
    
$this->commandBus->dispatch(new RetrieveBillitFileCommand(/* ... */));
$this->commandBus->dispatch(new GuessBillCategoryCommand($bill->getId()));
$this->eventBus->dispatch(new CreatedEvent($bill->getId()));
...

Done. Fixed. The bills will be flying to my accountant in no time!

The objection flew into my mind just as fast: "Wait, the pdf retrieval won't have run yet when the event fires. But the accountant upload needs that file."

Ok, then we'll just dispatch the event from the RetrieveBillitFileCommandHandler, after we've saved the PDF file:

#[AsMessageHandler(handles: RetrieveBillitFileCommand::class)]
final class RetrieveBillitFileCommandHandler
{
    public function __construct(
        private readonly BillitApiHelper $billitApiHelper,
        private readonly EntityManagerInterface $entityManager,
        private readonly MessageBusInterface $eventBus,
    ) {
    }

    public function __invoke(RetrieveBillitFileCommand $message): void
    {
        // ... retrieve file, build attachment ...
        $this->entityManager->persist($attachment);
        $this->entityManager->persist($entity);
        $this->entityManager->flush();

        if ($entity instanceof Bill) {
            $this->eventBus->dispatch(new CreatedEvent($entity->getId()));
        }
    }
}

Worried about nothing

So that was what I started writing the first version of my post about, but it got me thinking:

Are different buses actually processed in parallel?

The answer is yes, but also no.

It depends on your messenger config, specifically the routing:

framework:
    messenger:
        default_bus: command.bus

        buses:
            command.bus:
            event.bus:

        failure_transport: failed

        transports:
            async: '%env(MESSENGER_TRANSPORT_DSN)%'
            failed: '%env(MESSENGER_TRANSPORT_DSN)%&queue_name=failed'

        routing:
            # Route your messages to the transports
            # 'App\Message\YourMessage': async

In my case, both buses are sent to the same async transport. This means the command/event split is one in name only right now and buys nothing in regard to priority or isolation. A single worker will process the messages in the transport one by one, in the order they were put in the queue.
So as long as I dispatch the CreatedEvent after the commands, there is no need to worry about the event being fired before the commands were processed.

Of course, if I were to change my config to send either to a different transport or add more workers processing from the transport, my worry would become valid again. That brings me to my biggest realisation:

'Created' and 'complete' are not the same

Thanks to writing about this I read up on how Symfony Messenger actually works. I found out that my worry was unfounded in the first place because of my setup (but also, only because of that). Because I now understood the whole mechanism better I also realised something more important that changes the design for the better: 'created' and 'complete' are not the same. And, they should have their own signals.

When I changed where my CreatedEvent was dispatched, I actually changed its meaning. Just like the initial worry with the crossing message buses, it's an understandable reflex. An important process needs the PDF file, and it's an integral part of the bill indeed, so only notify when the bill is complete.

Complete. That word itself shows the shift in meaning.

In a queue-driven system, an event is a promise about state. A CreatedEvent reads like 'finished' or 'complete', and in a synchronous request it usually is, because everything happens before the response goes out. Split the work across async messages and the record appearing in the database drifts apart from the record being usable, and the event you fire is only as truthful as the step you fire it from.

The better design

After realising that events promise state, I knew how to clean up the entire design.

First, we actually do dispatch the event the second the bill is created:

$bill = new Bill()
    ->setAmount((float)$amount)
    ->setDate($date)
    ->...;

$duplicates = $this->entityManager->getRepository(Bill::class)->findDuplicates($bill);
if (empty($duplicates)) {
    $this->entityManager->persist($bill);
    $this->entityManager->flush();
    
    $this->eventBus->dispatch(new CreatedEvent($bill->getId());
    $created++;
}

After that, we remove the previous commands that were dispatched here because we'll turn those into listeners of the created event.