feature #34156 Adding DoctrineClearEntityManagerWorkerSubscriber to reset EM in worker (weaverryan)

This PR was merged into the 4.4 branch.

Discussion
----------

Adding DoctrineClearEntityManagerWorkerSubscriber to reset EM in worker

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes & no :)
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #34073
| License       | MIT
| Doc PR        | symfony/symfony-docs#12575
| DoctrineBundle PR | doctrine/DoctrineBundle#1043

Hi!

I've seen a few developers get "bit" by an issue recently: after running `messenger:consume`, the 2nd, 3rd, 4th, etc message that are handled are getting out-of-date data from Doctrine. The reason is simple:

A) Consume the 1st message, it queries for `Foo id=1` to do something
B) 10 minutes go by
C) Consume the 2nd message. It also queries for `Foo id=1`, but because this is already in the identity map, Doctrine re-uses the data that is now *10* minutes old.

Even though one worker process handles many messages, the system should (as much as possible) isolate each handler from each other. This is one very practical place we can help people. Also, checking the code, I don't think clearing the entity manager will cause any issues for an EM whose Connection has not been "connected" yet (i.e. it will not cause a connection to be established).

We would wire this in DoctrineBundle, and could make it disable-able... in case that's something that's needed.

Commits
-------

e7b98880aa Adding DoctrineClearEntityManagerWorkerSubscriber to reset entity manager in worker

src/Symfony/Bridge/Doctrine/CHANGELOG.md

@@ -4,7 +4,7 @@ CHANGELOG
 4.4.0
 -----
 
- * added `DoctrineClearEntityManagerMiddleware`
+ * added `DoctrineClearEntityManagerWorkerSubscriber`
  * deprecated `RegistryInterface`, use `Doctrine\Common\Persistence\ManagerRegistry`
  * added support for invokable event listeners
  * added `getMetadataDriverClass` method to deprecate class parameters in service configuration files

src/Symfony/Bridge/Doctrine/Messenger/DoctrineClearEntityManagerMiddleware.php

@@ -1,36 +0,0 @@
-<?php
-
-/*
- * This file is part of the Symfony package.
- *
- * (c) Fabien Potencier <fabien@symfony.com>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Symfony\Bridge\Doctrine\Messenger;
-
-use Doctrine\ORM\EntityManagerInterface;
-use Symfony\Component\Messenger\Envelope;
-use Symfony\Component\Messenger\Middleware\StackInterface;
-use Symfony\Component\Messenger\Stamp\ConsumedByWorkerStamp;
-
-/**
- * Clears entity manager after calling all handlers.
- *
- * @author Konstantin Myakshin <molodchick@gmail.com>
- */
-class DoctrineClearEntityManagerMiddleware extends AbstractDoctrineMiddleware
-{
-    protected function handleForManager(EntityManagerInterface $entityManager, Envelope $envelope, StackInterface $stack): Envelope
-    {
-        try {
-            return $stack->next()->handle($envelope, $stack);
-        } finally {
-            if (null !== $envelope->last(ConsumedByWorkerStamp::class)) {
-                $entityManager->clear();
-            }
-        }
-    }
-}

src/Symfony/Bridge/Doctrine/Messenger/DoctrineClearEntityManagerWorkerSubscriber.php

@@ -0,0 +1,55 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bridge\Doctrine\Messenger;
+
+use Doctrine\Common\Persistence\ManagerRegistry;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
+use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent;
+
+/**
+ * Clears entity managers between messages being handled to avoid outdated data.
+ *
+ * @author Ryan Weaver <ryan@symfonycasts.com>
+ */
+class DoctrineClearEntityManagerWorkerSubscriber implements EventSubscriberInterface
+{
+    private $managerRegistry;
+
+    public function __construct(ManagerRegistry $managerRegistry)
+    {
+        $this->managerRegistry = $managerRegistry;
+    }
+
+    public function onWorkerMessageHandled()
+    {
+        $this->clearEntityManagers();
+    }
+
+    public function onWorkerMessageFailed()
+    {
+        $this->clearEntityManagers();
+    }
+
+    public static function getSubscribedEvents()
+    {
+        yield WorkerMessageHandledEvent::class => 'onWorkerMessageHandled';
+        yield WorkerMessageFailedEvent::class => 'onWorkerMessageFailed';
+    }
+
+    private function clearEntityManagers()
+    {
+        foreach ($this->managerRegistry->getManagers() as $manager) {
+            $manager->clear();
+        }
+    }
+}

