Fix doctrine#10880 – changeset computation broken in 2.16.0 for entit…

…ies loaded through fetch=EAGER + using inheritance

 doctrine#10880 reports a case where the changes from doctrine#10785 cause entity updates to be missed.

Upon closer inspection, this change seems to be causing it:

https://github.com/doctrine/orm/pull/10785/files#diff-55a900494fc8033ab498c53929716caf0aa39d6bdd7058e7d256787a24412ee4L2990-L3003

The code was changed to use `registerManaged()` instead, which basically does the same things, but (since doctrine#10785) also includes an additional check against duplicate entity instances.

But, one detail slipped through tests and reviews: `registerManaged()` also updates `\Doctrine\ORM\UnitOfWork::$originalEntityData`, which is used to compute entity changesets. An empty array `[]` was passed for $data here.

This will make the changeset computation assume that a partial object was loaded and effectively ignore all field updates here:

https://github.com/doctrine/orm/blob/a616914887ea160db4158d2c67752e99624f7c8a/lib/Doctrine/ORM/UnitOfWork.php#L762-L764

I think that, effectively, it is sufficient to call `registerManaged()` only in the two cases where a proxy was created.

Calling `registerManaged()` with `[]` as data for a proxy object is consistent with e. g. `\Doctrine\ORM\EntityManager::getReference()`.

In the case that a full entity has to be loaded, we need not call `registerManaged()` at all, since that will already happen inside `EntityManager::find()` (or, more specifically, `UnitOfWork::createEntity()` called inside it).

Note that the test case has to make some provisions so that we actually reach this case:
* Load an entity that uses `fetch="EAGER"` on a to-one association
* That association being against a class that uses inheritance (why's that?)

lib/Doctrine/ORM/UnitOfWork.php

@@ -3048,6 +3048,7 @@ public function createEntity($className, array $data, &$hints = [])
                                 // We are negating the condition here. Other cases will assume it is valid!
                                 case $hints['fetchMode'][$class->name][$field] !== ClassMetadata::FETCH_EAGER:
                                     $newValue = $this->em->getProxyFactory()->getProxy($assoc['targetEntity'], $normalizedAssociatedId);
+                                    $this->registerManaged($newValue, $associatedId, []);
                                     break;
 
                                 // Deferred eager load only works for single identifier classes
@@ -3056,20 +3057,14 @@ public function createEntity($className, array $data, &$hints = [])
                                     $this->eagerLoadingEntities[$targetClass->rootEntityName][$relatedIdHash] = current($normalizedAssociatedId);
 
                                     $newValue = $this->em->getProxyFactory()->getProxy($assoc['targetEntity'], $normalizedAssociatedId);
+                                    $this->registerManaged($newValue, $associatedId, []);
                                     break;
 
                                 default:
                                     // TODO: This is very imperformant, ignore it?
                                     $newValue = $this->em->find($assoc['targetEntity'], $normalizedAssociatedId);
                                     break;
                             }
-
-                            if ($newValue === null) {
-                                break;
-                            }
-
-                            $this->registerManaged($newValue, $associatedId, []);
-                            break;
                     }
 
                     $this->originalEntityData[$oid][$field] = $newValue;

tests/Doctrine/Tests/ORM/Functional/Ticket/GH10880Test.php

@@ -0,0 +1,116 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\ORM\Mapping as ORM;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+class GH10880Test extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->setUpEntitySchema([
+            GH10880BaseProcess::class,
+            GH10880Process::class,
+            GH10880ProcessOwner::class,
+        ]);
+    }
+
+    public function testProcessShouldBeUpdated(): void
+    {
+        $process              = new GH10880Process();
+        $process->description = 'first value';
+
+        $owner          = new GH10880ProcessOwner();
+        $owner->process = $process;
+
+        $this->_em->persist($process);
+        $this->_em->persist($owner);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $ownerLoaded                       = $this->_em->getRepository(GH10880ProcessOwner::class)->find($owner->id);
+        $ownerLoaded->process->description = 'other description';
+
+        $queryLog = $this->getQueryLog();
+        $queryLog->reset()->enable();
+        $this->_em->flush();
+
+        $this->removeTransactionCommandsFromQueryLog();
+
+        self::assertCount(1, $queryLog->queries);
+        self::assertSame('UPDATE GH10880BaseProcess SET description = ? WHERE id = ?', $queryLog->queries[0]['sql']);
+    }
+
+    private function removeTransactionCommandsFromQueryLog(): void
+    {
+        $log = $this->getQueryLog();
+
+        foreach ($log->queries as $key => $entry) {
+            if ($entry['sql'] === '"START TRANSACTION"' || $entry['sql'] === '"COMMIT"') {
+                unset($log->queries[$key]);
+            }
+        }
+    }
+}
+
+/**
+ * @ORM\Entity
+ */
+class GH10880ProcessOwner
+{
+    /**
+     * @ORM\Id
+     * @ORM\GeneratedValue
+     * @ORM\Column(type="integer")
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * fetch=EAGER is important to reach the part of \Doctrine\ORM\UnitOfWork::createEntity()
+     * that is important for this regression test
+     *
+     * @ORM\ManyToOne(targetEntity="GH10880Process", fetch="EAGER")
+     *
+     * @var GH10880Process
+     */
+    public $process;
+}
+
+/**
+ * @ORM\Entity()
+ * @ORM\InheritanceType("SINGLE_TABLE")
+ * @ORM\DiscriminatorColumn(name="type", type="string")
+ * @ORM\DiscriminatorMap({"process" = "GH10880Process"})
+ */
+abstract class GH10880BaseProcess
+{
+    /**
+     * @ORM\Id
+     * @ORM\GeneratedValue
+     * @ORM\Column(type="integer")
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * @ORM\Column(type="text")
+     *
+     * @var string
+     */
+    public $description;
+}
+
+/**
+ * @ORM\Entity
+ */
+class GH10880Process extends GH10880BaseProcess
+{
+}