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

Fix docs unnecessarily undergo limbo resolution while resuming query bug #5506

Merged
merged 11 commits into from
Nov 21, 2023
1 change: 1 addition & 0 deletions firebase-firestore/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# Unreleased
* [fixed] Fixed an issue in the local cache synchronization logic where all locally-cached documents that matched a resumed query would be unnecessarily re-downloaded; with the fix it now only downloads the documents that are known to be out-of-sync. [#5506](//github.com/firebase/firebase-android-sdk/pull/5506)


# 24.9.1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,13 @@ private void emitNewSnapsAndNotifyLocalStore(
}
TargetChange targetChange =
remoteEvent == null ? null : remoteEvent.getTargetChanges().get(queryView.getTargetId());
ViewChange viewChange = queryView.getView().applyChanges(viewDocChanges, targetChange);

boolean targetIsPendingReset =
remoteEvent != null
&& remoteEvent.getTargetMismatches().get(queryView.getTargetId()) != null;

ViewChange viewChange =
queryView.getView().applyChanges(viewDocChanges, targetChange, targetIsPendingReset);
updateTrackedLimboDocuments(viewChange.getLimboChanges(), queryView.getTargetId());

if (viewChange.getSnapshot() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,21 @@ public ViewChange applyChanges(DocumentChanges docChanges) {
* @return A new ViewChange with the given docs, changes, and sync state.
*/
public ViewChange applyChanges(DocumentChanges docChanges, TargetChange targetChange) {
return applyChanges(docChanges, targetChange, false);
}

/**
* Updates the view with the given ViewDocumentChanges and updates limbo docs and sync state from
* the given (optional) target change.
*
* @param docChanges The set of changes to make to the view's docs.
* @param targetChange A target change to apply for computing limbo docs and sync state.
* @param targetIsPendingReset - Whether the target is pending to reset due to existence filter
* mismatch. If not explicitly specified, it is treated equivalently to `false`.
* @return A new ViewChange with the given docs, changes, and sync state.
*/
public ViewChange applyChanges(
DocumentChanges docChanges, TargetChange targetChange, boolean targetIsPendingReset) {
hardAssert(!docChanges.needsRefill, "Cannot apply changes that need a refill");

DocumentSet oldDocumentSet = documentSet;
Expand All @@ -293,8 +308,12 @@ public ViewChange applyChanges(DocumentChanges docChanges, TargetChange targetCh
return query.comparator().compare(o1.getDocument(), o2.getDocument());
});
applyTargetChange(targetChange);
List<LimboDocumentChange> limboDocumentChanges = updateLimboDocuments();
boolean synced = limboDocuments.size() == 0 && current;
List<LimboDocumentChange> limboDocumentChanges =
targetIsPendingReset ? Collections.emptyList() : updateLimboDocuments();

// We are at synced state if there is no limbo docs are waiting to be resolved, view is current
// with the backend, and the query is not pending to reset due to existence filter mismatch.
boolean synced = limboDocuments.size() == 0 && current && !targetIsPendingReset;
SyncState newSyncState = synced ? SyncState.SYNCED : SyncState.LOCAL;
boolean syncStatedChanged = newSyncState != syncState;
syncState = newSyncState;
Expand Down