diff --git a/src/Core/Manifest/VersionProvider.php b/src/Core/Manifest/VersionProvider.php index 65cfc73768f..4a0d4b48e44 100644 --- a/src/Core/Manifest/VersionProvider.php +++ b/src/Core/Manifest/VersionProvider.php @@ -189,6 +189,9 @@ public function getModuleVersionFromComposer($modules = []) { $versions = []; foreach ($modules as $module) { + if (!InstalledVersions::isInstalled($module)) { + continue; + } $versions[$module] = InstalledVersions::getPrettyVersion($module); } return $versions; diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index 9029866680a..3dd0f435e5c 100644 --- 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,10 +1151,8 @@ 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)) { @@ -1162,11 +1160,8 @@ private function fetchEagerLoadHasOne( } 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/Core/Manifest/VersionProviderTest.php b/tests/php/Core/Manifest/VersionProviderTest.php index 9e585f5ea45..7b7959a9cbd 100644 --- a/tests/php/Core/Manifest/VersionProviderTest.php +++ b/tests/php/Core/Manifest/VersionProviderTest.php @@ -102,6 +102,27 @@ public function testGetModuleVersion() $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'); diff --git a/tests/php/ORM/DataListEagerLoadingTest.php b/tests/php/ORM/DataListEagerLoadingTest.php index 32d56b5b29f..44ce1e36eeb 100644 --- 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(); + } }