Skip to content

Commit

Permalink
Add a safeguard against multiple objects competing for the same ident…
Browse files Browse the repository at this point in the history
…ity map entry

While trying to understand #3037, I found that it may happen that we have more entries in `\Doctrine\ORM\UnitOfWork::$entityIdentifiers` than in `\Doctrine\ORM\UnitOfWork::$identityMap`.

The former is a mapping from `spl_object_id` values to ID hashes, the latter an array first of entity class names and then from ID hash to entity object instances.

(Basically, "ID hash" is a concatenation of all field values making up the `@Id` for a given entity.)

This means that at some point, we must have _different_ objects representing the same entity.

I don't think it makes sense to overwrite an entity in the identity map, since that may leave `\Doctrine\ORM\UnitOfWork::$entityIdentifiers` in an inconsistent state.

If it makes sense at all to _replace_ an entity, that should happen through dedicated management methods to first detach the old entity before persisting, merging or otherwise adding the new one. This way we could make sure the internal structures remain consistent.
  • Loading branch information
mpdude committed Jun 22, 2023
1 parent 41f704c commit 8cd6a40
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 2 deletions.
8 changes: 8 additions & 0 deletions lib/Doctrine/ORM/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -1569,6 +1569,14 @@ public function addToIdentityMap($entity)
$className = $classMetadata->rootEntityName;

if (isset($this->identityMap[$className][$idHash])) {
if ($this->identityMap[$className][$idHash] !== $entity) {
throw new RuntimeException(sprintf(
'While adding an entity of class %s with an ID hash of "%s" to the identity map, another object was already present for the same ID. This exception is a safeguard against an internal inconsistency - IDs should uniquely map to entity object instances. This problem may occur if you create proxy instances directly using the ProxyFactory, instead of using \Doctrine\ORM\EntityManagerInterface::getReference(). Otherwise, it might be an ORM-internal inconsistency, please report it.',
$className,
$idHash
));
}

return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public function testEntityWithIdentifier(): void
*/
public function testProxyAsDqlParameterPersist(): void
{
$proxy = $this->_em->getProxyFactory()->getProxy(CmsUser::class, ['id' => $this->user->getId()]);
$proxy = $this->_em->getReference(CmsUser::class, ['id' => $this->user->getId()]);
$proxy->id = $this->user->getId();
$result = $this
->_em
Expand Down
2 changes: 1 addition & 1 deletion tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function testIssueProxyClear(): void

$user2 = $this->_em->getReference(DDC1238User::class, $userId);

$user->__load();
//$user->__load();

self::assertIsInt($user->getId(), 'Even if a proxy is detached, it should still have an identifier');

Expand Down
2 changes: 2 additions & 0 deletions tests/Doctrine/Tests/ORM/UnitOfWorkTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ protected function setUp(): void
$driverConnection = $this->createMock(Driver\Connection::class);
$driverConnection->method('prepare')
->willReturn($driverStatement);
$driverConnection->method('lastInsertId')
->willReturnOnConsecutiveCalls(1, 2, 3, 4, 5, 6);

$driver = $this->createMock(Driver::class);
$driver->method('getDatabasePlatform')
Expand Down

0 comments on commit 8cd6a40

Please sign in to comment.