From 34c16b69fcc58ea120d2a61c82968883209e3639 Mon Sep 17 00:00:00 2001 From: Constantine Karnaukhov Date: Mon, 21 Jun 2021 19:32:57 +0400 Subject: [PATCH 1/4] test(select): add test case which shows that scope not applied by default --- tests/ORM/Fixtures/SortByIdDescConstrain.php | 23 +++++++ tests/ORM/HasManyRelationTest.php | 72 ++++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 tests/ORM/Fixtures/SortByIdDescConstrain.php diff --git a/tests/ORM/Fixtures/SortByIdDescConstrain.php b/tests/ORM/Fixtures/SortByIdDescConstrain.php new file mode 100644 index 000000000..93d176dfe --- /dev/null +++ b/tests/ORM/Fixtures/SortByIdDescConstrain.php @@ -0,0 +1,23 @@ +orderBy('id', 'DESC'); + } +} diff --git a/tests/ORM/HasManyRelationTest.php b/tests/ORM/HasManyRelationTest.php index 0a03bbcf6..da2278f4c 100644 --- a/tests/ORM/HasManyRelationTest.php +++ b/tests/ORM/HasManyRelationTest.php @@ -20,6 +20,7 @@ use Cycle\ORM\Select\JoinableLoader; use Cycle\ORM\Tests\Fixtures\Comment; use Cycle\ORM\Tests\Fixtures\SortByIDConstrain; +use Cycle\ORM\Tests\Fixtures\SortByIdDescConstrain; use Cycle\ORM\Tests\Fixtures\User; use Cycle\ORM\Tests\Traits\TableTrait; use Cycle\ORM\Transaction; @@ -143,6 +144,77 @@ public function testFetchRelation(): void ], $selector->fetchData()); } + public function testFetchRelationWithScopeAppliedByDefault(): void + { + $this->withSchema(new Schema([ + User::class => [ + Schema::ROLE => 'user', + Schema::MAPPER => Mapper::class, + Schema::DATABASE => 'default', + Schema::TABLE => 'user', + Schema::PRIMARY_KEY => 'id', + Schema::COLUMNS => ['id', 'email', 'balance'], + Schema::SCHEMA => [], + Schema::RELATIONS => [ + 'comments' => [ + Relation::TYPE => Relation::HAS_MANY, + Relation::TARGET => Comment::class, + Relation::SCHEMA => [ + Relation::CASCADE => true, + Relation::INNER_KEY => 'id', + Relation::OUTER_KEY => 'user_id', + ], + ] + ], + Schema::CONSTRAIN => SortByIdDescConstrain::class, + ], + Comment::class => [ + Schema::ROLE => 'comment', + Schema::MAPPER => Mapper::class, + Schema::DATABASE => 'default', + Schema::TABLE => 'comment', + Schema::PRIMARY_KEY => 'id', + Schema::COLUMNS => ['id', 'user_id', 'message'], + Schema::SCHEMA => [], + Schema::RELATIONS => [], + Schema::CONSTRAIN => SortByIdDescConstrain::class, + ] + ])); + + $selector = new Select($this->orm, User::class); + $selector->load('comments'); + + $this->assertEquals([ + [ + 'id' => 2, + 'email' => 'another@world.com', + 'balance' => 200.0, + 'comments' => [], + ], + [ + 'id' => 1, + 'email' => 'hello@world.com', + 'balance' => 100.0, + 'comments' => [ + [ + 'id' => 3, + 'user_id' => 1, + 'message' => 'msg 3', + ], + [ + 'id' => 2, + 'user_id' => 1, + 'message' => 'msg 2', + ], + [ + 'id' => 1, + 'user_id' => 1, + 'message' => 'msg 1', + ], + ], + ], + ], $selector->fetchData()); + } public function testFetchRelationInload(): void { From ec55fb0790ae7aee53e64876ad2a8c385979a7b0 Mon Sep 17 00:00:00 2001 From: Constantine Karnaukhov Date: Mon, 21 Jun 2021 19:37:08 +0400 Subject: [PATCH 2/4] refactor(select): apply scope by default --- src/Select/AbstractLoader.php | 9 +++------ src/Select/JoinableLoader.php | 3 --- src/Select/QueryBuilder.php | 2 +- src/Select/RootLoader.php | 1 - 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/Select/AbstractLoader.php b/src/Select/AbstractLoader.php index 6cbafd73c..d6303f8e9 100644 --- a/src/Select/AbstractLoader.php +++ b/src/Select/AbstractLoader.php @@ -20,6 +20,7 @@ use Cycle\ORM\Schema; use Cycle\ORM\Select\Traits\AliasTrait; use Cycle\ORM\Select\Traits\ChainTrait; +use Cycle\ORM\Select\Traits\ConstrainTrait; use Spiral\Database\Query\SelectQuery; /** @@ -45,6 +46,7 @@ abstract class AbstractLoader implements LoaderInterface { use ChainTrait; use AliasTrait; + use ConstrainTrait; // Loading methods for data loaders. public const INLOAD = 1; @@ -81,6 +83,7 @@ public function __construct(ORMInterface $orm, string $target) { $this->orm = $orm; $this->target = $target; + $this->setConstrain($this->getSource()->getConstrain()); } /** @@ -294,12 +297,6 @@ protected function configureQuery(SelectQuery $query): SelectQuery return $query; } - /** - * @param SelectQuery $query - * @return SelectQuery - */ - abstract protected function applyConstrain(SelectQuery $query): SelectQuery; - /** * Define schema option associated with the entity. * diff --git a/src/Select/JoinableLoader.php b/src/Select/JoinableLoader.php index c29633d54..0874f7aa9 100644 --- a/src/Select/JoinableLoader.php +++ b/src/Select/JoinableLoader.php @@ -26,7 +26,6 @@ abstract class JoinableLoader extends AbstractLoader implements JoinableInterface { use ColumnsTrait; - use ConstrainTrait; /** * Default set of relation options. Child implementation might defined their of default options. @@ -128,8 +127,6 @@ public function withContext(LoaderInterface $parent, array $options = []): Loade } elseif (is_string($loader->options['constrain'])) { $loader->setConstrain($this->orm->getFactory()->make($loader->options['constrain'])); } - } else { - $loader->setConstrain($this->getSource()->getConstrain()); } if ($loader->isLoaded()) { diff --git a/src/Select/QueryBuilder.php b/src/Select/QueryBuilder.php index 031d4e1bc..69523f34f 100644 --- a/src/Select/QueryBuilder.php +++ b/src/Select/QueryBuilder.php @@ -151,7 +151,7 @@ public function resolve(string $identifier, bool $autoload = true): string $loader = $this->findLoader(substr($identifier, 0, $split), $autoload); if ($loader !== null) { return sprintf( - '%s.%s.', + '%s.%s', $loader->getAlias(), $loader->fieldAlias(substr($identifier, $split + 1)) ); diff --git a/src/Select/RootLoader.php b/src/Select/RootLoader.php index de7b46e54..dcdbe12f0 100644 --- a/src/Select/RootLoader.php +++ b/src/Select/RootLoader.php @@ -30,7 +30,6 @@ final class RootLoader extends AbstractLoader { use ColumnsTrait; - use ConstrainTrait; /** @var array */ protected $options = [ From ce20eb2052057d0667cf4cc59e3c94d68f8840ba Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Tue, 22 Jun 2021 10:40:57 +0300 Subject: [PATCH 3/4] Cleanup --- src/Select/JoinableLoader.php | 1 - src/Select/RootLoader.php | 1 - tests/ORM/HasManyRelationTest.php | 109 ++++++++++++------------------ 3 files changed, 42 insertions(+), 69 deletions(-) diff --git a/src/Select/JoinableLoader.php b/src/Select/JoinableLoader.php index 0874f7aa9..c864167d0 100644 --- a/src/Select/JoinableLoader.php +++ b/src/Select/JoinableLoader.php @@ -16,7 +16,6 @@ use Cycle\ORM\Parser\AbstractNode; use Cycle\ORM\Schema; use Cycle\ORM\Select\Traits\ColumnsTrait; -use Cycle\ORM\Select\Traits\ConstrainTrait; use Spiral\Database\Query\SelectQuery; use Spiral\Database\StatementInterface; diff --git a/src/Select/RootLoader.php b/src/Select/RootLoader.php index dcdbe12f0..d964f4fde 100644 --- a/src/Select/RootLoader.php +++ b/src/Select/RootLoader.php @@ -17,7 +17,6 @@ use Cycle\ORM\Parser\Typecast; use Cycle\ORM\Schema; use Cycle\ORM\Select\Traits\ColumnsTrait; -use Cycle\ORM\Select\Traits\ConstrainTrait; use Spiral\Database\Query\SelectQuery; use Spiral\Database\StatementInterface; diff --git a/tests/ORM/HasManyRelationTest.php b/tests/ORM/HasManyRelationTest.php index da2278f4c..4feef5b68 100644 --- a/tests/ORM/HasManyRelationTest.php +++ b/tests/ORM/HasManyRelationTest.php @@ -66,39 +66,7 @@ public function setUp(): void ] ); - $this->orm = $this->withSchema(new Schema([ - User::class => [ - Schema::ROLE => 'user', - Schema::MAPPER => Mapper::class, - Schema::DATABASE => 'default', - Schema::TABLE => 'user', - Schema::PRIMARY_KEY => 'id', - Schema::COLUMNS => ['id', 'email', 'balance'], - Schema::SCHEMA => [], - Schema::RELATIONS => [ - 'comments' => [ - Relation::TYPE => Relation::HAS_MANY, - Relation::TARGET => Comment::class, - Relation::SCHEMA => [ - Relation::CASCADE => true, - Relation::INNER_KEY => 'id', - Relation::OUTER_KEY => 'user_id', - ], - ] - ] - ], - Comment::class => [ - Schema::ROLE => 'comment', - Schema::MAPPER => Mapper::class, - Schema::DATABASE => 'default', - Schema::TABLE => 'comment', - Schema::PRIMARY_KEY => 'id', - Schema::COLUMNS => ['id', 'user_id', 'message'], - Schema::SCHEMA => [], - Schema::RELATIONS => [], - Schema::CONSTRAIN => SortByIDConstrain::class - ] - ])); + $this->orm = $this->withSchema(new Schema($this->getSchemaArray())); } public function testInitRelation(): void @@ -146,40 +114,10 @@ public function testFetchRelation(): void public function testFetchRelationWithScopeAppliedByDefault(): void { - $this->withSchema(new Schema([ - User::class => [ - Schema::ROLE => 'user', - Schema::MAPPER => Mapper::class, - Schema::DATABASE => 'default', - Schema::TABLE => 'user', - Schema::PRIMARY_KEY => 'id', - Schema::COLUMNS => ['id', 'email', 'balance'], - Schema::SCHEMA => [], - Schema::RELATIONS => [ - 'comments' => [ - Relation::TYPE => Relation::HAS_MANY, - Relation::TARGET => Comment::class, - Relation::SCHEMA => [ - Relation::CASCADE => true, - Relation::INNER_KEY => 'id', - Relation::OUTER_KEY => 'user_id', - ], - ] - ], - Schema::CONSTRAIN => SortByIdDescConstrain::class, - ], - Comment::class => [ - Schema::ROLE => 'comment', - Schema::MAPPER => Mapper::class, - Schema::DATABASE => 'default', - Schema::TABLE => 'comment', - Schema::PRIMARY_KEY => 'id', - Schema::COLUMNS => ['id', 'user_id', 'message'], - Schema::SCHEMA => [], - Schema::RELATIONS => [], - Schema::CONSTRAIN => SortByIdDescConstrain::class, - ] - ])); + $schemaArray = $this->getSchemaArray(); + $schemaArray[User::class][Schema::CONSTRAIN] = SortByIdDescConstrain::class; + $schemaArray[Comment::class][Schema::CONSTRAIN] = SortByIdDescConstrain::class; + $this->withSchema(new Schema($schemaArray)); $selector = new Select($this->orm, User::class); $selector->load('comments'); @@ -544,4 +482,41 @@ public function testSliceAndSaveToAnotherParent(): void $this->assertEquals('new b', $b->comments[0]->message); } + + private function getSchemaArray(): array + { + return [ + User::class => [ + Schema::ROLE => 'user', + Schema::MAPPER => Mapper::class, + Schema::DATABASE => 'default', + Schema::TABLE => 'user', + Schema::PRIMARY_KEY => 'id', + Schema::COLUMNS => ['id', 'email', 'balance'], + Schema::SCHEMA => [], + Schema::RELATIONS => [ + 'comments' => [ + Relation::TYPE => Relation::HAS_MANY, + Relation::TARGET => Comment::class, + Relation::SCHEMA => [ + Relation::CASCADE => true, + Relation::INNER_KEY => 'id', + Relation::OUTER_KEY => 'user_id', + ], + ] + ] + ], + Comment::class => [ + Schema::ROLE => 'comment', + Schema::MAPPER => Mapper::class, + Schema::DATABASE => 'default', + Schema::TABLE => 'comment', + Schema::PRIMARY_KEY => 'id', + Schema::COLUMNS => ['id', 'user_id', 'message'], + Schema::SCHEMA => [], + Schema::RELATIONS => [], + Schema::CONSTRAIN => SortByIDConstrain::class + ] + ]; + } } From 55d7978830665f716d690ad8bb20ac1204fa6276 Mon Sep 17 00:00:00 2001 From: Constantine Karnaukhov Date: Wed, 23 Jun 2021 17:43:55 +0400 Subject: [PATCH 4/4] refactor: source scope applied in one and only place --- src/ORM.php | 7 +------ src/Relation/Pivoted/PivotedPromise.php | 1 - tests/ORM/Fixtures/ProfilePromise.php | 4 +--- tests/ORM/Fixtures/UserPromise.php | 4 +--- 4 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/ORM.php b/src/ORM.php index 87ec1e885..f6078db37 100644 --- a/src/ORM.php +++ b/src/ORM.php @@ -261,12 +261,7 @@ public function getRepository($entity): RepositoryInterface return $this->repositories[$role]; } - $select = null; - - if ($this->schema->define($role, Schema::TABLE) !== null) { - $select = new Select($this, $role); - $select->constrain($this->getSource($role)->getConstrain()); - } + $select = new Select($this, $role); return $this->repositories[$role] = $this->factory->repository($this, $this->schema, $role, $select); } diff --git a/src/Relation/Pivoted/PivotedPromise.php b/src/Relation/Pivoted/PivotedPromise.php index 6478f2b59..c8777316f 100644 --- a/src/Relation/Pivoted/PivotedPromise.php +++ b/src/Relation/Pivoted/PivotedPromise.php @@ -113,7 +113,6 @@ public function __resolve() /** @var ManyToManyLoader $loader */ $loader = $loader->withContext($loader, [ - 'constrain' => $this->orm->getSource($this->target)->getConstrain(), 'as' => $this->target, 'method' => JoinableLoader::POSTLOAD ]); diff --git a/tests/ORM/Fixtures/ProfilePromise.php b/tests/ORM/Fixtures/ProfilePromise.php index 6b8d7f31d..3da6f1142 100644 --- a/tests/ORM/Fixtures/ProfilePromise.php +++ b/tests/ORM/Fixtures/ProfilePromise.php @@ -69,9 +69,7 @@ public function __resolve() { if (!is_null($this->orm)) { $select = new Select($this->orm, $this->target); - $data = $select->constrain( - $this->orm->getSource($this->target)->getConstrain() - )->where($this->scope)->fetchData(); + $data = $select->where($this->scope)->fetchData(); $this->orm->getMapper($this->target)->hydrate($this, $data[0]); diff --git a/tests/ORM/Fixtures/UserPromise.php b/tests/ORM/Fixtures/UserPromise.php index f57d28295..8d38d79e8 100644 --- a/tests/ORM/Fixtures/UserPromise.php +++ b/tests/ORM/Fixtures/UserPromise.php @@ -79,9 +79,7 @@ public function __resolve() // Fetching from the database $select = new Select($this->orm, $this->target); - $this->resolved = $select->constrain( - $this->orm->getSource($this->target)->getConstrain() - )->fetchOne($this->scope); + $this->resolved = $select->fetchOne($this->scope); $this->orm = null; }