Merge branch '5.2' into 5

FIX Ensure eagerLoading don't load has_one twice (#11170)

src/Core/Manifest/VersionProvider.php

@@ -189,6 +189,9 @@ class VersionProvider
     {
         $versions = [];
         foreach ($modules as $module) {
+            if (!InstalledVersions::isInstalled($module)) {
+                continue;
+            }
             $versions[$module] = InstalledVersions::getPrettyVersion($module);
         }
         return $versions;

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");
             }
         }

tests/php/Core/Manifest/VersionProviderTest.php

@@ -102,6 +102,27 @@ class VersionProviderTest extends SapphireTest
         $this->assertStringNotContainsString('Framework: 1.2.3', $result);
     }
 
+    public function testGetModuleVersionWhenPackageMayNotBeInstalled()
+    {
+        if (!class_exists(VersionParser::class)) {
+            $this->markTestSkipped('This test requires composer/semver to be installed');
+        }
+        $provider = $this->getProvider();
+        // VersionProvider::getModuleVersion() will loop over the modules defined in the "modules" config value, which
+        // may sometimes include packages that are optional (e.g. recipe-core). This tests that the version can still
+        // be found even if non-existent modules are encountered
+        Config::modify()->set(VersionProvider::class, 'modules', [
+            'this/module/cannot/possibly/exist' => 'Oopsies',
+            'silverstripe/framework' => 'Framework',
+            'silverstripe/another-module-that-does-not-exist' => 'Sapphire',
+        ]);
+        $moduleVersion = $provider->getModuleVersion('silverstripe/framework');
+        $parser = new VersionParser();
+        $this->assertIsString($parser->normalize($moduleVersion), "Expected a valid semver but got $moduleVersion");
+        $result = $provider->getVersion();
+        $this->assertStringNotContainsString('Framework: 1.2.3', $result);
+    }
+
     private function clearCache()
     {
         $cache = Injector::inst()->get(CacheInterface::class . '.VersionProvider');

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();
+    }
 }