From 8bc74c624ac13ce3430b8704be249d13952e68fb Mon Sep 17 00:00:00 2001
From: Matthias Pigulla <mp@webfactory.de>
Date: Fri, 2 Jun 2023 09:59:44 +0000
Subject: [PATCH] Make EntityPersisters tell the UoW about post insert IDs
 early

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 #10735, where we can use the IDs to skip extra updates.
---
 UPGRADE.md                                    |  6 ++
 .../Entity/BasicEntityPersister.php           | 16 ++---
 .../ORM/Persisters/Entity/EntityPersister.php | 10 ++-
 .../Entity/JoinedSubclassPersister.php        | 16 ++---
 lib/Doctrine/ORM/UnitOfWork.php               | 62 ++++++++++++-------
 .../JoinedSubclassPersisterTest.php           | 38 ------------
 6 files changed, 63 insertions(+), 85 deletions(-)
 delete mode 100644 tests/Doctrine/Tests/ORM/Persisters/JoinedSubclassPersisterTest.php

diff --git a/UPGRADE.md b/UPGRADE.md
index a583a83fffc..bf2d9b385db 100644
--- 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
index 623c2cb61c1..1de3d203d17 100644
--- 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
index 7f9d54450bf..a69fbbdae12 100644
--- 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
index 4a84ce5786f..c872f0a0f69 100644
--- 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
index bc8af25fbff..f192bd4b680 100644
--- 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
deleted file mode 100644
index 896a35a9a62..00000000000
--- a/tests/Doctrine/Tests/ORM/Persisters/JoinedSubclassPersisterTest.php
+++ /dev/null
@@ -1,38 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Doctrine\Tests\ORM\Persisters;
-
-use Doctrine\ORM\Persisters\Entity\JoinedSubclassPersister;
-use Doctrine\Tests\Mocks\EntityManagerMock;
-use Doctrine\Tests\Models\JoinedInheritanceType\RootClass;
-use Doctrine\Tests\OrmTestCase;
-
-/**
- * Tests for {@see \Doctrine\ORM\Persisters\Entity\JoinedSubclassPersister}
- *
- * @covers \Doctrine\ORM\Persisters\Entity\JoinedSubclassPersister
- */
-class JoinedSubclassPersisterTest extends OrmTestCase
-{
-    /** @var JoinedSubclassPersister */
-    protected $persister;
-
-    /** @var EntityManagerMock */
-    protected $em;
-
-    protected function setUp(): void
-    {
-        parent::setUp();
-
-        $this->em        = $this->getTestEntityManager();
-        $this->persister = new JoinedSubclassPersister($this->em, $this->em->getClassMetadata(RootClass::class));
-    }
-
-    /** @group DDC-3470 */
-    public function testExecuteInsertsWillReturnEmptySetWithNoQueuedInserts(): void
-    {
-        self::assertSame([], $this->persister->executeInserts());
-    }
-}