Skip to content

Commit

Permalink
Prepare entity-level commit order computation in the UnitOfWork
Browse files Browse the repository at this point in the history 
This is the second chunk to break #10547 into smaller PRs suitable for reviewing. It prepares the `UnitOfWork` to work with a commit order computed on the entity level, as opposed to a class-based ordering as before.

#### Background

#10531 and #10532 show that it is not always possible to run `UnitOfWork::commit()` with a commit order given in terms of  entity _classes_. Instead it is necessary to process entities in an order computed on the _object_ level. That may include

* a particular order for multiple entities of the _same_ class
* a particular order for entities of _different_ classes, possibly even going back and forth (entity `$a1` of class `A`, then `$b` of class `B`, then `$a2` of class `A` – revisiting that class).

This PR is about preparing the `UnitOfWork` so that its methods will be able to perform inserts and deletions on that level of granularity. Subsequent PRs will deal with implementing that particular order computation.

#### Suggested change

Change the private `executeInserts` and `executeDeletions` methods so that they do not take a `ClassMetadata` identifying the class of entities that shall be processed, but pass them the list of entities directly.

The lists of entities are built in two dedicated methods. That happens basically as previously, by iterating over `$this->entityInsertions` or `$this->entityDeletions` and filtering those by class.

#### Potential (BC breaking?) changes that need review scrutiny

* `\Doctrine\ORM\Persisters\Entity\EntityPersister::addInsert()` was previously called multiple times, before the insert would be performed by a call to `\Doctrine\ORM\Persisters\Entity\EntityPersister::executeInserts()`. With the changes here, this batching effectively no longer takes place. `executeInserts()` will always see one entity/insert only, and there may be multiple `executeInserts()` calls during a single `UoW::commit()` phase.
* The caching in `\Doctrine\ORM\Cache\Persister\Entity\AbstractEntityPersister` previously would cache entities from the last executed insert batch only. Now it will collect entities across multiple batches. I don't know if that poses a problem.
* Overhead in `\Doctrine\ORM\Persisters\Entity\BasicEntityPersister::executeInserts` is incurred multiple times; that may, however, only be about SQL statement preparation and might be salvageable.
* The `postPersist` event previously was not dispatched before _all_ instances of a particular entity class had been written to the database. Now, it will be dispatched immediately after every single entity that has been inserted.
  • Loading branch information
@mpdude
mpdude committed Apr 24, 2023
1 parent 33a19f1 commit b42cf99
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 76 deletions.
9 changes: 8 additions & 1 deletion lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use Doctrine\ORM\Persisters\Entity\EntityPersister;
use Doctrine\ORM\UnitOfWork;

use function array_merge;
use function assert;
use function serialize;
use function sha1;
Expand Down Expand Up @@ -314,7 +315,13 @@ public function getOwningTable($fieldName)
*/
public function executeInserts()
{
$this->queuedCache['insert'] = $this->persister->getInserts();
// The commit order/foreign key relationships may make it necessary that multiple calls to executeInsert()
// are performed, so collect all the new entities.
$newInserts = $this->persister->getInserts();

if ($newInserts) {
$this->queuedCache['insert'] = array_merge($this->queuedCache['insert'] ?? [], $newInserts);
}

return $this->persister->executeInserts();
}
Expand Down
180 changes: 109 additions & 71 deletions lib/Doctrine/ORM/UnitOfWork.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@
use function array_map;
use function array_merge;
use function array_pop;
use function array_reverse;
use function array_sum;
use function array_values;
use function assert;
use function count;
use function current;
use function func_get_arg;
use function func_num_args;
Expand Down Expand Up @@ -426,32 +426,37 @@ public function commit($entity = null)
}

if ($this->entityInsertions) {
foreach ($commitOrder as $class) {
$this->executeInserts($class);
}
// Perform entity insertions first, so that all new entities have their rows in the database
// and can be referred to by foreign keys. The commit order only needs to take new entities
// into account (new entities referring to other new entities), since all other types (entities
// with updates or scheduled deletions) are currently not a problem, since they are already
// in the database.
$this->executeInserts($this->computeInsertExecutionOrder($commitOrder));
}

