From 7971a5316426e8408be8fa13da637cc32a51fede Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Ols=CC=8Cavsky=CC=81?= <molsavsky1@gmail.com>
Date: Thu, 18 Feb 2021 09:13:40 +0100
Subject: [PATCH] Method hydrateAll() does not take into account possible
 exception from hydrateAllData() which in turn does not call cleanup()

---
 .../Internal/Hydration/AbstractHydrator.php   |  9 +--
 .../ORM/Hydration/AbstractHydratorTest.php    | 71 ++++++++++++++++---
 2 files changed, 68 insertions(+), 12 deletions(-)

diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php
index e1f27dcb050..5e1cef960bb 100644
--- 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
index 00aefa398d9..c995361e96b 100644
--- 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);
     }
 }