From 5599b3fc8371e5739959d1db88b705eb963a351e 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 | 4 ++-- .../InstanceOfTheWrongTypeEncountered.php | 19 +++++++++++++++++++ psalm-baseline.xml | 4 ++-- .../ORM/Functional/Ticket/DDC1041Test.php | 15 ++++++++++++--- 6 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php diff --git a/UPGRADE.md b/UPGRADE.md index 4946a9302d7..fad505318e7 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -601,6 +601,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.16 ## Changing the way how reflection-based mapping drivers report fields, deprecated the "old" mode diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index fdabf133b5d..cc3fa0ea5cb 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -11,6 +11,7 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\LockMode; use Doctrine\ORM\Exception\EntityManagerClosed; +use Doctrine\ORM\Exception\InstanceOfTheWrongTypeEncountered; use Doctrine\ORM\Exception\InvalidHydrationMode; use Doctrine\ORM\Exception\MissingIdentifierField; use Doctrine\ORM\Exception\MissingMappingDriverImplementation; @@ -386,7 +387,11 @@ public function getReference(string $entityName, mixed $id): object|null // 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) { @@ -408,7 +413,11 @@ public function getPartialReference(string $entityName, mixed $identifier): obje // 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 c0e3492fad5..8039e25de10 100644 --- a/lib/Doctrine/ORM/EntityManagerInterface.php +++ b/lib/Doctrine/ORM/EntityManagerInterface.php @@ -155,7 +155,7 @@ public function refresh(object $object, LockMode|int|null $lockMode = null): voi * @param mixed $id The entity identifier. * @psalm-param class-string $entityName * - * @psalm-return T|null + * @psalm-return T * * @throws ORMException * @@ -182,7 +182,7 @@ public function getReference(string $entityName, mixed $id): object|null; * @param mixed $identifier The entity identifier. * @psalm-param class-string $entityName * - * @psalm-return T|null + * @psalm-return T * * @template T of object */ diff --git a/lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php b/lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php new file mode 100644 index 00000000000..02eafa238d4 --- /dev/null +++ b/lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php @@ -0,0 +1,19 @@ +$entity $entity $entity - name ? $entity : null]]> - name ? $entity : null]]> + + load($sortedId, null, null, [], $lockMode)]]> loadById($sortedId)]]> metadataFactory]]> diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1041Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1041Test.php index 07e41044a29..8e89ecc9731 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\Company\CompanyFixContract; use Doctrine\Tests\Models\Company\CompanyFlexContract; use Doctrine\Tests\OrmFunctionalTestCase; @@ -19,7 +20,7 @@ protected function setUp(): void parent::setUp(); } - public function testGrabWrongSubtypeReturnsNull(): void + public function testGrabWrongSubtypeReturnsNullOrThrows(): void { $fix = new CompanyFixContract(); $fix->setFixPrice(2000); @@ -30,7 +31,15 @@ public function testGrabWrongSubtypeReturnsNull(): void $id = $fix->getId(); self::assertNull($this->_em->find(CompanyFlexContract::class, $id)); - self::assertNull($this->_em->getReference(CompanyFlexContract::class, $id)); - self::assertNull($this->_em->getPartialReference(CompanyFlexContract::class, $id)); + + try { + $this->_em->getReference(CompanyFlexContract::class, $id); + } catch (InstanceOfTheWrongTypeEncountered) { + } + + try { + $this->_em->getPartialReference(CompanyFlexContract::class, $id); + } catch (InstanceOfTheWrongTypeEncountered) { + } } }