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
90 changes: 90 additions & 0 deletions src/Eloquent/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Support\Str;
use InvalidArgumentException;
use MongoDB\BSON\Document;
use MongoDB\Builder\Type\QueryInterface;
use MongoDB\Builder\Type\SearchOperatorInterface;
Expand All @@ -15,15 +18,20 @@
use MongoDB\Laravel\Connection;
use MongoDB\Laravel\Helpers\QueriesRelationships;
use MongoDB\Laravel\Query\AggregationBuilder;
use MongoDB\Laravel\Relations\EmbedsOneOrMany;
use MongoDB\Laravel\Relations\HasMany;
use MongoDB\Model\BSONDocument;

use function array_key_exists;
use function array_merge;
use function collect;
use function count;
use function explode;
use function is_array;
use function is_object;
use function iterator_to_array;
use function property_exists;
use function sprintf;

/**
* @method \MongoDB\Laravel\Query\Builder toBase()
Expand All @@ -34,6 +42,9 @@
private const DUPLICATE_KEY_ERROR = 11000;
use QueriesRelationships;

/** @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

/**
* The methods that should be returned from query builder.
*
Expand Down Expand Up @@ -294,6 +305,85 @@
}
}

public function withAggregate($relations, $column, $function = null)
jmikola marked this conversation as resolved.
Show resolved Hide resolved
{
if (empty($relations)) {
return $this;
}

$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.

// For "count" and "exist" we can use the embedded list of ids
// for embedded relations, everything can be computed directly using a projection.
$segments = explode(' ', $name);

$name = $segments[0];
$alias = (count($segments) === 3 && Str::lower($segments[1]) === 'as' ? $segments[2] : Str::snake($name) . '_count');

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

if ($relation instanceof EmbedsOneOrMany) {
switch ($function) {
case 'count':
$this->project([$alias => ['$size' => ['$ifNull' => ['$' . $relation->getQualifiedForeignKeyName(), []]]]]);
Fixed Show fixed Hide fixed
break;
case 'exists':
$this->project([$alias => ['$exists' => '$' . $relation->getQualifiedForeignKeyName()]]);
Fixed Show fixed Hide fixed
break;
default:
throw new InvalidArgumentException(sprintf('Invalid aggregate function "%s"', $function));
}
} else {
$this->withAggregate[$alias] = [
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
'relation' => $relation,
'function' => $function,
'constraints' => $constraints,
'column' => $column,
'alias' => $alias,
];
}

// @todo HasMany ?

// Otherwise, we need to store the aggregate request to run during "eagerLoadRelation"
// after the root results are retrieved.
}

return $this;
}

public function eagerLoadRelations(array $models)
{
if ($this->withAggregate) {
$modelIds = collect($models)->pluck($this->model->getKeyName())->all();

foreach ($this->withAggregate as $withAggregate) {
if ($withAggregate['relation'] instanceof HasMany) {
Copy link
Member

Choose a reason for hiding this comment

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

Given that this only applies some processing for HasMany relations, under what circumstances would different relations get appended to $this->withAggregate? And are they OK being left as-is?

Copy link
Member Author

Choose a reason for hiding this comment

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

Adding an exception in the "else" case. I thing the only other case if "HasOne", but I need to check "BelongsToMany".

$results = $withAggregate['relation']->newQuery()
->where($withAggregate['constraints'])
->whereIn($withAggregate['relation']->getForeignKeyName(), $modelIds)
->groupBy($withAggregate['relation']->getForeignKeyName())
->aggregate($withAggregate['function'], [$withAggregate['column']]);

foreach ($models as $model) {
$value = $withAggregate['function'] === 'count' ? 0 : null;
foreach ($results as $result) {
if ($model->getKey() === $result->{$withAggregate['relation']->getForeignKeyName()}) {
$value = $result->aggregate;
jmikola marked this conversation as resolved.
Show resolved Hide resolved
break;
}
}

$model->setAttribute($withAggregate['alias'], $value);
}
}
}
}

return parent::eagerLoadRelations($models);
}

/**
* Add the "updated at" column to an array of values.
* TODO Remove if https://github.com/laravel/framework/commit/6484744326531829341e1ff886cc9b628b20d73e
Expand Down
12 changes: 10 additions & 2 deletions src/Query/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ public function toMql(): array

$aggregations = blank($this->aggregate['columns']) ? [] : $this->aggregate['columns'];

if ($column === '*' && $function === 'count' && ! $this->groups) {
if (in_array('*', $aggregations) && $function === 'count' && empty($group['_id'])) {
$options = $this->inheritConnectionOptions($this->options);

return ['countDocuments' => [$wheres, $options]];
Expand Down Expand Up @@ -611,7 +611,7 @@ public function aggregate($function = null, $columns = ['*'])

$this->bindings['select'] = [];

$results = $this->get($columns);
$results = $this->get();

// Once we have executed the query, we will reset the aggregate property so
// that more select queries can be executed against the database without
Expand Down Expand Up @@ -650,6 +650,14 @@ public function aggregateByGroup(string $function, array $columns = ['*'])
return $this->aggregate($function, $columns);
}

public function count($columns = '*')
{
// Can be removed when available in Laravel: https://github.com/laravel/framework/pull/53209
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this PR was merged, then reverted, and then superseded by laravel/framework#53679. Should this be updated?

Copy link
Member Author

Choose a reason for hiding this comment

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

Then reverted again: laravel/framework#54196

Copy link
Member

Choose a reason for hiding this comment

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

In either case, the reference to laravel/framework#53209 looks outdated. Please adjust accordingly and then feel free to resolve this thread.

$results = $this->aggregate(__FUNCTION__, Arr::wrap($columns));

return $results instanceof Collection ? $results : (int) $results;
}

/** @inheritdoc */
public function exists()
{
Expand Down
176 changes: 176 additions & 0 deletions tests/Eloquent/EloquentWithCountTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
<?php

namespace MongoDB\Laravel\Tests\Eloquent;

use Illuminate\Support\Facades\DB;
use MongoDB\Laravel\Eloquent\Model;
use MongoDB\Laravel\Tests\TestCase;

use function count;

/** Copied from {@see \Illuminate\Tests\Integration\Database\EloquentWithCountTest\EloquentWithCountTest} */
class EloquentWithCountTest extends TestCase
{
protected function tearDown(): void
{
EloquentWithCountModel1::truncate();
EloquentWithCountModel2::truncate();
EloquentWithCountModel3::truncate();
EloquentWithCountModel4::truncate();

parent::tearDown();
}

public function testItBasic()
{
$one = EloquentWithCountModel1::create(['id' => 123]);
$two = $one->twos()->create(['value' => 456]);
$two->threes()->create();

$results = EloquentWithCountModel1::withCount([
'twos' => function ($query) {
$query->where('value', '>=', 456);
},
]);

$this->assertEquals([
['id' => 123, 'twos_count' => 1],
], $results->get()->toArray());
}

public function testWithMultipleResults()
{
$connection = DB::connection('mongodb');
$ones = [
EloquentWithCountModel1::create(['id' => 1]),
EloquentWithCountModel1::create(['id' => 2]),
EloquentWithCountModel1::create(['id' => 3]),
];

$ones[0]->twos()->create(['value' => 1]);
$ones[0]->twos()->create(['value' => 2]);
$ones[0]->twos()->create(['value' => 3]);
$ones[0]->twos()->create(['value' => 1]);
$ones[2]->twos()->create(['value' => 1]);
$ones[2]->twos()->create(['value' => 2]);

$connection->enableQueryLog();
$results = EloquentWithCountModel1::withCount([
'twos' => function ($query) {
$query->where('value', '>=', 2);
},
]);

$this->assertEquals([
['id' => 1, 'twos_count' => 2],
['id' => 2, 'twos_count' => 0],
['id' => 3, 'twos_count' => 1],
], $results->get()->toArray());

$connection->disableQueryLog();
$this->assertEquals(2, count($connection->getQueryLog()));
}

public function testGlobalScopes()
{
$one = EloquentWithCountModel1::create();
$one->fours()->create();

$result = EloquentWithCountModel1::withCount('fours')->first();
$this->assertEquals(0, $result->fours_count);

$result = EloquentWithCountModel1::withCount('allFours')->first();
$this->assertEquals(1, $result->all_fours_count);
}

public function testSortingScopes()
{
$one = EloquentWithCountModel1::create();
$one->twos()->create();

$query = EloquentWithCountModel1::withCount('twos')->getQuery();

$this->assertNull($query->orders);
$this->assertSame([], $query->getRawBindings()['order']);
}
}

class EloquentWithCountModel1 extends Model

Check failure on line 98 in tests/Eloquent/EloquentWithCountTest.php

View workflow job for this annotation

GitHub Actions / phpcs

Each class must be in a file by itself
{
protected $connection = 'mongodb';
public $table = 'one';
public $timestamps = false;
protected $guarded = [];

public function twos()
{
return $this->hasMany(EloquentWithCountModel2::class, 'one_id');
}

public function fours()
{
return $this->hasMany(EloquentWithCountModel4::class, 'one_id');
}

public function allFours()
{
return $this->fours()->withoutGlobalScopes();
}
}

class EloquentWithCountModel2 extends Model

Check failure on line 121 in tests/Eloquent/EloquentWithCountTest.php

View workflow job for this annotation

GitHub Actions / phpcs

Each class must be in a file by itself
{
protected $connection = 'mongodb';
public $table = 'two';
public $timestamps = false;
protected $guarded = [];
protected $withCount = ['threes'];

protected static function boot()
{
parent::boot();

static::addGlobalScope('app', function ($builder) {
$builder->latest();
});
}

public function threes()
{
return $this->hasMany(EloquentWithCountModel3::class, 'two_id');
}
}

class EloquentWithCountModel3 extends Model

Check failure on line 144 in tests/Eloquent/EloquentWithCountTest.php

View workflow job for this annotation

GitHub Actions / phpcs

Each class must be in a file by itself
{
protected $connection = 'mongodb';
public $table = 'three';
public $timestamps = false;
protected $guarded = [];

protected static function boot()
{
parent::boot();

static::addGlobalScope('app', function ($builder) {
$builder->where('id', '>', 0);
});
}
}

class EloquentWithCountModel4 extends Model

Check failure on line 161 in tests/Eloquent/EloquentWithCountTest.php

View workflow job for this annotation

GitHub Actions / phpcs

Each class must be in a file by itself
{
protected $connection = 'mongodb';
public $table = 'four';
public $timestamps = false;
protected $guarded = [];

protected static function boot()
{
parent::boot();

static::addGlobalScope('app', function ($builder) {
$builder->where('id', '>', 1);
});
}
}
5 changes: 5 additions & 0 deletions tests/HybridRelationsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ public function testHybridWhereHas()

public function testHybridWith()
{
DB::connection('mongodb')->enableQueryLog();
jmikola marked this conversation as resolved.
Show resolved Hide resolved
$user = new SqlUser();
$otherUser = new SqlUser();
$this->assertInstanceOf(SqlUser::class, $user);
Expand Down Expand Up @@ -206,6 +207,10 @@ 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);
// });
jmikola marked this conversation as resolved.
Show resolved Hide resolved

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