Turn identity map collisions from exception to deprecation notice (do…

…ctrine#10878)

In doctrine#10785, a check was added that prevents entity instances from getting into the identity map when another object for the same ID is already being tracked.

This caused regressions for users that work with application-provided IDs and expect this condition to fail with `UniqueConstraintViolationExceptions` when flushing to the database.

Thus, this PR turns the exception into a deprecation notice. Users can opt-in to the new behavior. In 3.0, the exception will be used.

Implements doctrine#10871.

UPGRADE.md

@@ -1,5 +1,19 @@
 # Upgrade to 2.16
 
+## Deprecated accepting duplicate IDs in the identity map
+
+For any given entity class and ID value, there should be only one object instance
+representing the entity. 
+
+In https://github.com/doctrine/orm/pull/10785, a check was added that will guard this
+in the identity map. The most probable cause for violations of this rule are collisions
+of application-provided IDs.
+
+In ORM 2.16.0, the check was added by throwing an exception. In ORM 2.16.1, this will be
+changed to a deprecation notice. ORM 3.0 will make it an exception again. Use
+`\Doctrine\ORM\Configuration::setRejectIdCollisionInIdentityMap()` if you want to opt-in
+to the new mode.
+
 ## Potential changes to the order in which `INSERT`s are executed
 
 In https://github.com/doctrine/orm/pull/10547, the commit order computation was improved

lib/Doctrine/ORM/Configuration.php

@@ -1117,4 +1117,14 @@ public function setLazyGhostObjectEnabled(bool $flag): void
 
         $this->_attributes['isLazyGhostObjectEnabled'] = $flag;
     }
+
+    public function setRejectIdCollisionInIdentityMap(bool $flag): void
+    {
+        $this->_attributes['rejectIdCollisionInIdentityMap'] = $flag;
+    }
+
+    public function isRejectIdCollisionInIdentityMapEnabled(): bool
+    {
+        return $this->_attributes['rejectIdCollisionInIdentityMap'] ?? false;
+    }
 }

lib/Doctrine/ORM/UnitOfWork.php

@@ -1635,8 +1635,10 @@ public function addToIdentityMap($entity)
 
         if (isset($this->identityMap[$className][$idHash])) {
             if ($this->identityMap[$className][$idHash] !== $entity) {
-                throw new RuntimeException(sprintf(
-                    <<<'EXCEPTION'
+                if ($this->em->getConfiguration()->isRejectIdCollisionInIdentityMapEnabled()) {
+                    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
@@ -1651,11 +1653,42 @@ public function addToIdentityMap($entity)
 
 Otherwise, it might be an ORM-internal inconsistency, please report it. 
 EXCEPTION
-                    ,
-                    get_class($entity),
-                    $idHash,
-                    get_class($this->identityMap[$className][$idHash])
-                ));
+                            ,
+                            get_class($entity),
+                            $idHash,
+                            get_class($this->identityMap[$className][$idHash])
+                        )
+                    );
+                } else {
+                    Deprecation::trigger(
+                        'doctrine/orm',
+                        'https://github.com/doctrine/orm/pull/10785',
+                        <<<'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 will trigger
+an exception in ORM 3.0.
+
+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.
+
+To opt-in to the new exception, call 
+\Doctrine\ORM\Configuration::setRejectIdCollisionInIdentityMap on the entity
+manager's configuration.
+EXCEPTION
+                        ,
+                        get_class($entity),
+                        $idHash,
+                        get_class($this->identityMap[$className][$idHash])
+                    );
+                }
             }
 
             return false;

tests/Doctrine/Tests/ORM/Functional/BasicFunctionalTest.php

@@ -1329,6 +1329,8 @@ public function testWrongAssociationInstance(): void
 
     public function testItThrowsWhenReferenceUsesIdAssignedByDatabase(): void
     {
+        $this->_em->getConfiguration()->setRejectIdCollisionInIdentityMap(true);
+
         $user           = new CmsUser();
         $user->name     = 'test';
         $user->username = 'test';

tests/Doctrine/Tests/ORM/UnitOfWorkTest.php

@@ -927,8 +927,28 @@ public function testRemovedEntityIsRemovedFromOneToManyCollection(): void
         self::assertEmpty($user->phonenumbers->getSnapshot());
     }
 
+    public function testItTriggersADeprecationNoticeWhenApplicationProvidedIdsCollide(): 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->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/pull/10785');
+
+        $this->_unitOfWork->persist($phone2);
+    }
+
     public function testItThrowsWhenApplicationProvidedIdsCollide(): void
     {
+        $this->_emMock->getConfiguration()->setRejectIdCollisionInIdentityMap(true);
+
         // 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.