From 243f80733818f596b8222c7e707f1fe334f30e6c Mon Sep 17 00:00:00 2001 From: Kacper Pruszynski Date: Fri, 29 Mar 2024 23:55:35 +0100 Subject: [PATCH] Allow passing query Expression as column in Many-to-Many relationships (#50787) --- .../Eloquent/Relations/BelongsToMany.php | 38 +++-- ...aseEloquentBelongsToManyExpressionTest.php | 152 ++++++++++++++++++ ...tBelongsToManyWithCastedAttributesTest.php | 4 + ...BelongsToManyWithDefaultAttributesTest.php | 2 + ...oquentBelongsToManyWithoutTouchingTest.php | 2 + tests/Database/DatabaseEloquentModelTest.php | 2 + .../DatabaseEloquentMorphToManyTest.php | 51 ++++-- 7 files changed, 222 insertions(+), 29 deletions(-) create mode 100644 tests/Database/DatabaseEloquentBelongsToManyExpressionTest.php diff --git a/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php b/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php index 9fe0d301918b..a030b059a60b 100755 --- a/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php +++ b/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php @@ -65,7 +65,7 @@ class BelongsToMany extends Relation /** * The pivot table columns to retrieve. * - * @var array + * @var array */ protected $pivotColumns = []; @@ -356,7 +356,7 @@ public function as($accessor) /** * Set a where clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param mixed $operator * @param mixed $value * @param string $boolean @@ -372,7 +372,7 @@ public function wherePivot($column, $operator = null, $value = null, $boolean = /** * Set a "where between" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param array $values * @param string $boolean * @param bool $not @@ -386,7 +386,7 @@ public function wherePivotBetween($column, array $values, $boolean = 'and', $not /** * Set a "or where between" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param array $values * @return $this */ @@ -398,7 +398,7 @@ public function orWherePivotBetween($column, array $values) /** * Set a "where pivot not between" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param array $values * @param string $boolean * @return $this @@ -411,7 +411,7 @@ public function wherePivotNotBetween($column, array $values, $boolean = 'and') /** * Set a "or where not between" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param array $values * @return $this */ @@ -423,7 +423,7 @@ public function orWherePivotNotBetween($column, array $values) /** * Set a "where in" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param mixed $values * @param string $boolean * @param bool $not @@ -439,7 +439,7 @@ public function wherePivotIn($column, $values, $boolean = 'and', $not = false) /** * Set an "or where" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param mixed $operator * @param mixed $value * @return $this @@ -454,7 +454,7 @@ public function orWherePivot($column, $operator = null, $value = null) * * In addition, new pivot records will receive this value. * - * @param string|array $column + * @param string|\Illuminate\Contracts\Database\Query\Expression|array $column * @param mixed $value * @return $this * @@ -494,7 +494,7 @@ public function orWherePivotIn($column, $values) /** * Set a "where not in" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param mixed $values * @param string $boolean * @return $this @@ -519,7 +519,7 @@ public function orWherePivotNotIn($column, $values) /** * Set a "where null" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param string $boolean * @param bool $not * @return $this @@ -534,7 +534,7 @@ public function wherePivotNull($column, $boolean = 'and', $not = false) /** * Set a "where not null" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param string $boolean * @return $this */ @@ -546,7 +546,7 @@ public function wherePivotNotNull($column, $boolean = 'and') /** * Set a "or where null" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param bool $not * @return $this */ @@ -558,7 +558,7 @@ public function orWherePivotNull($column, $not = false) /** * Set a "or where not null" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @return $this */ public function orWherePivotNotNull($column) @@ -569,7 +569,7 @@ public function orWherePivotNotNull($column) /** * Add an "order by" clause for a pivot table column. * - * @param string $column + * @param string|\Illuminate\Contracts\Database\Query\Expression $column * @param string $direction * @return $this */ @@ -1558,11 +1558,15 @@ public function getPivotColumns() /** * Qualify the given column name by the pivot table. * - * @param string $column - * @return string + * @param string|\Illuminate\Contracts\Database\Query\Expression $column + * @return string|\Illuminate\Contracts\Database\Query\Expression */ public function qualifyPivotColumn($column) { + if ($this->getGrammar()->isExpression($column)) { + return $column; + } + return str_contains($column, '.') ? $column : $this->table.'.'.$column; diff --git a/tests/Database/DatabaseEloquentBelongsToManyExpressionTest.php b/tests/Database/DatabaseEloquentBelongsToManyExpressionTest.php new file mode 100644 index 000000000000..647757bb3ff3 --- /dev/null +++ b/tests/Database/DatabaseEloquentBelongsToManyExpressionTest.php @@ -0,0 +1,152 @@ +addConnection([ + 'driver' => 'sqlite', + 'database' => ':memory:', + ]); + + $db->bootEloquent(); + $db->setAsGlobal(); + + $this->createSchema(); + } + + public function testAmbiguousColumnsExpression(): void + { + $this->seedData(); + + $tags = DatabaseEloquentBelongsToManyExpressionTestTestPost::findOrFail(1) + ->tags() + ->wherePivotNotIn(new Expression("tag_id || '_' || type"), ['1_t1']) + ->get(); + + $this->assertCount(1, $tags); + $this->assertEquals(2, $tags->first()->getKey()); + } + + public function testQualifiedColumnExpression(): void + { + $this->seedData(); + + $tags = DatabaseEloquentBelongsToManyExpressionTestTestPost::findOrFail(2) + ->tags() + ->wherePivotNotIn(new Expression("taggables.tag_id || '_' || taggables.type"), ['2_t2']) + ->get(); + + $this->assertCount(1, $tags); + $this->assertEquals(3, $tags->first()->getKey()); + } + + /** + * Setup the database schema. + * + * @return void + */ + public function createSchema() + { + $this->schema()->create('posts', fn (Blueprint $t) => $t->id()); + $this->schema()->create('tags', fn (Blueprint $t) => $t->id()); + $this->schema()->create('taggables', function (Blueprint $t) { + $t->unsignedBigInteger('tag_id'); + $t->unsignedBigInteger('taggable_id'); + $t->string('type', 10); + $t->string('taggable_type'); + } + ); + } + + /** + * Tear down the database schema. + * + * @return void + */ + protected function tearDown(): void + { + $this->schema()->drop('posts'); + $this->schema()->drop('tags'); + $this->schema()->drop('taggables'); + } + + /** + * Helpers... + */ + protected function seedData(): void + { + $p1 = DatabaseEloquentBelongsToManyExpressionTestTestPost::query()->create(); + $p2 = DatabaseEloquentBelongsToManyExpressionTestTestPost::query()->create(); + $t1 = DatabaseEloquentBelongsToManyExpressionTestTestTag::query()->create(); + $t2 = DatabaseEloquentBelongsToManyExpressionTestTestTag::query()->create(); + $t3 = DatabaseEloquentBelongsToManyExpressionTestTestTag::query()->create(); + + $p1->tags()->sync([ + $t1->getKey() => ['type' => 't1'], + $t2->getKey() => ['type' => 't2'], + ]); + $p2->tags()->sync([ + $t2->getKey() => ['type' => 't2'], + $t3->getKey() => ['type' => 't3'], + ]); + } + + /** + * Get a database connection instance. + * + * @return \Illuminate\Database\ConnectionInterface + */ + protected function connection() + { + return Eloquent::getConnectionResolver()->connection(); + } + + /** + * Get a schema builder instance. + * + * @return \Illuminate\Database\Schema\Builder + */ + protected function schema() + { + return $this->connection()->getSchemaBuilder(); + } +} + +class DatabaseEloquentBelongsToManyExpressionTestTestPost extends Eloquent +{ + protected $table = 'posts'; + protected $fillable = ['id']; + public $timestamps = false; + + public function tags(): MorphToMany + { + return $this->morphToMany( + DatabaseEloquentBelongsToManyExpressionTestTestTag::class, + 'taggable', + 'taggables', + 'taggable_id', + 'tag_id', + 'id', + 'id', + ); + } +} + +class DatabaseEloquentBelongsToManyExpressionTestTestTag extends Eloquent +{ + protected $table = 'tags'; + protected $fillable = ['id']; + public $timestamps = false; +} diff --git a/tests/Database/DatabaseEloquentBelongsToManyWithCastedAttributesTest.php b/tests/Database/DatabaseEloquentBelongsToManyWithCastedAttributesTest.php index 7e1047ac9a38..34f89e8c49ef 100644 --- a/tests/Database/DatabaseEloquentBelongsToManyWithCastedAttributesTest.php +++ b/tests/Database/DatabaseEloquentBelongsToManyWithCastedAttributesTest.php @@ -6,6 +6,7 @@ use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsToMany; +use Illuminate\Database\Query\Grammars\Grammar; use Mockery as m; use PHPUnit\Framework\TestCase; @@ -61,6 +62,9 @@ protected function getRelation() $builder->shouldReceive('getModel')->andReturn($related); $related->shouldReceive('qualifyColumn'); $builder->shouldReceive('join', 'where'); + $builder->shouldReceive('getGrammar')->andReturn( + m::mock(Grammar::class, ['isExpression' => false]) + ); return new BelongsToMany( $builder, diff --git a/tests/Database/DatabaseEloquentBelongsToManyWithDefaultAttributesTest.php b/tests/Database/DatabaseEloquentBelongsToManyWithDefaultAttributesTest.php index 512f87454691..90174ce3e771 100644 --- a/tests/Database/DatabaseEloquentBelongsToManyWithDefaultAttributesTest.php +++ b/tests/Database/DatabaseEloquentBelongsToManyWithDefaultAttributesTest.php @@ -5,6 +5,7 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsToMany; +use Illuminate\Database\Query\Grammars\Grammar; use Mockery as m; use PHPUnit\Framework\TestCase; use stdClass; @@ -55,6 +56,7 @@ public function getRelationArguments() $builder->shouldReceive('join')->once()->with('club_user', 'users.id', '=', 'club_user.user_id'); $builder->shouldReceive('where')->once()->with('club_user.club_id', '=', 1); $builder->shouldReceive('where')->once()->with('club_user.is_admin', '=', 1, 'and'); + $builder->shouldReceive('getGrammar')->andReturn(m::mock(Grammar::class, ['isExpression' => false])); return [$builder, $parent, 'club_user', 'club_id', 'user_id', 'id', 'id', null, false]; } diff --git a/tests/Database/DatabaseEloquentBelongsToManyWithoutTouchingTest.php b/tests/Database/DatabaseEloquentBelongsToManyWithoutTouchingTest.php index 32d617e36fb2..6eafa9668671 100644 --- a/tests/Database/DatabaseEloquentBelongsToManyWithoutTouchingTest.php +++ b/tests/Database/DatabaseEloquentBelongsToManyWithoutTouchingTest.php @@ -7,6 +7,7 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsToMany; +use Illuminate\Database\Query\Grammars\Grammar; use Mockery as m; use PHPUnit\Framework\TestCase; @@ -31,6 +32,7 @@ public function testItWillNotTouchRelatedModelsWhenUpdatingChild(): void $parent->shouldReceive('getAttribute')->with('id')->andReturn(1); $builder->shouldReceive('getModel')->andReturn($related); $builder->shouldReceive('where'); + $builder->shouldReceive('getGrammar')->andReturn(m::mock(Grammar::class, ['isExpression' => false])); $relation = new BelongsToMany($builder, $parent, 'article_users', 'user_id', 'article_id', 'id', 'id'); $builder->shouldReceive('update')->never(); diff --git a/tests/Database/DatabaseEloquentModelTest.php b/tests/Database/DatabaseEloquentModelTest.php index cbfe8f74b1f9..2b8f11eaee8e 100755 --- a/tests/Database/DatabaseEloquentModelTest.php +++ b/tests/Database/DatabaseEloquentModelTest.php @@ -2860,6 +2860,7 @@ protected function addMockConnection($model) $resolver->shouldReceive('connection')->andReturn($connection = m::mock(Connection::class)); $connection->shouldReceive('getQueryGrammar')->andReturn($grammar = m::mock(Grammar::class)); $grammar->shouldReceive('getBitwiseOperators')->andReturn([]); + $grammar->shouldReceive('isExpression')->andReturnFalse(); $connection->shouldReceive('getPostProcessor')->andReturn($processor = m::mock(Processor::class)); $connection->shouldReceive('query')->andReturnUsing(function () use ($connection, $grammar, $processor) { return new BaseBuilder($connection, $grammar, $processor); @@ -3214,6 +3215,7 @@ public function getConnection() $mock = m::mock(Connection::class); $mock->shouldReceive('getQueryGrammar')->andReturn($grammar = m::mock(Grammar::class)); $grammar->shouldReceive('getBitwiseOperators')->andReturn([]); + $grammar->shouldReceive('isExpression')->andReturnFalse(); $mock->shouldReceive('getPostProcessor')->andReturn($processor = m::mock(Processor::class)); $mock->shouldReceive('getName')->andReturn('name'); $mock->shouldReceive('query')->andReturnUsing(function () use ($mock, $grammar, $processor) { diff --git a/tests/Database/DatabaseEloquentMorphToManyTest.php b/tests/Database/DatabaseEloquentMorphToManyTest.php index 32ccbe4b6705..36f4466ee943 100644 --- a/tests/Database/DatabaseEloquentMorphToManyTest.php +++ b/tests/Database/DatabaseEloquentMorphToManyTest.php @@ -5,18 +5,15 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\MorphToMany; +use Illuminate\Database\Query\Expression; +use Illuminate\Database\Query\Grammars\Grammar; +use Mockery\Adapter\Phpunit\MockeryTestCase as TestCase; use Mockery as m; -use PHPUnit\Framework\TestCase; use stdClass; class DatabaseEloquentMorphToManyTest extends TestCase { - protected function tearDown(): void - { - m::close(); - } - - public function testEagerConstraintsAreProperlyAdded() + public function testEagerConstraintsAreProperlyAdded(): void { $relation = $this->getRelation(); $relation->getParent()->shouldReceive('getKeyName')->andReturn('id'); @@ -30,7 +27,7 @@ public function testEagerConstraintsAreProperlyAdded() $relation->addEagerConstraints([$model1, $model2]); } - public function testAttachInsertsPivotTableRecord() + public function testAttachInsertsPivotTableRecord(): void { $relation = $this->getMockBuilder(MorphToMany::class)->onlyMethods(['touchIfTouching'])->setConstructorArgs($this->getRelationArguments())->getMock(); $query = m::mock(stdClass::class); @@ -43,7 +40,7 @@ public function testAttachInsertsPivotTableRecord() $relation->attach(2, ['foo' => 'bar']); } - public function testDetachRemovesPivotTableRecord() + public function testDetachRemovesPivotTableRecord(): void { $relation = $this->getMockBuilder(MorphToMany::class)->onlyMethods(['touchIfTouching'])->setConstructorArgs($this->getRelationArguments())->getMock(); $query = m::mock(stdClass::class); @@ -59,7 +56,7 @@ public function testDetachRemovesPivotTableRecord() $this->assertTrue($relation->detach([1, 2, 3])); } - public function testDetachMethodClearsAllPivotRecordsWhenNoIDsAreGiven() + public function testDetachMethodClearsAllPivotRecordsWhenNoIDsAreGiven(): void { $relation = $this->getMockBuilder(MorphToMany::class)->onlyMethods(['touchIfTouching'])->setConstructorArgs($this->getRelationArguments())->getMock(); $query = m::mock(stdClass::class); @@ -75,14 +72,39 @@ public function testDetachMethodClearsAllPivotRecordsWhenNoIDsAreGiven() $this->assertTrue($relation->detach()); } - public function getRelation() + public function testQueryExpressionCanBePassedToDifferentPivotQueryBuilderClauses(): void + { + $value = 'pivot_value'; + $column = new Expression("CONCAT(foo, '_', bar)"); + $relation = $this->getRelation(); + /** @var Builder|m\MockInterface $builder */ + $builder = $relation->getQuery(); + + $builder->shouldReceive('where')->with($column, '=', $value, 'and')->times(2)->andReturnSelf(); + $relation->wherePivot($column, '=', $value); + $relation->withPivotValue($column, $value); + + $builder->shouldReceive('whereBetween')->with($column, [$value, $value], 'and', false)->once()->andReturnSelf(); + $relation->wherePivotBetween($column, [$value, $value]); + + $builder->shouldReceive('whereIn')->with($column, [$value], 'and', false)->once()->andReturnSelf(); + $relation->wherePivotIn($column, [$value]); + + $builder->shouldReceive('whereNull')->with($column, 'and', false)->once()->andReturnSelf(); + $relation->wherePivotNull($column); + + $builder->shouldReceive('orderBy')->with($column, 'asc')->once()->andReturnSelf(); + $relation->orderByPivot($column); + } + + public function getRelation(): MorphToMany { [$builder, $parent] = $this->getRelationArguments(); return new MorphToMany($builder, $parent, 'taggable', 'taggables', 'taggable_id', 'tag_id', 'id', 'id'); } - public function getRelationArguments() + public function getRelationArguments(): array { $parent = m::mock(Model::class); $parent->shouldReceive('getMorphClass')->andReturn(get_class($parent)); @@ -105,6 +127,11 @@ public function getRelationArguments() $builder->shouldReceive('where')->once()->with('taggables.taggable_id', '=', 1); $builder->shouldReceive('where')->once()->with('taggables.taggable_type', get_class($parent)); + $grammar = m::mock(Grammar::class); + $grammar->shouldReceive('isExpression')->with(m::type(Expression::class))->andReturnTrue(); + $grammar->shouldReceive('isExpression')->with(m::type('string'))->andReturnFalse(); + $builder->shouldReceive('getGrammar')->andReturn($grammar); + return [$builder, $parent, 'taggable', 'taggables', 'taggable_id', 'tag_id', 'id', 'id', 'relation_name', false]; } }