PHPORM-155 Fluent aggregation builder #2738

GromNaN · 2024-02-26T17:08:28Z

With this integration, it's possible to derivate any Query builder into an aggregation pipeline. The conditions built with the query builder are used as first stages (match, project, limit ...)

Requires mongodb/mongo-php-builder#67

Checklist

Add tests and ensure they pass
Add an entry to the CHANGELOG.md file
Update documentation for new features

GromNaN · 2024-02-26T17:15:30Z

src/Eloquent/Builder.php

+    {
+        $result = $this->toBase()->aggregate($function, $columns);
+
+        return $result ?? $this;


This method must be overrided, because by default this is a fluent interface.

Does @return need to be @psalm-return?

We use phpstan on this project, and it get promoted for Laravel projects.

I have updated the annotation to @return ($function is null ? AggregationBuilder : self), which is the documented way.

GromNaN · 2024-03-08T08:43:03Z

$near, and $nearSphere query operators are not supported in $match stage. They must be replaced by a $geoNear. This is something we can't do automatically, especially when nested in a $and or $or query.
$slice projection operator has a different signature than the $slice expression (array field as 1st argument). This needs to be translated in the context of a $project stage.
$where is not available within aggregation pipeline. They can be replaced by the $function expression.

List of limitations: https://www.mongodb.com/docs/manual/reference/operator/aggregation/match/#restrictions

Since this operators are currently supported by the query builder, we need to keep building a find command with the query builder, and only create an aggregation pipeline when necessary.

composer.json

GromNaN · 2024-03-11T16:45:14Z

src/Eloquent/Model.php

@@ -49,6 +49,7 @@
 use function uniqid;
 use function var_export;

+/** @mixin Builder */


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

src/Query/AggregationBuilder.php

jmikola · 2024-03-13T21:16:29Z

src/Eloquent/Builder.php

+    {
+        $result = $this->toBase()->aggregate($function, $columns);
+
+        return $result ?? $this;


Does @return need to be @psalm-return?

src/Query/Builder.php

src/Eloquent/Builder.php

src/Query/Builder.php

GromNaN · 2024-03-19T17:47:14Z

After review with @jmikola we decided to remove the feature of initializing the aggregation pipeline from the query builder with $match, $limit, $sort and $project stages. Library users are invited to use the Aggregation Builder from the start.

The AggregationBuilder class is enhancer with execution methods similar to Query\Builder and Eloquent\Builder:

AggregationBuilder::get($options) returns the results in a Laravel\Support\Collection
AggregationBuilder::cursor($options) returns the results in a Laravel\Support\LazyCollection which is a lazy-loading feature.
AggregationBuilder::first($options) returns the first result, after appending the stage $limit: 1 to the pipeline.

GromNaN · 2024-03-21T13:50:24Z

src/Query/AggregationBuilder.php

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


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

alcaeus

LGTM in general, just a couple of minor suggestions.

composer.json

src/Query/Builder.php

jmikola · 2024-03-22T18:56:16Z

src/Query/Builder.php

+                throw new BadMethodCallException('Aggregation builder requires package mongodb/builder 0.2+');
+            }
+
+            if ($columns !== [] && $columns !== ['*']) {


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.

jmikola · 2024-03-22T19:02:09Z

src/Query/AggregationBuilder.php

+    use FluentFactoryTrait;
+
+    public function __construct(
+        private MongoDBCollection|LaravelMongoDBCollection $collection,


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.

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.

src/Query/AggregationBuilder.php

jmikola · 2024-03-22T19:09:49Z

src/Query/AggregationBuilder.php

+    {
+        return (clone $this)
+            ->limit(1)
+            ->cursor($options)


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

jmikola · 2024-03-22T19:17:42Z

src/Query/AggregationBuilder.php

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


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.

tests/Query/AggregationBuilderTest.php

jmikola

Please follow up on both open questions, but feel free to merge after doing so. Code changes LGTM.

tests/Query/AggregationBuilderTest.php

GromNaN added the aggregation label Feb 26, 2024

GromNaN self-assigned this Feb 26, 2024

GromNaN force-pushed the PHPORM-155 branch 2 times, most recently from c1450e2 to a2859a6 Compare February 26, 2024 17:14

GromNaN commented Feb 26, 2024

View reviewed changes

GromNaN changed the title ~~PHPORM-155~~ PHPORM-155 Fluent aggregation builder Feb 27, 2024

GromNaN force-pushed the PHPORM-155 branch from ae70ec5 to 7835e8a Compare February 27, 2024 13:12

GromNaN force-pushed the PHPORM-155 branch 4 times, most recently from 438ccd7 to 0e82e24 Compare March 11, 2024 14:33

GromNaN marked this pull request as ready for review March 11, 2024 16:43

GromNaN requested a review from a team as a code owner March 11, 2024 16:43

GromNaN requested a review from jmikola March 11, 2024 16:43

GromNaN commented Mar 11, 2024

View reviewed changes

composer.json Outdated Show resolved Hide resolved

GromNaN commented Mar 11, 2024

View reviewed changes

src/Query/AggregationBuilder.php Show resolved Hide resolved

GromNaN force-pushed the PHPORM-155 branch 2 times, most recently from c46344f to 3328e93 Compare March 14, 2024 10:00

GromNaN changed the base branch from 4.2 to 4.3 March 14, 2024 10:38

jmikola reviewed Mar 14, 2024

View reviewed changes

GromNaN force-pushed the PHPORM-155 branch from 63a2e40 to 058d298 Compare March 15, 2024 14:28

jmikola reviewed Mar 15, 2024

View reviewed changes

src/Eloquent/Builder.php Show resolved Hide resolved

GromNaN force-pushed the PHPORM-155 branch from 058d298 to e3dfa89 Compare March 18, 2024 13:54

jmikola reviewed Mar 18, 2024

View reviewed changes

src/Query/Builder.php Outdated Show resolved Hide resolved

src/Query/Builder.php Outdated Show resolved Hide resolved

src/Query/Builder.php Outdated Show resolved Hide resolved

src/Query/Builder.php Outdated Show resolved Hide resolved

GromNaN added 4 commits March 19, 2024 18:41

PHPORM-103 Use Aggregation Builder for the query builder

8a982fd

Update all Query\Builder tests to aggregation

b1696f4

Factorize tests

e247fe6

PHPORM-155 Integrate aggregation builder to the Query builder

f9a44f0

GromNaN and others added 12 commits March 19, 2024 18:41

Update aggregation example for mongodb 4.4 support

55f05f4

Improve return phpdoc for static analysis and completion

ec251c7

Switch to trait

cbd6284

Add error message if mongodb/builder is not installed

363de83

Update whereAll/Any tests

f6b207c

Add test on adding raw stage

4f0894f

apply phpcbf formatting

a30d529

phpstan fixes

28ab37f

Update changelog

defb21f

Validate $columns parameter is not set when creating an agg builder

8ebd6bb

Add tests on distinct

8a0937e

Remove pipeline creation from query builder and add execution methods

a3ec4a4

GromNaN force-pushed the PHPORM-155 branch from 491dbc2 to a3ec4a4 Compare March 19, 2024 17:41

GromNaN commented Mar 21, 2024

View reviewed changes

alcaeus reviewed Mar 21, 2024

View reviewed changes

composer.json Show resolved Hide resolved

src/Query/Builder.php Show resolved Hide resolved

jmikola reviewed Mar 22, 2024

View reviewed changes

GromNaN mentioned this pull request Mar 22, 2024

Fix accepted stage type in FluentFactoryTrait mongodb/mongo-php-builder#71

Merged

GromNaN added 2 commits March 22, 2024 21:19

Add tests on aggregate method errors and optimize first method

554a1d7

Suggests mongodb/builder package

d9a3273

GromNaN requested a review from jmikola March 22, 2024 20:22

GromNaN closed this in mongodb/mongo-php-builder#71 Mar 25, 2024

GromNaN reopened this Mar 26, 2024

jmikola approved these changes Mar 26, 2024

View reviewed changes

tests/Query/AggregationBuilderTest.php Show resolved Hide resolved

GromNaN merged commit fdfb5e5 into mongodb:4.3 Mar 26, 2024
46 checks passed

GromNaN deleted the PHPORM-155 branch March 26, 2024 16:58

GromNaN mentioned this pull request Apr 4, 2024

whereColumn feature missing #1651

Open

GromNaN added this to the 4.3 milestone Apr 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PHPORM-155 Fluent aggregation builder #2738

PHPORM-155 Fluent aggregation builder #2738

GromNaN commented Feb 26, 2024 •

edited

Loading

GromNaN Feb 26, 2024

jmikola Mar 13, 2024

GromNaN Mar 18, 2024

GromNaN commented Mar 8, 2024

GromNaN Mar 11, 2024 •

edited

Loading

jmikola Mar 13, 2024

GromNaN commented Mar 19, 2024

GromNaN Mar 21, 2024

alcaeus left a comment

jmikola Mar 22, 2024

jmikola Mar 22, 2024 •

edited

Loading

GromNaN Mar 26, 2024

jmikola Mar 22, 2024

jmikola Mar 22, 2024

jmikola left a comment

PHPORM-155 Fluent aggregation builder #2738

PHPORM-155 Fluent aggregation builder #2738

Conversation

GromNaN commented Feb 26, 2024 • edited Loading

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GromNaN commented Mar 8, 2024

GromNaN Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GromNaN commented Mar 19, 2024

Choose a reason for hiding this comment

alcaeus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmikola Mar 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmikola left a comment

Choose a reason for hiding this comment

GromNaN commented Feb 26, 2024 •

edited

Loading

GromNaN Mar 11, 2024 •

edited

Loading

jmikola Mar 22, 2024 •

edited

Loading