[Messenger] Change doctrine middleware order

[wtorsi]

Please, change doctrine middleware order in docs https://symfony.com/doc/current/messenger.html#middleware-for-doctrine
The valid order for full stack of doctrine middleware is: (also, please include default_bus option)

framework:
    messenger:
        default_bus: command.bus
        buses:
            command.bus:
                middleware:
                    - doctrine_ping_connection
                    - doctrine_close_connection
                    - doctrine_transaction

[weaverryan]

@wtorsi Can you tell us why this is the correct order? We're not necessarily experts on this, so having some validation behind this will make it much easier to do :).

Cheers!

[wtorsi]

Sure @weaverryan,
Middleware in messenger has stack behaviour, it means that top middleware calls next (below) middleware.
In our case:
DoctrinePingConnectionMiddleware previously checks connection and reopen it if needed, then it calls next middleware in the stack.

 public function handle(Envelope $envelope, StackInterface $stack): Envelope
    {
        try {
            $entityManager = $this->managerRegistry->getManager($this->entityManagerName);
        } catch (\InvalidArgumentException $e) {
            throw new UnrecoverableMessageHandlingException($e->getMessage(), 0, $e);
        }

        $connection = $entityManager->getConnection();

        if (!$connection->ping()) {
            $connection->close();
            $connection->connect();
        }

        if (!$entityManager->isOpen()) {
            $this->managerRegistry->resetManager($this->entityManagerName);
        }

        return $stack->next()->handle($envelope, $stack);
    }

DoctrineCloseConnectionMiddleware previously calls next middleware in the stack, and then it closes the connection.

 public function handle(Envelope $envelope, StackInterface $stack): Envelope
    {
        try {
            $entityManager = $this->managerRegistry->getManager($this->entityManagerName);
        } catch (\InvalidArgumentException $e) {
            throw new UnrecoverableMessageHandlingException($e->getMessage(), 0, $e);
        }

        try {
            $connection = $entityManager->getConnection();

            return $stack->next()->handle($envelope, $stack);
        } finally {
            $connection->close();
        }
    }

DoctrineTransactionMiddleware firstly starts the transaction, secondly calls next middleware in the stack (usually it is HandleMessageMiddleware which doing anything we want in our custom handler), and finally it flushes changes and commits the transaction.

    public function handle(Envelope $envelope, StackInterface $stack): Envelope
    {
        try {
            $entityManager = $this->managerRegistry->getManager($this->entityManagerName);
        } catch (\InvalidArgumentException $e) {
            throw new UnrecoverableMessageHandlingException($e->getMessage(), 0, $e);
        }

        $entityManager->getConnection()->beginTransaction();
        try {
            $envelope = $stack->next()->handle($envelope, $stack);
            $entityManager->flush();
            $entityManager->getConnection()->commit();

            return $envelope;
        } catch (\Throwable $exception) {
            $entityManager->getConnection()->rollBack();

            if ($exception instanceof HandlerFailedException) {
                // Remove all HandledStamp from the envelope so the retry will execute all handlers again.
                // When a handler fails, the queries of allegedly successful previous handlers just got rolled back.
                throw new HandlerFailedException($exception->getEnvelope()->withoutAll(HandledStamp::class), $exception->getNestedExceptions());
            }

            throw $exception;
        }
    }

The right business logic should be: check connection -> start transaction, do things, commit -> close connection.
According to the above short middleware description, the middleware stack should be

1. DoctrinePingConnectionMiddleware - it will check connection, then it will call next middleware DoctrineCloseConnectionMiddleware
2. DoctrineCloseConnectionMiddleware- it will call next middleware (wrap) DoctrineTransactionMiddleware, and then will close the connection
3. DoctrineTransactionMiddleware - will be called inside DoctrineCloseConnectionMiddleware, will do anything we want in one transaction.

[OskarStark]

@weaverryan can you please confirm this?

[javiereguiluz]

@Nyholm could you please check if the proposed change is correct? Thanks!

[Nyholm]

(Thank Javier you for the ping)

Well spotted @wtorsi. You are very much correct. We should update the order.

Do you want to make a PR to fix this?

[xabbuh]

fixed in #12357