feat(mongodb): Add pagination metadata to the aggregation results #6912
Conversation
| } | ||
|
|
||
| $resultsAggregationBuilderProphecy = $this->prophesize(Builder::class); | ||
| $resultsAggregationBuilderProphecy->skip($expectedOffset)->shouldBeCalled()->willReturn($skipProphecy->reveal()); | ||
|
|
||
| $addFieldsProphecy = $this->prophesize(AddFields::class); |
There was a problem hiding this comment.
@alcaeus The class Doctrine\ODM\MongoDB\Aggregation\Stage\AddFields is final, it's impossible to mock. I had to duplicate it here.
I hate how this tests are mocking the full aggregation builder. It's very hard to read and to update. It should be refactored to use an actual MongoDB connection.
There was a problem hiding this comment.
I removed the duplicated class thanks to doctrine/mongodb-odm#2717.
| public const LIMIT_ZERO_MARKER_FIELD = '___'; | ||
| public const LIMIT_ZERO_MARKER = 'limit0'; |
There was a problem hiding this comment.
This constants are removed. They should have been marked as internal. We can keep them for backward compatibility, but they are useless now that they are not used in the aggregation pipeline.
GromNaN
changed the title
GromNaN
force-pushed
the
odm-paginator
branch
2 times, most recently
from
aeb7cc3 to
eaa37fe
Compare
|
It looks like the patch is responsible for the failing CI |
GromNaN
force-pushed
the
odm-paginator
branch
from
b19b61c to
4932f30
Compare
GromNaN
force-pushed
the
odm-paginator
branch
from
56c3a0e to
adc939b
Compare
| @@ -124,7 +124,7 @@ | |||
| "doctrine/common": "^3.2.2", | |||
| "doctrine/dbal": "^4.0", | |||
| "doctrine/doctrine-bundle": "^2.11", | |||
| "doctrine/mongodb-odm": "^2.6", | |||
| "doctrine/mongodb-odm": "^2.9.2", | |||
There was a problem hiding this comment.
Required for mocking AddFields. See release 2.9.2
Simplifies code by removing the need to extract
firstResultandmaxResultsfrom the pipeline, by adding a$setFieldsstage to add the litteral informations.This also remove references to the MongoDB
Cursorand theUnitOfWorkstored in thePaginatorclass. I hope this helps preventing memory leak issues.Since I'm modifying the aggregation pipeline, the parameters of the
Paginatorconstructor and deleting 2 constants from thePaginatorclass, it's best not to merge this as a bugfix.