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

[firestore] Set the cursor to empty when the end is reached #7448

Merged
merged 5 commits into from
Jul 9, 2021

Conversation

xacrimon
Copy link
Contributor

@xacrimon xacrimon commented Jun 30, 2021

The pagination contract now requires that the cursor is set to empty once the end of a query is reached. Previously we didn't run all tests in CI and it worked somewhat wierd so I had to resort to running tests manually and switching them around which was not very reliable nor good. Firestore CI recently got fixed and this enables the rest of the tests to now run correctly. This PR makes sure the cursor is set to empty correctly once the end of the events have been reached. New UI changes rely on this behaviour.

@xacrimon xacrimon self-assigned this Jun 30, 2021
Comment on lines +506 to +507
// This solution here seems to be the the most common but I haven't been able to find
// any documented hard guarantees on firestore not early returning for some reason like response
Copy link
Contributor

Choose a reason for hiding this comment

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

Took a peek at the source code of firestore.DocumentItarator. Looks like GetAll() is just a convenience wrapper around Next(), which collects all results until iterator.Done reached. Since the docs guarantee that Next() only returns iterator.Done if there are actually no more documents matching the query, I think we can treat this as effectively guaranteed.

Copy link
Contributor

@russjones russjones left a comment

Choose a reason for hiding this comment

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

Bot.

@xacrimon xacrimon merged commit 2462c09 into master Jul 9, 2021
@xacrimon xacrimon deleted the joel/firestore-cursor-end branch July 9, 2021 13:47
xacrimon added a commit that referenced this pull request Jul 9, 2021
* nil firestore cursor on end

* added explainer
xacrimon added a commit that referenced this pull request Jul 9, 2021
* nil firestore cursor on end

* added explainer
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.

4 participants