From 135f9c6d3025259b519dfec351e3e85ecf2f46c5 Mon Sep 17 00:00:00 2001
From: Dominik Beerbohm <beerbohm@ims.de>
Date: Tue, 26 Mar 2024 14:31:27 +0100
Subject: [PATCH] FIX Ensure eagerLoading don't load has_one twice (#11170)

---
 src/ORM/DataList.php                       | 17 ++----
 tests/php/ORM/DataListEagerLoadingTest.php | 68 ++++++++++++++++++++++
 2 files changed, 74 insertions(+), 11 deletions(-)

diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php
index 902986668..3dd0f435e 100644
--- a/src/ORM/DataList.php
+++ b/src/ORM/DataList.php
@@ -1129,18 +1129,18 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
                 // $parentData represents a record in this DataList
                 $hasOneID = $parentData[$hasOneIDField];
                 $fetchedIDs[] = $hasOneID;
-                $addTo[$hasOneID] = $parentData['ID'];
+                $addTo[$hasOneID][] = $parentData['ID'];
             } elseif ($parentData instanceof DataObject) {
                 // $parentData represents another has_one record
                 $hasOneID = $parentData->$hasOneIDField;
                 $fetchedIDs[] = $hasOneID;
-                $addTo[$hasOneID] = $parentData;
+                $addTo[$hasOneID][] = $parentData;
             } elseif ($parentData instanceof EagerLoadedList) {
                 // $parentData represents a has_many or many_many relation
                 foreach ($parentData->getRows() as $parentRow) {
                     $hasOneID = $parentRow[$hasOneIDField];
                     $fetchedIDs[] = $hasOneID;
-                    $addTo[$hasOneID] = ['ID' => $parentRow['ID'], 'list' => $parentData];
+                    $addTo[$hasOneID][] = ['ID' => $parentRow['ID'], 'list' => $parentData];
                 }
             } else {
                 throw new LogicException("Invalid parent for eager loading $relationType relation $relationName");
@@ -1151,10 +1151,8 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
 
         // Add each fetched record to the appropriate place
         foreach ($fetchedRecords as $fetched) {
-            $fetchedID = $fetched->ID;
-            $added = false;
-            foreach ($addTo as $matchID => $addHere) {
-                if ($matchID === $fetchedID) {
+            if (isset($addTo[$fetched->ID])) {
+                foreach ($addTo[$fetched->ID] as $addHere) {
                     if ($addHere instanceof DataObject) {
                         $addHere->setEagerLoadedData($relationName, $fetched);
                     } elseif (is_array($addHere)) {
@@ -1162,11 +1160,8 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
                     } else {
                         $this->eagerLoadedData[$relationChain][$addHere][$relationName] = $fetched;
                     }
-                    $added = true;
-                    break;
                 }
-            }
-            if (!$added) {
+            } else {
                 throw new LogicException("Couldn't find parent for record $fetchedID on $relationType relation $relationName");
             }
         }
diff --git a/tests/php/ORM/DataListEagerLoadingTest.php b/tests/php/ORM/DataListEagerLoadingTest.php
index 32d56b5b2..44ce1e36e 100644
--- a/tests/php/ORM/DataListEagerLoadingTest.php
+++ b/tests/php/ORM/DataListEagerLoadingTest.php
@@ -1588,4 +1588,72 @@ class DataListEagerLoadingTest extends SapphireTest
             }
         }
     }
+
+    public function testHasOneMultipleAppearance(): void
+    {
+        $this->provideHasOneObjects();
+        $this->validateMultipleAppearance(6, EagerLoadObject::get());
+        $this->validateMultipleAppearance(2, EagerLoadObject::get()->eagerLoad('HasOneEagerLoadObject'));
+    }
+
+    protected function validateMultipleAppearance(int $expected, DataList $list): void
+    {
+        try {
+            $this->startCountingSelectQueries();
+
+            /** @var EagerLoadObject $item */
+            foreach ($list as $item) {
+                $item->HasOneEagerLoadObject()->Title;
+            }
+
+            $this->assertSame($expected, $this->stopCountingSelectQueries());
+        } finally {
+            $this->resetShowQueries();
+        }
+    }
+
+    protected function provideHasOneObjects(): void
+    {
+        $subA = new HasOneEagerLoadObject();
+        $subA->Title = 'A';
+        $subA->write();
+
+        $subB = new HasOneEagerLoadObject();
+        $subB->Title = 'B';
+        $subB->write();
+
+        $subC = new HasOneEagerLoadObject();
+        $subC->Title = 'C';
+        $subC->write();
+
+        $baseA = new EagerLoadObject();
+        $baseA->Title = 'A';
+        $baseA->HasOneEagerLoadObjectID = $subA->ID;
+        $baseA->write();
+
+        $baseB = new EagerLoadObject();
+        $baseB->Title = 'B';
+        $baseB->HasOneEagerLoadObjectID = $subA->ID;
+        $baseB->write();
+
+        $baseC = new EagerLoadObject();
+        $baseC->Title = 'C';
+        $baseC->HasOneEagerLoadObjectID = $subB->ID;
+        $baseC->write();
+
+        $baseD = new EagerLoadObject();
+        $baseD->Title = 'D';
+        $baseD->HasOneEagerLoadObjectID = $subC->ID;
+        $baseD->write();
+
+        $baseE = new EagerLoadObject();
+        $baseE->Title = 'E';
+        $baseE->HasOneEagerLoadObjectID = $subB->ID;
+        $baseE->write();
+
+        $baseF = new EagerLoadObject();
+        $baseF->Title = 'F';
+        $baseF->HasOneEagerLoadObjectID = 0;
+        $baseF->write();
+    }
 }