diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index 2063f4edb..397192b1b 100644 --- a/src/ORM/DataList.php +++ b/src/ORM/DataList.php @@ -1007,10 +1007,15 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li } $belongsToComponent = $schema->belongsToComponent($parentDataClass, $relationName); if ($belongsToComponent) { + $joinField = $schema->getRemoteJoinField($parentDataClass, $relationName, 'belongs_to', $polymorphic); return [ $belongsToComponent, 'belongs_to', - $schema->getRemoteJoinField($parentDataClass, $relationName, 'belongs_to'), + [ + 'joinField' => $joinField, + 'polymorphic' => $polymorphic, + 'parentClass' => $parentDataClass + ], ]; } $hasManyComponent = $schema->hasManyComponent($parentDataClass, $relationName); @@ -1061,7 +1066,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li foreach ($this->eagerLoadRelationChains as $relationChain) { $parentDataClass = $this->dataClass(); $parentIDs = $topLevelIDs; - $parentRelationName = ''; /** @var Query|array */ $parentRelationData = $query; $chainToDate = []; @@ -1080,7 +1084,8 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li $relationComponent, $relationDataClass, implode('.', $chainToDate), - $relationName + $relationName, + $relationType ); break; case 'belongs_to': @@ -1090,7 +1095,8 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li $relationComponent, $relationDataClass, implode('.', $chainToDate), - $relationName + $relationName, + $relationType ); break; case 'has_many': @@ -1101,7 +1107,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li $relationDataClass, implode('.', $chainToDate), $relationName, - $parentRelationName + $relationType ); break; case 'many_many': @@ -1112,15 +1118,14 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li $relationDataClass, implode('.', $chainToDate), $relationName, - $parentRelationName, - $parentDataClass + $parentDataClass, + $relationType ); break; default: throw new LogicException("Unexpected relation type $relationType"); } $parentDataClass = $relationDataClass; - $parentRelationName = $relationName; } } } @@ -1130,7 +1135,8 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li string $hasOneIDField, string $relationDataClass, string $relationChain, - string $relationName + string $relationName, + string $relationType ): array { $fetchedIDs = []; $addTo = []; @@ -1155,7 +1161,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li $addTo[$hasOneID] = ['ID' => $parentRow['ID'], 'list' => $parentData]; } } else { - throw new LogicException("Invalid parent for eager loading has_one relation $relationName"); + throw new LogicException("Invalid parent for eager loading $relationType relation $relationName"); } } @@ -1179,34 +1185,54 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li } } if (!$added) { - throw new LogicException("Couldn't find parent for record $fetchedID on has_one relation $relationName"); + throw new LogicException("Couldn't find parent for record $fetchedID on $relationType relation $relationName"); } } + // NOTE: Unlike the other relation types, we don't have to explicitly fill empty DataObject records + // into the has_one components - DataObject does that for us in getComponent() without any extra + // db calls. + return [$fetchedRecords, $fetchedIDs]; } private function fetchEagerLoadBelongsTo( Query|array $parents, array $parentIDs, - string $belongsToIDField, + array $component, string $relationDataClass, string $relationChain, - string $relationName + string $relationName, + string $relationType ): array { + $belongsToIDField = $component['joinField']; // Get ALL of the items for this relation up front, for ALL of the parents // Fetched as an array to avoid sporadic additional queries when the DataList is looped directly $fetchedRecords = DataObject::get($relationDataClass)->filter([$belongsToIDField => $parentIDs])->toArray(); $fetchedIDs = []; - + $foundParentIDs = []; // Add fetched record to the correct place foreach ($fetchedRecords as $fetched) { $fetchedIDs[] = $fetched->ID; $parentID = $fetched->$belongsToIDField; - $this->addEagerLoadedDataToParent($parents, $parentID, $relationChain, $relationName, $fetched, 'has_one'); + $foundParentIDs[] = $parentID; + $this->addEagerLoadedDataToParent($parents, $parentID, $relationChain, $relationName, $fetched, $relationType); } + // Load empty DataObject records into any parents which have no child records + $missingParentIDs = array_diff($parentIDs, $foundParentIDs); + $this->fillEmptyEagerLoadedRelations( + $parents, + $missingParentIDs, + $relationChain, + $relationName, + $relationType, + $relationDataClass, + null, + $component + ); + return [$fetchedRecords, $fetchedIDs]; } @@ -1216,7 +1242,8 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li string $hasManyIDField, string $relationDataClass, string $relationChain, - string $relationName + string $relationName, + string $relationType ): array { // Get ALL of the items for this relation up front, for ALL of the parents // Fetched as an array to avoid sporadic additional queries when the DataList is looped directly @@ -1234,12 +1261,24 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li // If we haven't created a list yet, create it and add it to the correct parent list/record $eagerLoadedList = EagerLoadedList::create($relationDataClass, HasManyList::class, $parentID); $eagerLoadedLists[$parentID] = $eagerLoadedList; - $this->addEagerLoadedDataToParent($parents, $parentID, $relationChain, $relationName, $eagerLoadedList, 'has_many'); + $this->addEagerLoadedDataToParent($parents, $parentID, $relationChain, $relationName, $eagerLoadedList, $relationType); } // Add this row to the list $eagerLoadedList->addRow($row); } + // Load empty EagerLoadedLists into any parents which have no child records + $missingParentIDs = array_diff($parentIDs, array_keys($eagerLoadedLists)); + $this->fillEmptyEagerLoadedRelations( + $parents, + $missingParentIDs, + $relationChain, + $relationName, + $relationType, + $relationDataClass, + HasManyList::class + ); + return [$eagerLoadedLists, $fetchedIDs]; } @@ -1250,7 +1289,8 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li string $relationDataClass, string $relationChain, string $relationName, - string $parentDataClass + string $parentDataClass, + string $relationType ): array { $parentIDField = $manyManyLastComponent['parentField']; $childIDField = $manyManyLastComponent['childField']; @@ -1288,6 +1328,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li $extraFields ); } + $relationListClass = get_class($relationList); // Get ALL of the items for this relation up front, for ALL of the parents $fetchedRows = $relationList->forForeignID($parentIDs)->getFinalisedQuery(); @@ -1307,14 +1348,27 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li $eagerLoadedList = $eagerLoadedLists[$parentID]; } else { // If we haven't created a list yet, create it and add it to the correct parent list/record - $eagerLoadedList = EagerLoadedList::create($relationDataClass, get_class($relationList), $parentID, $manyManyLastComponent); + $eagerLoadedList = EagerLoadedList::create($relationDataClass, $relationListClass, $parentID, $manyManyLastComponent); $eagerLoadedLists[$parentID] = $eagerLoadedList; - $this->addEagerLoadedDataToParent($parents, $parentID, $relationChain, $relationName, $eagerLoadedList, 'many_many'); + $this->addEagerLoadedDataToParent($parents, $parentID, $relationChain, $relationName, $eagerLoadedList, $relationType); } // Add this row to the list $eagerLoadedList->addRow($relationItem); } + // Load empty EagerLoadedLists into any parents which have no child records + $missingParentIDs = array_diff($parentIDs, array_keys($eagerLoadedLists)); + $this->fillEmptyEagerLoadedRelations( + $parents, + $missingParentIDs, + $relationChain, + $relationName, + $relationType, + $relationDataClass, + $relationListClass, + $manyManyLastComponent + ); + return [$eagerLoadedLists, $fetchedIDs]; } @@ -1367,6 +1421,46 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li } } + private function fillEmptyEagerLoadedRelations( + Query|array $parents, + array $missingParentIDs, + string $relationChain, + string $relationName, + string $relationType, + string $relationDataClass, + ?string $relationListClass = null, + ?array $component = null + ): void { + foreach ($missingParentIDs as $id) { + // Build the empty list or record + switch ($relationType) { + case 'has_one': + $dummyData = Injector::inst()->create($relationDataClass); + break; + case 'belongs_to': + $dummyData = Injector::inst()->create($relationDataClass); + $joinField = $component['joinField']; + if ($component['polymorphic']) { + $dummyData->{$joinField . 'ID'} = $id; + $dummyData->{$joinField . 'Class'} = $component['parentClass']; + } else { + $dummyData->$joinField = $id; + } + break; + case 'has_many': + $dummyData = EagerLoadedList::create($relationDataClass, $relationListClass, $id); + break; + case 'many_many': + $dummyData = EagerLoadedList::create($relationDataClass, $relationListClass, $id, $component); + break; + default: + throw new LogicException("Unexpected relation type $relationType"); + } + // Add the empty list or record to this parent + $this->addEagerLoadedDataToParent($parents, $id, $relationChain, $relationName, $dummyData, $relationType); + } + } + /** * Eager load relations for DataObjects in this DataList including nested relations * diff --git a/tests/php/ORM/DataListEagerLoadingTest.php b/tests/php/ORM/DataListEagerLoadingTest.php index d9b945f2a..7dda2ec9e 100644 --- a/tests/php/ORM/DataListEagerLoadingTest.php +++ b/tests/php/ORM/DataListEagerLoadingTest.php @@ -3,6 +3,7 @@ namespace SilverStripe\ORM\Tests; use InvalidArgumentException; +use LogicException; use SilverStripe\Dev\SapphireTest; use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; @@ -37,9 +38,12 @@ use SilverStripe\ORM\Tests\DataListTest\EagerLoading\MixedManyManyEagerLoadObjec class DataListEagerLoadingTest extends SapphireTest { - protected $usesDatabase = true; + private const SHOW_QUERIES_RESET = 'SET_TO_THIS_VALUE_WHEN_FINISHED'; + + private $showQueries = self::SHOW_QUERIES_RESET; + public static function getExtraDataObjects() { return [ @@ -100,6 +104,51 @@ class DataListEagerLoadingTest extends SapphireTest } } + /** + * Start counting the number of SELECT database queries being run + */ + private function startCountingSelectQueries(): void + { + if ($this->showQueries !== self::SHOW_QUERIES_RESET) { + throw new LogicException('showQueries wasnt reset, you did something wrong'); + } + $this->showQueries = $_REQUEST['showqueries'] ?? null; + // force showqueries on to count the number of SELECT statements via output-buffering + // if this approach turns out to be too brittle later on, switch to what debugbar + // does and use tractorcow/proxy-db which should be installed as a dev-dependency + // https://github.com/lekoala/silverstripe-debugbar/blob/master/code/Collector/DatabaseCollector.php#L79 + $_REQUEST['showqueries'] = 1; + ob_start(); + echo '__START_ITERATE__'; + } + + /** + * Stop counting database queries and return the count + */ + private function stopCountingSelectQueries(): int + { + $s = ob_get_clean(); + $s = preg_replace('/.*__START_ITERATE__/s', '', $s); + $this->resetShowQueries(); + return substr_count($s, ': SELECT'); + } + + /** + * Reset the "showqueries" request var + */ + private function resetShowQueries(): void + { + if ($this->showQueries === self::SHOW_QUERIES_RESET) { + return; + } + if ($this->showQueries) { + $_REQUEST['showqueries'] = $this->showQueries; + } else { + unset($_REQUEST['showqueries']); + } + $this->showQueries = self::SHOW_QUERIES_RESET; + } + /** * @dataProvider provideEagerLoadRelations */ @@ -615,17 +664,9 @@ class DataListEagerLoadingTest extends SapphireTest { $results = []; $selectCount = -1; - $showqueries = $_REQUEST['showqueries'] ?? null; try { - // force showqueries on to count the number of SELECT statements via output-buffering - // if this approach turns out to be too brittle later on, switch to what debugbar - // does and use tractorcow/proxy-db which should be installed as a dev-dependency - // https://github.com/lekoala/silverstripe-debugbar/blob/master/code/Collector/DatabaseCollector.php#L79 - $_REQUEST['showqueries'] = 1; - ob_start(); - echo '__START_ITERATE__'; + $this->startCountingSelectQueries(); $results = []; - $i = 0; if ($chunks) { $dataList = $dataList->chunkedFetch($chunks); } @@ -695,15 +736,9 @@ class DataListEagerLoadingTest extends SapphireTest } } } - $s = ob_get_clean(); - $s = preg_replace('/.*__START_ITERATE__/s', '', $s); - $selectCount = substr_count($s, ': SELECT'); + $selectCount = $this->stopCountingSelectQueries(); } finally { - if ($showqueries) { - $_REQUEST['showqueries'] = $showqueries; - } else { - unset($_REQUEST['showqueries']); - } + $this->resetShowQueries(); } return [$results, $selectCount]; } @@ -994,11 +1029,11 @@ class DataListEagerLoadingTest extends SapphireTest } /** - * @dataProvider provideEagerLoadingEmptyRelationLists + * @dataProvider provideEagerLoadingEmptyRelations */ - public function testEagerLoadingEmptyRelationLists(string $iden, string $eagerLoad): void + public function testEagerLoadingEmptyRelations(string $iden, string $eagerLoad): void { - $numBaseRecords = 2; + $numBaseRecords = 3; $numLevel1Records = 2; $numLevel2Records = 2; // Similar to createEagerLoadData(), except with less relations and @@ -1008,9 +1043,34 @@ class DataListEagerLoadingTest extends SapphireTest $obj = new EagerLoadObject(); $obj->Title = "obj $i"; $objID = $obj->write(); + if ($i > 1) { + continue; + } + // has_one - level1 + $hasOneObj = new HasOneEagerLoadObject(); + $hasOneObj->Title = "hasOneObj $i"; + $hasOneObjID = $hasOneObj->write(); + $obj->HasOneEagerLoadObjectID = $hasOneObjID; + $obj->write(); + // belongs_to - level1 + $belongsToObj = new BelongsToEagerLoadObject(); + $belongsToObj->EagerLoadObjectID = $objID; + $belongsToObj->Title = "belongsToObj $i"; + $belongsToObjID = $belongsToObj->write(); if ($i > 0) { continue; } + // has_one - level2 + $hasOneSubObj = new HasOneSubEagerLoadObject(); + $hasOneSubObj->Title = "hasOneSubObj $i"; + $hasOneSubObjID = $hasOneSubObj->write(); + $hasOneObj->HasOneSubEagerLoadObjectID = $hasOneSubObjID; + $hasOneObj->write(); + // belongs_to - level2 + $belongsToSubObj = new BelongsToSubEagerLoadObject(); + $belongsToSubObj->BelongsToEagerLoadObjectID = $belongsToObjID; + $belongsToSubObj->Title = "belongsToSubObj $i"; + $belongsToSubObj->write(); // has_many for ($j = 0; $j < $numLevel1Records; $j++) { $hasManyObj = new HasManyEagerLoadObject(); @@ -1065,31 +1125,81 @@ class DataListEagerLoadingTest extends SapphireTest } } + // The actual test starts here - everything above is for creating fixtures $i = 0; $relations = explode('.', $eagerLoad); - foreach (EagerLoadObject::get()->eagerLoad($eagerLoad) as $parentRecord) { - $relation = $relations[0]; - $list = $parentRecord->$relation(); - // For any record after the first one, there should be nothing in the related list. - $this->assertCount($i > 0 ? 0 : 2, $list); - $i++; + try { + foreach (EagerLoadObject::get()->eagerLoad($eagerLoad) as $parentRecord) { + if ($i === 0) { + $this->startCountingSelectQueries(); + } - if (count($relations) > 1) { - $j = 0; - foreach ($list as $relatedRecord) { - $relation = $relations[1]; - $list2 = $relatedRecord->$relation(); + // Test first level items are handled correctly + $relation = $relations[0]; + $listOrRecord = $parentRecord->$relation(); + if (str_starts_with($iden, 'hasone') || str_starts_with($iden, 'belongsto')) { + $class = str_starts_with($iden, 'hasone') ? HasOneEagerLoadObject::class : BelongsToEagerLoadObject::class; + if ($i > 1) { + $this->assertSame(0, $listOrRecord->ID); + } + $this->assertInstanceOf($class, $listOrRecord); + } else { // For any record after the first one, there should be nothing in the related list. - $this->assertCount($j > 0 ? 0 : 2, $list2); - $j++; + // All lists, even empty ones, should be an instance of EagerLoadedList + $this->assertCount($i > 0 ? 0 : 2, $listOrRecord); + $this->assertInstanceOf(EagerLoadedList::class, $listOrRecord); + } + $i++; + + // Test second level items are handled correctly + if (count($relations) > 1) { + $j = 0; + $relation = $relations[1]; + if (str_starts_with($iden, 'hasone') || str_starts_with($iden, 'belongsto')) { + $record2 = $listOrRecord->$relation(); + $class = str_starts_with($iden, 'hasone') ? HasOneSubEagerLoadObject::class : BelongsToSubEagerLoadObject::class; + if ($j > 0) { + $this->assertSame(0, $record2->ID); + } + $this->assertInstanceOf($class, $record2); + } else { + // For any record after the first one, there should be nothing in the related list. + // All lists, even empty ones, should be an instance of EagerLoadedList + foreach ($listOrRecord as $relatedRecord) { + $list2 = $relatedRecord->$relation(); + $this->assertCount($j > 0 ? 0 : 2, $list2); + $this->assertInstanceOf(EagerLoadedList::class, $list2); + $j++; + } + } } } + // No queries should have been run after initiating the loop + $this->assertSame(0, $this->stopCountingSelectQueries()); + } finally { + $this->resetShowQueries(); } } - public function provideEagerLoadingEmptyRelationLists(): array + public function provideEagerLoadingEmptyRelations(): array { return [ + [ + 'hasone-onelevel', + 'HasOneEagerLoadObject', + ], + [ + 'hasone-twolevels', + 'HasOneEagerLoadObject.HasOneSubEagerLoadObject', + ], + [ + 'belongsto-onelevel', + 'BelongsToEagerLoadObject', + ], + [ + 'belongsto-twolevels', + 'BelongsToEagerLoadObject.BelongsToSubEagerLoadObject', + ], [ 'hasmany-onelevel', 'HasManyEagerLoadObjects',