headsnet / domain-events-bundle

DDD Domain events for Symfony
MIT License
39 stars 4 forks source link

Events get published while rollback transaction is open #27

Open wazum opened 2 years ago

wazum commented 2 years ago

Dispatching domain events after a terminate event with an open rollback transaction might lead to dispatched events which aren't really persisted yet (which contradicts the outbox pattern).

I stumbled upon this by accident when I saw a failing command producing Symfony messenger entries and solved it with this modified version of \Headsnet\DomainEventsBundle\EventSubscriber\PublishDomainEventSubscriber::publishEvents (an instance of EntityManager is additionally injected via constructor)

    private function publishEvents(): void
    {
        // We definitely should not publish events during an open transaction (as events might not be persisted yet)
        if ($this->entityManager->getConnection()->isTransactionActive()) {
            return;
        }

        foreach ($this->eventStore->allUnpublished() as $event) {
            $this->publishEvent($event);
        }
    }

Is this the correct solution, or are there any other drawbacks I didn't think about @benr77 ?

wazum commented 2 years ago

The first idea was to add ->setCacheMode(Cache::MODE_REFRESH) to the query, but that had no effect and I found no other nice way to force a query on the actual database.

    $qb = $this->em->createQueryBuilder()
            ->setCacheMode(Cache::MODE_REFRESH)
            ->select('e')
            ->from(StoredEvent::class, 'e')
            ->where('e.publishedOn IS NULL')
            ->andWhere('e.occurredOn < :now')
            ->setParameter('now', $now)
            ->orderBy('e.occurredOn');
benr77 commented 2 years ago

I'm really not sure - I'm not very familiar with any of this. Is this part of the docs describing the Doctrine behaviour you are trying to manage:

https://www.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/transactions.html#transaction-nesting

wazum commented 2 years ago

Yes, let's see if I can come up with a test case for this.