NEW Allow manipulating eagerloading queries (#11140)

src/ORM/DataList.php

@@ -1115,6 +1115,11 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
         string $relationName,
         string $relationType
     ): array {
+        // Throw exception if developers try to manipulate a has_one relation as a list
+        if ($this->eagerLoadAllRelations[$relationChain] !== null) {
+            throw new LogicException("Cannot manipulate eagerloading query for $relationType relation $relationName");
+        }
+
         $fetchedIDs = [];
         $addTo = [];
 
@@ -1182,6 +1187,11 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
         string $relationName,
         string $relationType
     ): array {
+        // Throw exception if developers try to manipulate a belongs_to relation as a list
+        if ($this->eagerLoadAllRelations[$relationChain] !== null) {
+            throw new LogicException("Cannot manipulate eagerloading query for $relationType relation $relationName");
+        }
+
         $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
@@ -1222,9 +1232,11 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
         string $relationName,
         string $relationType
     ): array {
+        $fetchList = DataObject::get($relationDataClass)->filter([$hasManyIDField => $parentIDs]);
+        $fetchList = $this->manipulateEagerLoadingQuery($fetchList, $relationChain, $relationType);
         // 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
-        $fetchedRows = DataObject::get($relationDataClass)->filter([$hasManyIDField => $parentIDs])->getFinalisedQuery();
+        $fetchedRows = $fetchList->getFinalisedQuery();
         $fetchedIDs = [];
         $eagerLoadedLists = [];
 
@@ -1278,10 +1290,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
         $fetchedIDs = [];
         $eagerLoadedLists = [];
 
-        // Get the join records so we can correctly identify which children belong to which parents
-        // This also holds extra fields data
-        $joinRows = DB::query('SELECT * FROM "' . $joinTable . '" WHERE "' . $parentIDField . '" IN (' . implode(',', $parentIDs) . ')');
-
         // Use a real RelationList here so that the extraFields and join record are correctly fetched for all relations
         // There's a lot of special handling for things like DBComposite extra fields, etc.
         if ($joinClass !== null) {
@@ -1308,13 +1316,28 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
         $relationListClass = get_class($relationList);
 
         // Get ALL of the items for this relation up front, for ALL of the parents
-        $fetchedRows = $relationList->forForeignID($parentIDs)->getFinalisedQuery();
+        $fetchList = $relationList->forForeignID($parentIDs);
+        $fetchList = $this->manipulateEagerLoadingQuery($fetchList, $relationChain, $relationType);
+        $fetchedRows = $fetchList->getFinalisedQuery();
 
         foreach ($fetchedRows as $row) {
             $fetchedRowsArray[$row['ID']] = $row;
             $fetchedIDs[] = $row['ID'];
         }
 
+        // Get the join records so we can correctly identify which children belong to which parents
+        // This also holds extra fields data
+        $fetchedIDsAsString = implode(',', $fetchedIDs);
+        $joinRows = DB::query(
+            'SELECT * FROM "' . $joinTable
+            // Only get joins relevant for the parent list
+            . '" WHERE "' . $parentIDField . '" IN (' . implode(',', $parentIDs) . ')'
+            // Exclude any children that got filtered out
+            . ' AND ' . $childIDField . ' IN (' . $fetchedIDsAsString . ')'
+            // Respect sort order of fetched items
+            . ' ORDER BY FIELD(' . $childIDField . ', ' . $fetchedIDsAsString . ')'
+        );
+
         // Store the children in an EagerLoadedList against the correct parent
         foreach ($joinRows as $row) {
             $parentID = $row[$parentIDField];
@@ -1398,6 +1421,33 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
         }
     }
 
+    /**
+     * NOTE: Do not change `DataList` to `static` in this method signature.
+     * Subclasses of DataList must still accept DataList arguments and return DataList!
+     */
+    private function manipulateEagerLoadingQuery(
+        DataList $fetchList,
+        string $relationChain,
+        string $relationType
+    ): DataList {
+        $filterCallback = $this->eagerLoadAllRelations[$relationChain];
+        if ($filterCallback !== null) {
+            $fetchList = $filterCallback($fetchList);
+        }
+        if (!($fetchList instanceof DataList)) {
+            throw new LogicException(
+                "Eagerloading callback for $relationType relation $relationChain must return a DataList."
+            );
+        }
+        $limit = $fetchList->dataQuery->query()->getLimit();
+        if (!empty($limit) && ($limit['start'] !== 0 || $limit['limit'] !== null)) {
+            throw new LogicException(
+                "Cannot apply limit to eagerloaded data for $relationType relation $relationChain."
+            );
+        }
+        return $fetchList;
+    }
+
     private function fillEmptyEagerLoadedRelations(
         Query|array $parents,
         array $missingParentIDs,
@@ -1447,8 +1497,17 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      * You can specify nested relations by using dot notation, and you can also pass in multiple relations.
      * When specifying nested relations there is a maximum of 3 levels of relations allowed i.e. 2 dots
      *
-     * Example:
+     * Examples:
+     * <code>
      * $myDataList->eagerLoad('MyRelation.NestedRelation.EvenMoreNestedRelation', 'DifferentRelation')
+     * </code>
+     *
+     * <code>
+     * $myDataList->eagerLoad([
+     *     'MyRelation.NestedRelation.EvenMoreNestedRelation',
+     *     'DifferentRelation' => fn (DataList $list) => $list->filter($filterArgs),
+     * ]);
+     * </code>
      *
      * IMPORTANT: Calling eagerLoad() will cause any relations on DataObjects to be returned as an EagerLoadedList
      * instead of a subclass of DataList such as HasManyList i.e. MyDataObject->MyHasManyRelation() returns an EagerLoadedList
@@ -1458,8 +1517,29 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
     public function eagerLoad(...$relationChains): static
     {
         $list = clone $this;
-        foreach ($relationChains as $relationChain) {
+
-            // Don't add any relations we've added before
+        // If an array is passed in directly, treat it as though $relationChains wasn't spread.
+        if (count($relationChains) === 1 && is_array($relationChains[array_key_first($relationChains)])) {
+            $relationChains = $relationChains[array_key_first($relationChains)];
+        }
+
+        foreach ($relationChains as $relationChain => $callback) {
+            // Allow non-associative arrays
+            if (is_numeric($relationChain)) {
+                $relationChain = $callback;
+                $callback = null;
+            }
+
+            // Reject non-callable in associative array
+            if ($callback !== null && !is_callable($callback)) {
+                throw new LogicException(
+                    'Value of associative array must be a callable.'
+                    . 'If you don\'t want to pre-filter the list, use an indexed array.'
+                );
+            }
+
+            // Don't add any relations we've added before.
+            // Note we explicitly cannot use `isset` here, because most of the values are set to `null`.
             if (array_key_exists($relationChain, $list->eagerLoadAllRelations)) {
                 continue;
             }
@@ -1480,8 +1560,11 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
                 // Keep track of what we've seen before so we don't accidentally add a level 1 relation
                 // (e.g. "Players") to the chains list when we already have it as part of a longer chain
                 // (e.g. "Players.Teams")
-                $list->eagerLoadAllRelations[$item] = $item;
+                $list->eagerLoadAllRelations[$item] ??= null;
             }
+            // Set the callback for this chain
+            $list->eagerLoadAllRelations[$relationChain] = $callback;
+            // Set the relation chain to be loaded
             $list->eagerLoadRelationChains[$relationChain] = $relationChain;
         }
         return $list;

tests/php/ORM/DataListEagerLoadingTest.php

@@ -8,6 +8,7 @@ use SilverStripe\Dev\SapphireTest;
 use SilverStripe\ORM\Connect\MySQLDatabase;
 use SilverStripe\ORM\DataList;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\DataQuery;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\EagerLoadedList;
 use SilverStripe\ORM\ManyManyThroughList;
@@ -897,7 +898,7 @@ class DataListEagerLoadingTest extends SapphireTest
     {
         $this->createEagerLoadData(5);
         $filter = ['Title:GreaterThan' => 'obj 0'];
-        $dataList = EagerLoadObject::get()->filter($filter)->eagerLoad(...$eagerLoad);
+        $dataList = EagerLoadObject::get()->filter($filter)->eagerLoad($eagerLoad);
 
         // Validate that filtering results still actually works on the base list
         $this->assertListEquals([
@@ -1288,5 +1289,303 @@ class DataListEagerLoadingTest extends SapphireTest
                 $this->assertInstanceOf(EagerLoadedList::class, $eagerLoaded2);
             }
         }
+        $this->assertGreaterThan(1, $loop1Count);
+        $this->assertGreaterThan(1, $loop2Count);
+    }
+
+    public function testInvalidAssociativeArray(): void
+    {
+        $this->expectException(LogicException::class);
+        $this->expectExceptionMessage(
+            'Value of associative array must be a callable.'
+            . 'If you don\'t want to pre-filter the list, use an indexed array.'
+        );
+        EagerLoadObject::get()->eagerLoad(['HasManyEagerLoadObjects' => 'HasManyEagerLoadObjects']);
+    }
+
+    public function provideNoLimitEagerLoadingQuery(): array
+    {
+        // Note we don't test has_one or belongs_to because those don't accept a callback at all.
+        return [
+            'limit list directly - has_many' => [
+                'relation' => 'HasManyEagerLoadObjects',
+                'relationType' => 'has_many',
+                'callback' => fn (DataList $list) => $list->limit(1),
+            ],
+            'limit list directly - many_many' => [
+                'relation' => 'ManyManyEagerLoadObjects',
+                'relationType' => 'many_many',
+                'callback' => fn (DataList $list) => $list->limit(1),
+            ],
+            'limit underlying dataquery - has_many' => [
+                'relation' => 'HasManyEagerLoadObjects',
+                'relationType' => 'has_many',
+                'callback' => fn (DataList $list) => $list->alterDataQuery(fn (DataQuery $query) => $query->limit(1)),
+            ],
+            'limit underlying dataquery - many_many' => [
+                'relation' => 'ManyManyEagerLoadObjects',
+                'relationType' => 'many_many',
+                'callback' => fn (DataList $list) => $list->alterDataQuery(fn (DataQuery $query) => $query->limit(1)),
+            ],
+        ];
+    }
+
+    /**
+     * Tests that attempting to limit an eagerloading query will throw an exception.
+     *
+     * @dataProvider provideNoLimitEagerLoadingQuery
+     */
+    public function testNoLimitEagerLoadingQuery(string $relation, string $relationType, callable $callback): void
+    {
+        // Need to have at least one record in the main list for eagerloading to even be triggered.
+        $record = new EagerLoadObject();
+        $record->write();
+
+        $this->expectException(LogicException::class);
+        $this->expectExceptionMessage(
+            "Cannot apply limit to eagerloaded data for $relationType relation $relation."
+        );
+        EagerLoadObject::get()->eagerLoad([$relation => $callback])->toArray();
+    }
+
+    public function provideCannotManipulateUnaryRelationQuery(): array
+    {
+        return [
+            'has_one' => [
+                'relation' => 'HasOneEagerLoadObject',
+                'relationType' => 'has_one',
+            ],
+            'belongs_to' => [
+                'relation' => 'BelongsToEagerLoadObject',
+                'relationType' => 'belongs_to',
+            ],
+        ];
+    }
+
+    /**
+     * Tests that attempting to manipulate a has_one or belongs_to eagerloading query will throw an exception.
+     *
+     * @dataProvider provideCannotManipulateUnaryRelationQuery
+     */
+    public function testCannotManipulateUnaryRelationQuery(string $relation, string $relationType): void
+    {
+        // Need to have at least one record in the main list for eagerloading to even be triggered.
+        $record = new EagerLoadObject();
+        $record->write();
+
+        $this->expectException(LogicException::class);
+        $this->expectExceptionMessage(
+            "Cannot manipulate eagerloading query for $relationType relation $relation"
+        );
+        EagerLoadObject::get()->eagerLoad([$relation => fn (DataList $list) => $list])->toArray();
+    }
+
+    /**
+     * Tests that attempting to manipulate an eagerloading query without returning the list will throw an exception.
+     */
+    public function testManipulatingEagerloadingQueryNoReturn(): void
+    {
+        // Need to have at least one record in the main list for eagerloading to even be triggered.
+        $record = new EagerLoadObject();
+        $record->write();
+        $this->expectException(LogicException::class);
+        $this->expectExceptionMessage(
+            'Eagerloading callback for has_many relation HasManyEagerLoadObjects must return a DataList.'
+        );
+        EagerLoadObject::get()->eagerLoad([
+            'HasManyEagerLoadObjects' => function (DataList $list) {
+                $list->filter('ID', 1);
+            }
+        ])->toArray();
+    }
+
+    public function provideManipulatingEagerloadingQuery(): array
+    {
+        return [
+            'nested has_many' => [
+                'relationType' => 'has_many',
+                'relations' => [
+                    'HasManyEagerLoadObjects' => HasManyEagerLoadObject::class,
+                    'HasManySubEagerLoadObjects' => HasManySubEagerLoadObject::class,
+                ],
+                'eagerLoad' => [
+                    'HasManyEagerLoadObjects' => fn (DataList $list) => $list->filter(['Title:StartsWith' => 'HasMany T'])->Sort('Title', 'ASC'),
+                    'HasManyEagerLoadObjects.HasManySubEagerLoadObjects' => fn (DataList $list) => $list->Sort(['Title' => 'DESC']),
+                ],
+                'expected' => [
+                    'first loop' => ['HasMany Three', 'HasMany Two'],
+                    'second loop' => ['Sub B', 'Sub A'],
+                ],
+            ],
+            'nested has_many (reverse sort)' => [
+                'relationType' => 'has_many',
+                'relations' => [
+                    'HasManyEagerLoadObjects' => HasManyEagerLoadObject::class,
+                    'HasManySubEagerLoadObjects' => HasManySubEagerLoadObject::class,
+                ],
+                'eagerLoad' => [
+                    'HasManyEagerLoadObjects' => fn (DataList $list) => $list->filter(['Title:StartsWith' => 'HasMany T'])->Sort('Title', 'DESC'),
+                    'HasManyEagerLoadObjects.HasManySubEagerLoadObjects' => fn (DataList $list) => $list->Sort(['Title' => 'ASC']),
+                ],
+                'expected' => [
+                    'first loop' => ['HasMany Two', 'HasMany Three'],
+                    'second loop' => ['Sub A', 'Sub B'],
+                ],
+            ],
+            'nested many_many' => [
+                'relationType' => 'many_many',
+                'relations' => [
+                    'ManyManyEagerLoadObjects' => ManyManyEagerLoadObject::class,
+                    'ManyManySubEagerLoadObjects' => ManyManySubEagerLoadObject::class,
+                ],
+                'eagerLoad' => [
+                    'ManyManyEagerLoadObjects' => fn (DataList $list) => $list->filter(['Title:StartsWith' => 'ManyMany T'])->Sort('Title', 'ASC'),
+                    'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects' => fn (DataList $list) => $list->Sort(['Title' => 'DESC']),
+                ],
+                'expected' => [
+                    'first loop' => ['ManyMany Three', 'ManyMany Two'],
+                    'second loop' => ['Sub B', 'Sub A'],
+                ],
+            ],
+            'nested many_many (reverse sort)' => [
+                'relationType' => 'many_many',
+                'relations' => [
+                    'ManyManyEagerLoadObjects' => ManyManyEagerLoadObject::class,
+                    'ManyManySubEagerLoadObjects' => ManyManySubEagerLoadObject::class,
+                ],
+                'eagerLoad' => [
+                    'ManyManyEagerLoadObjects' => fn (DataList $list) => $list->filter(['Title:StartsWith' => 'ManyMany T'])->Sort('Title', 'DESC'),
+                    'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects' => fn (DataList $list) => $list->Sort(['Title' => 'ASC']),
+                ],
+                'expected' => [
+                    'first loop' => ['ManyMany Two', 'ManyMany Three'],
+                    'second loop' => ['Sub A', 'Sub B'],
+                ],
+            ],
+            'nested belongs_many_many' => [
+                'relationType' => 'belongs_many_many',
+                'relations' => [
+                    'BelongsManyManyEagerLoadObjects' => BelongsManyManyEagerLoadObject::class,
+                    'BelongsManyManySubEagerLoadObjects' => BelongsManyManySubEagerLoadObject::class,
+                ],
+                'eagerLoad' => [
+                    'BelongsManyManyEagerLoadObjects' => fn (DataList $list) => $list->filter(['Title:StartsWith' => 'ManyMany T'])->Sort('Title', 'ASC'),
+                    'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects' => fn (DataList $list) => $list->Sort(['Title' => 'DESC']),
+                ],
+                'expected' => [
+                    'first loop' => ['ManyMany Three', 'ManyMany Two'],
+                    'second loop' => ['Sub B', 'Sub A'],
+                ],
+            ],
+            'nested belongs_many_many (reverse sort)' => [
+                'relationType' => 'belongs_many_many',
+                'relations' => [
+                    'BelongsManyManyEagerLoadObjects' => BelongsManyManyEagerLoadObject::class,
+                    'BelongsManyManySubEagerLoadObjects' => BelongsManyManySubEagerLoadObject::class,
+                ],
+                'eagerLoad' => [
+                    'BelongsManyManyEagerLoadObjects' => fn (DataList $list) => $list->filter(['Title:StartsWith' => 'ManyMany T'])->Sort('Title', 'DESC'),
+                    'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects' => fn (DataList $list) => $list->Sort(['Title' => 'ASC']),
+                ],
+                'expected' => [
+                    'first loop' => ['ManyMany Two', 'ManyMany Three'],
+                    'second loop' => ['Sub A', 'Sub B'],
+                ],
+            ],
+            'nested many_many_through' => [
+                'relationType' => 'many_many_through',
+                'relations' => [
+                    'ManyManyThroughEagerLoadObjects' => ManyManyThroughEagerLoadObject::class,
+                    'ManyManyThroughSubEagerLoadObjects' => ManyManyThroughSubEagerLoadObject::class,
+                ],
+                'eagerLoad' => [
+                    'ManyManyThroughEagerLoadObjects' => fn (DataList $list) => $list->filter(['Title:StartsWith' => 'ManyMany T'])->Sort('Title', 'ASC'),
+                    'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects' => fn (DataList $list) => $list->Sort(['Title' => 'DESC']),
+                ],
+                'expected' => [
+                    'first loop' => ['ManyMany Three', 'ManyMany Two'],
+                    'second loop' => ['Sub B', 'Sub A'],
+                ],
+            ],
+            'nested many_many_through (reverse sort)' => [
+                'relationType' => 'many_many_through',
+                'relations' => [
+                    'ManyManyThroughEagerLoadObjects' => ManyManyThroughEagerLoadObject::class,
+                    'ManyManyThroughSubEagerLoadObjects' => ManyManyThroughSubEagerLoadObject::class,
+                ],
+                'eagerLoad' => [
+                    'ManyManyThroughEagerLoadObjects' => fn (DataList $list) => $list->filter(['Title:StartsWith' => 'ManyMany T'])->Sort('Title', 'DESC'),
+                    'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects' => fn (DataList $list) => $list->Sort(['Title' => 'ASC']),
+                ],
+                'expected' => [
+                    'first loop' => ['ManyMany Two', 'ManyMany Three'],
+                    'second loop' => ['Sub A', 'Sub B'],
+                ],
+            ],
+        ];
+    }
+
+    /**
+     * Tests that callbacks can be used to manipulate eagerloading queries
+     *
+     * @dataProvider provideManipulatingEagerloadingQuery
+     */
+    public function testManipulatingEagerloadingQuery(string $relationType, array $relations, array $eagerLoad, array $expected): void
+    {
+        $relationNames = array_keys($relations);
+        $relationOne = $relationNames[0];
+        $relationTwo = $relationNames[1];
+        $classOne = $relations[$relationOne];
+        $classTwo = $relations[$relationTwo];
+        // Prepare fixtures.
+        // Eager loading is different to most tests - we build fixtures at run time per test
+        // to avoid wasting a bunch of CI time building test-specific YAML fixtures.
+        $record = new EagerLoadObject();
+        $record->write();
+        if ($relationType === 'has_many') {
+            $hasMany1 = new $classOne(['Title' => 'HasMany One']);
+            $hasMany2 = new $classOne(['Title' => 'HasMany Two']);
+            $hasMany3 = new $classOne(['Title' => 'HasMany Three']);
+            $hasMany = [$hasMany1, $hasMany2, $hasMany3];
+            foreach ($hasMany as $hasManyRecord) {
+                $hasManyRecord->write();
+                // Since these are has_many they can't share the same records, so build
+                // separate records for each list.
+                $hasManySub1 = new $classTwo(['Title' => 'Sub A']);
+                $hasManySub2 = new $classTwo(['Title' => 'Sub B']);
+                $hasManySub1->write();
+                $hasManySub2->write();
+                $hasManyRecord->$relationTwo()->addMany([$hasManySub1, $hasManySub2]);
+            }
+            $record->$relationOne()->addMany($hasMany);
+        } elseif (str_contains($relationType, 'many_many')) {
+            $manyMany1 = new $classOne(['Title' => 'ManyMany One']);
+            $manyMany2 = new $classOne(['Title' => 'ManyMany Two']);
+            $manyMany3 = new $classOne(['Title' => 'ManyMany Three']);
+            $manyManySub1 = new $classTwo(['Title' => 'Sub A']);
+            $manyManySub2 = new $classTwo(['Title' => 'Sub B']);
+            $manyManySub1->write();
+            $manyManySub2->write();
+            $manyMany = [$manyMany1, $manyMany2, $manyMany3];
+            foreach ($manyMany as $manyManyRecord) {
+                $manyManyRecord->write();
+                $manyManyRecord->$relationTwo()->addMany([$manyManySub1, $manyManySub2]);
+            }
+            $record->$relationOne()->addMany($manyMany);
+        } else {
+            throw new LogicException("Unexpected relation type: $relationType");
+        }
+
+        // Loop through the relations and make assertions
+        foreach (EagerLoadObject::get()->filter(['ID' => $record->ID])->eagerLoad($eagerLoad) as $eagerLoadObject) {
+            $list = $eagerLoadObject->$relationOne();
+            $this->assertInstanceOf(EagerLoadedList::class, $list);
+            $this->assertSame($expected['first loop'], $list->column('Title'));
+            foreach ($list as $relatedObject) {
+                $list = $relatedObject->$relationTwo();
+                $this->assertInstanceOf(EagerLoadedList::class, $list);
+                $this->assertSame($expected['second loop'], $list->column('Title'));
+            }
+        }
     }
 }