-
Notifications
You must be signed in to change notification settings - Fork 580
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
Fix Firestore failing to return empty results from the local cache #4207
Fix Firestore failing to return empty results from the local cache #4207
Conversation
Coverage Report 1Affected Products
Test Logs |
Size Report 1Affected Products
Test Logs |
…-results-from-the-local-cache
…-results-from-the-local-cache
Please update the description of this PR. Feel free to (mostly) copy the description from firebase/firebase-js-sdk#6624. Also, mention that this is a port of firebase/firebase-js-sdk#6624. This really helps enabling cross linking between PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Just two very minor comments, plus changes to the PR's description.
firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java
Outdated
Show resolved
Hide resolved
firebase-firestore/src/main/java/com/google/firebase/firestore/core/View.java
Outdated
Show resolved
Hide resolved
Oh, and please add a change log entry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one tiny fix to the change log. Otherwise, LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved!
Fix a bug where Firestore fails to return results from the local cache if the result set was empty.
This bug is due to the following logic in
QueryListener.java
:firebase-android-sdk/firebase-firestore/src/main/java/com/google/firebase/firestore/core/QueryListener.java
Line 143 in ae3bdb8
which assumes that if the result set from the local cache is empty that there is no cached result; however, if the result set of the query is indeed empty then it should be returning that empty result set from the cache.
This fix improves the logic to use the presence of a hasCachedResults flag to indicate that the empty result set is cached data and should be raised to the client.
This bug fix is ported from firebase/firebase-js-sdk#6624, which fixed #5873