Make EntityPersisters tell the UoW about post insert IDs early

mpdude · mpdude · commit 8bc74c624ac1 · 2023-06-23T09:08:07.000+02:00
This refactoring does two things: * We can avoid collecting the post insert IDs in a cumbersome array structure that will be returned by the EntityPersisters and processed by the UoW right after. Instead, use a more expressive API: Make the EntityPersisters tell the UoW about the IDs immediately. * IDs will be available in inserted entities a tad sooner. That may help to resolve doctrine#10735, where we can use the IDs to skip extra updates.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -1,5 +1,11 @@
 # Upgrade to 2.16
 
+## Deprecated returning post insert IDs from `EntityPersister::executeInserts()`
+
+Persisters implementing `\Doctrine\ORM\Persisters\Entity\EntityPersister` should no longer
+return an array of post insert IDs from their `::executeInserts()` method. Make the
+persister call `Doctrine\ORM\UnitOfWork::assignPostInsertId()` instead.
+
 ## Changing the way how reflection-based mapping drivers report fields, deprecated the "old" mode
 
 In ORM 3.0, a change will be made regarding how the `AttributeDriver` reports field mappings.
diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
@@ -256,10 +256,10 @@ public function getInserts()
     public function executeInserts()
     {
         if (! $this->queuedInserts) {
-            return [];
+            return;
         }
 
-        $postInsertIds  = [];
+        $uow            = $this->em->getUnitOfWork();
         $idGenerator    = $this->class->idGenerator;
         $isPostInsertId = $idGenerator->isPostInsertGenerator();
 
@@ -280,12 +280,10 @@ public function executeInserts()
             $stmt->executeStatement();
 
             if ($isPostInsertId) {
-                $generatedId     = $idGenerator->generateId($this->em, $entity);
-                $id              = [$this->class->identifier[0] => $generatedId];
-                $postInsertIds[] = [
-                    'generatedId' => $generatedId,
-                    'entity' => $entity,
-                ];
+                $generatedId = $idGenerator->generateId($this->em, $entity);
+                $id          = [$this->class->identifier[0] => $generatedId];
+
+                $uow->assignPostInsertId($entity, $generatedId);
             } else {
                 $id = $this->class->getIdentifierValues($entity);
             }
@@ -296,8 +294,6 @@ public function executeInserts()
         }
 
         $this->queuedInserts = [];
-
-        return $postInsertIds;
     }
 
     /**
diff --git a/lib/Doctrine/ORM/Persisters/Entity/EntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/EntityPersister.php
@@ -109,17 +109,15 @@ public function getSelectConditionStatementSQL($field, $value, $assoc = null, $c
     public function addInsert($entity);
 
     /**
-     * Executes all queued entity insertions and returns any generated post-insert
-     * identifiers that were created as a result of the insertions.
+     * Executes all queued entity insertions.
      *
      * If no inserts are queued, invoking this method is a NOOP.
      *
-     * @psalm-return list<array{
+     * @psalm-return void|list<array{
      *                   generatedId: int,
      *                   entity: object
-     *               }> An array of any generated post-insert IDs. This will be
-     *                  an empty array if the entity class does not use the
-     *                  IDENTITY generation strategy.
+     *               }> Returning an array of generated post-insert IDs is deprecated, implementations
+     *                  should call UnitOfWork::assignPostInsertId() and return void.
      */
     public function executeInserts();
 
diff --git a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php
@@ -109,10 +109,10 @@ public function getOwningTable($fieldName)
     public function executeInserts()
     {
         if (! $this->queuedInserts) {
-            return [];
+            return;
         }
 
-        $postInsertIds  = [];
+        $uow            = $this->em->getUnitOfWork();
         $idGenerator    = $this->class->idGenerator;
         $isPostInsertId = $idGenerator->isPostInsertGenerator();
         $rootClass      = $this->class->name !== $this->class->rootEntityName
@@ -157,12 +157,10 @@ public function executeInserts()
             $rootTableStmt->executeStatement();
 
             if ($isPostInsertId) {
-                $generatedId     = $idGenerator->generateId($this->em, $entity);
-                $id              = [$this->class->identifier[0] => $generatedId];
-                $postInsertIds[] = [
-                    'generatedId' => $generatedId,
-                    'entity' => $entity,
-                ];
+                $generatedId = $idGenerator->generateId($this->em, $entity);
+                $id          = [$this->class->identifier[0] => $generatedId];
+
+                $uow->assignPostInsertId($entity, $generatedId);
             } else {
                 $id = $this->em->getUnitOfWork()->getEntityIdentifier($entity);
             }
@@ -194,8 +192,6 @@ public function executeInserts()
         }
 
         $this->queuedInserts = [];
