Skip to content

Commit

Permalink
Avoid returning null from EntityManager::getReference() and `getPar…
Browse files Browse the repository at this point in the history 
…tialReference()`
  • Loading branch information
@simPod
simPod committed Nov 20, 2021
1 parent 203cd6e commit a4f9c16
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 23 deletions.
4 changes: 4 additions & 0 deletions UPGRADE.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ Because of that, functionality that aims to do so has been deprecated:
* `Configuration::ensureProductionSettings()`
* the `orm:ensure-production-settings` console command

## 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 `TABLE` id generator strategy
Expand Down
13 changes: 11 additions & 2 deletions lib/Doctrine/ORM/EntityManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,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;
Expand Down Expand Up @@ -529,7 +530,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) {
Expand All @@ -554,7 +559,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)) {
Expand Down
8 changes: 4 additions & 4 deletions lib/Doctrine/ORM/EntityManagerInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,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
*
Expand All @@ -201,8 +201,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
*/
Expand Down
19 changes: 19 additions & 0 deletions lib/Doctrine/ORM/Exception/InstanceOfTheWrongTypeEncountered.php
View file
Original file line number Diff line number Diff line change
@@ -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 ');
}
}
104 changes: 92 additions & 12 deletions phpstan-baseline.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,22 +156,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

Expand Down Expand Up @@ -220,16 +215,31 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSqlPlatform referenced with incorrect case\\: Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSQLPlatform\\.$#"
count: 1
path: lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php

-
message: "#^Parameter \\#2 \\$discrMap of static method Doctrine\\\\ORM\\\\Internal\\\\Hydration\\\\HydrationException\\:\\:invalidDiscriminatorValue\\(\\) expects array\\<string, string\\>, array\\<int, int\\|string\\> given\\.$#"
count: 1
path: lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSqlPlatform referenced with incorrect case\\: Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSQLPlatform\\.$#"
count: 1
path: lib/Doctrine/ORM/Internal/SQLResultCasing.php

-
message: "#^Call to an undefined method Doctrine\\\\Common\\\\Collections\\\\Collection\\<\\(int\\|string\\), mixed\\>\\:\\:matching\\(\\)\\.$#"
count: 1
path: lib/Doctrine/ORM/LazyCriteriaCollection.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSqlPlatform referenced with incorrect case\\: Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSQLPlatform\\.$#"
count: 1
path: lib/Doctrine/ORM/Mapping/AnsiQuoteStrategy.php

-
message: "#^Access to an undefined property Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\:\\:\\$cache\\.$#"
count: 2
Expand Down Expand Up @@ -400,6 +410,11 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSqlPlatform referenced with incorrect case\\: Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSQLPlatform\\.$#"
count: 1
path: lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

-
message: "#^If condition is always true\\.$#"
count: 2
Expand Down Expand Up @@ -521,12 +536,12 @@ parameters:
path: lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

-
message: "#^Array \\(array\\('name' \\=\\> string, 'schema' \\=\\> string, 'indexes' \\=\\> array, 'uniqueConstraints' \\=\\> array\\)\\) does not accept key 'options'\\.$#"
message: "#^Array \\(array\\<string, array\\|string\\>\\) does not accept key 'options'\\.$#"
count: 1
path: lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php

-
message: "#^Array \\(array\\('name' \\=\\> string, 'schema' \\=\\> string, 'indexes' \\=\\> array, 'uniqueConstraints' \\=\\> array\\)\\) does not accept key 'quoted'\\.$#"
message: "#^Array \\(array\\<string, array\\|string\\>\\) does not accept key 'quoted'\\.$#"
count: 2
path: lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php

Expand Down Expand Up @@ -570,6 +585,11 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Mapping/DefaultEntityListenerResolver.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSqlPlatform referenced with incorrect case\\: Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSQLPlatform\\.$#"
count: 1
path: lib/Doctrine/ORM/Mapping/DefaultQuoteStrategy.php

-
message: "#^Access to an undefined property Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<T of object\\>\\:\\:\\$inheritanceType\\.$#"
count: 1
Expand Down Expand Up @@ -1115,6 +1135,11 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php

-
message: "#^Attribute class ReturnTypeWillChange does not exist\\.$#"
count: 2
path: lib/Doctrine/ORM/Mapping/ReflectionEmbeddedProperty.php

-
message: "#^Call to function is_int\\(\\) with string will always evaluate to false\\.$#"
count: 1
Expand Down Expand Up @@ -1185,6 +1210,11 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Persisters/Entity/CachedPersisterContext.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSqlPlatform referenced with incorrect case\\: Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSQLPlatform\\.$#"
count: 1
path: lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php

-
message: "#^If condition is always true\\.$#"
count: 1
Expand All @@ -1200,6 +1230,11 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSqlPlatform referenced with incorrect case\\: Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSQLPlatform\\.$#"
count: 1
path: lib/Doctrine/ORM/Persisters/Entity/SingleTablePersister.php

-
message: "#^Access to an undefined property Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\:\\:\\$isEmbeddedClass\\.$#"
count: 1
Expand Down Expand Up @@ -1381,7 +1416,7 @@ parameters:
path: lib/Doctrine/ORM/Query/FilterCollection.php

