tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 12:05:37 +00:00
Code Issues Projects Releases Wiki Activity

FIX Don't throw exception for empty eagerloaded relation (#11220)

Browse Source
This commit is contained in:
Guy Sartorelli 2024-05-06 18:06:54 +12:00 committed by GitHub
parent a92baeaf6f
commit a198c91628
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 70 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/ORM/DataList.php
View File

@ -1162,7 +1162,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
} }
} }
} else { } else {
throw new LogicException("Couldn't find parent for record $fetchedID on $relationType relation $relationName"); throw new LogicException("Couldn't find parent for record {$fetched->ID} on $relationType relation $relationName");
} }
} }
@ -1321,17 +1321,21 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
} }
// Get the join records so we can correctly identify which children belong to which parents // Get the join records so we can correctly identify which children belong to which parents
// This also holds extra fields data // If there are no parents and no children, skip this to avoid an error (and to skip an unnecessary DB call)
$fetchedIDsAsString = implode(',', $fetchedIDs); // Note that $joinRows also holds extra fields data
$joinRows = DB::query( $joinRows = [];
'SELECT * FROM "' . $joinTable if (!empty($parentIDs) && !empty($fetchedIDs)) {
// Only get joins relevant for the parent list $fetchedIDsAsString = implode(',', $fetchedIDs);
. '" WHERE "' . $parentIDField . '" IN (' . implode(',', $parentIDs) . ')' $joinRows = DB::query(
// Exclude any children that got filtered out 'SELECT * FROM "' . $joinTable
. ' AND ' . $childIDField . ' IN (' . $fetchedIDsAsString . ')' // Only get joins relevant for the parent list
// Respect sort order of fetched items . '" WHERE "' . $parentIDField . '" IN (' . implode(',', $parentIDs) . ')'
. ' ORDER BY FIELD(' . $childIDField . ', ' . $fetchedIDsAsString . ')' // 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 // Store the children in an EagerLoadedList against the correct parent
foreach ($joinRows as $row) { foreach ($joinRows as $row) {

54
tests/php/ORM/DataListEagerLoadingTest.php
View File

@ -12,6 +12,7 @@ use SilverStripe\ORM\DataQuery;
use SilverStripe\ORM\DB; use SilverStripe\ORM\DB;
use SilverStripe\ORM\EagerLoadedList; use SilverStripe\ORM\EagerLoadedList;
use SilverStripe\ORM\ManyManyThroughList; use SilverStripe\ORM\ManyManyThroughList;
use SilverStripe\ORM\SS_List;
use SilverStripe\ORM\Tests\DataListTest\EagerLoading\EagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\EagerLoadObject;
use SilverStripe\ORM\Tests\DataListTest\EagerLoading\HasOneEagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\HasOneEagerLoadObject;
use SilverStripe\ORM\Tests\DataListTest\EagerLoading\HasOneSubEagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\HasOneSubEagerLoadObject;
@ -754,6 +755,59 @@ class DataListEagerLoadingTest extends SapphireTest
return [$results, $selectCount]; return [$results, $selectCount];
} }
/**
* @dataProvider provideEagerLoadRelationsEmpty
*/
public function testEagerLoadRelationsEmpty(string $eagerLoadRelation, int $expectedNumQueries): void
{
EagerLoadObject::create(['Title' => 'test object'])->write();
$dataList = EagerLoadObject::get()->eagerLoad($eagerLoadRelation);
$this->startCountingSelectQueries();
foreach ($dataList as $record) {
$relation = $record->$eagerLoadRelation();
if ($relation instanceof SS_List) {
// The list should be an empty eagerloaded list
$this->assertInstanceOf(EagerLoadedList::class, $relation);
$this->assertCount(0, $relation);
} elseif ($relation !== null) {
// There should be no record here
$this->assertSame($relation->ID, 0);
}
}
$numQueries = $this->stopCountingSelectQueries();
$this->assertSame($expectedNumQueries, $numQueries);
}
public function provideEagerLoadRelationsEmpty(): array
{
return [
'has_one' => [
'eagerLoad' => 'HasOneEagerLoadObject',
'expectedNumQueries' => 2,
],
'belongs_to' => [
'eagerLoad' => 'BelongsToEagerLoadObject',
'expectedNumQueries' => 2,
],
'has_many' => [
'eagerLoad' => 'HasManyEagerLoadObjects',
'expectedNumQueries' => 2,
],
'many_many' => [
'eagerLoad' => 'ManyManyEagerLoadObjects',
'expectedNumQueries' => 2,
],
'many_many through' => [
'eagerLoad' => 'ManyManyThroughEagerLoadObjects',
'expectedNumQueries' => 2,
],
'belongs_many_many' => [
'eagerLoad' => 'BelongsManyManyEagerLoadObjects',
'expectedNumQueries' => 2,
],
];
}
public function testEagerLoadFourthLevelException(): void public function testEagerLoadFourthLevelException(): void
{ {
$eagerLoadRelation = implode('.', [ $eagerLoadRelation = implode('.', [