Method hydrateAll() does not take into account possible exception

from hydrateAllData() which in turn does not call cleanup()
doctrine · Feb 20, 2021 · 7971a53 · 7971a53
1 parent 1ffc0ca
commit 7971a53
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 12 deletions.
diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php
@@ -205,12 +205,13 @@ public function hydrateAll($stmt, $resultSetMapping, array $hints = [])
         $this->_hints = $hints;
 
         $this->_em->getEventManager()->addEventListener([Events::onClear], $this);
-
         $this->prepare();
 
-        $result = $this->hydrateAllData();
-
-        $this->cleanup();
+        try {
+            $result = $this->hydrateAllData();
+        } finally {
+            $this->cleanup();
+        }
 
         return $result;
     }

diff --git a/tests/Doctrine/Tests/ORM/Hydration/AbstractHydratorTest.php b/tests/Doctrine/Tests/ORM/Hydration/AbstractHydratorTest.php
@@ -10,6 +10,7 @@
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Events;
 use Doctrine\ORM\Internal\Hydration\AbstractHydrator;
+use Doctrine\ORM\ORMException;
 use Doctrine\ORM\Query\ResultSetMapping;
 use Doctrine\Tests\OrmFunctionalTestCase;
 use PHPUnit_Framework_MockObject_MockObject;
@@ -72,17 +73,26 @@ protected function setUp(): void
      */
     public function testOnClearEventListenerIsDetachedOnCleanup(): void
     {
+        $eventListenerHasBeenRegistered = false;
+
         $this
             ->mockEventManager
-            ->expects(self::at(0))
+            ->expects(self::once())
             ->method('addEventListener')
-            ->with([Events::onClear], $this->hydrator);
+            ->with([Events::onClear], $this->hydrator)
+            ->willReturnCallback(function () use (&$eventListenerHasBeenRegistered): void {
+                $this->assertFalse($eventListenerHasBeenRegistered);
+                $eventListenerHasBeenRegistered = true;
+            });
 
         $this
             ->mockEventManager
-            ->expects(self::at(1))
+            ->expects(self::once())
             ->method('removeEventListener')
-            ->with([Events::onClear], $this->hydrator);
+            ->with([Events::onClear], $this->hydrator)
+            ->willReturnCallback(function () use (&$eventListenerHasBeenRegistered): void {
+                $this->assertTrue($eventListenerHasBeenRegistered);
+            });
 
         iterator_to_array($this->hydrator->iterate($this->mockStatement, $this->mockResultMapping));
     }
@@ -92,18 +102,63 @@ public function testOnClearEventListenerIsDetachedOnCleanup(): void
      */
     public function testHydrateAllRegistersAndClearsAllAttachedListeners(): void
     {
+        $eventListenerHasBeenRegistered = false;
+
         $this
             ->mockEventManager
-            ->expects(self::at(0))
+            ->expects(self::once())
             ->method('addEventListener')
-            ->with([Events::onClear], $this->hydrator);
+            ->with([Events::onClear], $this->hydrator)
+            ->willReturnCallback(function () use (&$eventListenerHasBeenRegistered): void {
+                $this->assertFalse($eventListenerHasBeenRegistered);
+                $eventListenerHasBeenRegistered = true;
+            });
 
         $this
             ->mockEventManager
-            ->expects(self::at(1))
+            ->expects(self::once())
             ->method('removeEventListener')
-            ->with([Events::onClear], $this->hydrator);
+            ->with([Events::onClear], $this->hydrator)
+            ->willReturnCallback(function () use (&$eventListenerHasBeenRegistered): void {
+                $this->assertTrue($eventListenerHasBeenRegistered);
+            });
+
+        $this->hydrator->hydrateAll($this->mockStatement, $this->mockResultMapping);
+    }
+
+    /**
+     * @group #8482
+     */
+    public function testHydrateAllClearsAllAttachedListenersEvenOnError(): void
+    {
+        $eventListenerHasBeenRegistered = false;
+
+        $this
+            ->mockEventManager
+            ->expects(self::once())
+            ->method('addEventListener')
+            ->with([Events::onClear], $this->hydrator)
+            ->willReturnCallback(function () use (&$eventListenerHasBeenRegistered): void {
+                $this->assertFalse($eventListenerHasBeenRegistered);
+                $eventListenerHasBeenRegistered = true;
+            });
+
+        $this
+            ->mockEventManager
+            ->expects(self::once())
+            ->method('removeEventListener')
+            ->with([Events::onClear], $this->hydrator)
+            ->willReturnCallback(function () use (&$eventListenerHasBeenRegistered): void {
+                $this->assertTrue($eventListenerHasBeenRegistered);
+            });
+
+        $this
+            ->hydrator
+            ->expects(self::once())
+            ->method('hydrateAllData')
+            ->willThrowException(new ORMException());
 
+        $this->expectException(ORMException::class);
         $this->hydrator->hydrateAll($this->mockStatement, $this->mockResultMapping);
     }
 }