Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHPORM-238 Add support for withCount and other aggregations #3182

Open
wants to merge 11 commits into
base: 5.x
Choose a base branch
from
34 changes: 32 additions & 2 deletions src/Eloquent/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,19 @@
use MongoDB\Laravel\Relations\EmbedsOneOrMany;
use MongoDB\Laravel\Relations\HasMany;
use MongoDB\Model\BSONDocument;
use RuntimeException;
use TypeError;

use function array_key_exists;
use function array_merge;
use function assert;
use function collect;
use function count;
use function explode;
use function get_debug_type;
use function is_array;
use function is_object;
use function is_string;
use function iterator_to_array;
use function property_exists;
use function sprintf;
Expand All @@ -43,7 +48,11 @@
private const DUPLICATE_KEY_ERROR = 11000;
use QueriesRelationships;

/** @var array{relation: Relation, function: string, constraints: array, column: string, alias: string}[] */
/**
* List of aggregations on the related models after the main query.
*
* @var array{relation: Relation, function: string, constraints: array, column: string, alias: string}[]
*/
private array $withAggregate = [];
jmikola marked this conversation as resolved.
Show resolved Hide resolved

/**
Expand Down Expand Up @@ -306,19 +315,37 @@
}
}

/**
* Add subsequent queries to include an aggregate value for a relationship.
* For embedded relations, a projection is used to calculate the aggregate.
*
* @see \Illuminate\Database\Eloquent\Concerns\QueriesRelationships::withAggregate()
*
* @param mixed $relations Name of the relationship or an array of relationships to closure for constraint
* @param string $column Name of the field to aggregate
* @param string $function Required aggregation function name (count, min, max, avg)
*
* @return $this
*/
public function withAggregate($relations, $column, $function = null)
jmikola marked this conversation as resolved.
Show resolved Hide resolved
{
if (empty($relations)) {
return $this;
}

assert(is_string($function), new TypeError('Argument 3 ($function) passed to withAggregate must be of the type string, ' . get_debug_type($function) . ' given'));

$relations = is_array($relations) ? $relations : [$relations];

foreach ($this->parseWithRelations($relations) as $name => $constraints) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this calls this method on the base class but it's not clear what "parse a list of relations into individuals" means for the structure of the return value, which is only described as an array. In particular, it's not clear how "<field> as <alias>" would be returned.

Would a $name with exactly three space-delimited segments always have that structure?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's adding the constraint closure from the scope and the relationships.

$segments = explode(' ', $name);

$alias = match (true) {
count($segments) === 1 => Str::snake($segments[0]) . '_' . $function,
count($segments) === 3 && Str::lower($segments[1]) => $segments[2],
jmikola marked this conversation as resolved.
Show resolved Hide resolved
default => throw new InvalidArgumentException(sprintf('Invalid relation name format. Expected "relation as alias" or "relation", got "%s"', $name)),
};
$name = $segments[0];
$alias = (count($segments) === 3 && Str::lower($segments[1]) === 'as' ? $segments[2] : Str::snake($name) . '_' . $function);

$relation = $this->getRelationWithoutConstraints($name);

Expand Down Expand Up @@ -347,6 +374,7 @@
throw new InvalidArgumentException(sprintf('Invalid aggregate function "%s"', $function));
}
} else {
// The aggregation will be performed after the main query, during eager loading.
$this->withAggregate[$alias] = [
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
'relation' => $relation,
'function' => $function,
Expand Down Expand Up @@ -384,6 +412,8 @@

$model->setAttribute($withAggregate['alias'], $value);
}
} else {
throw new RuntimeException(sprintf('Unsupported relation type for aggregation', $withAggregate['relation']::class));
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
}
}
}
Expand Down
1 change: 1 addition & 0 deletions tests/Eloquent/EloquentWithAggregateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ public function testWithAggregateMultipleResults()
['id' => 4, 'twos_count' => 0],
], $results->get());

// Only 2 queries should be executed: the main query and the aggregate grouped by foreign id
self::assertSame(2, count($connection->getQueryLog()));
jmikola marked this conversation as resolved.
Show resolved Hide resolved
$connection->flushQueryLog();

Expand Down
5 changes: 0 additions & 5 deletions tests/HybridRelationsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ public function testHybridWhereHas()

public function testHybridWith()
{
DB::connection('mongodb')->enableQueryLog();
$user = new SqlUser();
$otherUser = new SqlUser();
$this->assertInstanceOf(SqlUser::class, $user);
Expand Down Expand Up @@ -207,10 +206,6 @@ public function testHybridWith()
->each(function ($user) {
$this->assertEquals($user->id, $user->books->count());
});
//SqlUser::withCount('books')->get()
// ->each(function ($user) {
// $this->assertEquals($user->id, $user->books_count);
// });

SqlUser::whereHas('sqlBooks', function ($query) {
return $query->where('title', 'LIKE', 'Harry%');
Expand Down
Loading