FIX many_many extraFields and join records weren't in eagerloading

src/ORM/DataList.php

@@ -1030,28 +1030,37 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
         $hasManyIDField = null;
         $manyManyLastComponent = null;
         for ($i = 0; $i < count($relations) - 1; $i++) {
-            $hasOneComponent = $schema->hasOneComponent($dataClasses[$i], $relations[$i + 1]);
+            $parentDataClass = $dataClasses[$i];
+            $relationName = $relations[$i + 1];
+            $hasOneComponent = $schema->hasOneComponent($parentDataClass, $relationName);
             if ($hasOneComponent) {
                 $dataClasses[] = $hasOneComponent;
                 $hasOneIDField = $relations[$i + 1] . 'ID';
                 continue;
             }
-            $belongsToComponent = $schema->belongsToComponent($dataClasses[$i], $relations[$i + 1]);
+            $belongsToComponent = $schema->belongsToComponent($parentDataClass, $relationName);
             if ($belongsToComponent) {
                 $dataClasses[] = $belongsToComponent;
-                $belongsToIDField = $schema->getRemoteJoinField($dataClasses[$i], $relations[$i + 1], 'belongs_to');
+                $belongsToIDField = $schema->getRemoteJoinField($parentDataClass, $relationName, 'belongs_to');
                 continue;
             }
-            $hasManyComponent = $schema->hasManyComponent($dataClasses[$i], $relations[$i + 1]);
+            $hasManyComponent = $schema->hasManyComponent($parentDataClass, $relationName);
             if ($hasManyComponent) {
                 $dataClasses[] = $hasManyComponent;
-                $hasManyIDField = $schema->getRemoteJoinField($dataClasses[$i], $relations[$i + 1], 'has_many');
+                $hasManyIDField = $schema->getRemoteJoinField($parentDataClass, $relationName, 'has_many');
                 continue;
             }
             // this works for both many_many and belongs_many_many
-            $manyManyComponent = $schema->manyManyComponent($dataClasses[$i], $relations[$i + 1]);
+            $manyManyComponent = $schema->manyManyComponent($parentDataClass, $relationName);
             if ($manyManyComponent) {
                 $dataClasses[] = $manyManyComponent['childClass'];
+                $manyManyComponent['extraFields'] = $schema->manyManyExtraFieldsForComponent($parentDataClass, $relationName) ?: [];
+                if (is_a($manyManyComponent['relationClass'], ManyManyThroughList::class, true)) {
+                    $manyManyComponent['joinClass'] = $manyManyComponent['join'];
+                    $manyManyComponent['join'] = $schema->baseDataTable($manyManyComponent['joinClass']);
+                } else {
+                    $manyManyComponent['joinClass'] = null;
+                }
                 $manyManyLastComponent = $manyManyComponent;
                 continue;
             }
@@ -1130,7 +1139,8 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
                     $parentIDs,
                     $relationDataClass,
                     $eagerLoadRelation,
-                    $relationName
+                    $relationName,
+                    $parentDataClass
                 );
             } else {
                 throw new LogicException('Something went wrong with the eager loading');
@@ -1221,7 +1231,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
             $eagerLoadID = $relationItem->$hasManyIDField;
             if (!isset($this->eagerLoadedData[$eagerLoadRelation][$eagerLoadID][$relationName])) {
                 $arrayList = ArrayList::create();
-                $arrayList->setDataClass($relationItem->dataClass);
+                $arrayList->setDataClass($relationDataClass);
                 $this->eagerLoadedData[$eagerLoadRelation][$eagerLoadID][$relationName] = $arrayList;
             }
             $this->eagerLoadedData[$eagerLoadRelation][$eagerLoadID][$relationName]->push($relationItem);
@@ -1235,39 +1245,46 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
         array $parentIDs,
         string $relationDataClass,
         string $eagerLoadRelation,
-        string $relationName
+        string $relationName,
+        string $parentDataClass
     ): array {
         $parentIDField = $manyManyLastComponent['parentField'];
         $childIDField = $manyManyLastComponent['childField'];
-        // $join will either be:
+        $joinTable = $manyManyLastComponent['join'];
-        // - the join table name for many-many
+        $extraFields = $manyManyLastComponent['extraFields'];
-        // - the join data class for many-many-through
+        $joinClass = $manyManyLastComponent['joinClass'];
-        $join = $manyManyLastComponent['join'];
+
+        // Get the join records so we can correctly identify which children belong to which parents
+        $joinRows = DB::query('SELECT * FROM "' . $joinTable . '" WHERE "' . $parentIDField . '" IN (' . implode(',', $parentIDs) . ')');
 
         // many_many_through
-        if (is_a($manyManyLastComponent['relationClass'], ManyManyThroughList::class, true)) {
+        if ($joinClass !== null) {
-            $joinThroughObjs = DataObject::get($join)->filter([$parentIDField => $parentIDs]);
+            $relationList = ManyManyThroughList::create(
-            $relationItemIDs = [];
+                $relationDataClass,
-            $joinRows = [];
+                $joinClass,
-            foreach ($joinThroughObjs as $joinThroughObj) {
+                $childIDField,
-                $joinRows[] = [
+                $parentIDField,
-                    $parentIDField => $joinThroughObj->$parentIDField,
+                $extraFields,
-                    $childIDField => $joinThroughObj->$childIDField
+                $relationDataClass,
-                ];
+                $parentDataClass
-                $relationItemIDs[] = $joinThroughObj->$childIDField;
+            )->forForeignID($parentIDs);
-            }
         // many_many + belongs_many_many
         } else {
-            $joinTableQuery = DB::query('SELECT * FROM "' . $join . '" WHERE "' . $parentIDField . '" IN (' . implode(',', $parentIDs) . ')');
+            $relationList = ManyManyList::create(
-            $relationItemIDs = $joinTableQuery->column($childIDField);
+                $relationDataClass,
-            $joinRows = $joinTableQuery;
+                $joinTable,
+                $childIDField,
+                $parentIDField,
+                $extraFields
+            )->forForeignID($parentIDs);
         }
 
         // Get ALL of the items for this relation up front, for ALL of the parents
+        // Use a real RelationList here so that the extraFields and join record are correctly set for all relations
         // Fetched as a map so we can get the ID for all records up front (instead of in another nested loop)
         // Fetched after that as an array because for some reason that performs better in the loop
         // Note that "Me" is a method on ViewableData that returns $this - i.e. that is the actual DataObject record
-        $relationArray = DataObject::get($relationDataClass)->byIDs($relationItemIDs)->map('ID', 'Me')->toArray();
+        $relationArray = $relationList->map('ID', 'Me')->toArray();
 
         // Store the children in an ArrayList against the correct parent
         foreach ($joinRows as $row) {
@@ -1277,13 +1294,13 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
 
             if (!isset($this->eagerLoadedData[$eagerLoadRelation][$parentID][$relationName])) {
                 $arrayList = ArrayList::create();
-                $arrayList->setDataClass($relationItem->dataClass);
+                $arrayList->setDataClass($relationDataClass);
                 $this->eagerLoadedData[$eagerLoadRelation][$parentID][$relationName] = $arrayList;
             }
             $this->eagerLoadedData[$eagerLoadRelation][$parentID][$relationName]->push($relationItem);
         }
 
-        return [$relationArray, $relationItemIDs];
+        return [$relationArray, array_keys($relationArray)];
     }
 
     /**

tests/php/ORM/DataListEagerLoadingTest.php

@@ -5,6 +5,7 @@ namespace SilverStripe\ORM\Tests;
 use InvalidArgumentException;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\ORM\DataList;
+use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\Tests\DataListTest\EagerLoading\EagerLoadObject;
 use SilverStripe\ORM\Tests\DataListTest\EagerLoading\HasOneEagerLoadObject;
 use SilverStripe\ORM\Tests\DataListTest\EagerLoading\HasOneSubEagerLoadObject;
@@ -34,14 +35,11 @@ use SilverStripe\ORM\Tests\DataListTest\EagerLoading\MixedManyManyEagerLoadObjec
 class DataListEagerLoadingTest extends SapphireTest
 {
 
-    // Borrow the model from DataObjectTest
-    // protected static $fixture_file = 'DataObjectTest.yml';
-
     protected $usesDatabase = true;
 
     public static function getExtraDataObjects()
     {
-        $eagerLoadDataObjects = [
+        return [
             EagerLoadObject::class,
             HasOneEagerLoadObject::class,
             HasOneSubEagerLoadObject::class,
@@ -68,11 +66,6 @@ class DataListEagerLoadingTest extends SapphireTest
             MixedHasOneEagerLoadObject::class,
             MixedManyManyEagerLoadObject::class,
         ];
-        return array_merge(
-            // DataObjectTest::$extra_data_objects,
-            // ManyManyListTest::$extra_data_objects,
-            $eagerLoadDataObjects
-        );
     }
 
     /**
@@ -337,12 +330,27 @@ class DataListEagerLoadingTest extends SapphireTest
                     }
                 }
             }
+            // many_many with extraFields
+            for ($j = 0; $j < 2; $j++) {
+                $manyManyObj = new ManyManyEagerLoadObject();
+                $manyManyObj->Title = "manyManyObj $i $j";
+                $manyManyObj->write();
+                $obj->ManyManyEagerLoadWithExtraFields()->add($manyManyObj, [
+                    'SomeText' => "Some text here $i $j",
+                    'SomeBool' => $j % 2 === 0, // true if even
+                    'SomeInt' => $j,
+                ]);
+            }
             // many_many_through
             for ($j = 0; $j < 2; $j++) {
                 $manyManyThroughObj = new ManyManyThroughEagerLoadObject();
                 $manyManyThroughObj->Title = "manyManyThroughObj $i $j";
                 $manyManyThroughObj->write();
-                $obj->ManyManyThroughEagerLoadObjects()->add($manyManyThroughObj);
+                $obj->ManyManyThroughEagerLoadObjects()->add($manyManyThroughObj, [
+                    'Title' => "Some text here $i $j",
+                    'SomeBool' => $j % 2 === 0, // true if even
+                    'SomeInt' => $j,
+                ]);
                 for ($k = 0; $k < 2; $k++) {
                     $manyManyThroughSubObj = new ManyManyThroughSubEagerLoadObject();
                     $manyManyThroughSubObj->Title = "manyManyThroughSubObj $i $j $k";
@@ -686,4 +694,98 @@ class DataListEagerLoadingTest extends SapphireTest
             ]
         ];
     }
+
+    /**
+     * @dataProvider provideEagerLoadManyManyExtraFields
+     */
+    public function testEagerLoadManyManyExtraFields(string $parentClass, string $eagerLoadRelation): void
+    {
+        $this->createEagerLoadData();
+
+        foreach (DataObject::get($parentClass)->eagerLoad($eagerLoadRelation) as $parentRecord) {
+            if ($parentClass === EagerLoadObject::class) {
+                $this->validateEagerLoadManyManyExtraFields($parentRecord->ManyManyEagerLoadWithExtraFields());
+            } else {
+                foreach ($parentRecord->EagerLoadObjects() as $relationRecord) {
+                    $this->validateEagerLoadManyManyExtraFields($relationRecord->ManyManyEagerLoadWithExtraFields());
+                }
+            }
+        }
+    }
+
+    private function validateEagerLoadManyManyExtraFields($relationList): void
+    {
+        foreach ($relationList as $record) {
+            preg_match('/manyManyObj (?<i>\d+) (?<j>\d+)/', $record->Title, $matches);
+            $i = (int) $matches['i'];
+            $j = (int) $matches['j'];
+            $this->assertSame("Some text here $i $j", $record->SomeText);
+            // Bool fields are just small ints, so the data is either 1 or 0
+            $this->assertSame($j % 2 === 0 ? 1 : 0, $record->SomeBool);
+            $this->assertSame($j, $record->SomeInt);
+        }
+    }
+
+    public function provideEagerLoadManyManyExtraFields(): array
+    {
+        return [
+            [
+                EagerLoadObject::class,
+                'ManyManyEagerLoadWithExtraFields',
+            ],
+            [
+                BelongsManyManyEagerLoadObject::class,
+                'EagerLoadObjects.ManyManyEagerLoadWithExtraFields',
+            ]
+        ];
+    }
+
+    /**
+     * @dataProvider provideEagerLoadManyManyThroughJoinRecords
+     */
+    public function testEagerLoadManyManyThroughJoinRecords(string $parentClass, string $eagerLoadRelation): void
+    {
+        $this->createEagerLoadData();
+
+        foreach (DataObject::get($parentClass)->eagerLoad($eagerLoadRelation) as $parentRecord) {
+            if ($parentClass === EagerLoadObject::class) {
+                $this->validateEagerLoadManyManyThroughJoinRecords($parentRecord->ManyManyThroughEagerLoadObjects());
+            } else {
+                foreach ($parentRecord->EagerLoadObjects() as $relationRecord) {
+                    $this->validateEagerLoadManyManyThroughJoinRecords($relationRecord->ManyManyThroughEagerLoadObjects());
+                }
+            }
+        }
+    }
+
+    private function validateEagerLoadManyManyThroughJoinRecords($relationList): void
+    {
+        /** @var DataObject $record */
+        foreach ($relationList as $record) {
+            $joinRecord = $record->getJoin();
+            $this->assertNotNull($joinRecord);
+
+            preg_match('/manyManyThroughObj (?<i>\d+) (?<j>\d+)/', $record->Title, $matches);
+            $i = (int) $matches['i'];
+            $j = (int) $matches['j'];
+            $this->assertSame("Some text here $i $j", $joinRecord->Title);
+            // Bool fields are just small ints, so the data is either 1 or 0
+            $this->assertSame($j % 2 === 0 ? 1 : 0, $joinRecord->SomeBool);
+            $this->assertSame($j, $joinRecord->SomeInt);
+        }
+    }
+
+    public function provideEagerLoadManyManyThroughJoinRecords(): array
+    {
+        return [
+            [
+                EagerLoadObject::class,
+                'ManyManyThroughEagerLoadObjects',
+            ],
+            [
+                BelongsManyManyEagerLoadObject::class,
+                'EagerLoadObjects.ManyManyThroughEagerLoadObjects',
+            ]
+        ];
+    }
 }

