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-155 Fluent aggregation builder #2738

Merged
merged 24 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8a982fd
PHPORM-103 Use Aggregation Builder for the query builder
GromNaN Oct 30, 2023
b1696f4
Update all Query\Builder tests to aggregation
GromNaN Feb 22, 2024
e247fe6
Factorize tests
GromNaN Feb 22, 2024
f9a44f0
PHPORM-155 Integrate aggregation builder to the Query builder
GromNaN Feb 26, 2024
6ad5a56
Make it possible to run the aggregation pipeline
GromNaN Feb 27, 2024
706365f
Rework built-in aggregations using aggregation builder
GromNaN Feb 27, 2024
5b3d1cc
Use PipelineBuilder
GromNaN Mar 1, 2024
19542dd
Support options
GromNaN Mar 8, 2024
ef935d7
Restore find query
GromNaN Mar 11, 2024
4aba26d
Rename AggregationBuilder class
GromNaN Mar 11, 2024
55f05f4
Update aggregation example for mongodb 4.4 support
GromNaN Mar 11, 2024
ec251c7
Improve return phpdoc for static analysis and completion
GromNaN Mar 11, 2024
cbd6284
Switch to trait
GromNaN Mar 13, 2024
363de83
Add error message if mongodb/builder is not installed
GromNaN Mar 14, 2024
f6b207c
Update whereAll/Any tests
GromNaN Mar 14, 2024
4f0894f
Add test on adding raw stage
GromNaN Mar 15, 2024
a30d529
apply phpcbf formatting
GromNaN Mar 15, 2024
28ab37f
phpstan fixes
GromNaN Mar 18, 2024
defb21f
Update changelog
GromNaN Mar 18, 2024
8ebd6bb
Validate $columns parameter is not set when creating an agg builder
GromNaN Mar 18, 2024
8a0937e
Add tests on distinct
GromNaN Mar 18, 2024
a3ec4a4
Remove pipeline creation from query builder and add execution methods
GromNaN Mar 19, 2024
554a1d7
Add tests on aggregate method errors and optimize first method
GromNaN Mar 22, 2024
d9a3273
Suggests mongodb/builder package
GromNaN Mar 22, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file.

## [unreleased]

* New aggregation pipeline builder by @GromNaN in [#2738](https://github.com/mongodb/laravel-mongodb/pull/2738)

## [4.2.0] - 2024-12-14

Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"mongodb/mongodb": "^1.15"
},
"require-dev": {
"mongodb/builder": "^0.2",
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
"phpunit/phpunit": "^10.3",
"orchestra/testbench": "^8.0|^9.0",
"mockery/mockery": "^1.4.4",
Expand Down
15 changes: 14 additions & 1 deletion src/Eloquent/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use MongoDB\Laravel\Collection;
use MongoDB\Laravel\Helpers\QueriesRelationships;
use MongoDB\Laravel\Internal\FindAndModifyCommandSubscriber;
use MongoDB\Laravel\Query\AggregationBuilder;
use MongoDB\Model\BSONDocument;
use MongoDB\Operation\FindOneAndUpdate;

Expand Down Expand Up @@ -56,6 +57,18 @@ class Builder extends EloquentBuilder
'tomql',
];

/**
* @return ($function is null ? AggregationBuilder : self)
*
* @inheritdoc
*/
public function aggregate($function = null, $columns = ['*'])
{
$result = $this->toBase()->aggregate($function, $columns);
jmikola marked this conversation as resolved.
Show resolved Hide resolved

return $result ?: $this;
}