if ($this->entityUpdates) {
foreach ($commitOrder as $class) {
$this->executeUpdates($class);
}
// Updates do not need to follow a particular order
$this->executeUpdates();
}

// Extra updates that were requested by persisters.
// This may include foreign keys that could not be set when an entity was inserted,
// which may happen in the case of circular foreign key relationships.
if ($this->extraUpdates) {
$this->executeExtraUpdates();
}

// Collection updates (deleteRows, updateRows, insertRows)
// No particular order is necessary, since all entities themselves are already
// in the database
foreach ($this->collectionUpdates as $collectionToUpdate) {
$this->getCollectionPersister($collectionToUpdate->getMapping())->update($collectionToUpdate);
}

// Entity deletions come last and need to be in reverse commit order
// Entity deletions come last. Their order only needs to take care of other deletions
// (first delete entities depending upon others, before deleting depended-upon entities).
if ($this->entityDeletions) {
for ($count = count($commitOrder), $i = $count - 1; $i >= 0 && $this->entityDeletions; --$i) {
$this->executeDeletions($commitOrder[$i]);
}
$this->executeDeletions($this->computeDeleteExecutionOrder($commitOrder));
}

// Commit failed silently
Expand Down Expand Up @@ -1114,64 +1119,52 @@ public function recomputeSingleEntityChangeSet(ClassMetadata $class, $entity)
}