-
-        return $postInsertIds;
     }
 
     /**
diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
@@ -1143,30 +1143,24 @@ private function executeInserts(ClassMetadata $class): void
 
         $postInsertIds = $persister->executeInserts();
 
-        if ($postInsertIds) {
+        if (is_array($postInsertIds)) {
+            Deprecation::trigger(
+                'doctrine/orm',
+                'https://github.com/doctrine/orm/pull/10743/',
+                'Returning post insert IDs from \Doctrine\ORM\Persisters\Entity\EntityPersister::executeInserts() is deprecated and will not be supported in Doctrine ORM 3.0. Make the persister call Doctrine\ORM\UnitOfWork::assignPostInsertId() instead.'
+            );
+
             // Persister returned post-insert IDs
             foreach ($postInsertIds as $postInsertId) {
-                $idField = $class->getSingleIdentifierFieldName();
-                $idValue = $this->convertSingleFieldIdentifierToPHPValue($class, $postInsertId['generatedId']);
-
-                $entity = $postInsertId['entity'];
-                $oid    = spl_object_id($entity);
-
-                $class->reflFields[$idField]->setValue($entity, $idValue);
-
-                $this->entityIdentifiers[$oid]            = [$idField => $idValue];
-                $this->entityStates[$oid]                 = self::STATE_MANAGED;
-                $this->originalEntityData[$oid][$idField] = $idValue;
-
-                $this->addToIdentityMap($entity);
+                $this->assignPostInsertId($postInsertId['entity'], $postInsertId['generatedId']);
             }
-        } else {
-            foreach ($insertionsForClass as $oid => $entity) {
-                if (! isset($this->entityIdentifiers[$oid])) {
-                    //entity was not added to identity map because some identifiers are foreign keys to new entities.
-                    //add it now
-                    $this->addToEntityIdentifiersAndEntityMap($class, $oid, $entity);
-                }
+        }
+
+        foreach ($insertionsForClass as $oid => $entity) {
+            if (! isset($this->entityIdentifiers[$oid])) {
+                //entity was not added to identity map because some identifiers are foreign keys to new entities.
+                //add it now
+                $this->addToEntityIdentifiersAndEntityMap($class, $oid, $entity);
             }
         }
 
@@ -3790,4 +3784,30 @@ private function normalizeIdentifier(ClassMetadata $targetClass, array $flatIden
 
         return $normalizedAssociatedId;
     }
+
+    /**
+     * Assign a post-insert generated ID to an entity
+     *
+     * This is used by EntityPersisters after they inserted entities into the database.
+     * It will place the assigned ID values in the entity's fields and start tracking
+     * the entity in the identity map.
+     *
+     * @param object $entity
+     * @param mixed  $generatedId
+     */
+    final public function assignPostInsertId($entity, $generatedId): void
+    {
+        $class   = $this->em->getClassMetadata(get_class($entity));
+        $idField = $class->getSingleIdentifierFieldName();
+        $idValue = $this->convertSingleFieldIdentifierToPHPValue($class, $generatedId);
+        $oid     = spl_object_id($entity);
+
+        $class->reflFields[$idField]->setValue($entity, $idValue);
+
+        $this->entityIdentifiers[$oid]            = [$idField => $idValue];
+        $this->entityStates[$oid]                 = self::STATE_MANAGED;
+        $this->originalEntityData[$oid][$idField] = $idValue;
+
+        $this->addToIdentityMap($entity);
+    }
 }
diff --git a/tests/Doctrine/Tests/ORM/Persisters/JoinedSubclassPersisterTest.php b/tests/Doctrine/Tests/ORM/Persisters/JoinedSubclassPersisterTest.php
