Make it possible to have non-NULLable self-referencing associations when using application-provided IDs

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;

tests/Doctrine/Tests/ORM/Functional/GH7877Test.php

@@ -0,0 +1,135 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\ORM\Mapping as ORM;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+use function uniqid;
+
+/**
+ * @group GH7877
+ */
+class GH7877Test extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->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();
+    }
+}