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
97 changes: 97 additions & 0 deletions src/Eloquent/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,32 @@
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;
use MongoDB\Driver\CursorInterface;
use MongoDB\Driver\Exception\WriteException;
use MongoDB\Laravel\Connection;
use MongoDB\Laravel\Eloquent\Model as DocumentModel;
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 +43,9 @@ class Builder extends EloquentBuilder
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 +306,91 @@ public function createOrFirst(array $attributes = [], array $values = [])
}
}

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.

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

$name = $segments[0];
$alias = (count($segments) === 3 && Str::lower($segments[1]) === 'as' ? $segments[2] : Str::snake($name) . '_' . $function);
jmikola marked this conversation as resolved.
Show resolved Hide resolved

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

if (! DocumentModel::isDocumentModel($relation->getRelated())) {
throw new InvalidArgumentException('WithAggregate does not support hybrid relations');
}

if ($relation instanceof EmbedsOneOrMany) {
$subQuery = $this->newQuery();
$constraints($subQuery);
if ($subQuery->getQuery()->wheres) {
// @see https://jira.mongodb.org/browse/PHPORM-292
throw new InvalidArgumentException('Constraints are not supported for embedded relations');
}

switch ($function) {
case 'count':
$this->getQuery()->project([$alias => ['$size' => ['$ifNull' => ['$' . $name, []]]]]);
jmikola marked this conversation as resolved.
Show resolved Hide resolved
break;
case 'min':
case 'max':
case 'avg':
$this->getQuery()->project([$alias => ['$' . $function => '$' . $name . '.' . $column]]);
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,
];
}
}

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
Loading
Loading