diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 37b1412f195..58bb27a8651 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1591,6 +1591,30 @@ public function addToIdentityMap($entity) $className = $classMetadata->rootEntityName; if (isset($this->identityMap[$className][$idHash])) { + if ($this->identityMap[$className][$idHash] !== $entity) { + throw new RuntimeException(sprintf( + <<<'EXCEPTION' +While adding an entity of class %s with an ID hash of "%s" to the identity map, +another object of class %s 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 use application-provided IDs and reuse ID values; +- database-provided IDs are reassigned after truncating the database without + clearing the EntityManager; +- you might have been using EntityManager#getReference() to create a reference + for a nonexistent ID that was subsequently (by the RDBMS) assigned to another + entity. + +Otherwise, it might be an ORM-internal inconsistency, please report it. +EXCEPTION + , + get_class($entity), + $idHash, + get_class($this->identityMap[$className][$idHash]) + )); + } + return false; } @@ -2811,25 +2835,20 @@ public function createEntity($className, array $data, &$hints = []) } $this->originalEntityData[$oid] = $data; + + if ($entity instanceof NotifyPropertyChanged) { + $entity->addPropertyChangedListener($this); + } } else { $entity = $this->newInstance($class); $oid = spl_object_id($entity); - - $this->entityIdentifiers[$oid] = $id; - $this->entityStates[$oid] = self::STATE_MANAGED; - $this->originalEntityData[$oid] = $data; - - $this->identityMap[$class->rootEntityName][$idHash] = $entity; + $this->registerManaged($entity, $id, $data); if (isset($hints[Query::HINT_READ_ONLY])) { $this->readOnlyObjects[$oid] = true; } } - if ($entity instanceof NotifyPropertyChanged) { - $entity->addPropertyChangedListener($this); - } - foreach ($data as $field => $value) { if (isset($class->fieldMappings[$field])) { $class->reflFields[$field]->setValue($entity, $value); @@ -2987,20 +3006,7 @@ public function createEntity($className, array $data, &$hints = []) break; } - // PERF: Inlined & optimized code from UnitOfWork#registerManaged() - $newValueOid = spl_object_id($newValue); - $this->entityIdentifiers[$newValueOid] = $associatedId; - $this->identityMap[$targetClass->rootEntityName][$relatedIdHash] = $newValue; - - if ( - $newValue instanceof NotifyPropertyChanged && - ( ! $newValue instanceof Proxy || $newValue->__isInitialized()) - ) { - $newValue->addPropertyChangedListener($this); - } - - $this->entityStates[$newValueOid] = self::STATE_MANAGED; - // make sure that when an proxy is then finally loaded, $this->originalEntityData is set also! + $this->registerManaged($newValue, $associatedId, []); break; } diff --git a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php index 8689dd5735f..f2681258018 100644 --- a/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php @@ -19,6 +19,7 @@ use Doctrine\Tests\Models\CMS\CmsUser; use Doctrine\Tests\OrmFunctionalTestCase; use InvalidArgumentException; +use RuntimeException; use function get_class; @@ -1291,4 +1292,33 @@ public function testWrongAssociationInstance(): void $this->_em->flush(); } + + public function testItThrowsWhenReferenceUsesIdAssignedByDatabase(): void + { + $user = new CmsUser(); + $user->name = 'test'; + $user->username = 'test'; + $this->_em->persist($user); + $this->_em->flush(); + + // Obtain a reference object for the next ID. This is a user error - references + // should be fetched only for existing IDs + $ref = $this->_em->getReference(CmsUser::class, $user->id + 1); + + $user2 = new CmsUser(); + $user2->name = 'test2'; + $user2->username = 'test2'; + + // Now the database will assign an ID to the $user2 entity, but that place + // in the identity map is already taken by user error. + $this->expectException(RuntimeException::class); + $this->expectExceptionMessageMatches('/another object .* was already present for the same ID/'); + + // depending on ID generation strategy, the ID may be asssigned already here + // and the entity be put in the identity map + $this->_em->persist($user2); + + // post insert IDs will be assigned during flush + $this->_em->flush(); + } } diff --git a/tests/Doctrine/Tests/ORM/Functional/ProxiesLikeEntitiesTest.php b/tests/Doctrine/Tests/ORM/Functional/ProxiesLikeEntitiesTest.php index c15306efc3a..24b61d8654e 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ProxiesLikeEntitiesTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ProxiesLikeEntitiesTest.php @@ -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 diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php index 082b40f1967..bdb1d70231a 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1238Test.php @@ -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'); diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7869Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7869Test.php index 11a5eec69e2..534bb6c1c37 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7869Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH7869Test.php @@ -55,7 +55,7 @@ public function getClassMetadata($className): ClassMetadata $uow->clear(); $uow->triggerEagerLoads(); - self::assertSame(2, $em->getClassMetadataCalls); + self::assertSame(4, $em->getClassMetadataCalls); } } diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 5e5fc29b7f9..e47af9d57df 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -41,6 +41,7 @@ use Doctrine\Tests\OrmTestCase; use Doctrine\Tests\PHPUnitCompatibility\MockBuilderCompatibilityTools; use PHPUnit\Framework\MockObject\MockObject; +use RuntimeException; use stdClass; use function assert; @@ -108,6 +109,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') @@ -923,6 +926,25 @@ public function testRemovedEntityIsRemovedFromOneToManyCollection(): void self::assertFalse($user->phonenumbers->isDirty()); self::assertEmpty($user->phonenumbers->getSnapshot()); } + + public function testItThrowsWhenApplicationProvidedIdsCollide(): void + { + // We're using application-provided IDs and assign the same ID twice + // Note this is about colliding IDs in the identity map in memory. + // Duplicate database-level IDs would be spotted when the EM is flushed. + + $phone1 = new CmsPhonenumber(); + $phone1->phonenumber = '1234'; + $this->_unitOfWork->persist($phone1); + + $phone2 = new CmsPhonenumber(); + $phone2->phonenumber = '1234'; + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessageMatches('/another object .* was already present for the same ID/'); + + $this->_unitOfWork->persist($phone2); + } } /** @Entity */