Skip to content

Commit

Permalink
Improvements to BelongsToManyThrough
Browse files Browse the repository at this point in the history 
Add more strict typing and improved tests
  • Loading branch information
@lcharette
lcharette committed Dec 28, 2023
1 parent a429064 commit 69138f9
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 57 deletions.
91 changes: 52 additions & 39 deletions app/src/Database/Relations/BelongsToManyThrough.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@
/**
* A BelongsToMany relationship that queries through an additional intermediate model.
*
* @author Alex Weissman (https://alexanderweissman.com)
*
* @see https://github.com/laravel/framework/blob/5.8/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php
* @see https://github.com/illuminate/database/blob/10.x/Eloquent/Relations/BelongsToMany.php
*/
class BelongsToManyThrough extends BelongsToMany
{
Expand All @@ -33,25 +31,34 @@ class BelongsToManyThrough extends BelongsToMany
/**
* The relation through which we are joining.
*
* @var Relation
* @var BelongsToMany
*/
protected $intermediateRelation;
protected BelongsToMany $intermediateRelation;

/**
* Create a new belongs to many relationship instance.
*
* @param \Illuminate\Database\Eloquent\Builder $query
* @param \Illuminate\Database\Eloquent\Model $parent
* @param \Illuminate\Database\Eloquent\Relations\Relation $intermediateRelation
* @param string $table
* @param string $foreignPivotKey
* @param string $relatedPivotKey
* @param string $parentKey
* @param string $relatedKey
* @param string $relationName
* @param Builder $query
* @param Model $parent
* @param BelongsToMany $intermediateRelation
* @param string|class-string<\Illuminate\Database\Eloquent\Model> $table
* @param string $foreignPivotKey
* @param string $relatedPivotKey
* @param string $parentKey
* @param string $relatedKey
* @param string|null $relationName
*/
public function __construct(Builder $query, Model $parent, Relation $intermediateRelation, $table, $foreignPivotKey, $relatedPivotKey, $parentKey, $relatedKey, $relationName = null)
{
public function __construct(
Builder $query,
Model $parent,
BelongsToMany $intermediateRelation,
string $table,
string $foreignPivotKey,
string $relatedPivotKey,
string $parentKey,
string $relatedKey,
?string $relationName = null
) {
$this->intermediateRelation = $intermediateRelation;

parent::__construct($query, $parent, $table, $foreignPivotKey, $relatedPivotKey, $parentKey, $relatedKey, $relationName);
Expand All @@ -65,7 +72,7 @@ public function __construct(Builder $query, Model $parent, Relation $intermediat
*
* @return string
*/
public function getParentKeyName()
public function getParentKeyName(): string
{
return $this->intermediateRelation->newExistingPivot()->getForeignKey();
}
Expand All @@ -77,20 +84,20 @@ public function getParentKeyName()
*
* @return string
*/
public function getExistenceCompareKey()
public function getExistenceCompareKey(): string
{
return $this->intermediateRelation->getQualifiedForeignPivotKeyName();
}

/**
* Add a "via" query to load the intermediate models through which the child models are related.
*
* @param string $viaRelationName
* @param callable $viaCallback
* @param string|null $viaRelationName
* @param callable $viaCallback
*
* @return self
*/
public function withVia($viaRelationName = null, $viaCallback = null)
public function withVia(?string $viaRelationName = null, callable $viaCallback = null): self
{
$this->tertiaryRelated = $this->intermediateRelation->getRelated();

Expand All @@ -111,22 +118,24 @@ public function withVia($viaRelationName = null, $viaCallback = null)
/**
* Set the constraints for an eager load of the relation.
*
* @param array $models
* @param Model[] $models
*/
// @phpstan-ignore-next-line - Issue is on Laravel's side
public function addEagerConstraints(array $models)
{
// Constraint to only load models where the intermediate relation's foreign key matches the parent model
$intermediateForeignKeyName = $this->intermediateRelation->getQualifiedForeignPivotKeyName();

return $this->query->whereIn($intermediateForeignKeyName, $this->getKeys($models));
// @phpstan-ignore-next-line - Laravel limitation
$this->query->whereIn($intermediateForeignKeyName, $this->getKeys($models));
}

/**
* Set the where clause for the relation query.
*
* @return self
* @return $this
*/
protected function addWhereConstraints()
protected function addWhereConstraints(): self
{
$parentKeyName = $this->getParentKeyName();

Expand All @@ -142,13 +151,14 @@ protected function addWhereConstraints()
/**
* Match the eagerly loaded results to their parents.
*
* @param array $models
* @param \Illuminate\Database\Eloquent\Collection $results
* @param string $relation
* @param Model[] $models
* @param Collection<int, Model> $results
* @param string $relation
*
* @return array
* @return Model[]
*/
public function match(array $models, Collection $results, $relation)
// @phpstan-ignore-next-line - Issue is on Laravel's side
public function match(array $models, Collection $results, $relation): array
{
// Build dictionary of parent (e.g. user) to related (e.g. permission) models
list($dictionary, $nestedViaDictionary) = $this->buildDictionary($results, $this->getParentKeyName());
Expand All @@ -158,7 +168,7 @@ public function match(array $models, Collection $results, $relation)
// the parent models. Then we will return the hydrated models back out.
foreach ($models as $model) {
if (isset($dictionary[$key = $model->getKey()])) {
/** @var array */
/** @var Model[] */
$items = $dictionary[$key];

// Eliminate any duplicates
Expand All @@ -171,6 +181,7 @@ public function match(array $models, Collection $results, $relation)

// Remove the tertiary pivot key from the condensed models
foreach ($items as $relatedModel) {
// @phpstan-ignore-next-line
unset($relatedModel->pivot->{$this->foreignPivotKey});
}

Expand All @@ -187,25 +198,26 @@ public function match(array $models, Collection $results, $relation)
/**
* Unset tertiary pivots on a collection or array of models.
*
* @param \Illuminate\Database\Eloquent\Collection $models
* @param Collection<int, Model> $models
*/
protected function unsetTertiaryPivots(Collection $models)
protected function unsetTertiaryPivots(Collection $models): void
{
foreach ($models as $model) {
// @phpstan-ignore-next-line
unset($model->pivot->{$this->foreignPivotKey});
}
}

/**
* Set the join clause for the relation query.
*
* @param \Illuminate\Database\Eloquent\Builder|null $query
* @param Builder|null $query
*
* @return self
* @return $this
*/
protected function performJoin($query = null)
protected function performJoin($query = null): self
{
$query = $query ?: $this->query;
$query = $query ?? $this->query;

parent::performJoin($query);

Expand All @@ -216,6 +228,7 @@ protected function performJoin($query = null)

$key = $this->intermediateRelation->getQualifiedRelatedPivotKeyName();

// @phpstan-ignore-next-line - Laravel magic functions
$query->join($intermediateTable, $key, '=', $this->getQualifiedForeignPivotKeyName());

return $this;
Expand All @@ -226,9 +239,9 @@ protected function performJoin($query = null)
*
* "pivot_" is prefixed to each column for easy removal later.
*
* @return array
* @return string[]
*/
protected function aliasedPivotColumns()
protected function aliasedPivotColumns(): array
{
$defaults = [$this->foreignPivotKey, $this->relatedPivotKey];
$aliasedPivotColumns = collect(array_merge($defaults, $this->pivotColumns))->map(function ($column) {
Expand Down
34 changes: 33 additions & 1 deletion ...ts/Integration/Database/DatabaseTests.php → ...sts/Integration/Database/DatabaseTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
/**
* Test custom relations in `/src/Database/Relations`.
*/
class DatabaseTests extends TestCase
class DatabaseTest extends TestCase
{
/**
* @var Builder
Expand Down Expand Up @@ -794,6 +794,38 @@ public function testBelongsToManyThroughWithVia(): void
$this->assertBelongsToManyThroughForAlex($usersWithPermissions[1]['permissions']);
}

/**
* Test withVia and custom callback
* @depends testTableCreation
* @depends testBelongsToManyThrough
* @depends testBelongsToManyThroughWithVia
*/
public function testBelongsToManyThroughWithViaCallback(): void
{
$this->generateRolesWithPermissions();

$user = EloquentTestUser::create(['name' => 'David']);

$user->roles()->attach([1, 2]);

// Test retrieval of via models as well
$permissions = $user->permissions()->withVia('roles_via', function (&$roleQuery) {
$roleQuery->with('permissions');
})->get()->toArray();
$this->assertEquals('uri_harvest', $permissions[0]['roles_via'][0]['permissions'][0]['slug']);

$user2 = EloquentTestUser::create(['name' => 'Alex']);
$user2->roles()->attach([2, 3]);

// Test eager loading
$users = EloquentTestUser::with(['permissions' => function ($query) {
return $query->withVia('roles_via', function (&$roleQuery) {
$roleQuery->with('permissions');
});
}])->get()->toArray();
$this->assertEquals('uri_harvest', $users[0]['permissions'][0]['roles_via'][0]['permissions'][0]['slug']);
}

/**
* @depends testTableCreation
* testQueryExclude
Expand Down
29 changes: 12 additions & 17 deletions app/tests/Unit/Database/Relations/BelongsToManyThroughTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
use Illuminate\Database\Eloquent\Relations\BelongsToMany;
use Mockery;
use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
use Mockery as m;
use PHPUnit\Framework\TestCase;
use UserFrosting\Sprinkle\Core\Database\Builder as QueryBuilder;
use UserFrosting\Sprinkle\Core\Database\Models\Model;
Expand All @@ -28,22 +28,17 @@ class BelongsToManyThroughTest extends TestCase
{
use MockeryPHPUnitIntegration;

public function tearDown(): void
{
parent::tearDown();
m::close();
}

public function testPaginatedQuery()
public function testPaginatedQuery(): void
{
// Creates a real BelongsToManyThrough object
$relation = $this->getRelation();

// We need to define a mock base query, because Eloquent\Builder will pass through many calls
// to this underlying Query\Builder object.
$baseQuery = m::mock(QueryBuilder::class);
$builder = m::mock(EloquentBuilder::class, [$baseQuery])->makePartial();
$baseQuery = Mockery::mock(QueryBuilder::class);
$builder = Mockery::mock(EloquentBuilder::class, [$baseQuery])->makePartial();

/** @var \Mockery\MockInterface */
$related = $relation->getRelated();
$related->shouldReceive('getQualifiedKeyName')->once()->andReturn('users.id');

Expand All @@ -53,25 +48,25 @@ public function testPaginatedQuery()
$builder->shouldReceive('offset')->once()->with(1)->andReturnSelf();

// Mock the collection generated by the constrained query
$collection = m::mock('Illuminate\Database\Eloquent\Collection');
$collection = Mockery::mock('Illuminate\Database\Eloquent\Collection');
$collection->shouldReceive('pluck')->once()->with('id')->andReturn($collection);
$collection->shouldReceive('toArray')->once()->andReturn([1, 2]);
$builder->shouldReceive('get')->once()->andReturn($collection);

// Test the final modification to the original unpaginated query
$builder->shouldReceive('whereIn')->once()->with('users.id', [1, 2])->andReturnSelf();

$paginatedQuery = $relation->getPaginatedQuery($builder, 2, 1);
$relation->getPaginatedQuery($builder, 2, 1);
}

/**
* Set up and simulate base expectations for arguments to relationship.
*/
protected function getRelation()
protected function getRelation(): BelongsToManyThrough
{
// We simulate a BelongsToManyThrough relationship that gets all related users for a specified permission(s).
$builder = m::mock(EloquentBuilder::class);
$related = m::mock('Illuminate\Database\Eloquent\Model')->makePartial();
$builder = Mockery::mock(EloquentBuilder::class);
$related = Mockery::mock('Illuminate\Database\Eloquent\Model')->makePartial();
$related->shouldReceive('getKey')->andReturn(1);
$related->shouldReceive('getTable')->andReturn('users');
$related->shouldReceive('getKeyName')->andReturn('id');
Expand All @@ -80,11 +75,11 @@ protected function getRelation()
$builder->shouldReceive('getModel')->andReturn($related);

// Mock the intermediate role->permission BelongsToMany relation
$intermediateRelationship = m::mock(BelongsToMany::class);
$intermediateRelationship = Mockery::mock(BelongsToMany::class);
$intermediateRelationship->shouldReceive('getTable')->once()->andReturn('permission_roles');
$intermediateRelationship->shouldReceive('getQualifiedRelatedPivotKeyName')->once()->andReturn('permission_roles.role_id');
// Crazy pivot query stuff
$newPivot = m::mock('\Illuminate\Database\Eloquent\Relations\Pivot');
$newPivot = Mockery::mock('\Illuminate\Database\Eloquent\Relations\Pivot');
$newPivot->shouldReceive('getForeignKey')->andReturn('permission_id');
$intermediateRelationship->shouldReceive('newExistingPivot')->andReturn($newPivot);

Expand Down

0 comments on commit 69138f9

Please sign in to comment.