-
message: "#^Array \\(array\\<int, Doctrine\\\\ORM\\\\Query\\\\AST\\\\SelectExpression\\>\\) does not accept key non\\-empty\\-string\\.$#"
message: "#^Array \\(array\\<int, Doctrine\\\\ORM\\\\Query\\\\AST\\\\SelectExpression\\>\\) does not accept key string\\.$#"
count: 1
path: lib/Doctrine/ORM/Query/Parser.php

Expand Down Expand Up @@ -1426,11 +1461,21 @@ parameters:
count: 4
path: lib/Doctrine/ORM/Query/Parser.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSqlPlatform referenced with incorrect case\\: Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSQLPlatform\\.$#"
count: 1
path: lib/Doctrine/ORM/Query/ResultSetMappingBuilder.php

-
message: "#^Parameter \\#2 \\$class of static method Doctrine\\\\ORM\\\\Utility\\\\PersisterHelper\\:\\:getTypeOfColumn\\(\\) expects Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadata, Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataInfo given\\.$#"
count: 1
path: lib/Doctrine/ORM/Query/ResultSetMappingBuilder.php

-
message: "#^Parameter \\#2 \\$mode of method Doctrine\\\\ORM\\\\Query\\\\ResultSetMappingBuilder\\:\\:getColumnAliasMap\\(\\) expects 1\\|2\\|3, int given\\.$#"
count: 2
path: lib/Doctrine/ORM/Query/ResultSetMappingBuilder.php

-
message: "#^Array \\(array\\<string, array\\<int, string\\>\\|string\\>\\) does not accept key int\\.$#"
count: 1
Expand Down Expand Up @@ -1966,6 +2011,11 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Query/TreeWalkerChain.php

-
message: "#^Attribute class ReturnTypeWillChange does not exist\\.$#"
count: 9
path: lib/Doctrine/ORM/Query/TreeWalkerChainIterator.php

-
message: "#^Parameter \\#2 \\$value \\(string\\) of method Doctrine\\\\ORM\\\\Query\\\\TreeWalkerChainIterator\\:\\:offsetSet\\(\\) should be compatible with parameter \\$value \\(Doctrine\\\\ORM\\\\Query\\\\TreeWalker\\) of method ArrayAccess\\<int,Doctrine\\\\ORM\\\\Query\\\\TreeWalker\\>\\:\\:offsetSet\\(\\)$#"
count: 1
Expand Down Expand Up @@ -2011,6 +2061,11 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Tools/Console/Command/ConvertMappingCommand.php

-
message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Connection\\:\\:createSchemaManager\\(\\)\\.$#"
count: 1
path: lib/Doctrine/ORM/Tools/Console/Command/ConvertMappingCommand.php

-
message: "#^Method Doctrine\\\\ORM\\\\Tools\\\\Console\\\\Command\\\\ConvertMappingCommand\\:\\:execute\\(\\) should return int but empty return statement found\\.$#"
count: 1
Expand Down Expand Up @@ -2046,6 +2101,11 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Tools/Console/Command/MappingDescribeCommand.php

-
message: "#^Attribute class ReturnTypeWillChange does not exist\\.$#"
count: 3
path: lib/Doctrine/ORM/Tools/Console/MetadataFilter.php

-
message: "#^If condition is always true\\.$#"
count: 1
Expand Down Expand Up @@ -2116,11 +2176,26 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSqlPlatform referenced with incorrect case\\: Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSQLPlatform\\.$#"
count: 2
path: lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryOutputWalker.php

-
message: "#^Return type \\(void\\) of method Doctrine\\\\ORM\\\\Tools\\\\Pagination\\\\LimitSubqueryWalker\\:\\:walkSelectStatement\\(\\) should be compatible with return type \\(string\\) of method Doctrine\\\\ORM\\\\Query\\\\TreeWalker\\:\\:walkSelectStatement\\(\\)$#"
count: 1
path: lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php

-
message: "#^Attribute class ReturnTypeWillChange does not exist\\.$#"
count: 2
path: lib/Doctrine/ORM/Tools/Pagination/Paginator.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSqlPlatform referenced with incorrect case\\: Doctrine\\\\DBAL\\\\Platforms\\\\PostgreSQLPlatform\\.$#"
count: 1
path: lib/Doctrine/ORM/Tools/Pagination/Paginator.php

-
message: "#^Instanceof between \\*NEVER\\* and Doctrine\\\\ORM\\\\Query\\\\AST\\\\ConditionalFactor will always evaluate to false\\.$#"
count: 1
Expand All @@ -2141,6 +2216,11 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Tools/Pagination/WhereInWalker.php

-
message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Connection\\:\\:createSchemaManager\\(\\)\\.$#"
count: 1
path: lib/Doctrine/ORM/Tools/SchemaTool.php

-
message: "#^Else branch is unreachable because ternary operator condition is always true\\.$#"
count: 1
Expand Down
4 changes: 2 additions & 2 deletions psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,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>
Expand Down
11 changes: 8 additions & 3 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1041Test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\ORM\Exception\InstanceOfTheWrongTypeEncountered;
use Doctrine\Tests\Models;
use Doctrine\Tests\OrmFunctionalTestCase;

Expand All @@ -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);
Expand All @@ -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);
}
}

0 comments on commit a4f9c16

Please sign in to comment.