Add a safeguard against multiple objects competing for the same identity map entry

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;
                     }
 

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();
+    }
 }

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

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');
 

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);
     }
 }
 

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 */