src/Symfony/Bridge/Doctrine/Tests/Messenger/DoctrineClearEntityManagerMiddlewareTest.php

@@ -1,76 +0,0 @@
-<?php
-
-/*
- * This file is part of the Symfony package.
- *
- * (c) Fabien Potencier <fabien@symfony.com>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Symfony\Bridge\Doctrine\Tests\Messenger;
-
-use Doctrine\Common\Persistence\ManagerRegistry;
-use Doctrine\ORM\EntityManagerInterface;
-use Symfony\Bridge\Doctrine\Messenger\DoctrineClearEntityManagerMiddleware;
-use Symfony\Component\Messenger\Envelope;
-use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
-use Symfony\Component\Messenger\Stamp\ConsumedByWorkerStamp;
-use Symfony\Component\Messenger\Test\Middleware\MiddlewareTestCase;
-
-class DoctrineClearEntityManagerMiddlewareTest extends MiddlewareTestCase
-{
-    public function testMiddlewareClearEntityManager()
-    {
-        $entityManager = $this->createMock(EntityManagerInterface::class);
-        $entityManager->expects($this->once())
-            ->method('clear');
-
-        $managerRegistry = $this->createMock(ManagerRegistry::class);
-        $managerRegistry
-            ->method('getManager')
-            ->with('default')
-            ->willReturn($entityManager);
-
-        $middleware = new DoctrineClearEntityManagerMiddleware($managerRegistry, 'default');
-
-        $envelope = new Envelope(new \stdClass(), [
-            new ConsumedByWorkerStamp(),
-        ]);
-        $middleware->handle($envelope, $this->getStackMock());
-    }
-
-    public function testInvalidEntityManagerThrowsException()
-    {
-        $managerRegistry = $this->createMock(ManagerRegistry::class);
-        $managerRegistry
-            ->method('getManager')
-            ->with('unknown_manager')
-            ->will($this->throwException(new \InvalidArgumentException()));
-
-        $middleware = new DoctrineClearEntityManagerMiddleware($managerRegistry, 'unknown_manager');
-
-        $this->expectException(UnrecoverableMessageHandlingException::class);
-
-        $middleware->handle(new Envelope(new \stdClass()), $this->getStackMock(false));
-    }
-
-    public function testMiddlewareDoesNotClearInNonWorkerContext()
-    {
-        $entityManager = $this->createMock(EntityManagerInterface::class);
-        $entityManager->expects($this->never())
-            ->method('clear');
-
-        $managerRegistry = $this->createMock(ManagerRegistry::class);
-        $managerRegistry
-            ->method('getManager')
-            ->with('default')
-            ->willReturn($entityManager);
-
-        $middleware = new DoctrineClearEntityManagerMiddleware($managerRegistry, 'default');
-
-        $envelope = new Envelope(new \stdClass());
-        $middleware->handle($envelope, $this->getStackMock());
-    }
-}

src/Symfony/Bridge/Doctrine/Tests/Messenger/DoctrineClearEntityManagerWorkerSubscriberTest.php

@@ -0,0 +1,40 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bridge\Doctrine\Tests\Messenger;
+
+use Doctrine\Common\Persistence\ManagerRegistry;
+use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Bridge\Doctrine\Messenger\DoctrineClearEntityManagerWorkerSubscriber;
+use Symfony\Component\Messenger\Test\Middleware\MiddlewareTestCase;
+
+class DoctrineClearEntityManagerWorkerSubscriberTest extends MiddlewareTestCase
+{
+    public function testMiddlewareClearEntityManager()
+    {
+        $entityManager1 = $this->createMock(EntityManagerInterface::class);
+        $entityManager1->expects($this->once())
+            ->method('clear');
+
+        $entityManager2 = $this->createMock(EntityManagerInterface::class);
+        $entityManager2->expects($this->once())
+            ->method('clear');
+
+        $managerRegistry = $this->createMock(ManagerRegistry::class);
+        $managerRegistry
+            ->method('getManagers')
+            ->with()
+            ->willReturn([$entityManager1, $entityManager2]);
+
+        $subscriber = new DoctrineClearEntityManagerWorkerSubscriber($managerRegistry);
+        $subscriber->onWorkerMessageHandled();
+    }
+}