This change makes the BasicEntityPersister not schedule extra updat…

…es for association-target entities that use application-provided IDs (the "NONE" generator strategy). For this generator strategy, we may assume that IDs are already present at the time the insert is executed.

The benefit is that now the join column can be defined as not-NULLable, since the ORM no longer uses temporary NULL values.

However, care must be taken:

Extra updates are not only about the question of having IDs for the associated entities available, but also about having those entities in the database already (to satisfy foreign key constaints).

The latter will become easier with #10547, since the UoW will take care of that detail when computing the commit order.

But without that detail, we might break use cases that currently work – that's cases where the extra update gives us the extra time to insert the referred-to entity first.

Fixes #7877, closes #7882.

Co-authored-by: Sylvain Fabre <[email protected]>

lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php

@@ -43,6 +43,7 @@
 use function array_values;
 use function assert;
 use function count;
+use function get_class;
 use function implode;
 use function is_array;
 use function is_object;
@@ -683,10 +684,18 @@ protected function prepareUpdateData($entity, bool $isInsert = false)
             if ($newVal !== null) {
                 $oid = spl_object_id($newVal);
 
+                // If the associated entity $newVal is not yet persisted, we must
+                // set $newVal = null, in order to insert a null value and schedule an
+                // extra update on the UnitOfWork.
+                //
+                // This gives us extra time to a) obtain the database-generated identifier value
+                // for $newVal, and b) insert $newVal into the database before the foreign key
+                // reference is being made.
+
+                // TODO: Use condition like this?
+                // if ((isset($this->queuedInserts[$oid]) || $uow->isScheduledForInsert($newVal)) && ! $this->em->getClassMetadata(get_class($newVal))->isIdentifierNatural()) {
+
                 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.
                     $uow->scheduleExtraUpdate($entity, [$field => [null, $newVal]]);
 
                     $newVal = null;

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

@@ -0,0 +1,133 @@
+<?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;
+
+    /**
+     * @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();
+    }
+}