tests/php/ORM/DataListTest/EagerLoading/EagerLoadObject.php

@@ -33,6 +33,15 @@ class EagerLoadObject extends DataObject implements TestOnly
             'to' => 'ManyManyThroughEagerLoadObject',
         ],
         'MixedManyManyEagerLoadObjects' => MixedManyManyEagerLoadObject::class,
+        'ManyManyEagerLoadWithExtraFields' => ManyManyEagerLoadObject::class,
+    ];
+
+    private static $many_many_extraFields = [
+        'ManyManyEagerLoadWithExtraFields' => [
+            'SomeText' => 'Varchar',
+            'SomeBool' => 'Boolean',
+            'SomeInt' => 'Int',
+        ],
     ];
 
     private static $belongs_many_many = [

tests/php/ORM/DataListTest/EagerLoading/EagerLoadObjectManyManyThroughEagerLoadObject.php

@@ -10,7 +10,9 @@ class EagerLoadObjectManyManyThroughEagerLoadObject extends DataObject implement
     private static $table_name = 'EagerLoadObjectManyManyThroughEagerLoadObject';
 
     private static $db = [
-        'Title' => 'Varchar'
+        'Title' => 'Varchar',
+        'SomeBool' => 'Boolean',
+        'SomeInt' => 'Int',
     ];
 
     private static $has_one = [