Skip to content

Commit

Permalink
Compute entity-level commit order for entity insertions
Browse files Browse the repository at this point in the history
This is the third step to break #10547 into smaller PRs suitable for reviewing. It uses the new topological sort implementation from #10592 and the refactoring from #10651 to compute the UoW's commit order for entity insertions not on the entity class level, but for single entities and their actual dependencies instead.

 #### Current situation

`UnitOfWork::getCommitOrder()` would compute the entity sequence on the class level with the following code:

https://github.com/doctrine/orm/blob/70477d81e96c0044ad6fd8c13c37b2270d082792/lib/Doctrine/ORM/UnitOfWork.php#L1310-L1325

 #### Suggested change

* Instead of considering the classes of all entities that need to be inserted, updated or deleted, consider the new (inserted) entities only. We only need to find a sequence in situations where there are foreign key relationships between two _new_ entities.
* In the dependency graph, add edges for all to-one association target entities.
* Make edges "optional" when the association is nullable.

 #### Test changes

I have not tried to fully understand the few changes necessary to fix the tests. My guess is that those are edge cases where the insert order changed and we need to consider this during clean-up.

Keep in mind that many of the functional tests we have assume that entities have IDs assigned in the order that they were added to the EntityManager. That does not change – so the order of entities is generally stable, equal to the previous implementation.
  • Loading branch information
mpdude committed May 31, 2023
1 parent ae60cf0 commit d76fc4e
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 10 deletions.
47 changes: 39 additions & 8 deletions lib/Doctrine/ORM/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -1266,20 +1266,51 @@ private function executeDeletions(array $entities): void
/** @return list<object> */
private function computeInsertExecutionOrder(): array
{
$commitOrder = $this->getCommitOrder();
$result = [];
foreach ($commitOrder as $class) {
$className = $class->name;
foreach ($this->entityInsertions as $entity) {
if ($this->em->getClassMetadata(get_class($entity))->name !== $className) {
$sort = new TopologicalSort();

// First make sure we have all the nodes
foreach ($this->entityInsertions as $entity) {
$sort->addNode($entity);
}

// Now add edges
foreach ($this->entityInsertions as $entity) {
$class = $this->em->getClassMetadata(get_class($entity));

foreach ($class->associationMappings as $assoc) {
// We only need to consider the owning sides of to-one associations,
// since many-to-many associations are persisted at a later step and
// have no insertion order problems (all entities already in the database
// at that time).
if (! ($assoc['isOwningSide'] && $assoc['type'] & ClassMetadata::TO_ONE)) {
continue;
}

$targetEntity = $class->getFieldValue($entity, $assoc['fieldName']);

// If there is no entity that we need to refer to, or it is already in the
// database (i. e. does not have to be inserted), no need to consider it.
if ($targetEntity === null || ! $sort->hasNode($targetEntity)) {
continue;
}

$result[] = $entity;
// According to https://www.doctrine-project.org/projects/doctrine-orm/en/2.14/reference/annotations-reference.html#annref_joincolumn,
// the default for "nullable" is true. Unfortunately, it seems this default is not applied at the metadata driver, factory or other
// level, but in fact we may have an undefined 'nullable' key here, so we must assume that default here as well.
//
// Same in \Doctrine\ORM\Tools\EntityGenerator::isAssociationIsNullable or \Doctrine\ORM\Persisters\Entity\BasicEntityPersister::getJoinSQLForJoinColumns,
// to give two examples.
assert(isset($assoc['joinColumns']));
$joinColumns = reset($assoc['joinColumns']);
$isNullable = ! isset($joinColumns['nullable']) || $joinColumns['nullable'];

// Add dependency. The dependency direction implies that "$targetEntity has to go before $entity",
// so we can work through the topo sort result from left to right (with all edges pointing right).
$sort->addEdge($targetEntity, $entity, $isNullable);
}
}

return $result;
return $sort->sort();
}

/** @return list<object> */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,11 @@ public function testFind(): void

public function testEagerLoadsAssociation(): void
{
$this->createFixture();
$customerId = $this->createFixture();

$query = $this->_em->createQuery('select c, m from Doctrine\Tests\Models\ECommerce\ECommerceCustomer c left join c.mentor m where c.id = :id');
$query->setParameter('id', $customerId);

$query = $this->_em->createQuery('select c, m from Doctrine\Tests\Models\ECommerce\ECommerceCustomer c left join c.mentor m order by c.id asc');
$result = $query->getResult();
$customer = $result[0];
$this->assertLoadingOfAssociation($customer);
Expand Down
1 change: 1 addition & 0 deletions tests/Doctrine/Tests/OrmFunctionalTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@ protected function tearDown(): void
$conn->executeStatement('DELETE FROM ecommerce_products_categories');
$conn->executeStatement('DELETE FROM ecommerce_products_related');
$conn->executeStatement('DELETE FROM ecommerce_carts');
$conn->executeStatement('DELETE FROM ecommerce_customers WHERE mentor_id IS NOT NULL');
$conn->executeStatement('DELETE FROM ecommerce_customers');
$conn->executeStatement('DELETE FROM ecommerce_features');
$conn->executeStatement('DELETE FROM ecommerce_products');
Expand Down

0 comments on commit d76fc4e

Please sign in to comment.