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

feat(mongodb): Add pagination metadata to the aggregation results #6912

Merged
merged 4 commits into from
Jan 29, 2025

Conversation

GromNaN
Copy link
Contributor

@GromNaN GromNaN commented Jan 10, 2025

Q A
Branch? 4.1
Tickets Closes #6878
License MIT
Doc PR -

Simplifies code by removing the need to extract firstResult and maxResults from the pipeline, by adding a $setFields stage to add the litteral informations.

This also remove references to the MongoDB Cursor and the UnitOfWork stored in the Paginator class. I hope this helps preventing memory leak issues.

Since I'm modifying the aggregation pipeline, the parameters of the Paginator constructor and deleting 2 constants from the Paginator class, it's best not to merge this as a bugfix.

Comment on lines -30 to -31
public const LIMIT_ZERO_MARKER_FIELD = '___';
public const LIMIT_ZERO_MARKER = 'limit0';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 GromNaN changed the title MongoDB: Add pagination metadata to the aggregation results feat(mongodb): Add pagination metadata to the aggregation results Jan 11, 2025
@GromNaN GromNaN force-pushed the odm-paginator branch 2 times, most recently from aeb7cc3 to eaa37fe Compare January 11, 2025 11:20
@soyuka
Copy link
Member

soyuka commented Jan 17, 2025

It looks like the patch is responsible for the failing CI

@@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Required for mocking AddFields. See release 2.9.2

@GromNaN GromNaN force-pushed the odm-paginator branch 3 times, most recently from dd44008 to abfa2e9 Compare January 28, 2025 20:22
@GromNaN
Copy link
Contributor Author

GromNaN commented Jan 28, 2025

Thanks to Behat tests. I had totally misunderstood the meaning of limit=0. I thought it meant “infinite”, whereas it actually means zero (I think it should be a error to put zero results per page, but maybe there's a use case).

So, to solve this problem, I add an empty array directly to the results rather than building a query that returns nothing.

@soyuka soyuka merged commit 4bdf042 into api-platform:4.1 Jan 29, 2025
59 checks passed
@soyuka
Copy link
Member

soyuka commented Jan 29, 2025

Thanks @GromNaN !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants