Avoid returning null from EntityManager::getReference() and `getPar…

…tialReference()`
doctrine · Dec 28, 2021 · 05d1161 · 05d1161
1 parent 3f2cc10
commit 05d1161
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 20 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -100,6 +100,10 @@ following classes and methods:
 
 Use `toIterable()` instead.
 
+## BC BREAK: `Doctrine\ORM\EntityManagerInterface#getReference()` and `getPartialReference()` does not return `null` anymore
+
+Method `Doctrine\ORM\EntityManagerInterface#getReference()` and `getPartialReference()` throws `InstanceOfTheWrongTypeEncountered` in 3.0 when different entity type is found in inheritance hierachy.
+
 # Upgrade to 2.11
 
 ## Deprecated: `Setup::registerAutoloadDirectory()`

diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php
@@ -12,6 +12,7 @@
 use Doctrine\DBAL\LockMode;
 use Doctrine\Deprecations\Deprecation;
 use Doctrine\ORM\Exception\EntityManagerClosed;
+use Doctrine\ORM\Exception\InstanceOfTheWrongTypeEncountered;
 use Doctrine\ORM\Exception\InvalidHydrationMode;
 use Doctrine\ORM\Exception\MismatchedEventManager;
 use Doctrine\ORM\Exception\MissingIdentifierField;
@@ -503,7 +504,11 @@ public function getReference($entityName, $id)
 
         // Check identity map first, if its already in there just return it.
         if ($entity !== false) {
-            return $entity instanceof $class->name ? $entity : null;
+            if (! $entity instanceof $class->name) {
+                throw InstanceOfTheWrongTypeEncountered::forInstance($entity);
+            }
+
+            return $entity;
         }
 
         if ($class->subClasses) {
@@ -528,7 +533,11 @@ public function getPartialReference($entityName, $identifier)
 
         // Check identity map first, if its already in there just return it.
         if ($entity !== false) {
-            return $entity instanceof $class->name ? $entity : null;
+            if (! $entity instanceof $class->name) {
+                throw InstanceOfTheWrongTypeEncountered::forInstance($entity);
+            }
+
+            return $entity;
         }
 
         if (! is_array($identifier)) {

diff --git a/lib/Doctrine/ORM/EntityManagerInterface.php b/lib/Doctrine/ORM/EntityManagerInterface.php
@@ -154,8 +154,8 @@ public function createQueryBuilder();
      * @param mixed  $id         The entity identifier.
      * @psalm-param class-string<T> $entityName
      *
-     * @return object|null The entity reference.
-     * @psalm-return T|null
+     * @return object The entity reference.
+     * @psalm-return T
      *
      * @throws ORMException
      *
@@ -182,8 +182,8 @@ public function getReference($entityName, $id);
      * @param mixed  $identifier The entity identifier.
      * @psalm-param class-string<T> $entityName
      *
-     * @return object|null The (partial) entity reference
-     * @psalm-return T|null
+     * @return object The (partial) entity reference
+     * @psalm-return T
      *
      * @template T
      */

diff --git a/lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php b/lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php
@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Exception;
+
+use LogicException;
+
+use function get_class;
+
+final class InstanceOfTheWrongTypeEncountered extends LogicException implements ORMException
+{
+    /** @param object $instance */
+    public static function forInstance($instance): self
+    {
+        return new self('Instance of the wrong type (' . get_class($instance) . ') was retrieved in inheritance hierachy.' .
+                        'This happens because identity map aggregates instances by rootEntityName ');
+    }
+}
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -146,22 +146,17 @@ parameters:
 			path: lib/Doctrine/ORM/EntityManager.php
 
 		-
-			message: "#^Method Doctrine\\\\ORM\\\\EntityManager\\:\\:getPartialReference\\(\\) should return T\\|null but returns object\\.$#"
-			count: 1
-			path: lib/Doctrine/ORM/EntityManager.php
-
-		-
-			message: "#^Method Doctrine\\\\ORM\\\\EntityManager\\:\\:getPartialReference\\(\\) should return T\\|null but returns object\\|null\\.$#"
-			count: 1
+			message: "#^Method Doctrine\\\\ORM\\\\EntityManager\\:\\:getPartialReference\\(\\) should return T but returns object\\.$#"
+			count: 2
 			path: lib/Doctrine/ORM/EntityManager.php
 
 		-
-			message: "#^Method Doctrine\\\\ORM\\\\EntityManager\\:\\:getReference\\(\\) should return T\\|null but returns Doctrine\\\\Common\\\\Proxy\\\\Proxy\\.$#"
+			message: "#^Method Doctrine\\\\ORM\\\\EntityManager\\:\\:getReference\\(\\) should return T but returns Doctrine\\\\Common\\\\Proxy\\\\Proxy\\.$#"
 			count: 1
 			path: lib/Doctrine/ORM/EntityManager.php
 
 		-
-			message: "#^Method Doctrine\\\\ORM\\\\EntityManager\\:\\:getReference\\(\\) should return T\\|null but returns object\\|null\\.$#"
+			message: "#^Method Doctrine\\\\ORM\\\\EntityManager\\:\\:getReference\\(\\) should return T but returns object\\.$#"
 			count: 1
 			path: lib/Doctrine/ORM/EntityManager.php
 

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -409,8 +409,8 @@
       <code>$entity</code>
       <code>$entity</code>
       <code>$entity</code>
-      <code>$entity instanceof $class-&gt;name ? $entity : null</code>
-      <code>$entity instanceof $class-&gt;name ? $entity : null</code>
+      <code>$entity</code>
+      <code>$entity</code>
       <code>$persister-&gt;load($sortedId, null, null, [], $lockMode)</code>
       <code>$persister-&gt;loadById($sortedId)</code>
       <code>$this-&gt;metadataFactory-&gt;getMetadataFor($className)</code>

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1041Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1041Test.php
@@ -4,6 +4,7 @@
 
 namespace Doctrine\Tests\ORM\Functional\Ticket;
 
+use Doctrine\ORM\Exception\InstanceOfTheWrongTypeEncountered;
 use Doctrine\Tests\Models;
 use Doctrine\Tests\OrmFunctionalTestCase;
 
@@ -18,7 +19,7 @@ protected function setUp(): void
         parent::setUp();
     }
 
-    public function testGrabWrongSubtypeReturnsNull(): void
+    public function testGrabWrongSubtypeReturnsNullOrThrows(): void
     {
         $fix = new Models\Company\CompanyFixContract();
         $fix->setFixPrice(2000);
@@ -29,7 +30,11 @@ public function testGrabWrongSubtypeReturnsNull(): void
         $id = $fix->getId();
 
         self::assertNull($this->_em->find(Models\Company\CompanyFlexContract::class, $id));
-        self::assertNull($this->_em->getReference(Models\Company\CompanyFlexContract::class, $id));
-        self::assertNull($this->_em->getPartialReference(Models\Company\CompanyFlexContract::class, $id));
+
+        $this->expectException(InstanceOfTheWrongTypeEncountered::class);
+        $this->_em->getReference(Models\Company\CompanyFlexContract::class, $id);
+
+        $this->expectException(InstanceOfTheWrongTypeEncountered::class);
+        $this->_em->getPartialReference(Models\Company\CompanyFlexContract::class, $id);
     }
 }