Merge pull request #11185 from beerbohmdo/fix/issue-11170

FIX Ensure eagerLoading don't load has_one twice (#11170)
silverstripe · Apr 18, 2024 · cfeb678 · cfeb678
2 parents 908bdcf + 135f9c6
commit cfeb678
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 11 deletions.
diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php
@@ -1129,18 +1129,18 @@ private function fetchEagerLoadHasOne(
                 // $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,22 +1151,17 @@ private function fetchEagerLoadHasOne(
 
         // 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)) {
                         $addHere['list']->addEagerLoadedData($relationName, $addHere['ID'], $fetched);
                     } 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
@@ -1588,4 +1588,72 @@ public function testManipulatingEagerloadingQuery(string $relationType, array $r
             }
         }
     }
+
+    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();
+    }
 }