From 92b5ac00bedc1021f2ec64087a42776705564ffa Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Mon, 16 Aug 2021 10:27:14 +0200 Subject: [PATCH] Avoid returning null from `EntityManager::getReference()` and `getPartialReference()` --- UPGRADE.md | 4 ++++ lib/Doctrine/ORM/EntityManager.php | 13 +++++++++++-- lib/Doctrine/ORM/EntityManagerInterface.php | 6 +++--- .../InstanceOfTheWrongTypeEncountered.php | 17 +++++++++++++++++ phpstan-baseline.neon | 4 ++-- psalm-baseline.xml | 4 ++-- .../Tests/ORM/Functional/Ticket/DDC1041Test.php | 11 ++++++++--- 7 files changed, 47 insertions(+), 12 deletions(-) create mode 100644 lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php diff --git a/UPGRADE.md b/UPGRADE.md index ead83b26ae8..3d724c958f0 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -21,6 +21,10 @@ Signatures of overridden methods should be changed accordingly Method `Doctrine\ORM\EntityManagerInterface#copy()` never got its implementation and is removed in 3.0. +## 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.10 ## BC Break: Removed possibility to extend the doctrine mapping xml schema with anything diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index b5a7d839b94..478c5b168ec 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -14,6 +14,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; @@ -528,7 +529,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) { @@ -553,7 +558,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 index 0be30ecd3f5..f4eb59a0d64 100644 --- a/lib/Doctrine/ORM/EntityManagerInterface.php +++ b/lib/Doctrine/ORM/EntityManagerInterface.php @@ -170,8 +170,8 @@ public function createQueryBuilder(); * @param mixed $id The entity identifier. * @psalm-param class-string $entityName * - * @return object|null The entity reference. - * @psalm-return ?T + * @return object The entity reference. + * @psalm-return T * * @throws ORMException * @@ -197,7 +197,7 @@ public function getReference($entityName, $id); * @param string $entityName The name of the entity type. * @param mixed $identifier The entity identifier. * - * @return object|null The (partial) entity reference. + * @return object The (partial) entity reference. */ public function getPartialReference($entityName, $identifier); diff --git a/lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php b/lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php new file mode 100644 index 00000000000..3ebebc38d9d --- /dev/null +++ b/lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php @@ -0,0 +1,17 @@ +$entity $entity $entity - $entity instanceof $class->name ? $entity : null + $entity $persister->load($sortedId, null, null, [], $lockMode) $persister->loadById($sortedId) $this->metadataFactory->getMetadataFor($className) - ?T + T getClassMetadata getReference diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1041Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1041Test.php index 7769a883882..2dd1b1dd6bb 100644 --- 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); } }