FIX Empty relations don't have extra DB calls with eager-loading (#10886)

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<DataObject|EagerLoadedList> */
             $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
      *

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
+            $this->startCountingSelectQueries();
-            // 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__';
             $results = [];
-            $i = 0;
             if ($chunks) {
                 $dataList = $dataList->chunkedFetch($chunks);
             }
@@ -695,15 +736,9 @@ class DataListEagerLoadingTest extends SapphireTest
                     }
                 }
             }
-            $s = ob_get_clean();
+            $selectCount = $this->stopCountingSelectQueries();
-            $s = preg_replace('/.*__START_ITERATE__/s', '', $s);
-            $selectCount = substr_count($s, ': SELECT');
         } finally {
-            if ($showqueries) {
+            $this->resetShowQueries();
-                $_REQUEST['showqueries'] = $showqueries;
-            } else {
-                unset($_REQUEST['showqueries']);
-            }
         }
         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);
+        try {
             foreach (EagerLoadObject::get()->eagerLoad($eagerLoad) as $parentRecord) {
+                if ($i === 0) {
+                    $this->startCountingSelectQueries();
+                }
+
+                // Test first level items are handled correctly
                 $relation = $relations[0];
-            $list = $parentRecord->$relation();
+                $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($i > 0 ? 0 : 2, $list);
+                    // 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;
-                foreach ($list as $relatedRecord) {
                     $relation = $relations[1];
-                    $list2 = $relatedRecord->$relation();
+                    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',