diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index b84fcb9a688..0fbde8bacb3 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -266,7 +266,7 @@ public function executeInserts() $stmt = $this->conn->prepare($this->getInsertSQL()); $tableName = $this->class->getTableName(); - foreach ($this->queuedInserts as $entity) { + foreach ($this->queuedInserts as $key => $entity) { $insertData = $this->prepareInsertData($entity); if (isset($insertData[$tableName])) { @@ -291,9 +291,16 @@ public function executeInserts() if ($this->class->requiresFetchAfterChange) { $this->assignDefaultVersionAndUpsertableValues($entity, $id); } - } - $this->queuedInserts = []; + // Unset this queued insert, so that the prepareUpdateData() method knows right away + // (for the next entity already) that the current entity has been written to the database + // and no extra updates need to be scheduled to refer to it. + // + // In \Doctrine\ORM\UnitOfWork::executeInserts(), the UoW already removed entities + // from its own list (\Doctrine\ORM\UnitOfWork::$entityInsertions) right after they + // were given to our addInsert() method. + unset($this->queuedInserts[$key]); + } } /** @@ -671,10 +678,30 @@ protected function prepareUpdateData($entity, bool $isInsert = false) if ($newVal !== null) { $oid = spl_object_id($newVal); - if (isset($this->queuedInserts[$oid]) || $uow->isScheduledForInsert($newVal)) { - // The associated entity $newVal is not yet persisted, so we must - // set $newVal = null, in order to insert a null value and schedule an - // extra update on the UnitOfWork. + // If the associated entity $newVal is not yet persisted and/or does not yet have + // an ID assigned, we must set $newVal = null. This will insert a null value and + // schedule an extra update on the UnitOfWork. + // + // This gives us extra time to a) possibly obtain a database-generated identifier + // value for $newVal, and b) insert $newVal into the database before the foreign + // key reference is being made. + // + // When looking at $this->queuedInserts and $uow->isScheduledForInsert, be aware + // of the implementation details that our own executeInserts() method will remove + // entities from the former as soon as the insert statement has been executed and + // a post-insert ID has been assigned (if necessary), and that the UnitOfWork has + // already removed entities from its own list at the time they were passed to our + // addInsert() method. + // + // Then, there is one extra exception we can make: An entity that references back to itself + // _and_ uses an application-provided ID (the "NONE" generator strategy) also does not + // need the extra update, although it is still in the list of insertions itself. + // This looks like a minor optimization at first, but is the capstone for being able to + // use non-NULLable, self-referencing associations in applications that provide IDs (like UUIDs). + if ( + (isset($this->queuedInserts[$oid]) || $uow->isScheduledForInsert($newVal)) + && ! ($newVal === $entity && $this->class->isIdentifierNatural()) + ) { $uow->scheduleExtraUpdate($entity, [$field => [null, $newVal]]); $newVal = null; diff --git a/tests/Doctrine/Tests/ORM/Functional/GH7877Test.php b/tests/Doctrine/Tests/ORM/Functional/GH7877Test.php new file mode 100644 index 00000000000..6461c686527 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/GH7877Test.php @@ -0,0 +1,135 @@ +createSchemaForModels( + GH7877ApplicationGeneratedIdEntity::class, + GH7877EntityWithNullableAssociation::class + ); + } + + public function testSelfReferenceWithApplicationGeneratedIdMayBeNotNullable(): void + { + $entity = new GH7877ApplicationGeneratedIdEntity(); + $entity->parent = $entity; + + $this->expectNotToPerformAssertions(); + + $this->_em->persist($entity); + $this->_em->flush(); + } + + public function testCrossReferenceWithApplicationGeneratedIdMayBeNotNullable(): void + { + $entity1 = new GH7877ApplicationGeneratedIdEntity(); + $entity1->parent = $entity1; + $entity2 = new GH7877ApplicationGeneratedIdEntity(); + $entity2->parent = $entity1; + + $this->expectNotToPerformAssertions(); + + // As long as we do not have entity-level commit order computation + // (see https://github.com/doctrine/orm/pull/10547), + // this only works when the UoW processes $entity1 before $entity2, + // so that the foreign key constraint E2 -> E1 can be satisfied. + + $this->_em->persist($entity1); + $this->_em->persist($entity2); + $this->_em->flush(); + } + + public function testNullableForeignKeysMakeInsertOrderLessRelevant(): void + { + $entity1 = new GH7877EntityWithNullableAssociation(); + $entity1->parent = $entity1; + $entity2 = new GH7877EntityWithNullableAssociation(); + $entity2->parent = $entity1; + + $this->expectNotToPerformAssertions(); + + // In contrast to the previous test, this case demonstrates that with NULLable + // associations, even without entity-level commit order computation + // (see https://github.com/doctrine/orm/pull/10547), we can get away with an + // insertion order of E2 before E1. That is because the UoW will schedule an extra + // update that saves the day - the foreign key reference will established only after + // all insertions have been performed. + + $this->_em->persist($entity2); + $this->_em->persist($entity1); + $this->_em->flush(); + } +} + +/** + * @ORM\Entity + */ +class GH7877ApplicationGeneratedIdEntity +{ + /** + * @ORM\Id + * @ORM\Column(type="string") + * @ORM\GeneratedValue(strategy="NONE") + * + * @var string + */ + public $id; + + /** + * (!) Note this uses "nullable=false" + * + * @ORM\ManyToOne(targetEntity="GH7877ApplicationGeneratedIdEntity") + * @ORM\JoinColumn(name="parent_id", referencedColumnName="id", nullable=false) + * + * @var self + */ + public $parent; + + public function __construct() + { + $this->id = uniqid(); + } +} + +/** + * @ORM\Entity + */ +class GH7877EntityWithNullableAssociation +{ + /** + * @ORM\Id + * @ORM\Column(type="string") + * @ORM\GeneratedValue(strategy="NONE") + * + * @var string + */ + public $id; + + /** + * @ORM\ManyToOne(targetEntity="GH7877EntityWithNullableAssociation") + * @ORM\JoinColumn(name="parent_id", referencedColumnName="id", nullable=true) + * + * @var self + */ + public $parent; + + public function __construct() + { + $this->id = uniqid(); + } +}