From d76fc4ebf61ce09c1dd759fa22ee9f4f72d336cb Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 8 May 2023 16:02:12 +0000 Subject: [PATCH] Compute entity-level commit order for entity insertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the third step to break https://github.com/doctrine/orm/pull/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. --- lib/Doctrine/ORM/UnitOfWork.php | 47 +++++++++++++++---- ...OneToOneSelfReferentialAssociationTest.php | 6 ++- .../Doctrine/Tests/OrmFunctionalTestCase.php | 1 + 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index d7f563c41ee..83bf461dc39 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1266,20 +1266,51 @@ private function executeDeletions(array $entities): void /** @return list */ 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 */ diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneSelfReferentialAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneSelfReferentialAssociationTest.php index 6c211aa603c..9cefa18ab7c 100644 --- a/tests/Doctrine/Tests/ORM/Functional/OneToOneSelfReferentialAssociationTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneSelfReferentialAssociationTest.php @@ -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); diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index c2054179d6f..b024f09f553 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -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');