From 8bc74c624ac13ce3430b8704be249d13952e68fb Mon Sep 17 00:00:00 2001 From: Matthias Pigulla 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 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 @@ -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()); - } -}