From 9bed2a3d98ac66e1741cffbef2faac823945f906 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Thu, 6 Jul 2023 14:21:28 +1200 Subject: [PATCH 1/6] MNT Refactor eagerloading tests into their own class --- tests/php/ORM/DataListEagerLoadingTest.php | 689 ++++++++++++++++++ tests/php/ORM/DataListTest.php | 641 +--------------- .../BelongsManyManyEagerLoadObject.php | 2 +- .../BelongsManyManySubEagerLoadObject.php | 2 +- .../BelongsManyManySubSubEagerLoadObject.php | 2 +- .../BelongsToEagerLoadObject.php | 2 +- .../BelongsToSubEagerLoadObject.php | 2 +- .../BelongsToSubSubEagerLoadObject.php | 2 +- .../{ => EagerLoading}/EagerLoadObject.php | 2 +- ...adObjectManyManyThroughEagerLoadObject.php | 2 +- .../HasManyEagerLoadObject.php | 2 +- .../HasManySubEagerLoadObject.php | 2 +- .../HasManySubSubEagerLoadObject.php | 2 +- .../HasOneEagerLoadObject.php | 2 +- .../HasOneSubEagerLoadObject.php | 2 +- .../HasOneSubSubEagerLoadObject.php | 2 +- .../ManyManyEagerLoadObject.php | 2 +- .../ManyManySubEagerLoadObject.php | 2 +- .../ManyManySubSubEagerLoadObject.php | 2 +- .../ManyManyThroughEagerLoadObject.php | 2 +- ...bjectManyManyThroughSubEagerLoadObject.php | 2 +- .../ManyManyThroughSubEagerLoadObject.php | 2 +- ...ctManyManyThroughSubSubEagerLoadObject.php | 2 +- .../ManyManyThroughSubSubEagerLoadObject.php | 2 +- .../MixedHasManyEagerLoadObject.php | 2 +- .../MixedHasOneEagerLoadObject.php | 2 +- .../MixedManyManyEagerLoadObject.php | 2 +- 27 files changed, 715 insertions(+), 665 deletions(-) create mode 100644 tests/php/ORM/DataListEagerLoadingTest.php rename tests/php/ORM/DataListTest/{ => EagerLoading}/BelongsManyManyEagerLoadObject.php (89%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/BelongsManyManySubEagerLoadObject.php (93%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/BelongsManyManySubSubEagerLoadObject.php (93%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/BelongsToEagerLoadObject.php (88%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/BelongsToSubEagerLoadObject.php (89%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/BelongsToSubSubEagerLoadObject.php (86%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/EagerLoadObject.php (94%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/EagerLoadObjectManyManyThroughEagerLoadObject.php (89%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/HasManyEagerLoadObject.php (88%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/HasManySubEagerLoadObject.php (89%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/HasManySubSubEagerLoadObject.php (89%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/HasOneEagerLoadObject.php (88%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/HasOneSubEagerLoadObject.php (89%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/HasOneSubSubEagerLoadObject.php (86%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/ManyManyEagerLoadObject.php (89%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/ManyManySubEagerLoadObject.php (90%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/ManyManySubSubEagerLoadObject.php (88%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/ManyManyThroughEagerLoadObject.php (92%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/ManyManyThroughEagerLoadObjectManyManyThroughSubEagerLoadObject.php (90%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/ManyManyThroughSubEagerLoadObject.php (92%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/ManyManyThroughSubEagerLoadObjectManyManyThroughSubSubEagerLoadObject.php (91%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/ManyManyThroughSubSubEagerLoadObject.php (87%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/MixedHasManyEagerLoadObject.php (88%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/MixedHasOneEagerLoadObject.php (87%) rename tests/php/ORM/DataListTest/{ => EagerLoading}/MixedManyManyEagerLoadObject.php (89%) diff --git a/tests/php/ORM/DataListEagerLoadingTest.php b/tests/php/ORM/DataListEagerLoadingTest.php new file mode 100644 index 000000000..46d012bb3 --- /dev/null +++ b/tests/php/ORM/DataListEagerLoadingTest.php @@ -0,0 +1,689 @@ +createEagerLoadData(); + $dataList = EagerLoadObject::get()->eagerLoad(...$eagerLoad); + list($results, $selectCount) = $this->iterateEagerLoadData($dataList); + $expectedResults = $this->expectedEagerLoadData(); + // Sort because the order of the results doesn't really matter - and has proven to be different in postgres + sort($expectedResults); + sort($results); + + $this->assertSame($expectedResults, $results); + $this->assertSame($expected, $selectCount); + } + + public function provideEagerLoadRelations(): array + { + return [ + [ + 'iden' => 'lazy-load', + 'eagerLoad' => [], + 'expected' => 83 + ], + [ + 'iden' => 'has-one-a', + 'eagerLoad' => [ + 'HasOneEagerLoadObject', + ], + 'expected' => 82 + ], + [ + 'iden' => 'has-one-b', + 'eagerLoad' => [ + 'HasOneEagerLoadObject.HasOneSubEagerLoadObject', + ], + 'expected' => 81 + ], + [ + 'iden' => 'has-one-c', + 'eagerLoad' => [ + 'HasOneEagerLoadObject.HasOneSubEagerLoadObject.HasOneSubSubEagerLoadObject', + ], + 'expected' => 80 + ], + [ + 'iden' => 'belongs-to-a', + 'eagerLoad' => [ + 'BelongsToEagerLoadObject', + ], + 'expected' => 82 + ], + [ + 'iden' => 'belongs-to-b', + 'eagerLoad' => [ + 'BelongsToEagerLoadObject.BelongsToSubEagerLoadObject', + ], + 'expected' => 81 + ], + [ + 'iden' => 'belongs-to-c', + 'eagerLoad' => [ + 'BelongsToEagerLoadObject.BelongsToSubEagerLoadObject.BelongsToSubSubEagerLoadObject', + ], + 'expected' => 80 + ], + [ + 'iden' => 'has-many-a', + 'eagerLoad' => [ + 'HasManyEagerLoadObjects', + ], + 'expected' => 82 + ], + [ + 'iden' => 'has-many-b', + 'eagerLoad' => [ + 'HasManyEagerLoadObjects.HasManySubEagerLoadObjects', + ], + 'expected' => 79 + ], + [ + 'iden' => 'has-many-c', + 'eagerLoad' => [ + 'HasManyEagerLoadObjects.HasManySubEagerLoadObjects.HasManySubSubEagerLoadObjects', + ], + 'expected' => 72 + ], + [ + 'iden' => 'many-many-a', + 'eagerLoad' => [ + 'ManyManyEagerLoadObjects', + ], + 'expected' => 83 // same number as lazy-load, though without an INNER JOIN + ], + [ + 'iden' => 'many-many-b', + 'eagerLoad' => [ + 'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects', + ], + 'expected' => 81 + ], + [ + 'iden' => 'many-many-c', + 'eagerLoad' => [ + 'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects.ManyManySubSubEagerLoadObjects', + ], + 'expected' => 75 + ], + [ + 'iden' => 'many-many-through-a', + 'eagerLoad' => [ + 'ManyManyThroughEagerLoadObjects', + ], + 'expected' => 83 + ], + [ + 'iden' => 'many-many-through-b', + 'eagerLoad' => [ + 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects', + ], + 'expected' => 81 + ], + [ + 'iden' => 'many-many-through-c', + 'eagerLoad' => [ + 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects.ManyManyThroughSubSubEagerLoadObjects', + ], + 'expected' => 75 + ], + [ + 'iden' => 'belongs-many-many-a', + 'eagerLoad' => [ + 'BelongsManyManyEagerLoadObjects', + ], + 'expected' => 83 + ], + [ + 'iden' => 'belongs-many-many-b', + 'eagerLoad' => [ + 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects', + ], + 'expected' => 81 + ], + [ + 'iden' => 'belongs-many-many-c', + 'eagerLoad' => [ + 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects.BelongsManyManySubSubEagerLoadObjects', + ], + 'expected' => 75 + ], + [ + 'iden' => 'mixed-a', + 'eagerLoad' => [ + 'MixedManyManyEagerLoadObjects', + ], + 'expected' => 83 + ], + [ + 'iden' => 'mixed-b', + 'eagerLoad' => [ + 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects', + ], + 'expected' => 80 + ], + [ + 'iden' => 'mixed-c', + 'eagerLoad' => [ + 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.MixedHasOneEagerLoadObject', + ], + 'expected' => 73 + ], + [ + 'iden' => 'all', + 'eagerLoad' => [ + 'HasOneEagerLoadObject.HasOneSubEagerLoadObject.HasOneSubSubEagerLoadObject', + 'BelongsToEagerLoadObject.BelongsToSubEagerLoadObject.BelongsToSubSubEagerLoadObject', + 'HasManyEagerLoadObjects.HasManySubEagerLoadObjects.HasManySubSubEagerLoadObjects', + 'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects.ManyManySubSubEagerLoadObjects', + 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects.ManyManyThroughSubSubEagerLoadObjects', + 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects.BelongsManyManySubSubEagerLoadObjects', + 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.MixedHasOneEagerLoadObject', + ], + 'expected' => 32 + ], + ]; + } + + private function createEagerLoadData(): void + { + for ($i = 0; $i < 2; $i++) { + // base object + $obj = new EagerLoadObject(); + $obj->Title = "obj $i"; + $objID = $obj->write(); + // has_one + $hasOneObj = new HasOneEagerLoadObject(); + $hasOneObj->Title = "hasOneObj $i"; + $hasOneObjID = $hasOneObj->write(); + $obj->HasOneEagerLoadObjectID = $hasOneObjID; + $obj->write(); + $hasOneSubObj = new HasOneSubEagerLoadObject(); + $hasOneSubObj->Title = "hasOneSubObj $i"; + $hasOneSubObjID = $hasOneSubObj->write(); + $hasOneObj->HasOneSubEagerLoadObjectID = $hasOneSubObjID; + $hasOneObj->write(); + $hasOneSubSubObj = new HasOneSubSubEagerLoadObject(); + $hasOneSubSubObj->Title = "hasOneSubSubObj $i"; + $hasOneSubSubObjID = $hasOneSubSubObj->write(); + $hasOneSubObj->HasOneSubSubEagerLoadObjectID = $hasOneSubSubObjID; + $hasOneSubObj->write(); + // belongs_to + $belongsToObj = new BelongsToEagerLoadObject(); + $belongsToObj->EagerLoadObjectID = $objID; + $belongsToObj->Title = "belongsToObj $i"; + $belongsToObjID = $belongsToObj->write(); + $belongsToSubObj = new BelongsToSubEagerLoadObject(); + $belongsToSubObj->BelongsToEagerLoadObjectID = $belongsToObjID; + $belongsToSubObj->Title = "belongsToSubObj $i"; + $belongsToSubObjID = $belongsToSubObj->write(); + $belongsToSubSubObj = new BelongsToSubSubEagerLoadObject(); + $belongsToSubSubObj->BelongsToSubEagerLoadObjectID = $belongsToSubObjID; + $belongsToSubSubObj->Title = "belongsToSubSubObj $i"; + $belongsToSubSubObj->write(); + // has_many + for ($j = 0; $j < 2; $j++) { + $hasManyObj = new HasManyEagerLoadObject(); + $hasManyObj->EagerLoadObjectID = $objID; + $hasManyObj->Title = "hasManyObj $i $j"; + $hasManyObjID = $hasManyObj->write(); + for ($k = 0; $k < 2; $k++) { + $hasManySubObj = new HasManySubEagerLoadObject(); + $hasManySubObj->HasManyEagerLoadObjectID = $hasManyObjID; + $hasManySubObj->Title = "hasManySubObj $i $j $k"; + $hasManySubObjID = $hasManySubObj->write(); + for ($l = 0; $l < 2; $l++) { + $hasManySubSubObj = new HasManySubSubEagerLoadObject(); + $hasManySubSubObj->HasManySubEagerLoadObjectID = $hasManySubObjID; + $hasManySubSubObj->Title = "hasManySubSubObj $i $j $k $l"; + $hasManySubSubObj->write(); + } + } + } + // many_many + for ($j = 0; $j < 2; $j++) { + $manyManyObj = new ManyManyEagerLoadObject(); + $manyManyObj->Title = "manyManyObj $i $j"; + $manyManyObj->write(); + $obj->ManyManyEagerLoadObjects()->add($manyManyObj); + for ($k = 0; $k < 2; $k++) { + $manyManySubObj = new ManyManySubEagerLoadObject(); + $manyManySubObj->Title = "manyManySubObj $i $j $k"; + $manyManySubObj->write(); + $manyManyObj->ManyManySubEagerLoadObjects()->add($manyManySubObj); + for ($l = 0; $l < 2; $l++) { + $manyManySubSubObj = new ManyManySubSubEagerLoadObject(); + $manyManySubSubObj->Title = "manyManySubSubObj $i $j $k $l"; + $manyManySubSubObj->write(); + $manyManySubObj->ManyManySubSubEagerLoadObjects()->add($manyManySubSubObj); + } + } + } + // many_many_through + for ($j = 0; $j < 2; $j++) { + $manyManyThroughObj = new ManyManyThroughEagerLoadObject(); + $manyManyThroughObj->Title = "manyManyThroughObj $i $j"; + $manyManyThroughObj->write(); + $obj->ManyManyThroughEagerLoadObjects()->add($manyManyThroughObj); + for ($k = 0; $k < 2; $k++) { + $manyManyThroughSubObj = new ManyManyThroughSubEagerLoadObject(); + $manyManyThroughSubObj->Title = "manyManyThroughSubObj $i $j $k"; + $manyManyThroughSubObj->write(); + $manyManyThroughObj->ManyManyThroughSubEagerLoadObjects()->add($manyManyThroughSubObj); + for ($l = 0; $l < 2; $l++) { + $manyManyThroughSubSubObj = new ManyManyThroughSubSubEagerLoadObject(); + $manyManyThroughSubSubObj->Title = "manyManyThroughSubSubObj $i $j $k $l"; + $manyManyThroughSubSubObj->write(); + $manyManyThroughSubObj->ManyManyThroughSubSubEagerLoadObjects()->add($manyManyThroughSubSubObj); + } + } + } + // belongs_many_many + for ($j = 0; $j < 2; $j++) { + $belongsManyManyObj = new BelongsManyManyEagerLoadObject(); + $belongsManyManyObj->Title = "belongsManyManyObj $i $j"; + $belongsManyManyObj->write(); + $obj->BelongsManyManyEagerLoadObjects()->add($belongsManyManyObj); + for ($k = 0; $k < 2; $k++) { + $belongsManyManySubObj = new BelongsManyManySubEagerLoadObject(); + $belongsManyManySubObj->Title = "belongsManyManySubObj $i $j $k"; + $belongsManyManySubObj->write(); + $belongsManyManyObj->BelongsManyManySubEagerLoadObjects()->add($belongsManyManySubObj); + for ($l = 0; $l < 2; $l++) { + $belongsManyManySubSubObj = new BelongsManyManySubSubEagerLoadObject(); + $belongsManyManySubSubObj->Title = "belongsManyManySubSubObj $i $j $k $l"; + $belongsManyManySubSubObj->write(); + $belongsManyManySubObj->BelongsManyManySubSubEagerLoadObjects()->add($belongsManyManySubSubObj); + } + } + } + // mixed + for ($j = 0; $j < 2; $j++) { + $mixedManyManyObj = new MixedManyManyEagerLoadObject(); + $mixedManyManyObj->Title = "mixedManyManyObj $i $j"; + $mixedManyManyObj->write(); + $obj->MixedManyManyEagerLoadObjects()->add($mixedManyManyObj); + for ($k = 0; $k < 2; $k++) { + $mixedHasManyObj = new MixedHasManyEagerLoadObject(); + $mixedHasManyObj->Title = "mixedHasManyObj $i $j $k"; + $mixedHasManyObjID = $mixedHasManyObj->write(); + $mixedManyManyObj->MixedHasManyEagerLoadObjects()->add($mixedHasManyObj); + for ($l = 0; $l < 2; $l++) { + $mixedHasOneObj = new MixedHasOneEagerLoadObject(); + $mixedHasOneObj->Title = "mixedHasOneObj $i $j $k $l"; + $mixedHasOneObjID = $mixedHasOneObj->write(); + $mixedHasManyObj->MixedHasOneEagerLoadObjectID = $mixedHasOneObjID; + $mixedHasManyObj->write(); + } + } + } + } + } + + private function iterateEagerLoadData(DataList $dataList): array + { + $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__'; + $results = []; + $i = 0; + foreach ($dataList as $obj) { + // base obj + $results[] = $obj->Title; + // has_one + $hasOneObj = $obj->HasOneEagerLoadObject(); + $hasOneSubObj = $hasOneObj->HasOneSubEagerLoadObject(); + $hasOneSubSubObj = $hasOneSubObj->HasOneSubSubEagerLoadObject(); + $results[] = $hasOneObj->Title; + $results[] = $hasOneSubObj->Title; + $results[] = $hasOneSubSubObj->Title; + // belongs_to + $belongsToObj = $obj->BelongsToEagerLoadObject(); + $belongsToSubObj = $belongsToObj->BelongsToSubEagerLoadObject(); + $belongsToSubSubObj = $belongsToSubObj->BelongsToSubSubEagerLoadObject(); + $results[] = $belongsToObj->Title; + $results[] = $belongsToSubObj->Title; + $results[] = $belongsToSubSubObj->Title; + // has_many + foreach ($obj->HasManyEagerLoadObjects() as $hasManyObj) { + $results[] = $hasManyObj->Title; + foreach ($hasManyObj->HasManySubEagerLoadObjects() as $hasManySubObj) { + $results[] = $hasManySubObj->Title; + foreach ($hasManySubObj->HasManySubSubEagerLoadObjects() as $hasManySubSubObj) { + $results[] = $hasManySubSubObj->Title; + } + } + } + // many_many + foreach ($obj->ManyManyEagerLoadObjects() as $manyManyObj) { + $results[] = $manyManyObj->Title; + foreach ($manyManyObj->ManyManySubEagerLoadObjects() as $manyManySubObj) { + $results[] = $manyManySubObj->Title; + foreach ($manyManySubObj->ManyManySubSubEagerLoadObjects() as $manyManySubSubObj) { + $results[] = $manyManySubSubObj->Title; + } + } + } + // many_many_through + foreach ($obj->ManyManyThroughEagerLoadObjects() as $manyManyThroughObj) { + $results[] = $manyManyThroughObj->Title; + foreach ($manyManyThroughObj->ManyManyThroughSubEagerLoadObjects() as $manyManyThroughSubObj) { + $results[] = $manyManyThroughSubObj->Title; + foreach ($manyManyThroughSubObj->ManyManyThroughSubSubEagerLoadObjects() as $manyManyThroughSubSubObj) { + $results[] = $manyManyThroughSubSubObj->Title; + } + } + } + // belongs_many_many + foreach ($obj->BelongsManyManyEagerLoadObjects() as $belongsManyManyObj) { + $results[] = $belongsManyManyObj->Title; + foreach ($belongsManyManyObj->BelongsManyManySubEagerLoadObjects() as $belongsManyManySubObj) { + $results[] = $belongsManyManySubObj->Title; + foreach ($belongsManyManySubObj->BelongsManyManySubSubEagerLoadObjects() as $belongsManyManySubSubObj) { + $results[] = $belongsManyManySubSubObj->Title; + } + } + } + // mixed + foreach ($obj->MixedManyManyEagerLoadObjects() as $mixedManyManyObj) { + $results[] = $mixedManyManyObj->Title; + foreach ($mixedManyManyObj->MixedHasManyEagerLoadObjects() as $mixedHasManyObj) { + $results[] = $mixedHasManyObj->Title; + $results[] = $mixedHasManyObj->MixedHasOneEagerLoadObject()->Title; + } + } + } + $s = ob_get_clean(); + $s = preg_replace('/.*__START_ITERATE__/s', '', $s); + $selectCount = substr_count($s, ': SELECT'); + } finally { + if ($showqueries) { + $_REQUEST['showqueries'] = $showqueries; + } else { + unset($_REQUEST['showqueries']); + } + } + return [$results, $selectCount]; + } + + private function expectedEagerLoadData(): array + { + return [ + 'obj 0', + 'hasOneObj 0', + 'hasOneSubObj 0', + 'hasOneSubSubObj 0', + 'belongsToObj 0', + 'belongsToSubObj 0', + 'belongsToSubSubObj 0', + 'hasManyObj 0 0', + 'hasManySubObj 0 0 0', + 'hasManySubSubObj 0 0 0 0', + 'hasManySubSubObj 0 0 0 1', + 'hasManySubObj 0 0 1', + 'hasManySubSubObj 0 0 1 0', + 'hasManySubSubObj 0 0 1 1', + 'hasManyObj 0 1', + 'hasManySubObj 0 1 0', + 'hasManySubSubObj 0 1 0 0', + 'hasManySubSubObj 0 1 0 1', + 'hasManySubObj 0 1 1', + 'hasManySubSubObj 0 1 1 0', + 'hasManySubSubObj 0 1 1 1', + 'manyManyObj 0 0', + 'manyManySubObj 0 0 0', + 'manyManySubSubObj 0 0 0 0', + 'manyManySubSubObj 0 0 0 1', + 'manyManySubObj 0 0 1', + 'manyManySubSubObj 0 0 1 0', + 'manyManySubSubObj 0 0 1 1', + 'manyManyObj 0 1', + 'manyManySubObj 0 1 0', + 'manyManySubSubObj 0 1 0 0', + 'manyManySubSubObj 0 1 0 1', + 'manyManySubObj 0 1 1', + 'manyManySubSubObj 0 1 1 0', + 'manyManySubSubObj 0 1 1 1', + 'manyManyThroughObj 0 0', + 'manyManyThroughSubObj 0 0 0', + 'manyManyThroughSubSubObj 0 0 0 0', + 'manyManyThroughSubSubObj 0 0 0 1', + 'manyManyThroughSubObj 0 0 1', + 'manyManyThroughSubSubObj 0 0 1 0', + 'manyManyThroughSubSubObj 0 0 1 1', + 'manyManyThroughObj 0 1', + 'manyManyThroughSubObj 0 1 0', + 'manyManyThroughSubSubObj 0 1 0 0', + 'manyManyThroughSubSubObj 0 1 0 1', + 'manyManyThroughSubObj 0 1 1', + 'manyManyThroughSubSubObj 0 1 1 0', + 'manyManyThroughSubSubObj 0 1 1 1', + 'belongsManyManyObj 0 0', + 'belongsManyManySubObj 0 0 0', + 'belongsManyManySubSubObj 0 0 0 0', + 'belongsManyManySubSubObj 0 0 0 1', + 'belongsManyManySubObj 0 0 1', + 'belongsManyManySubSubObj 0 0 1 0', + 'belongsManyManySubSubObj 0 0 1 1', + 'belongsManyManyObj 0 1', + 'belongsManyManySubObj 0 1 0', + 'belongsManyManySubSubObj 0 1 0 0', + 'belongsManyManySubSubObj 0 1 0 1', + 'belongsManyManySubObj 0 1 1', + 'belongsManyManySubSubObj 0 1 1 0', + 'belongsManyManySubSubObj 0 1 1 1', + 'mixedManyManyObj 0 0', + 'mixedHasManyObj 0 0 0', + 'mixedHasOneObj 0 0 0 1', + 'mixedHasManyObj 0 0 1', + 'mixedHasOneObj 0 0 1 1', + 'mixedManyManyObj 0 1', + 'mixedHasManyObj 0 1 0', + 'mixedHasOneObj 0 1 0 1', + 'mixedHasManyObj 0 1 1', + 'mixedHasOneObj 0 1 1 1', + 'obj 1', + 'hasOneObj 1', + 'hasOneSubObj 1', + 'hasOneSubSubObj 1', + 'belongsToObj 1', + 'belongsToSubObj 1', + 'belongsToSubSubObj 1', + 'hasManyObj 1 0', + 'hasManySubObj 1 0 0', + 'hasManySubSubObj 1 0 0 0', + 'hasManySubSubObj 1 0 0 1', + 'hasManySubObj 1 0 1', + 'hasManySubSubObj 1 0 1 0', + 'hasManySubSubObj 1 0 1 1', + 'hasManyObj 1 1', + 'hasManySubObj 1 1 0', + 'hasManySubSubObj 1 1 0 0', + 'hasManySubSubObj 1 1 0 1', + 'hasManySubObj 1 1 1', + 'hasManySubSubObj 1 1 1 0', + 'hasManySubSubObj 1 1 1 1', + 'manyManyObj 1 0', + 'manyManySubObj 1 0 0', + 'manyManySubSubObj 1 0 0 0', + 'manyManySubSubObj 1 0 0 1', + 'manyManySubObj 1 0 1', + 'manyManySubSubObj 1 0 1 0', + 'manyManySubSubObj 1 0 1 1', + 'manyManyObj 1 1', + 'manyManySubObj 1 1 0', + 'manyManySubSubObj 1 1 0 0', + 'manyManySubSubObj 1 1 0 1', + 'manyManySubObj 1 1 1', + 'manyManySubSubObj 1 1 1 0', + 'manyManySubSubObj 1 1 1 1', + 'manyManyThroughObj 1 0', + 'manyManyThroughSubObj 1 0 0', + 'manyManyThroughSubSubObj 1 0 0 0', + 'manyManyThroughSubSubObj 1 0 0 1', + 'manyManyThroughSubObj 1 0 1', + 'manyManyThroughSubSubObj 1 0 1 0', + 'manyManyThroughSubSubObj 1 0 1 1', + 'manyManyThroughObj 1 1', + 'manyManyThroughSubObj 1 1 0', + 'manyManyThroughSubSubObj 1 1 0 0', + 'manyManyThroughSubSubObj 1 1 0 1', + 'manyManyThroughSubObj 1 1 1', + 'manyManyThroughSubSubObj 1 1 1 0', + 'manyManyThroughSubSubObj 1 1 1 1', + 'belongsManyManyObj 1 0', + 'belongsManyManySubObj 1 0 0', + 'belongsManyManySubSubObj 1 0 0 0', + 'belongsManyManySubSubObj 1 0 0 1', + 'belongsManyManySubObj 1 0 1', + 'belongsManyManySubSubObj 1 0 1 0', + 'belongsManyManySubSubObj 1 0 1 1', + 'belongsManyManyObj 1 1', + 'belongsManyManySubObj 1 1 0', + 'belongsManyManySubSubObj 1 1 0 0', + 'belongsManyManySubSubObj 1 1 0 1', + 'belongsManyManySubObj 1 1 1', + 'belongsManyManySubSubObj 1 1 1 0', + 'belongsManyManySubSubObj 1 1 1 1', + 'mixedManyManyObj 1 0', + 'mixedHasManyObj 1 0 0', + 'mixedHasOneObj 1 0 0 1', + 'mixedHasManyObj 1 0 1', + 'mixedHasOneObj 1 0 1 1', + 'mixedManyManyObj 1 1', + 'mixedHasManyObj 1 1 0', + 'mixedHasOneObj 1 1 0 1', + 'mixedHasManyObj 1 1 1', + 'mixedHasOneObj 1 1 1 1', + ]; + } + + public function testEagerLoadFourthLevelException(): void + { + $eagerLoadRelation = implode('.', [ + 'MixedManyManyEagerLoadObjects', + 'MixedHasManyEagerLoadObjects', + 'MixedHasOneEagerLoadObject', + 'FourthLevel' + ]); + $expectedMessage = implode(' - ', [ + 'Eager loading only supports up to 3 levels of nesting, passed 4 levels', + $eagerLoadRelation + ]); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($expectedMessage); + EagerLoadObject::get()->eagerLoad($eagerLoadRelation); + } + + /** + * @dataProvider provideEagerLoadInvalidRelationException + */ + public function testEagerLoadInvalidRelationException(string $eagerLoadRelation): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Invalid relation passed to eagerLoad() - $eagerLoadRelation"); + $this->createEagerLoadData(); + EagerLoadObject::get()->eagerLoad($eagerLoadRelation)->toArray(); + } + + public function provideEagerLoadInvalidRelationException(): array + { + return [ + [ + 'Invalid', + ], + [ + 'MixedManyManyEagerLoadObjects.Invalid', + ], + [ + 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.Invalid' + ] + ]; + } +} diff --git a/tests/php/ORM/DataListTest.php b/tests/php/ORM/DataListTest.php index abe116f1b..20ded987e 100755 --- a/tests/php/ORM/DataListTest.php +++ b/tests/php/ORM/DataListTest.php @@ -26,31 +26,6 @@ use SilverStripe\ORM\Tests\DataObjectTest\Team; use SilverStripe\ORM\Tests\DataObjectTest\TeamComment; use SilverStripe\ORM\Tests\DataObjectTest\ValidatedObject; use SilverStripe\ORM\Tests\ManyManyListTest\Category; -use SilverStripe\ORM\Tests\DataListTest\EagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\HasOneEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\HasOneSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\HasOneSubSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\BelongsToEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\BelongsToSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\BelongsToSubSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\HasManyEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\HasManySubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\HasManySubSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\ManyManyEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\ManyManySubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\ManyManySubSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\ManyManyThroughEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\ManyManyThroughSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\ManyManyThroughSubSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\EagerLoadObjectManyManyThroughEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\ManyManyThroughEagerLoadObjectManyManyThroughSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\ManyManyThroughSubEagerLoadObjectManyManyThroughSubSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\BelongsManyManyEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\BelongsManyManySubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\BelongsManyManySubSubEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\MixedHasManyEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\MixedHasOneEagerLoadObject; -use SilverStripe\ORM\Tests\DataListTest\MixedManyManyEagerLoadObject; use SilverStripe\ORM\Connect\DatabaseException; class DataListTest extends SapphireTest @@ -61,37 +36,9 @@ class DataListTest extends SapphireTest public static function getExtraDataObjects() { - $eagerLoadDataObjects = [ - EagerLoadObject::class, - HasOneEagerLoadObject::class, - HasOneSubEagerLoadObject::class, - HasOneSubSubEagerLoadObject::class, - BelongsToEagerLoadObject::class, - BelongsToSubEagerLoadObject::class, - BelongsToSubSubEagerLoadObject::class, - HasManyEagerLoadObject::class, - HasManySubEagerLoadObject::class, - HasManySubSubEagerLoadObject::class, - ManyManyEagerLoadObject::class, - ManyManySubEagerLoadObject::class, - ManyManySubSubEagerLoadObject::class, - ManyManyThroughEagerLoadObject::class, - ManyManyThroughSubEagerLoadObject::class, - ManyManyThroughSubSubEagerLoadObject::class, - EagerLoadObjectManyManyThroughEagerLoadObject::class, - ManyManyThroughEagerLoadObjectManyManyThroughSubEagerLoadObject::class, - ManyManyThroughSubEagerLoadObjectManyManyThroughSubSubEagerLoadObject::class, - BelongsManyManyEagerLoadObject::class, - BelongsManyManySubEagerLoadObject::class, - BelongsManyManySubSubEagerLoadObject::class, - MixedHasManyEagerLoadObject::class, - MixedHasOneEagerLoadObject::class, - MixedManyManyEagerLoadObject::class, - ]; return array_merge( DataObjectTest::$extra_data_objects, - ManyManyListTest::$extra_data_objects, - $eagerLoadDataObjects + ManyManyListTest::$extra_data_objects ); } @@ -2251,590 +2198,4 @@ class DataListTest extends SapphireTest $this->assertEmpty($expectedIDs, 'chunk returns all the results that the regular iterator does'); $this->assertEquals($expectedQueryCount, $dataQuery->getCount()); } - - /** - * @dataProvider provideEagerLoadRelations - */ - public function testEagerLoadRelations(array $eagerLoad, int $expectedCount): void - { - $this->createEagerLoadData(); - $dataList = EagerLoadObject::get()->eagerLoad(...$eagerLoad); - list($results, $selectCount) = $this->iterateEagerLoadData($dataList); - $expectedResults=$this->expectedEagerLoadData(); - $this->assertSame(sort($expectedResults), sort($results)); - $this->assertSame($expectedCount, $selectCount); - } - - public function provideEagerLoadRelations(): array - { - return [ - 'lazy-load' => [ - 'eagerLoad' => [], - 'expected' => 83 - ], - 'has-one-a' => [ - 'eagerLoad' => [ - 'HasOneEagerLoadObject', - ], - 'expected' => 82 - ], - 'has-one-b' => [ - 'eagerLoad' => [ - 'HasOneEagerLoadObject.HasOneSubEagerLoadObject', - ], - 'expected' => 81 - ], - 'has-one-c' => [ - 'eagerLoad' => [ - 'HasOneEagerLoadObject.HasOneSubEagerLoadObject.HasOneSubSubEagerLoadObject', - ], - 'expected' => 80 - ], - 'belongs-to-a' => [ - 'eagerLoad' => [ - 'BelongsToEagerLoadObject', - ], - 'expected' => 82 - ], - 'belongs-to-b' => [ - 'eagerLoad' => [ - 'BelongsToEagerLoadObject.BelongsToSubEagerLoadObject', - ], - 'expected' => 81 - ], - 'belongs-to-c' => [ - 'eagerLoad' => [ - 'BelongsToEagerLoadObject.BelongsToSubEagerLoadObject.BelongsToSubSubEagerLoadObject', - ], - 'expected' => 80 - ], - 'has-many-a' => [ - 'eagerLoad' => [ - 'HasManyEagerLoadObjects', - ], - 'expected' => 82 - ], - 'has-many-b' => [ - 'eagerLoad' => [ - 'HasManyEagerLoadObjects.HasManySubEagerLoadObjects', - ], - 'expected' => 79 - ], - 'has-many-c' => [ - 'eagerLoad' => [ - 'HasManyEagerLoadObjects.HasManySubEagerLoadObjects.HasManySubSubEagerLoadObjects', - ], - 'expected' => 72 - ], - 'many-many-a' => [ - 'eagerLoad' => [ - 'ManyManyEagerLoadObjects', - ], - 'expected' => 83 // same number as lazy-load, though without an INNER JOIN - ], - 'many-many-b' => [ - 'eagerLoad' => [ - 'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects', - ], - 'expected' => 81 - ], - 'many-many-c' => [ - 'eagerLoad' => [ - 'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects.ManyManySubSubEagerLoadObjects', - ], - 'expected' => 75 - ], - 'many-many-through-a' => [ - 'eagerLoad' => [ - 'ManyManyThroughEagerLoadObjects', - ], - 'expected' => 83 - ], - 'many-many-through-b' => [ - 'eagerLoad' => [ - 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects', - ], - 'expected' => 81 - ], - 'many-many-through-c' => [ - 'eagerLoad' => [ - 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects.ManyManyThroughSubSubEagerLoadObjects', - ], - 'expected' => 75 - ], - 'belongs-many-many-a' => [ - 'eagerLoad' => [ - 'BelongsManyManyEagerLoadObjects', - ], - 'expected' => 83 - ], - 'belongs-many-many-b' => [ - 'eagerLoad' => [ - 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects', - ], - 'expected' => 81 - ], - 'belongs-many-many-c' => [ - 'eagerLoad' => [ - 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects.BelongsManyManySubSubEagerLoadObjects', - ], - 'expected' => 75 - ], - 'mixed-a' => [ - 'eagerLoad' => [ - 'MixedManyManyEagerLoadObjects', - ], - 'expected' => 83 - ], - 'mixed-b' => [ - 'eagerLoad' => [ - 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects', - ], - 'expected' => 80 - ], - 'mixed-c' => [ - 'eagerLoad' => [ - 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.MixedHasOneEagerLoadObject', - ], - 'expected' => 73 - ], - 'all' => [ - 'eagerLoad' => [ - 'HasOneEagerLoadObject.HasOneSubEagerLoadObject.HasOneSubSubEagerLoadObject', - 'BelongsToEagerLoadObject.BelongsToSubEagerLoadObject.BelongsToSubSubEagerLoadObject', - 'HasManyEagerLoadObjects.HasManySubEagerLoadObjects.HasManySubSubEagerLoadObjects', - 'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects.ManyManySubSubEagerLoadObjects', - 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects.ManyManyThroughSubSubEagerLoadObjects', - 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects.BelongsManyManySubSubEagerLoadObjects', - 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.MixedHasOneEagerLoadObject', - ], - 'expected' => 32 - ], - ]; - } - - private function createEagerLoadData(): void - { - for ($i = 0; $i < 2; $i++) { - // base object - $obj = new EagerLoadObject(); - $obj->Title = "obj $i"; - $objID = $obj->write(); - // has_one - $hasOneObj = new HasOneEagerLoadObject(); - $hasOneObj->Title = "hasOneObj $i"; - $hasOneObjID = $hasOneObj->write(); - $obj->HasOneEagerLoadObjectID = $hasOneObjID; - $obj->write(); - $hasOneSubObj = new HasOneSubEagerLoadObject(); - $hasOneSubObj->Title = "hasOneSubObj $i"; - $hasOneSubObjID = $hasOneSubObj->write(); - $hasOneObj->HasOneSubEagerLoadObjectID = $hasOneSubObjID; - $hasOneObj->write(); - $hasOneSubSubObj = new HasOneSubSubEagerLoadObject(); - $hasOneSubSubObj->Title = "hasOneSubSubObj $i"; - $hasOneSubSubObjID = $hasOneSubSubObj->write(); - $hasOneSubObj->HasOneSubSubEagerLoadObjectID = $hasOneSubSubObjID; - $hasOneSubObj->write(); - // belongs_to - $belongsToObj = new BelongsToEagerLoadObject(); - $belongsToObj->EagerLoadObjectID = $objID; - $belongsToObj->Title = "belongsToObj $i"; - $belongsToObjID = $belongsToObj->write(); - $belongsToSubObj = new BelongsToSubEagerLoadObject(); - $belongsToSubObj->BelongsToEagerLoadObjectID = $belongsToObjID; - $belongsToSubObj->Title = "belongsToSubObj $i"; - $belongsToSubObjID = $belongsToSubObj->write(); - $belongsToSubSubObj = new BelongsToSubSubEagerLoadObject(); - $belongsToSubSubObj->BelongsToSubEagerLoadObjectID = $belongsToSubObjID; - $belongsToSubSubObj->Title = "belongsToSubSubObj $i"; - $belongsToSubSubObj->write(); - // has_many - for ($j = 0; $j < 2; $j++) { - $hasManyObj = new HasManyEagerLoadObject(); - $hasManyObj->EagerLoadObjectID = $objID; - $hasManyObj->Title = "hasManyObj $i $j"; - $hasManyObjID = $hasManyObj->write(); - for ($k = 0; $k < 2; $k++) { - $hasManySubObj = new HasManySubEagerLoadObject(); - $hasManySubObj->HasManyEagerLoadObjectID = $hasManyObjID; - $hasManySubObj->Title = "hasManySubObj $i $j $k"; - $hasManySubObjID = $hasManySubObj->write(); - for ($l = 0; $l < 2; $l++) { - $hasManySubSubObj = new HasManySubSubEagerLoadObject(); - $hasManySubSubObj->HasManySubEagerLoadObjectID = $hasManySubObjID; - $hasManySubSubObj->Title = "hasManySubSubObj $i $j $k $l"; - $hasManySubSubObj->write(); - } - } - } - // many_many - for ($j = 0; $j < 2; $j++) { - $manyManyObj = new ManyManyEagerLoadObject(); - $manyManyObj->Title = "manyManyObj $i $j"; - $manyManyObj->write(); - $obj->ManyManyEagerLoadObjects()->add($manyManyObj); - for ($k = 0; $k < 2; $k++) { - $manyManySubObj = new ManyManySubEagerLoadObject(); - $manyManySubObj->Title = "manyManySubObj $i $j $k"; - $manyManySubObj->write(); - $manyManyObj->ManyManySubEagerLoadObjects()->add($manyManySubObj); - for ($l = 0; $l < 2; $l++) { - $manyManySubSubObj = new ManyManySubSubEagerLoadObject(); - $manyManySubSubObj->Title = "manyManySubSubObj $i $j $k $l"; - $manyManySubSubObj->write(); - $manyManySubObj->ManyManySubSubEagerLoadObjects()->add($manyManySubSubObj); - } - } - } - // many_many_through - for ($j = 0; $j < 2; $j++) { - $manyManyThroughObj = new ManyManyThroughEagerLoadObject(); - $manyManyThroughObj->Title = "manyManyThroughObj $i $j"; - $manyManyThroughObj->write(); - $obj->ManyManyThroughEagerLoadObjects()->add($manyManyThroughObj); - for ($k = 0; $k < 2; $k++) { - $manyManyThroughSubObj = new ManyManyThroughSubEagerLoadObject(); - $manyManyThroughSubObj->Title = "manyManyThroughSubObj $i $j $k"; - $manyManyThroughSubObj->write(); - - $manyManyThroughObj->ManyManyThroughSubEagerLoadObjects()->add($manyManyThroughSubObj); - for ($l = 0; $l < 2; $l++) { - $manyManyThroughSubSubObj = new ManyManyThroughSubSubEagerLoadObject(); - $manyManyThroughSubSubObj->Title = "manyManyThroughSubSubObj $i $j $k $l"; - $manyManyThroughSubSubObj->write(); - $manyManyThroughSubObj->ManyManyThroughSubSubEagerLoadObjects()->add($manyManyThroughSubSubObj); - } - } - } - // belongs_many_many - for ($j = 0; $j < 2; $j++) { - $belongsManyManyObj = new BelongsManyManyEagerLoadObject(); - $belongsManyManyObj->Title = "belongsManyManyObj $i $j"; - $belongsManyManyObj->write(); - $obj->BelongsManyManyEagerLoadObjects()->add($belongsManyManyObj); - for ($k = 0; $k < 2; $k++) { - $belongsManyManySubObj = new BelongsManyManySubEagerLoadObject(); - $belongsManyManySubObj->Title = "belongsManyManySubObj $i $j $k"; - $belongsManyManySubObj->write(); - $belongsManyManyObj->BelongsManyManySubEagerLoadObjects()->add($belongsManyManySubObj); - for ($l = 0; $l < 2; $l++) { - $belongsManyManySubSubObj = new BelongsManyManySubSubEagerLoadObject(); - $belongsManyManySubSubObj->Title = "belongsManyManySubSubObj $i $j $k $l"; - $belongsManyManySubSubObj->write(); - $belongsManyManySubObj->BelongsManyManySubSubEagerLoadObjects()->add($belongsManyManySubSubObj); - } - } - } - // mixed - for ($j = 0; $j < 2; $j++) { - $mixedManyManyObj = new MixedManyManyEagerLoadObject(); - $mixedManyManyObj->Title = "mixedManyManyObj $i $j"; - $mixedManyManyObj->write(); - $obj->MixedManyManyEagerLoadObjects()->add($mixedManyManyObj); - for ($k = 0; $k < 2; $k++) { - $mixedHasManyObj = new MixedHasManyEagerLoadObject(); - $mixedHasManyObj->Title = "mixedHasManyObj $i $j $k"; - $mixedHasManyObjID = $mixedHasManyObj->write(); - $mixedManyManyObj->MixedHasManyEagerLoadObjects()->add($mixedHasManyObj); - for ($l = 0; $l < 2; $l++) { - $mixedHasOneObj = new MixedHasOneEagerLoadObject(); - $mixedHasOneObj->Title = "mixedHasOneObj $i $j $k $l"; - $mixedHasOneObjID = $mixedHasOneObj->write(); - $mixedHasManyObj->MixedHasOneEagerLoadObjectID = $mixedHasOneObjID; - $mixedHasManyObj->write(); - } - } - } - } - } - - private function iterateEagerLoadData(DataList $dataList): array - { - $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__'; - $results = []; - $i = 0; - foreach ($dataList as $obj) { - // base obj - $results[] = $obj->Title; - // has_one - $hasOneObj = $obj->HasOneEagerLoadObject(); - $hasOneSubObj = $hasOneObj->HasOneSubEagerLoadObject(); - $hasOneSubSubObj = $hasOneSubObj->HasOneSubSubEagerLoadObject(); - $results[] = $hasOneObj->Title; - $results[] = $hasOneSubObj->Title; - $results[] = $hasOneSubSubObj->Title; - // belongs_to - $belongsToObj = $obj->BelongsToEagerLoadObject(); - $belongsToSubObj = $belongsToObj->BelongsToSubEagerLoadObject(); - $belongsToSubSubObj = $belongsToSubObj->BelongsToSubSubEagerLoadObject(); - $results[] = $belongsToObj->Title; - $results[] = $belongsToSubObj->Title; - $results[] = $belongsToSubSubObj->Title; - // has_many - foreach ($obj->HasManyEagerLoadObjects() as $hasManyObj) { - $results[] = $hasManyObj->Title; - foreach ($hasManyObj->HasManySubEagerLoadObjects() as $hasManySubObj) { - $results[] = $hasManySubObj->Title; - foreach ($hasManySubObj->HasManySubSubEagerLoadObjects() as $hasManySubSubObj) { - $results[] = $hasManySubSubObj->Title; - } - } - } - // many_many - foreach ($obj->ManyManyEagerLoadObjects() as $manyManyObj) { - $results[] = $manyManyObj->Title; - foreach ($manyManyObj->ManyManySubEagerLoadObjects() as $manyManySubObj) { - $results[] = $manyManySubObj->Title; - foreach ($manyManySubObj->ManyManySubSubEagerLoadObjects() as $manyManySubSubObj) { - $results[] = $manyManySubSubObj->Title; - } - } - } - // many_many_through - foreach ($obj->ManyManyThroughEagerLoadObjects() as $manyManyThroughObj) { - $results[] = $manyManyThroughObj->Title; - foreach ($manyManyThroughObj->ManyManyThroughSubEagerLoadObjects() as $manyManyThroughSubObj) { - $results[] = $manyManyThroughSubObj->Title; - foreach ($manyManyThroughSubObj->ManyManyThroughSubSubEagerLoadObjects() as $manyManyThroughSubSubObj) { - $results[] = $manyManyThroughSubSubObj->Title; - } - } - } - // belongs_many_many - foreach ($obj->BelongsManyManyEagerLoadObjects() as $belongsManyManyObj) { - $results[] = $belongsManyManyObj->Title; - foreach ($belongsManyManyObj->BelongsManyManySubEagerLoadObjects() as $belongsManyManySubObj) { - $results[] = $belongsManyManySubObj->Title; - foreach ($belongsManyManySubObj->BelongsManyManySubSubEagerLoadObjects() as $belongsManyManySubSubObj) { - $results[] = $belongsManyManySubSubObj->Title; - } - } - } - // mixed - foreach ($obj->MixedManyManyEagerLoadObjects() as $mixedManyManyObj) { - $results[] = $mixedManyManyObj->Title; - foreach ($mixedManyManyObj->MixedHasManyEagerLoadObjects() as $mixedHasManyObj) { - $results[] = $mixedHasManyObj->Title; - $results[] = $mixedHasManyObj->MixedHasOneEagerLoadObject()->Title; - } - } - } - $s = ob_get_clean(); - $s = preg_replace('/.*__START_ITERATE__/s', '', $s); - $selectCount = substr_count($s, ': SELECT'); - } finally { - if ($showqueries) { - $_REQUEST['showqueries'] = $showqueries; - } else { - unset($_REQUEST['showqueries']); - } - } - return [$results, $selectCount]; - } - - private function expectedEagerLoadData(): array - { - return [ - 'obj 0', - 'hasOneObj 0', - 'hasOneSubObj 0', - 'hasOneSubSubObj 0', - 'belongsToObj 0', - 'belongsToSubObj 0', - 'belongsToSubSubObj 0', - 'hasManyObj 0 0', - 'hasManySubObj 0 0 0', - 'hasManySubSubObj 0 0 0 0', - 'hasManySubSubObj 0 0 0 1', - 'hasManySubObj 0 0 1', - 'hasManySubSubObj 0 0 1 0', - 'hasManySubSubObj 0 0 1 1', - 'hasManyObj 0 1', - 'hasManySubObj 0 1 0', - 'hasManySubSubObj 0 1 0 0', - 'hasManySubSubObj 0 1 0 1', - 'hasManySubObj 0 1 1', - 'hasManySubSubObj 0 1 1 0', - 'hasManySubSubObj 0 1 1 1', - 'manyManyObj 0 0', - 'manyManySubObj 0 0 0', - 'manyManySubSubObj 0 0 0 0', - 'manyManySubSubObj 0 0 0 1', - 'manyManySubObj 0 0 1', - 'manyManySubSubObj 0 0 1 0', - 'manyManySubSubObj 0 0 1 1', - 'manyManyObj 0 1', - 'manyManySubObj 0 1 0', - 'manyManySubSubObj 0 1 0 0', - 'manyManySubSubObj 0 1 0 1', - 'manyManySubObj 0 1 1', - 'manyManySubSubObj 0 1 1 0', - 'manyManySubSubObj 0 1 1 1', - 'manyManyThroughObj 0 0', - 'manyManyThroughSubObj 0 0 0', - 'manyManyThroughSubSubObj 0 0 0 0', - 'manyManyThroughSubSubObj 0 0 0 1', - 'manyManyThroughSubObj 0 0 1', - 'manyManyThroughSubSubObj 0 0 1 0', - 'manyManyThroughSubSubObj 0 0 1 1', - 'manyManyThroughObj 0 1', - 'manyManyThroughSubObj 0 1 0', - 'manyManyThroughSubSubObj 0 1 0 0', - 'manyManyThroughSubSubObj 0 1 0 1', - 'manyManyThroughSubObj 0 1 1', - 'manyManyThroughSubSubObj 0 1 1 0', - 'manyManyThroughSubSubObj 0 1 1 1', - 'belongsManyManyObj 0 0', - 'belongsManyManySubObj 0 0 0', - 'belongsManyManySubSubObj 0 0 0 0', - 'belongsManyManySubSubObj 0 0 0 1', - 'belongsManyManySubObj 0 0 1', - 'belongsManyManySubSubObj 0 0 1 0', - 'belongsManyManySubSubObj 0 0 1 1', - 'belongsManyManyObj 0 1', - 'belongsManyManySubObj 0 1 0', - 'belongsManyManySubSubObj 0 1 0 0', - 'belongsManyManySubSubObj 0 1 0 1', - 'belongsManyManySubObj 0 1 1', - 'belongsManyManySubSubObj 0 1 1 0', - 'belongsManyManySubSubObj 0 1 1 1', - 'mixedManyManyObj 0 0', - 'mixedHasManyObj 0 0 0', - 'mixedHasOneObj 0 0 0 1', - 'mixedHasManyObj 0 0 1', - 'mixedHasOneObj 0 0 1 1', - 'mixedManyManyObj 0 1', - 'mixedHasManyObj 0 1 0', - 'mixedHasOneObj 0 1 0 1', - 'mixedHasManyObj 0 1 1', - 'mixedHasOneObj 0 1 1 1', - 'obj 1', - 'hasOneObj 1', - 'hasOneSubObj 1', - 'hasOneSubSubObj 1', - 'belongsToObj 1', - 'belongsToSubObj 1', - 'belongsToSubSubObj 1', - 'hasManyObj 1 0', - 'hasManySubObj 1 0 0', - 'hasManySubSubObj 1 0 0 0', - 'hasManySubSubObj 1 0 0 1', - 'hasManySubObj 1 0 1', - 'hasManySubSubObj 1 0 1 0', - 'hasManySubSubObj 1 0 1 1', - 'hasManyObj 1 1', - 'hasManySubObj 1 1 0', - 'hasManySubSubObj 1 1 0 0', - 'hasManySubSubObj 1 1 0 1', - 'hasManySubObj 1 1 1', - 'hasManySubSubObj 1 1 1 0', - 'hasManySubSubObj 1 1 1 1', - 'manyManyObj 1 0', - 'manyManySubObj 1 0 0', - 'manyManySubSubObj 1 0 0 0', - 'manyManySubSubObj 1 0 0 1', - 'manyManySubObj 1 0 1', - 'manyManySubSubObj 1 0 1 0', - 'manyManySubSubObj 1 0 1 1', - 'manyManyObj 1 1', - 'manyManySubObj 1 1 0', - 'manyManySubSubObj 1 1 0 0', - 'manyManySubSubObj 1 1 0 1', - 'manyManySubObj 1 1 1', - 'manyManySubSubObj 1 1 1 0', - 'manyManySubSubObj 1 1 1 1', - 'manyManyThroughObj 1 0', - 'manyManyThroughSubObj 1 0 0', - 'manyManyThroughSubSubObj 1 0 0 0', - 'manyManyThroughSubSubObj 1 0 0 1', - 'manyManyThroughSubObj 1 0 1', - 'manyManyThroughSubSubObj 1 0 1 0', - 'manyManyThroughSubSubObj 1 0 1 1', - 'manyManyThroughObj 1 1', - 'manyManyThroughSubObj 1 1 0', - 'manyManyThroughSubSubObj 1 1 0 0', - 'manyManyThroughSubSubObj 1 1 0 1', - 'manyManyThroughSubObj 1 1 1', - 'manyManyThroughSubSubObj 1 1 1 0', - 'manyManyThroughSubSubObj 1 1 1 1', - 'belongsManyManyObj 1 0', - 'belongsManyManySubObj 1 0 0', - 'belongsManyManySubSubObj 1 0 0 0', - 'belongsManyManySubSubObj 1 0 0 1', - 'belongsManyManySubObj 1 0 1', - 'belongsManyManySubSubObj 1 0 1 0', - 'belongsManyManySubSubObj 1 0 1 1', - 'belongsManyManyObj 1 1', - 'belongsManyManySubObj 1 1 0', - 'belongsManyManySubSubObj 1 1 0 0', - 'belongsManyManySubSubObj 1 1 0 1', - 'belongsManyManySubObj 1 1 1', - 'belongsManyManySubSubObj 1 1 1 0', - 'belongsManyManySubSubObj 1 1 1 1', - 'mixedManyManyObj 1 0', - 'mixedHasManyObj 1 0 0', - 'mixedHasOneObj 1 0 0 1', - 'mixedHasManyObj 1 0 1', - 'mixedHasOneObj 1 0 1 1', - 'mixedManyManyObj 1 1', - 'mixedHasManyObj 1 1 0', - 'mixedHasOneObj 1 1 0 1', - 'mixedHasManyObj 1 1 1', - 'mixedHasOneObj 1 1 1 1', - ]; - } - - public function testEagerLoadFourthLevelException(): void - { - $eagerLoadRelation = implode('.', [ - 'MixedManyManyEagerLoadObjects', - 'MixedHasManyEagerLoadObjects', - 'MixedHasOneEagerLoadObject', - 'FourthLevel' - ]); - $expectedMessage = implode(' - ', [ - 'Eager loading only supports up to 3 levels of nesting, passed 4 levels', - $eagerLoadRelation - ]); - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage($expectedMessage); - EagerLoadObject::get()->eagerLoad($eagerLoadRelation); - } - - /** - * @dataProvider provideEagerLoadInvalidRelationException - */ - public function testEagerLoadInvalidRelationException(string $eagerLoadRelation): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage("Invalid relation passed to eagerLoad() - $eagerLoadRelation"); - $this->createEagerLoadData(); - EagerLoadObject::get()->eagerLoad($eagerLoadRelation)->toArray(); - } - - public function provideEagerLoadInvalidRelationException(): array - { - return [ - [ - 'Invalid', - ], - [ - 'MixedManyManyEagerLoadObjects.Invalid', - ], - [ - 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.Invalid' - ] - ]; - } } diff --git a/tests/php/ORM/DataListTest/BelongsManyManyEagerLoadObject.php b/tests/php/ORM/DataListTest/EagerLoading/BelongsManyManyEagerLoadObject.php similarity index 89% rename from tests/php/ORM/DataListTest/BelongsManyManyEagerLoadObject.php rename to tests/php/ORM/DataListTest/EagerLoading/BelongsManyManyEagerLoadObject.php index d48783928..ad1620add 100644 --- a/tests/php/ORM/DataListTest/BelongsManyManyEagerLoadObject.php +++ b/tests/php/ORM/DataListTest/EagerLoading/BelongsManyManyEagerLoadObject.php @@ -1,6 +1,6 @@ Date: Thu, 6 Jul 2023 17:15:13 +1200 Subject: [PATCH 2/6] FIX many_many extraFields and join records weren't in eagerloading --- src/ORM/DataList.php | 77 ++++++----- tests/php/ORM/DataListEagerLoadingTest.php | 122 ++++++++++++++++-- .../EagerLoading/EagerLoadObject.php | 9 ++ ...adObjectManyManyThroughEagerLoadObject.php | 4 +- 4 files changed, 171 insertions(+), 41 deletions(-) diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index a0fb17d97..d6c93ec0a 100644 --- a/src/ORM/DataList.php +++ b/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: - // - the join table name for many-many - // - the join data class for many-many-through - $join = $manyManyLastComponent['join']; + $joinTable = $manyManyLastComponent['join']; + $extraFields = $manyManyLastComponent['extraFields']; + $joinClass = $manyManyLastComponent['joinClass']; + + // 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)) { - $joinThroughObjs = DataObject::get($join)->filter([$parentIDField => $parentIDs]); - $relationItemIDs = []; - $joinRows = []; - foreach ($joinThroughObjs as $joinThroughObj) { - $joinRows[] = [ - $parentIDField => $joinThroughObj->$parentIDField, - $childIDField => $joinThroughObj->$childIDField - ]; - $relationItemIDs[] = $joinThroughObj->$childIDField; - } + if ($joinClass !== null) { + $relationList = ManyManyThroughList::create( + $relationDataClass, + $joinClass, + $childIDField, + $parentIDField, + $extraFields, + $relationDataClass, + $parentDataClass + )->forForeignID($parentIDs); // many_many + belongs_many_many } else { - $joinTableQuery = DB::query('SELECT * FROM "' . $join . '" WHERE "' . $parentIDField . '" IN (' . implode(',', $parentIDs) . ')'); - $relationItemIDs = $joinTableQuery->column($childIDField); - $joinRows = $joinTableQuery; + $relationList = ManyManyList::create( + $relationDataClass, + $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)]; } /** diff --git a/tests/php/ORM/DataListEagerLoadingTest.php b/tests/php/ORM/DataListEagerLoadingTest.php index 46d012bb3..2982bbfa0 100644 --- a/tests/php/ORM/DataListEagerLoadingTest.php +++ b/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 (?\d+) (?\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 (?\d+) (?\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', + ] + ]; + } } diff --git a/tests/php/ORM/DataListTest/EagerLoading/EagerLoadObject.php b/tests/php/ORM/DataListTest/EagerLoading/EagerLoadObject.php index 13af762a2..9d753dc9f 100644 --- a/tests/php/ORM/DataListTest/EagerLoading/EagerLoadObject.php +++ b/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 = [ diff --git a/tests/php/ORM/DataListTest/EagerLoading/EagerLoadObjectManyManyThroughEagerLoadObject.php b/tests/php/ORM/DataListTest/EagerLoading/EagerLoadObjectManyManyThroughEagerLoadObject.php index d9a7a4c57..007be929d 100644 --- a/tests/php/ORM/DataListTest/EagerLoading/EagerLoadObjectManyManyThroughEagerLoadObject.php +++ b/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 = [ From 95d1c674a289224e7e4b6da1ce906479cc6d6286 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Mon, 10 Jul 2023 18:13:03 +1200 Subject: [PATCH 3/6] FIX Allow multiple iterations of eager-loaded DataLists Includes making sure toArray() uses the correct iteration logic, and cloning resets the eagerloaded data for the new clone. Previously, a second iteration would add every relation item to the relation list a second time - so with each iteration your relation list count doubled (though it was the same records time and again). --- src/ORM/DataList.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index d6c93ec0a..4b1598877 100644 --- a/src/ORM/DataList.php +++ b/src/ORM/DataList.php @@ -96,6 +96,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li { $this->dataQuery = clone $this->dataQuery; $this->finalisedQuery = null; + $this->eagerLoadedData = []; } /** @@ -831,11 +832,10 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li */ public function toArray() { - $rows = $this->executeQuery(); $results = []; - foreach ($rows as $row) { - $results[] = $this->createDataObject($row); + foreach ($this as $item) { + $results[] = $item; } return $results; @@ -1001,6 +1001,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li // Re-set the finaliseQuery so that it can be re-executed $this->finalisedQuery = null; + $this->eagerLoadedData = []; } /** From acad946b740db8cdef83bb512a1816c9c5dcee10 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Mon, 10 Jul 2023 18:17:24 +1200 Subject: [PATCH 4/6] MNT Add missing tests for eagerloaded DataList interactions Add tests for: - filtered, limited, and sorted DataLists with eagerloaded relations - iterating through the list multiple times - eagerLoad method can be called anywhere in a query chain - eager loaded relation lists can be correctly empty without throwing exceptions - repeating relations to be eagerloaded in various permutations doesn't cause issues - eagerloading doesn't negatively impact chunkedFetch functionality squash --- tests/php/ORM/DataListEagerLoadingTest.php | 522 ++++++++++++++------- 1 file changed, 349 insertions(+), 173 deletions(-) diff --git a/tests/php/ORM/DataListEagerLoadingTest.php b/tests/php/ORM/DataListEagerLoadingTest.php index 2982bbfa0..6a148ee24 100644 --- a/tests/php/ORM/DataListEagerLoadingTest.php +++ b/tests/php/ORM/DataListEagerLoadingTest.php @@ -6,6 +6,8 @@ use InvalidArgumentException; use SilverStripe\Dev\SapphireTest; use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DB; +use SilverStripe\ORM\ManyManyThroughList; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\EagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\HasOneEagerLoadObject; use SilverStripe\ORM\Tests\DataListTest\EagerLoading\HasOneSubEagerLoadObject; @@ -68,15 +70,46 @@ class DataListEagerLoadingTest extends SapphireTest ]; } + public static function setUpBeforeClass(): void + { + parent::setUpBeforeClass(); + // Set non-zero auto increment offset for each object type so we don't end up with the same IDs across + // the board. If all of the IDs are 0, 1, 2 then we have no way of knowing if we're accidentally mixing + // up relation ID lists between different relation lists for different classes. + $schema = DataObject::getSchema(); + /** @var DataObject $class */ + $numManyManyTables = 0; + foreach (static::getExtraDataObjects() as $i => $class) { + $autoIncrementStart = ($i + 1) * 100; + $table = $schema->baseDataTable($class); + DB::query("ALTER TABLE $table AUTO_INCREMENT = $autoIncrementStart;"); + + // Also adjust auto increment for join tables for the same reason. + foreach (array_keys($class::config()->get('many_many') ?? []) as $relationName) { + $manyManyComponent = $schema->manyManyComponent($class, $relationName); + + $joinTable = $manyManyComponent['join']; + if (is_a($manyManyComponent['relationClass'], ManyManyThroughList::class, true)) { + $joinTable = $schema->baseDataTable($joinTable); + } + $joinAutoIncrementStart = ($numManyManyTables + 1) * 100 + 50; + DB::query("ALTER TABLE $joinTable AUTO_INCREMENT = $joinAutoIncrementStart;"); + $numManyManyTables++; + } + } + } + /** * @dataProvider provideEagerLoadRelations */ public function testEagerLoadRelations(string $iden, array $eagerLoad, int $expected): void { $this->createEagerLoadData(); + $dataList = EagerLoadObject::get()->eagerLoad(...$eagerLoad); list($results, $selectCount) = $this->iterateEagerLoadData($dataList); - $expectedResults = $this->expectedEagerLoadData(); + // We can rely on the non-eager-loaded data being correct, since it's validated by other unit tests + list($expectedResults, $_) = $this->iterateEagerLoadData(EagerLoadObject::get()); // Sort because the order of the results doesn't really matter - and has proven to be different in postgres sort($expectedResults); sort($results); @@ -88,6 +121,9 @@ class DataListEagerLoadingTest extends SapphireTest public function provideEagerLoadRelations(): array { return [ + // Include the lazy-loaded expectation here, since if the number + // of queries changes for this we should expect the number + // to change for eager-loading as well. [ 'iden' => 'lazy-load', 'eagerLoad' => [], @@ -240,6 +276,19 @@ class DataListEagerLoadingTest extends SapphireTest ], 'expected' => 73 ], + [ + 'iden' => 'duplicates', + 'eagerLoad' => [ + 'MixedManyManyEagerLoadObjects', + 'MixedManyManyEagerLoadObjects', + 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects', + 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects', + 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.MixedManyManyEagerLoadObject', + 'BelongsManyManyEagerLoadObjects.BelongsManyManySubEagerLoadObjects', + 'MixedManyManyEagerLoadObjects.MixedHasManyEagerLoadObjects.MixedHasOneEagerLoadObject', + ], + 'expected' => 78 + ], [ 'iden' => 'all', 'eagerLoad' => [ @@ -256,9 +305,13 @@ class DataListEagerLoadingTest extends SapphireTest ]; } - private function createEagerLoadData(): void - { - for ($i = 0; $i < 2; $i++) { + private function createEagerLoadData( + int $numBaseRecords = 2, + int $numLevel1Records = 2, + int $numLevel2Records = 2, + int $numLevel3Records = 2 + ): void { + for ($i = 0; $i < $numBaseRecords; $i++) { // base object $obj = new EagerLoadObject(); $obj->Title = "obj $i"; @@ -293,17 +346,17 @@ class DataListEagerLoadingTest extends SapphireTest $belongsToSubSubObj->Title = "belongsToSubSubObj $i"; $belongsToSubSubObj->write(); // has_many - for ($j = 0; $j < 2; $j++) { + for ($j = 0; $j < $numLevel1Records; $j++) { $hasManyObj = new HasManyEagerLoadObject(); $hasManyObj->EagerLoadObjectID = $objID; $hasManyObj->Title = "hasManyObj $i $j"; $hasManyObjID = $hasManyObj->write(); - for ($k = 0; $k < 2; $k++) { + for ($k = 0; $k < $numLevel2Records; $k++) { $hasManySubObj = new HasManySubEagerLoadObject(); $hasManySubObj->HasManyEagerLoadObjectID = $hasManyObjID; $hasManySubObj->Title = "hasManySubObj $i $j $k"; $hasManySubObjID = $hasManySubObj->write(); - for ($l = 0; $l < 2; $l++) { + for ($l = 0; $l < $numLevel3Records; $l++) { $hasManySubSubObj = new HasManySubSubEagerLoadObject(); $hasManySubSubObj->HasManySubEagerLoadObjectID = $hasManySubObjID; $hasManySubSubObj->Title = "hasManySubSubObj $i $j $k $l"; @@ -312,17 +365,17 @@ class DataListEagerLoadingTest extends SapphireTest } } // many_many - for ($j = 0; $j < 2; $j++) { + for ($j = 0; $j < $numLevel1Records; $j++) { $manyManyObj = new ManyManyEagerLoadObject(); $manyManyObj->Title = "manyManyObj $i $j"; $manyManyObj->write(); $obj->ManyManyEagerLoadObjects()->add($manyManyObj); - for ($k = 0; $k < 2; $k++) { + for ($k = 0; $k < $numLevel2Records; $k++) { $manyManySubObj = new ManyManySubEagerLoadObject(); $manyManySubObj->Title = "manyManySubObj $i $j $k"; $manyManySubObj->write(); $manyManyObj->ManyManySubEagerLoadObjects()->add($manyManySubObj); - for ($l = 0; $l < 2; $l++) { + for ($l = 0; $l < $numLevel3Records; $l++) { $manyManySubSubObj = new ManyManySubSubEagerLoadObject(); $manyManySubSubObj->Title = "manyManySubSubObj $i $j $k $l"; $manyManySubSubObj->write(); @@ -331,7 +384,7 @@ class DataListEagerLoadingTest extends SapphireTest } } // many_many with extraFields - for ($j = 0; $j < 2; $j++) { + for ($j = 0; $j < $numLevel1Records; $j++) { $manyManyObj = new ManyManyEagerLoadObject(); $manyManyObj->Title = "manyManyObj $i $j"; $manyManyObj->write(); @@ -342,7 +395,7 @@ class DataListEagerLoadingTest extends SapphireTest ]); } // many_many_through - for ($j = 0; $j < 2; $j++) { + for ($j = 0; $j < $numLevel1Records; $j++) { $manyManyThroughObj = new ManyManyThroughEagerLoadObject(); $manyManyThroughObj->Title = "manyManyThroughObj $i $j"; $manyManyThroughObj->write(); @@ -351,12 +404,12 @@ class DataListEagerLoadingTest extends SapphireTest 'SomeBool' => $j % 2 === 0, // true if even 'SomeInt' => $j, ]); - for ($k = 0; $k < 2; $k++) { + for ($k = 0; $k < $numLevel2Records; $k++) { $manyManyThroughSubObj = new ManyManyThroughSubEagerLoadObject(); $manyManyThroughSubObj->Title = "manyManyThroughSubObj $i $j $k"; $manyManyThroughSubObj->write(); $manyManyThroughObj->ManyManyThroughSubEagerLoadObjects()->add($manyManyThroughSubObj); - for ($l = 0; $l < 2; $l++) { + for ($l = 0; $l < $numLevel3Records; $l++) { $manyManyThroughSubSubObj = new ManyManyThroughSubSubEagerLoadObject(); $manyManyThroughSubSubObj->Title = "manyManyThroughSubSubObj $i $j $k $l"; $manyManyThroughSubSubObj->write(); @@ -365,17 +418,17 @@ class DataListEagerLoadingTest extends SapphireTest } } // belongs_many_many - for ($j = 0; $j < 2; $j++) { + for ($j = 0; $j < $numLevel1Records; $j++) { $belongsManyManyObj = new BelongsManyManyEagerLoadObject(); $belongsManyManyObj->Title = "belongsManyManyObj $i $j"; $belongsManyManyObj->write(); $obj->BelongsManyManyEagerLoadObjects()->add($belongsManyManyObj); - for ($k = 0; $k < 2; $k++) { + for ($k = 0; $k < $numLevel2Records; $k++) { $belongsManyManySubObj = new BelongsManyManySubEagerLoadObject(); $belongsManyManySubObj->Title = "belongsManyManySubObj $i $j $k"; $belongsManyManySubObj->write(); $belongsManyManyObj->BelongsManyManySubEagerLoadObjects()->add($belongsManyManySubObj); - for ($l = 0; $l < 2; $l++) { + for ($l = 0; $l < $numLevel3Records; $l++) { $belongsManyManySubSubObj = new BelongsManyManySubSubEagerLoadObject(); $belongsManyManySubSubObj->Title = "belongsManyManySubSubObj $i $j $k $l"; $belongsManyManySubSubObj->write(); @@ -384,17 +437,17 @@ class DataListEagerLoadingTest extends SapphireTest } } // mixed - for ($j = 0; $j < 2; $j++) { + for ($j = 0; $j < $numLevel1Records; $j++) { $mixedManyManyObj = new MixedManyManyEagerLoadObject(); $mixedManyManyObj->Title = "mixedManyManyObj $i $j"; $mixedManyManyObj->write(); $obj->MixedManyManyEagerLoadObjects()->add($mixedManyManyObj); - for ($k = 0; $k < 2; $k++) { + for ($k = 0; $k < $numLevel2Records; $k++) { $mixedHasManyObj = new MixedHasManyEagerLoadObject(); $mixedHasManyObj->Title = "mixedHasManyObj $i $j $k"; $mixedHasManyObjID = $mixedHasManyObj->write(); $mixedManyManyObj->MixedHasManyEagerLoadObjects()->add($mixedHasManyObj); - for ($l = 0; $l < 2; $l++) { + for ($l = 0; $l < $numLevel3Records; $l++) { $mixedHasOneObj = new MixedHasOneEagerLoadObject(); $mixedHasOneObj->Title = "mixedHasOneObj $i $j $k $l"; $mixedHasOneObjID = $mixedHasOneObj->write(); @@ -406,7 +459,7 @@ class DataListEagerLoadingTest extends SapphireTest } } - private function iterateEagerLoadData(DataList $dataList): array + private function iterateEagerLoadData(DataList $dataList, int $chunks = 0): array { $results = []; $selectCount = -1; @@ -421,6 +474,9 @@ class DataListEagerLoadingTest extends SapphireTest echo '__START_ITERATE__'; $results = []; $i = 0; + if ($chunks) { + $dataList = $dataList->chunkedFetch($chunks); + } foreach ($dataList as $obj) { // base obj $results[] = $obj->Title; @@ -500,158 +556,6 @@ class DataListEagerLoadingTest extends SapphireTest return [$results, $selectCount]; } - private function expectedEagerLoadData(): array - { - return [ - 'obj 0', - 'hasOneObj 0', - 'hasOneSubObj 0', - 'hasOneSubSubObj 0', - 'belongsToObj 0', - 'belongsToSubObj 0', - 'belongsToSubSubObj 0', - 'hasManyObj 0 0', - 'hasManySubObj 0 0 0', - 'hasManySubSubObj 0 0 0 0', - 'hasManySubSubObj 0 0 0 1', - 'hasManySubObj 0 0 1', - 'hasManySubSubObj 0 0 1 0', - 'hasManySubSubObj 0 0 1 1', - 'hasManyObj 0 1', - 'hasManySubObj 0 1 0', - 'hasManySubSubObj 0 1 0 0', - 'hasManySubSubObj 0 1 0 1', - 'hasManySubObj 0 1 1', - 'hasManySubSubObj 0 1 1 0', - 'hasManySubSubObj 0 1 1 1', - 'manyManyObj 0 0', - 'manyManySubObj 0 0 0', - 'manyManySubSubObj 0 0 0 0', - 'manyManySubSubObj 0 0 0 1', - 'manyManySubObj 0 0 1', - 'manyManySubSubObj 0 0 1 0', - 'manyManySubSubObj 0 0 1 1', - 'manyManyObj 0 1', - 'manyManySubObj 0 1 0', - 'manyManySubSubObj 0 1 0 0', - 'manyManySubSubObj 0 1 0 1', - 'manyManySubObj 0 1 1', - 'manyManySubSubObj 0 1 1 0', - 'manyManySubSubObj 0 1 1 1', - 'manyManyThroughObj 0 0', - 'manyManyThroughSubObj 0 0 0', - 'manyManyThroughSubSubObj 0 0 0 0', - 'manyManyThroughSubSubObj 0 0 0 1', - 'manyManyThroughSubObj 0 0 1', - 'manyManyThroughSubSubObj 0 0 1 0', - 'manyManyThroughSubSubObj 0 0 1 1', - 'manyManyThroughObj 0 1', - 'manyManyThroughSubObj 0 1 0', - 'manyManyThroughSubSubObj 0 1 0 0', - 'manyManyThroughSubSubObj 0 1 0 1', - 'manyManyThroughSubObj 0 1 1', - 'manyManyThroughSubSubObj 0 1 1 0', - 'manyManyThroughSubSubObj 0 1 1 1', - 'belongsManyManyObj 0 0', - 'belongsManyManySubObj 0 0 0', - 'belongsManyManySubSubObj 0 0 0 0', - 'belongsManyManySubSubObj 0 0 0 1', - 'belongsManyManySubObj 0 0 1', - 'belongsManyManySubSubObj 0 0 1 0', - 'belongsManyManySubSubObj 0 0 1 1', - 'belongsManyManyObj 0 1', - 'belongsManyManySubObj 0 1 0', - 'belongsManyManySubSubObj 0 1 0 0', - 'belongsManyManySubSubObj 0 1 0 1', - 'belongsManyManySubObj 0 1 1', - 'belongsManyManySubSubObj 0 1 1 0', - 'belongsManyManySubSubObj 0 1 1 1', - 'mixedManyManyObj 0 0', - 'mixedHasManyObj 0 0 0', - 'mixedHasOneObj 0 0 0 1', - 'mixedHasManyObj 0 0 1', - 'mixedHasOneObj 0 0 1 1', - 'mixedManyManyObj 0 1', - 'mixedHasManyObj 0 1 0', - 'mixedHasOneObj 0 1 0 1', - 'mixedHasManyObj 0 1 1', - 'mixedHasOneObj 0 1 1 1', - 'obj 1', - 'hasOneObj 1', - 'hasOneSubObj 1', - 'hasOneSubSubObj 1', - 'belongsToObj 1', - 'belongsToSubObj 1', - 'belongsToSubSubObj 1', - 'hasManyObj 1 0', - 'hasManySubObj 1 0 0', - 'hasManySubSubObj 1 0 0 0', - 'hasManySubSubObj 1 0 0 1', - 'hasManySubObj 1 0 1', - 'hasManySubSubObj 1 0 1 0', - 'hasManySubSubObj 1 0 1 1', - 'hasManyObj 1 1', - 'hasManySubObj 1 1 0', - 'hasManySubSubObj 1 1 0 0', - 'hasManySubSubObj 1 1 0 1', - 'hasManySubObj 1 1 1', - 'hasManySubSubObj 1 1 1 0', - 'hasManySubSubObj 1 1 1 1', - 'manyManyObj 1 0', - 'manyManySubObj 1 0 0', - 'manyManySubSubObj 1 0 0 0', - 'manyManySubSubObj 1 0 0 1', - 'manyManySubObj 1 0 1', - 'manyManySubSubObj 1 0 1 0', - 'manyManySubSubObj 1 0 1 1', - 'manyManyObj 1 1', - 'manyManySubObj 1 1 0', - 'manyManySubSubObj 1 1 0 0', - 'manyManySubSubObj 1 1 0 1', - 'manyManySubObj 1 1 1', - 'manyManySubSubObj 1 1 1 0', - 'manyManySubSubObj 1 1 1 1', - 'manyManyThroughObj 1 0', - 'manyManyThroughSubObj 1 0 0', - 'manyManyThroughSubSubObj 1 0 0 0', - 'manyManyThroughSubSubObj 1 0 0 1', - 'manyManyThroughSubObj 1 0 1', - 'manyManyThroughSubSubObj 1 0 1 0', - 'manyManyThroughSubSubObj 1 0 1 1', - 'manyManyThroughObj 1 1', - 'manyManyThroughSubObj 1 1 0', - 'manyManyThroughSubSubObj 1 1 0 0', - 'manyManyThroughSubSubObj 1 1 0 1', - 'manyManyThroughSubObj 1 1 1', - 'manyManyThroughSubSubObj 1 1 1 0', - 'manyManyThroughSubSubObj 1 1 1 1', - 'belongsManyManyObj 1 0', - 'belongsManyManySubObj 1 0 0', - 'belongsManyManySubSubObj 1 0 0 0', - 'belongsManyManySubSubObj 1 0 0 1', - 'belongsManyManySubObj 1 0 1', - 'belongsManyManySubSubObj 1 0 1 0', - 'belongsManyManySubSubObj 1 0 1 1', - 'belongsManyManyObj 1 1', - 'belongsManyManySubObj 1 1 0', - 'belongsManyManySubSubObj 1 1 0 0', - 'belongsManyManySubSubObj 1 1 0 1', - 'belongsManyManySubObj 1 1 1', - 'belongsManyManySubSubObj 1 1 1 0', - 'belongsManyManySubSubObj 1 1 1 1', - 'mixedManyManyObj 1 0', - 'mixedHasManyObj 1 0 0', - 'mixedHasOneObj 1 0 0 1', - 'mixedHasManyObj 1 0 1', - 'mixedHasOneObj 1 0 1 1', - 'mixedManyManyObj 1 1', - 'mixedHasManyObj 1 1 0', - 'mixedHasOneObj 1 1 0 1', - 'mixedHasManyObj 1 1 1', - 'mixedHasOneObj 1 1 1 1', - ]; - } - public function testEagerLoadFourthLevelException(): void { $eagerLoadRelation = implode('.', [ @@ -788,4 +692,276 @@ class DataListEagerLoadingTest extends SapphireTest ] ]; } + + /** + * @dataProvider provideEagerLoadRelations + */ + public function testEagerLoadingFilteredList(string $iden, array $eagerLoad): void + { + $this->createEagerLoadData(5); + $filter = ['Title:GreaterThan' => 'obj 0']; + $dataList = EagerLoadObject::get()->filter($filter)->eagerLoad(...$eagerLoad); + + // Validate that filtering results still actually works on the base list + $this->assertListEquals([ + ['Title' => 'obj 1'], + ['Title' => 'obj 2'], + ['Title' => 'obj 3'], + ['Title' => 'obj 4'], + ], $dataList); + + $this->validateEagerLoadingResults($iden, EagerLoadObject::get()->filter($filter), $dataList); + } + + /** + * @dataProvider provideEagerLoadRelations + */ + public function testEagerLoadingSortedList(string $iden, array $eagerLoad): void + { + $this->createEagerLoadData(3); + $items = [ + 'obj 0', + 'obj 1', + 'obj 2', + ]; + + foreach (['ASC', 'DESC'] as $order) { + $sort = "Title $order"; + $dataList = EagerLoadObject::get()->sort($sort)->eagerLoad(...$eagerLoad); + + if ($order === 'DESC') { + $items = array_reverse($items); + } + + // Validate that sorting results still actually works on the base list + $this->assertSame($items, $dataList->column('Title')); + } + + // We don't care about the order after this point, so whichever order we've got will be fine. + // We just want to validate that the data was correctly eager loaded. + $this->validateEagerLoadingResults($iden, EagerLoadObject::get()->sort($sort), $dataList); + } + + /** + * @dataProvider provideEagerLoadRelations + */ + public function testEagerLoadingLimitedList(string $iden, array $eagerLoad): void + { + // Make sure to create more base records AND more records on at least one relation than the limit + // to ensure the limit isn't accidentally carried through to the relations. + $this->createEagerLoadData(6, numLevel3Records: 6); + $limit = 5; + $dataList = EagerLoadObject::get()->limit($limit)->eagerLoad(...$eagerLoad); + + // Validate that limiting results still actually works on the base list + $this->assertCount($limit, $dataList); + + $this->validateEagerLoadingResults($iden, EagerLoadObject::get()->limit($limit), $dataList); + } + + /** + * @dataProvider provideEagerLoadRelations + */ + public function testRepeatedIterationOfEagerLoadedList(string $iden, array $eagerLoad): void + { + // We need at least 3 base records for many_many relations to have fewer db queries than lazy-loaded lists. + $this->createEagerLoadData(3); + $dataList = EagerLoadObject::get()->eagerLoad(...$eagerLoad); + + // Validate twice - each validation requires a full iteration over all records including the base list. + $this->validateEagerLoadingResults($iden, EagerLoadObject::get(), $dataList); + $this->validateEagerLoadingResults($iden, EagerLoadObject::get(), $dataList); + } + + /** + * This test validates that you can call eagerLoad() anywhere on the list before + * execution, including before or after sort/limit/filter, etc - and it will + * work the same way regardless of when it was called. + * + * @dataProvider provideEagerLoadRelations + */ + public function testEagerLoadWorksAnywhereBeforeExecution(string $iden, array $eagerLoad): void + { + $this->createEagerLoadData(7); + $filter = ['Title:LessThan' => 'obj 5']; + $sort = 'Title DESC'; + $limit = 3; + + $lazyList = EagerLoadObject::get()->filter($filter)->sort($sort)->limit($limit); + $lazyListArray = $lazyList->map()->toArray(); + $eagerList1 = EagerLoadObject::get()->eagerLoad(...$eagerLoad)->filter($filter)->sort($sort)->limit($limit); + $eagerList2 = EagerLoadObject::get()->filter($filter)->eagerLoad(...$eagerLoad)->sort($sort)->limit($limit); + $eagerList3 = EagerLoadObject::get()->filter($filter)->sort($sort)->eagerLoad(...$eagerLoad)->limit($limit); + $eagerList4 = EagerLoadObject::get()->filter($filter)->sort($sort)->limit($limit)->eagerLoad(...$eagerLoad); + + // Validates that this list is set up correctly, and there are 3 records with this combination of filter/sort/limit. + $this->assertCount(3, $lazyListArray); + + // This will probably be really slow, but the idea is to validate that no matter when we call eagerLoad(), + // both the underlying DataList results and all of the eagerloaded data is the same. + $this->assertSame($lazyListArray, $eagerList1->map()->toArray()); + $this->assertSame($lazyListArray, $eagerList2->map()->toArray()); + $this->assertSame($lazyListArray, $eagerList3->map()->toArray()); + $this->assertSame($lazyListArray, $eagerList4->map()->toArray()); + $this->validateEagerLoadingResults($iden, $lazyList, $eagerList1); + $this->validateEagerLoadingResults($iden, $lazyList, $eagerList2); + $this->validateEagerLoadingResults($iden, $lazyList, $eagerList3); + $this->validateEagerLoadingResults($iden, $lazyList, $eagerList4); + } + + /** + * @dataProvider provideEagerLoadRelations + */ + public function testEagerLoadWithChunkedFetch(string $iden, array $eagerLoad): void + { + $this->createEagerLoadData(10); + $dataList = EagerLoadObject::get()->eagerLoad(...$eagerLoad); + + $this->validateEagerLoadingResults($iden, EagerLoadObject::get(), $dataList, 3); + } + + private function validateEagerLoadingResults(string $iden, DataList $lazyList, DataList $eagerList, int $chunks = 0): void + { + list($results, $eagerCount) = $this->iterateEagerLoadData($eagerList, $chunks); + // We can rely on the non-eager-loaded data being correct, since it's validated by other unit tests + list($expectedResults, $lazyCount) = $this->iterateEagerLoadData($lazyList, $chunks); + // Sort because the order of the results doesn't really matter - and has proven to be different in postgres + sort($expectedResults); + sort($results); + + $this->assertSame([], array_diff($expectedResults, $results)); + $this->assertSame([], array_diff($results, $expectedResults)); + $this->assertSame(count($expectedResults), count($results)); + + // Validate that we have the same eager-loaded data as the lazy-loaded list, and that lazy-loaded lists + // execute less database queries than lazy-loaded ones + $this->assertSame($expectedResults, $results); + if ($iden !== 'lazy-load') { + $this->assertLessThan($lazyCount, $eagerCount); + } + } + + /** + * @dataProvider provideEagerLoadingEmptyRelationLists + */ + public function testEagerLoadingEmptyRelationLists(string $iden, string $eagerLoad): void + { + $numBaseRecords = 2; + $numLevel1Records = 2; + $numLevel2Records = 2; + // Similar to createEagerLoadData(), except with less relations and + // making sure only the first record has any items in its relation lists. + for ($i = 0; $i < $numBaseRecords; $i++) { + // base object + $obj = new EagerLoadObject(); + $obj->Title = "obj $i"; + $objID = $obj->write(); + if ($i > 0) { + continue; + } + // has_many + for ($j = 0; $j < $numLevel1Records; $j++) { + $hasManyObj = new HasManyEagerLoadObject(); + $hasManyObj->EagerLoadObjectID = $objID; + $hasManyObj->Title = "hasManyObj $i $j"; + $hasManyObjID = $hasManyObj->write(); + if ($j > 0) { + continue; + } + for ($k = 0; $k < $numLevel2Records; $k++) { + $hasManySubObj = new HasManySubEagerLoadObject(); + $hasManySubObj->HasManyEagerLoadObjectID = $hasManyObjID; + $hasManySubObj->Title = "hasManySubObj $i $j $k"; + $hasManySubObj->write(); + } + } + // many_many + for ($j = 0; $j < $numLevel1Records; $j++) { + $manyManyObj = new ManyManyEagerLoadObject(); + $manyManyObj->Title = "manyManyObj $i $j"; + $manyManyObj->write(); + $obj->ManyManyEagerLoadObjects()->add($manyManyObj); + if ($j > 0) { + continue; + } + for ($k = 0; $k < $numLevel2Records; $k++) { + $manyManySubObj = new ManyManySubEagerLoadObject(); + $manyManySubObj->Title = "manyManySubObj $i $j $k"; + $manyManySubObj->write(); + $manyManyObj->ManyManySubEagerLoadObjects()->add($manyManySubObj); + } + } + // many_many_through + for ($j = 0; $j < $numLevel1Records; $j++) { + $manyManyThroughObj = new ManyManyThroughEagerLoadObject(); + $manyManyThroughObj->Title = "manyManyThroughObj $i $j"; + $manyManyThroughObj->write(); + $obj->ManyManyThroughEagerLoadObjects()->add($manyManyThroughObj, [ + 'Title' => "Some text here $i $j", + 'SomeBool' => $j % 2 === 0, // true if even + 'SomeInt' => $j, + ]); + if ($j > 0) { + continue; + } + for ($k = 0; $k < $numLevel2Records; $k++) { + $manyManyThroughSubObj = new ManyManyThroughSubEagerLoadObject(); + $manyManyThroughSubObj->Title = "manyManyThroughSubObj $i $j $k"; + $manyManyThroughSubObj->write(); + $manyManyThroughObj->ManyManyThroughSubEagerLoadObjects()->add($manyManyThroughSubObj); + } + } + } + + $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++; + + if (count($relations) > 1) { + $j = 0; + foreach ($list as $relatedRecord) { + $relation = $relations[1]; + $list2 = $relatedRecord->$relation(); + // For any record after the first one, there should be nothing in the related list. + $this->assertCount($j > 0 ? 0 : 2, $list2); + $j++; + } + } + } + } + + public function provideEagerLoadingEmptyRelationLists(): array + { + return [ + [ + 'hasmany-onelevel', + 'HasManyEagerLoadObjects', + ], + [ + 'hasmany-twolevels', + 'HasManyEagerLoadObjects.HasManySubEagerLoadObjects', + ], + [ + 'manymany-onelevel', + 'ManyManyEagerLoadObjects', + ], + [ + 'manymany-twolevels', + 'ManyManyEagerLoadObjects.ManyManySubEagerLoadObjects', + ], + [ + 'manymany-through-onelevel', + 'ManyManyThroughEagerLoadObjects', + ], + [ + 'manymany-through-twolevels', + 'ManyManyThroughEagerLoadObjects.ManyManyThroughSubEagerLoadObjects', + ], + ]; + } } From 3bf845a9e6c8ec2e38b3144626b0a7708d5cd6ea Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Mon, 10 Jul 2023 18:19:11 +1200 Subject: [PATCH 5/6] FIX Protect against loading incorrect eager-loaded relations. Don't fetch eager-loaded has_many or many_many unless we've validated that the relation IS actually a has_many or many_many relation. Also clear eager-loaded relations when flushing the record. --- src/ORM/DataObject.php | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 5a06c5c1c..0b29c689a 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -1954,12 +1954,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ public function getComponents($componentName, $id = null) { - if (isset($this->eagerLoadedData[$componentName])) { - return $this->eagerLoadedData[$componentName]; - } - if (!isset($id)) { - $id = $this->ID; - } $result = null; $schema = $this->getSchema(); @@ -1972,7 +1966,14 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity )); } + if (isset($this->eagerLoadedData[$componentName])) { + return $this->eagerLoadedData[$componentName]; + } + // If we haven't been written yet, we can't save these relations, so use a list that handles this case + if (!isset($id)) { + $id = $this->ID; + } if (!$id) { if (!isset($this->unsavedRelations[$componentName])) { $this->unsavedRelations[$componentName] = @@ -2174,12 +2175,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ public function getManyManyComponents($componentName, $id = null) { - if (isset($this->eagerLoadedData[$componentName])) { - return $this->eagerLoadedData[$componentName]; - } - if (!isset($id)) { - $id = $this->ID; - } $schema = static::getSchema(); $manyManyComponent = $schema->manyManyComponent(static::class, $componentName); if (!$manyManyComponent) { @@ -2190,7 +2185,14 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity )); } + if (isset($this->eagerLoadedData[$componentName])) { + return $this->eagerLoadedData[$componentName]; + } + // If we haven't been written yet, we can't save these relations, so use a list that handles this case + if (!isset($id)) { + $id = $this->ID; + } if (!$id) { if (!isset($this->unsavedRelations[$componentName])) { $this->unsavedRelations[$componentName] = new UnsavedRelationList( @@ -3447,6 +3449,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $this->extend('flushCache'); $this->components = []; + $this->eagerLoadedData = []; return $this; } From ed4c34b9d9574b4a949415fe859b2efa64841ec2 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Thu, 13 Jul 2023 10:44:12 +1200 Subject: [PATCH 6/6] MNT Re-implement static test data for main eagerloading test --- tests/php/ORM/DataListEagerLoadingTest.php | 155 ++++++++++++++++++++- 1 file changed, 153 insertions(+), 2 deletions(-) diff --git a/tests/php/ORM/DataListEagerLoadingTest.php b/tests/php/ORM/DataListEagerLoadingTest.php index 6a148ee24..256daec69 100644 --- a/tests/php/ORM/DataListEagerLoadingTest.php +++ b/tests/php/ORM/DataListEagerLoadingTest.php @@ -108,8 +108,7 @@ class DataListEagerLoadingTest extends SapphireTest $dataList = EagerLoadObject::get()->eagerLoad(...$eagerLoad); list($results, $selectCount) = $this->iterateEagerLoadData($dataList); - // We can rely on the non-eager-loaded data being correct, since it's validated by other unit tests - list($expectedResults, $_) = $this->iterateEagerLoadData(EagerLoadObject::get()); + $expectedResults = $this->expectedEagerLoadRelations(); // Sort because the order of the results doesn't really matter - and has proven to be different in postgres sort($expectedResults); sort($results); @@ -305,6 +304,158 @@ class DataListEagerLoadingTest extends SapphireTest ]; } + private function expectedEagerLoadRelations(): array + { + return [ + 'obj 0', + 'hasOneObj 0', + 'hasOneSubObj 0', + 'hasOneSubSubObj 0', + 'belongsToObj 0', + 'belongsToSubObj 0', + 'belongsToSubSubObj 0', + 'hasManyObj 0 0', + 'hasManySubObj 0 0 0', + 'hasManySubSubObj 0 0 0 0', + 'hasManySubSubObj 0 0 0 1', + 'hasManySubObj 0 0 1', + 'hasManySubSubObj 0 0 1 0', + 'hasManySubSubObj 0 0 1 1', + 'hasManyObj 0 1', + 'hasManySubObj 0 1 0', + 'hasManySubSubObj 0 1 0 0', + 'hasManySubSubObj 0 1 0 1', + 'hasManySubObj 0 1 1', + 'hasManySubSubObj 0 1 1 0', + 'hasManySubSubObj 0 1 1 1', + 'manyManyObj 0 0', + 'manyManySubObj 0 0 0', + 'manyManySubSubObj 0 0 0 0', + 'manyManySubSubObj 0 0 0 1', + 'manyManySubObj 0 0 1', + 'manyManySubSubObj 0 0 1 0', + 'manyManySubSubObj 0 0 1 1', + 'manyManyObj 0 1', + 'manyManySubObj 0 1 0', + 'manyManySubSubObj 0 1 0 0', + 'manyManySubSubObj 0 1 0 1', + 'manyManySubObj 0 1 1', + 'manyManySubSubObj 0 1 1 0', + 'manyManySubSubObj 0 1 1 1', + 'manyManyThroughObj 0 0', + 'manyManyThroughSubObj 0 0 0', + 'manyManyThroughSubSubObj 0 0 0 0', + 'manyManyThroughSubSubObj 0 0 0 1', + 'manyManyThroughSubObj 0 0 1', + 'manyManyThroughSubSubObj 0 0 1 0', + 'manyManyThroughSubSubObj 0 0 1 1', + 'manyManyThroughObj 0 1', + 'manyManyThroughSubObj 0 1 0', + 'manyManyThroughSubSubObj 0 1 0 0', + 'manyManyThroughSubSubObj 0 1 0 1', + 'manyManyThroughSubObj 0 1 1', + 'manyManyThroughSubSubObj 0 1 1 0', + 'manyManyThroughSubSubObj 0 1 1 1', + 'belongsManyManyObj 0 0', + 'belongsManyManySubObj 0 0 0', + 'belongsManyManySubSubObj 0 0 0 0', + 'belongsManyManySubSubObj 0 0 0 1', + 'belongsManyManySubObj 0 0 1', + 'belongsManyManySubSubObj 0 0 1 0', + 'belongsManyManySubSubObj 0 0 1 1', + 'belongsManyManyObj 0 1', + 'belongsManyManySubObj 0 1 0', + 'belongsManyManySubSubObj 0 1 0 0', + 'belongsManyManySubSubObj 0 1 0 1', + 'belongsManyManySubObj 0 1 1', + 'belongsManyManySubSubObj 0 1 1 0', + 'belongsManyManySubSubObj 0 1 1 1', + 'mixedManyManyObj 0 0', + 'mixedHasManyObj 0 0 0', + 'mixedHasOneObj 0 0 0 1', + 'mixedHasManyObj 0 0 1', + 'mixedHasOneObj 0 0 1 1', + 'mixedManyManyObj 0 1', + 'mixedHasManyObj 0 1 0', + 'mixedHasOneObj 0 1 0 1', + 'mixedHasManyObj 0 1 1', + 'mixedHasOneObj 0 1 1 1', + 'obj 1', + 'hasOneObj 1', + 'hasOneSubObj 1', + 'hasOneSubSubObj 1', + 'belongsToObj 1', + 'belongsToSubObj 1', + 'belongsToSubSubObj 1', + 'hasManyObj 1 0', + 'hasManySubObj 1 0 0', + 'hasManySubSubObj 1 0 0 0', + 'hasManySubSubObj 1 0 0 1', + 'hasManySubObj 1 0 1', + 'hasManySubSubObj 1 0 1 0', + 'hasManySubSubObj 1 0 1 1', + 'hasManyObj 1 1', + 'hasManySubObj 1 1 0', + 'hasManySubSubObj 1 1 0 0', + 'hasManySubSubObj 1 1 0 1', + 'hasManySubObj 1 1 1', + 'hasManySubSubObj 1 1 1 0', + 'hasManySubSubObj 1 1 1 1', + 'manyManyObj 1 0', + 'manyManySubObj 1 0 0', + 'manyManySubSubObj 1 0 0 0', + 'manyManySubSubObj 1 0 0 1', + 'manyManySubObj 1 0 1', + 'manyManySubSubObj 1 0 1 0', + 'manyManySubSubObj 1 0 1 1', + 'manyManyObj 1 1', + 'manyManySubObj 1 1 0', + 'manyManySubSubObj 1 1 0 0', + 'manyManySubSubObj 1 1 0 1', + 'manyManySubObj 1 1 1', + 'manyManySubSubObj 1 1 1 0', + 'manyManySubSubObj 1 1 1 1', + 'manyManyThroughObj 1 0', + 'manyManyThroughSubObj 1 0 0', + 'manyManyThroughSubSubObj 1 0 0 0', + 'manyManyThroughSubSubObj 1 0 0 1', + 'manyManyThroughSubObj 1 0 1', + 'manyManyThroughSubSubObj 1 0 1 0', + 'manyManyThroughSubSubObj 1 0 1 1', + 'manyManyThroughObj 1 1', + 'manyManyThroughSubObj 1 1 0', + 'manyManyThroughSubSubObj 1 1 0 0', + 'manyManyThroughSubSubObj 1 1 0 1', + 'manyManyThroughSubObj 1 1 1', + 'manyManyThroughSubSubObj 1 1 1 0', + 'manyManyThroughSubSubObj 1 1 1 1', + 'belongsManyManyObj 1 0', + 'belongsManyManySubObj 1 0 0', + 'belongsManyManySubSubObj 1 0 0 0', + 'belongsManyManySubSubObj 1 0 0 1', + 'belongsManyManySubObj 1 0 1', + 'belongsManyManySubSubObj 1 0 1 0', + 'belongsManyManySubSubObj 1 0 1 1', + 'belongsManyManyObj 1 1', + 'belongsManyManySubObj 1 1 0', + 'belongsManyManySubSubObj 1 1 0 0', + 'belongsManyManySubSubObj 1 1 0 1', + 'belongsManyManySubObj 1 1 1', + 'belongsManyManySubSubObj 1 1 1 0', + 'belongsManyManySubSubObj 1 1 1 1', + 'mixedManyManyObj 1 0', + 'mixedHasManyObj 1 0 0', + 'mixedHasOneObj 1 0 0 1', + 'mixedHasManyObj 1 0 1', + 'mixedHasOneObj 1 0 1 1', + 'mixedManyManyObj 1 1', + 'mixedHasManyObj 1 1 0', + 'mixedHasOneObj 1 1 0 1', + 'mixedHasManyObj 1 1 1', + 'mixedHasOneObj 1 1 1 1', + ]; + } + private function createEagerLoadData( int $numBaseRecords = 2, int $numLevel1Records = 2,