Merge pull request #10968 from greg0ire/reject-id-collisions-unconditionally

Reject ID collisions in identity map unconditionally

Author: greg0ire

UPGRADE.md

@@ -1,5 +1,11 @@
 # Upgrade to 3.0
 
+## BC BREAK: collisions in identity map are unconditionally rejected
+
+`Doctrine\ORM\Configuration::setRejectIdCollisionInIdentityMap()` and
+`Doctrine\ORM\Configuration::isRejectIdCollisionInIdentityMapEnabled()` are now
+no-ops and will be deprecated in 3.1.0.
+
 ## BC BREAK: Lifecycle callback mapping on embedded classes is now explicitly forbidden
 
 Lifecycle callback mapping on embedded classes produced no effect, and is now

lib/Doctrine/ORM/Configuration.php

@@ -605,13 +605,24 @@ public function setLazyGhostObjectEnabled(bool $flag): void
         $this->attributes['isLazyGhostObjectEnabled'] = $flag;
     }
 
+    /** To be deprecated in 3.1.0 */
     public function setRejectIdCollisionInIdentityMap(bool $flag): void
     {
-        $this->attributes['rejectIdCollisionInIdentityMap'] = $flag;
+        if (! $flag) {
+            throw new LogicException(<<<'EXCEPTION'
+                Rejecting ID collisions in the identity map cannot be disabled anymore.
+                Please remove the call to setRejectIdCollisionInIdentityMap(false).
+                EXCEPTION);
+        }
     }
 
+    /**
+     * To be deprecated in 3.1.0
+     *
+     * @return true
+     */
     public function isRejectIdCollisionInIdentityMapEnabled(): bool
     {
-        return $this->attributes['rejectIdCollisionInIdentityMap'] ?? false;
+        return true;
     }
 }

lib/Doctrine/ORM/UnitOfWork.php

@@ -1485,38 +1485,7 @@ public function addToIdentityMap(object $entity): bool
 
         if (isset($this->identityMap[$className][$idHash])) {
             if ($this->identityMap[$className][$idHash] !== $entity) {
-                if ($this->em->getConfiguration()->isRejectIdCollisionInIdentityMapEnabled()) {
-                    throw EntityIdentityCollisionException::create($this->identityMap[$className][$idHash], $entity, $idHash);
-                }
-
-                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]),
-                );
+                throw EntityIdentityCollisionException::create($this->identityMap[$className][$idHash], $entity, $idHash);
             }
 
             return false;

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

@@ -1079,8 +1079,6 @@ 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

@@ -628,28 +628,8 @@ 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.