/** @inheritdoc */
public function update(array $values, array $options = [])
{
Expand Down Expand Up @@ -215,7 +228,7 @@ public function createOrFirst(array $attributes = [], array $values = []): Model
$document = $collection->findOneAndUpdate(
$attributes,
// Before MongoDB 5.0, $setOnInsert requires a non-empty document.
// This is should not be an issue as $values includes the query filter.
// This should not be an issue as $values includes the query filter.
['$setOnInsert' => (object) $values],
[
'upsert' => true,
Expand Down
1 change: 1 addition & 0 deletions src/Eloquent/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
use function uniqid;
use function var_export;

/** @mixin Builder */
Copy link
Member Author

@GromNaN GromNaN Mar 11, 2024

Choose a reason for hiding this comment

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

Allows completions such as Model::where()->aggregate()->addField(

abstract class Model extends BaseModel
{
use HybridRelations;
Expand Down
98 changes: 98 additions & 0 deletions src/Query/AggregationBuilder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<?php

declare(strict_types=1);

namespace MongoDB\Laravel\Query;

use Illuminate\Support\Collection as LaravelCollection;
use Illuminate\Support\LazyCollection;
use InvalidArgumentException;
use Iterator;
use MongoDB\Builder\BuilderEncoder;
use MongoDB\Builder\Stage\FluentFactoryTrait;
use MongoDB\Collection as MongoDBCollection;
use MongoDB\Driver\CursorInterface;
use MongoDB\Laravel\Collection as LaravelMongoDBCollection;

use function array_replace;
use function collect;
use function sprintf;
use function str_starts_with;

class AggregationBuilder
{
use FluentFactoryTrait;

public function __construct(
private MongoDBCollection|LaravelMongoDBCollection $collection,
Copy link
Member

@jmikola jmikola Mar 22, 2024

Choose a reason for hiding this comment

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

Is there any reason to allow MongoDB\Collection here? It's only constructed as such in the test suite, but I presume you could just mock MongoDB\Laravel\Collection instead there.

I do realize MongoDB\Laravel\Collection just forwards method calls to the PHPLIB class, so there's no technical reason that a MongoDB\Collection wouldn't work as well.

OK to leave as-is. I just wanted to bring this up since it makes the API more permissive and I don't expect users would be constructing this on their own anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

I expect MongoDB\Laravel\Collection to be deprecated soon, as it's used only for logging commands. We need to switch to driver monitoring features: PHPORM-56. This union type is forward-compatible.

private readonly array $options = [],
) {
}

/**
* Add a stage without using the builder. Necessary if the stage is built
* outside the builder, or it is not yet supported by the library.
*/
public function addRawStage(string $operator, mixed $value): static
{
if (! str_starts_with($operator, '$')) {
throw new InvalidArgumentException(sprintf('The stage name "%s" is invalid. It must start with a "$" sign.', $operator));
}

$this->pipeline[] = [$operator => $value];
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
jmikola marked this conversation as resolved.
Show resolved Hide resolved

return $this;
}

/**
* Execute the aggregation pipeline and return the results.
*/
public function get(array $options = []): LaravelCollection|LazyCollection
{
$cursor = $this->execute($options);

return collect($cursor->toArray());
}

/**
* Execute the aggregation pipeline and return the results in a lazy collection.
*/
public function cursor($options = []): LazyCollection
{
$cursor = $this->execute($options);

return LazyCollection::make(function () use ($cursor) {
foreach ($cursor as $item) {
yield $item;
}
});
}

/**
* Execute the aggregation pipeline and return the first result.
*/
public function first(array $options = []): mixed
Copy link
Member Author

Choose a reason for hiding this comment

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

I was concerned of potential conflict with a stage name, but $first is an expression operator 😅

{
return (clone $this)
->limit(1)
->cursor($options)
Copy link
Member

Choose a reason for hiding this comment

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

If you're going to apply limit(1) I reckon it'd be more performant to just use get() here.

->first();
}

/**
* Execute the aggregation pipeline and return MongoDB cursor.
*/
private function execute(array $options): CursorInterface&Iterator
Copy link
Member

Choose a reason for hiding this comment

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

Even though this is internal, I suppose you can't use MongoDB\Driver\Cursor here since a CodecCursor might be returned?

Unrelated to this PR, but this made me wonder if we need to keep CursorInterface&Iterator in place in PHPLIB 2.0, or if there were plans to integrate the Iterator API into CursorInterface. I asked @alcaeus about this in PHPLIB-1114.

Nothing for you here. I merely wanted to cross-reference.

{
$encoder = new BuilderEncoder();
$pipeline = $encoder->encode($this->getPipeline());

$options = array_replace(
['typeMap' => ['root' => 'array', 'document' => 'array']],
$this->options,
$options,
);

return $this->collection->aggregate($pipeline, $options);
}
}
27 changes: 23 additions & 4 deletions src/Query/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use MongoDB\BSON\ObjectID;
use MongoDB\BSON\Regex;
use MongoDB\BSON\UTCDateTime;
use MongoDB\Builder\Stage\FluentFactoryTrait;
use MongoDB\Driver\Cursor;
use Override;
use RuntimeException;
Expand Down Expand Up @@ -65,6 +66,7 @@
use function strlen;
use function strtolower;
use function substr;
use function trait_exists;
use function var_export;

class Builder extends BaseBuilder
Expand All @@ -74,7 +76,7 @@ class Builder extends BaseBuilder
/**
* The database collection.
*
* @var \MongoDB\Collection
* @var \MongoDB\Laravel\Collection
jmikola marked this conversation as resolved.
Show resolved Hide resolved
*/
protected $collection;

Expand All @@ -83,7 +85,7 @@ class Builder extends BaseBuilder
*
* @var array
*/
public $projections;
public $projections = [];
jmikola marked this conversation as resolved.
Show resolved Hide resolved

/**
* The maximum amount of seconds to allow the query to run.
Expand Down Expand Up @@ -538,9 +540,26 @@ public function generateCacheKey()
return md5(serialize(array_values($key)));
}

/** @inheritdoc */
public function aggregate($function, $columns = [])
/** @return ($function is null ? AggregationBuilder : mixed) */
public function aggregate($function = null, $columns = ['*'])
{
if ($function === null) {
GromNaN marked this conversation as resolved.
Show resolved Hide resolved
if (! trait_exists(FluentFactoryTrait::class)) {
// This error will be unreachable when the mongodb/builder package will be merged into mongodb/mongodb
throw new BadMethodCallException('Aggregation builder requires package mongodb/builder 0.2+');
}

if ($columns !== [] && $columns !== ['*']) {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason to allow [] for $columns? IIRC, the previous signature changed the default value to an empty array, but you're preserving ['*'] now. Do you expect users to explicitly call aggregate(null, [])? If not, I'd stick to just ['*'] here.

throw new InvalidArgumentException('Columns cannot be specified to create an aggregation builder. Add a $project stage instead.');
}

if ($this->wheres) {
throw new BadMethodCallException('Aggregation builder does not support previous query-builder instructions. Use a $match stage instead.');
}

return new AggregationBuilder($this->collection, $this->options);
}

$this->aggregate = [
'function' => $function,
'columns' => $columns,
Expand Down
132 changes: 132 additions & 0 deletions tests/Query/AggregationBuilderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
<?php

declare(strict_types=1);

namespace MongoDB\Laravel\Tests\Query;

use DateTimeImmutable;
use Illuminate\Support\Collection;
use Illuminate\Support\LazyCollection;
use InvalidArgumentException;
use MongoDB\BSON\Document;
use MongoDB\BSON\ObjectId;
use MongoDB\BSON\UTCDateTime;
use MongoDB\Builder\BuilderEncoder;
use MongoDB\Builder\Expression;
use MongoDB\Builder\Pipeline;
use MongoDB\Builder\Type\Sort;
use MongoDB\Collection as MongoDBCollection;
use MongoDB\Laravel\Query\AggregationBuilder;
use MongoDB\Laravel\Tests\Models\User;
use MongoDB\Laravel\Tests\TestCase;

class AggregationBuilderTest extends TestCase
{
public function tearDown(): void
{
User::truncate();

parent::tearDown();
}

public function testCreateAggregationBuilder(): void
{
User::insert([
['name' => 'John Doe', 'birthday' => new UTCDateTime(new DateTimeImmutable('1989-01-01'))],
['name' => 'Jane Doe', 'birthday' => new UTCDateTime(new DateTimeImmutable('1990-01-01'))],
]);

// Create the aggregation pipeline from the query builder
$pipeline = User::aggregate();

$this->assertInstanceOf(AggregationBuilder::class, $pipeline);

$pipeline
->match(name: 'John Doe')
->limit(10)
->addFields(
// Requires MongoDB 5.0+
year: Expression::year(
Expression::dateFieldPath('birthday'),
),
)
->sort(year: Sort::Desc, name: Sort::Asc)
->unset('birthday');

// Compare with the expected pipeline
$expected = [
['$match' => ['name' => 'John Doe']],
['$limit' => 10],
[
'$addFields' => [
'year' => ['$year' => ['date' => '$birthday']],
],
],
['$sort' => ['year' => -1, 'name' => 1]],
['$unset' => ['birthday']],
];

$this->assertSamePipeline($expected, $pipeline->getPipeline());

// Execute the pipeline and validate the results
$results = $pipeline->get();
$this->assertInstanceOf(Collection::class, $results);
$this->assertCount(1, $results);
$this->assertInstanceOf(ObjectId::class, $results->first()['_id']);
$this->assertSame('John Doe', $results->first()['name']);
$this->assertIsInt($results->first()['year']);
$this->assertArrayNotHasKey('birthday', $results->first());

// Execute the pipeline and validate the results in a lazy collection
$results = $pipeline->cursor();
$this->assertInstanceOf(LazyCollection::class, $results);

// Execute the pipeline and return the first result
$result = $pipeline->first();
$this->assertIsArray($result);
$this->assertInstanceOf(ObjectId::class, $result['_id']);
$this->assertSame('John Doe', $result['name']);
}

public function testAddRawStage(): void
{
$collection = $this->createMock(MongoDBCollection::class);

$pipeline = new AggregationBuilder($collection);
$pipeline
->addRawStage('$match', ['name' => 'John Doe'])
->addRawStage('$limit', 10)
->addRawStage('$replaceRoot', (object) ['newRoot' => '$$ROOT']);
jmikola marked this conversation as resolved.
Show resolved Hide resolved

$expected = [
['$match' => ['name' => 'John Doe']],
['$limit' => 10],
['$replaceRoot' => ['newRoot' => '$$ROOT']],
];

$this->assertSamePipeline($expected, $pipeline->getPipeline());
}

public function testAddRawStageInvalid(): void
{
$collection = $this->createMock(MongoDBCollection::class);

$pipeline = new AggregationBuilder($collection);

$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('The stage name "match" is invalid. It must start with a "$" sign.');
$pipeline->addRawStage('match', ['name' => 'John Doe']);
}

private static function assertSamePipeline(array $expected, Pipeline $pipeline): void
{
$expected = Document::fromPHP(['pipeline' => $expected])->toCanonicalExtendedJSON();

$codec = new BuilderEncoder();
$actual = $codec->encode($pipeline);
// Normalize with BSON round-trip
$actual = Document::fromPHP(['pipeline' => $actual])->toCanonicalExtendedJSON();

self::assertJsonStringEqualsJsonString($expected, $actual);
jmikola marked this conversation as resolved.
Show resolved Hide resolved
}
}
Loading