/**
* Executes all entity insertions for entities of the specified type.
* Executes entity insertions in the given order
*
* @param list<object> $entities
*/
private function executeInserts(ClassMetadata $class): void
private function executeInserts(array $entities): void
{
$entities = [];
$className = $class->name;
$persister = $this->getEntityPersister($className);
$invoke = $this->listenersInvoker->getSubscribedSystems($class, Events::postPersist);

$insertionsForClass = [];

foreach ($this->entityInsertions as $oid => $entity) {
if ($this->em->getClassMetadata(get_class($entity))->name !== $className) {
continue;
}

$insertionsForClass[$oid] = $entity;
foreach ($entities as $entity) {
$oid = spl_object_id($entity);
$class = $this->em->getClassMetadata(get_class($entity));
$persister = $this->getEntityPersister($class->name);
$invoke = $this->listenersInvoker->getSubscribedSystems($class, Events::postPersist);

$persister->addInsert($entity);

unset($this->entityInsertions[$oid]);

if ($invoke !== ListenersInvoker::INVOKE_NONE) {
$entities[] = $entity;
}
}

$postInsertIds = $persister->executeInserts();
$postInsertIds = $persister->executeInserts();

if ($postInsertIds) {
// Persister returned post-insert IDs
foreach ($postInsertIds as $postInsertId) {
$idField = $class->getSingleIdentifierFieldName();
$idValue = $this->convertSingleFieldIdentifierToPHPValue($class, $postInsertId['generatedId']);
if ($postInsertIds) {
// 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);
$entity = $postInsertId['entity'];
$oid = spl_object_id($entity);

$class->reflFields[$idField]->setValue($entity, $idValue);
$class->reflFields[$idField]->setValue($entity, $idValue);

$this->entityIdentifiers[$oid] = [$idField => $idValue];
$this->entityStates[$oid] = self::STATE_MANAGED;
$this->originalEntityData[$oid][$idField] = $idValue;
$this->entityIdentifiers[$oid] = [$idField => $idValue];
$this->entityStates[$oid] = self::STATE_MANAGED;
$this->originalEntityData[$oid][$idField] = $idValue;

$this->addToIdentityMap($entity);
}
} else {
foreach ($insertionsForClass as $oid => $entity) {
$this->addToIdentityMap($entity);
}
} else {
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 ($entities as $entity) {
$this->listenersInvoker->invoke($class, Events::postPersist, $entity, new PostPersistEventArgs($entity, $this->em), $invoke);
if ($invoke !== ListenersInvoker::INVOKE_NONE) {
$this->listenersInvoker->invoke($class, Events::postPersist, $entity, new PostPersistEventArgs($entity, $this->em), $invoke);
}
}
}

Expand Down Expand Up @@ -1209,19 +1202,15 @@ private function addToEntityIdentifiersAndEntityMap(
}

/**
* Executes all entity updates for entities of the specified type.
* Executes all entity updates
*/
private function executeUpdates(ClassMetadata $class): void
private function executeUpdates(): void
{
$className = $class->name;
$persister = $this->getEntityPersister($className);
$preUpdateInvoke = $this->listenersInvoker->getSubscribedSystems($class, Events::preUpdate);
$postUpdateInvoke = $this->listenersInvoker->getSubscribedSystems($class, Events::postUpdate);

foreach ($this->entityUpdates as $oid => $entity) {
if ($this->em->getClassMetadata(get_class($entity))->name !== $className) {
continue;
}
$class = $this->em->getClassMetadata(get_class($entity));
$persister = $this->getEntityPersister($class->name);
$preUpdateInvoke = $this->listenersInvoker->getSubscribedSystems($class, Events::preUpdate);
$postUpdateInvoke = $this->listenersInvoker->getSubscribedSystems($class, Events::postUpdate);

if ($preUpdateInvoke !== ListenersInvoker::INVOKE_NONE) {
$this->listenersInvoker->invoke($class, Events::preUpdate, $entity, new PreUpdateEventArgs($entity, $this->em, $this->getEntityChangeSet($entity)), $preUpdateInvoke);
Expand All @@ -1242,18 +1231,17 @@ private function executeUpdates(ClassMetadata $class): void
}

/**
* Executes all entity deletions for entities of the specified type.
* Executes all entity deletions
*
* @param list<object> $entities
*/
private function executeDeletions(ClassMetadata $class): void
private function executeDeletions(array $entities): void
{
$className = $class->name;
$persister = $this->getEntityPersister($className);
$invoke = $this->listenersInvoker->getSubscribedSystems($class, Events::postRemove);

foreach ($this->entityDeletions as $oid => $entity) {
if ($this->em->getClassMetadata(get_class($entity))->name !== $className) {
continue;
}
foreach ($entities as $entity) {
$oid = spl_object_id($entity);
$class = $this->em->getClassMetadata(get_class($entity));
$persister = $this->getEntityPersister($class->name);
$invoke = $this->listenersInvoker->getSubscribedSystems($class, Events::postRemove);

$persister->delete($entity);

Expand All @@ -1277,6 +1265,50 @@ private function executeDeletions(ClassMetadata $class): void
}
}

/**
* @param list<ClassMetadata> $commitOrder
*
* @return list<object>
*/
private function computeInsertExecutionOrder(array $commitOrder): array
{
$result = [];
foreach ($commitOrder as $class) {
$className = $class->name;
foreach ($this->entityInsertions as $entity) {
if ($this->em->getClassMetadata(get_class($entity))->name !== $className) {
continue;
}

$result[] = $entity;
}
}

return $result;
}

/**
* @param list<ClassMetadata> $commitOrder
*
* @return list<object>
*/
private function computeDeleteExecutionOrder(array $commitOrder): array
{
$result = [];
foreach (array_reverse($commitOrder) as $class) {
$className = $class->name;
foreach ($this->entityDeletions as $entity) {
if ($this->em->getClassMetadata(get_class($entity))->name !== $className) {
continue;
}

$result[] = $entity;
}
}

return $result;
}

/**
* Gets the commit order.
*
Expand Down Expand Up @@ -1343,7 +1375,13 @@ private function getCommitOrder(): array
}
}

return $calc->sort();
// Remove duplicate class entries
$result = [];
foreach ($calc->sort() as $classMetadata) {
$result[$classMetadata->name] = $classMetadata;
}

return array_values($result);
}

/**
Expand Down
4 changes: 0 additions & 4 deletions psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3043,10 +3043,6 @@
<PropertyTypeCoercion>
<code><![CDATA[$this->nonCascadedNewDetectedEntities]]></code>
</PropertyTypeCoercion>
<RedundantCondition>
<code><![CDATA[$i >= 0 && $this->entityDeletions]]></code>
<code><![CDATA[$this->entityDeletions]]></code>
</RedundantCondition>
<RedundantConditionGivenDocblockType>
<code>is_array($entity)</code>
</RedundantConditionGivenDocblockType>
Expand Down

0 comments on commit b42cf99

Please sign in to comment.