diff --git a/Firestore/Example/Tests/SpecTests/json/listen_spec_test.json b/Firestore/Example/Tests/SpecTests/json/listen_spec_test.json index f94ee7627e2..f45f725afcb 100644 --- a/Firestore/Example/Tests/SpecTests/json/listen_spec_test.json +++ b/Firestore/Example/Tests/SpecTests/json/listen_spec_test.json @@ -1702,7 +1702,7 @@ } ] ], - "targets": [ + "removedTargets": [ 2 ] } @@ -1848,6 +1848,693 @@ } ] }, + "Individual (deleted) documents cannot revert": { + "describeName": "Listens:", + "itName": "Individual (deleted) documents cannot revert", + "tags": [], + "config": { + "useGarbageCollection": false + }, + "steps": [ + { + "userListen": [ + 2, + { + "path": "collection", + "filters": [ + [ + "visible", + "==", + true + ] + ], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": { + "2": { + "query": { + "path": "collection", + "filters": [ + [ + "visible", + "==", + true + ] + ], + "orderBys": [] + }, + "resumeToken": "" + } + } + } + }, + { + "watchAck": [ + 2 + ] + }, + { + "watchEntity": { + "docs": [ + [ + "collection/a", + 1000, + { + "v": "v1000", + "visible": true + } + ] + ], + "targets": [ + 2 + ] + } + }, + { + "watchCurrent": [ + [ + 2 + ], + "resume-token-1000" + ] + }, + { + "watchSnapshot": { + "version": 1000 + }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [ + [ + "visible", + "==", + true + ] + ], + "orderBys": [] + }, + "added": [ + [ + "collection/a", + 1000, + { + "v": "v1000", + "visible": true + } + ] + ], + "errorCode": 0, + "fromCache": false, + "hasPendingWrites": false + } + ] + }, + { + "userUnlisten": [ + 2, + { + "path": "collection", + "filters": [ + [ + "visible", + "==", + true + ] + ], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": {} + } + }, + { + "watchRemove": { + "targetIds": [ + 2 + ] + } + }, + { + "userListen": [ + 4, + { + "path": "collection", + "filters": [], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": { + "4": { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "resumeToken": "" + } + } + }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "added": [ + [ + "collection/a", + 1000, + { + "v": "v1000", + "visible": true + } + ] + ], + "errorCode": 0, + "fromCache": true, + "hasPendingWrites": false + } + ] + }, + { + "watchAck": [ + 4 + ] + }, + { + "watchEntity": { + "docs": [ + [ + "collection/a", + 3000, + null + ] + ], + "removedTargets": [ + 4 + ] + } + }, + { + "watchCurrent": [ + [ + 4 + ], + "resume-token-4000" + ] + }, + { + "watchSnapshot": { + "version": 4000 + }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "removed": [ + [ + "collection/a", + 1000, + { + "v": "v1000", + "visible": true + } + ] + ], + "errorCode": 0, + "fromCache": false, + "hasPendingWrites": false + } + ] + }, + { + "userUnlisten": [ + 4, + { + "path": "collection", + "filters": [], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": {} + } + }, + { + "watchRemove": { + "targetIds": [ + 4 + ] + } + }, + { + "userListen": [ + 2, + { + "path": "collection", + "filters": [ + [ + "visible", + "==", + true + ] + ], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": { + "2": { + "query": { + "path": "collection", + "filters": [ + [ + "visible", + "==", + true + ] + ], + "orderBys": [] + }, + "resumeToken": "resume-token-1000" + } + } + } + }, + { + "watchAck": [ + 2 + ] + }, + { + "watchEntity": { + "docs": [ + [ + "collection/a", + 2000, + { + "v": "v2000", + "visible": false + } + ] + ], + "removedTargets": [ + 2 + ] + } + }, + { + "watchCurrent": [ + [ + 2 + ], + "resume-token-5000" + ] + }, + { + "watchSnapshot": { + "version": 5000 + }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [ + [ + "visible", + "==", + true + ] + ], + "orderBys": [] + }, + "errorCode": 0, + "fromCache": false, + "hasPendingWrites": false + } + ] + }, + { + "userUnlisten": [ + 2, + { + "path": "collection", + "filters": [ + [ + "visible", + "==", + true + ] + ], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": {} + } + }, + { + "watchRemove": { + "targetIds": [ + 2 + ] + } + }, + { + "userListen": [ + 4, + { + "path": "collection", + "filters": [], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": { + "4": { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "resumeToken": "resume-token-4000" + } + } + } + }, + { + "watchAck": [ + 4 + ] + }, + { + "watchEntity": { + "docs": [], + "targets": [ + 4 + ] + } + }, + { + "watchCurrent": [ + [ + 4 + ], + "resume-token-6000" + ] + }, + { + "watchSnapshot": { + "version": 6000 + }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "errorCode": 0, + "fromCache": false, + "hasPendingWrites": false + } + ] + } + ] + }, + "Deleted documents in cache are fixed": { + "describeName": "Listens:", + "itName": "Deleted documents in cache are fixed", + "tags": [], + "config": { + "useGarbageCollection": false + }, + "steps": [ + { + "userListen": [ + 2, + { + "path": "collection", + "filters": [ + [ + "key", + "==", + "a" + ] + ], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": { + "2": { + "query": { + "path": "collection", + "filters": [ + [ + "key", + "==", + "a" + ] + ], + "orderBys": [] + }, + "resumeToken": "" + } + } + } + }, + { + "watchAck": [ + 2 + ] + }, + { + "watchEntity": { + "docs": [ + [ + "collection/a", + 1000, + { + "key": "a" + } + ] + ], + "targets": [ + 2 + ] + } + }, + { + "watchCurrent": [ + [ + 2 + ], + "resume-token-1000" + ] + }, + { + "watchSnapshot": { + "version": 1000 + }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [ + [ + "key", + "==", + "a" + ] + ], + "orderBys": [] + }, + "added": [ + [ + "collection/a", + 1000, + { + "key": "a" + } + ] + ], + "errorCode": 0, + "fromCache": false, + "hasPendingWrites": false + } + ] + }, + { + "watchEntity": { + "docs": [ + [ + "collection/a", + 2000, + null + ] + ], + "removedTargets": [ + 2 + ] + } + }, + { + "watchSnapshot": { + "version": 2000, + "targetIds": [ + 2 + ], + "resumeToken": "resume-token-2000" + } + }, + { + "watchSnapshot": { + "version": 2000 + }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [ + [ + "key", + "==", + "a" + ] + ], + "orderBys": [] + }, + "removed": [ + [ + "collection/a", + 1000, + { + "key": "a" + } + ] + ], + "errorCode": 0, + "fromCache": false, + "hasPendingWrites": false + } + ] + }, + { + "userUnlisten": [ + 2, + { + "path": "collection", + "filters": [ + [ + "key", + "==", + "a" + ] + ], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": {} + } + }, + { + "watchRemove": { + "targetIds": [ + 2 + ] + } + }, + { + "userListen": [ + 4, + { + "path": "collection", + "filters": [], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": { + "4": { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "resumeToken": "" + } + } + } + }, + { + "watchAck": [ + 4 + ] + }, + { + "watchEntity": { + "docs": [ + [ + "collection/a", + 1000, + { + "key": "a" + } + ] + ], + "targets": [ + 4 + ] + } + }, + { + "watchCurrent": [ + [ + 4 + ], + "resume-token-3000" + ] + }, + { + "watchSnapshot": { + "version": 3000 + }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "added": [ + [ + "collection/a", + 1000, + { + "key": "a" + } + ] + ], + "errorCode": 0, + "fromCache": false, + "hasPendingWrites": false + } + ] + } + ] + }, "Listens are reestablished after network disconnect": { "describeName": "Listens:", "itName": "Listens are reestablished after network disconnect", diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm index 6aab78e8b6e..aed0e552ef5 100644 --- a/Firestore/Source/Local/FSTLocalStore.mm +++ b/Firestore/Source/Local/FSTLocalStore.mm @@ -268,6 +268,7 @@ - (FSTMaybeDocumentDictionary *)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent { FSTListenSequenceNumber sequenceNumber = [self.listenSequence next]; id queryCache = self.queryCache; + DocumentKeySet authoritativeUpdates; for (const auto &entry : remoteEvent.targetChanges) { FSTTargetID targetID = entry.first; FSTBoxedTargetID *boxedTargetID = @(targetID); @@ -279,6 +280,21 @@ - (FSTMaybeDocumentDictionary *)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent { continue; } + // When a global snapshot contains updates (either add or modify) we can completely trust + // these updates as authoritative and blindly apply them to our cache (as a defensive measure + // to promote self-healing in the unfortunate case that our cache is ever somehow corrupted / + // out-of-sync). + // + // If the document is only updated while removing it from a target then watch isn't obligated + // to send the absolute latest version: it can send the first version that caused the document + // not to match. + for (const DocumentKey &key : change.addedDocuments) { + authoritativeUpdates = authoritativeUpdates.insert(key); + } + for (const DocumentKey &key : change.modifiedDocuments) { + authoritativeUpdates = authoritativeUpdates.insert(key); + } + [queryCache removeMatchingKeys:change.removedDocuments forTargetID:targetID]; [queryCache addMatchingKeys:change.addedDocuments forTargetID:targetID]; @@ -303,11 +319,12 @@ - (FSTMaybeDocumentDictionary *)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent { FSTMaybeDocument *doc = kv.second; changedDocKeys = changedDocKeys.insert(key); FSTMaybeDocument *existingDoc = [self.remoteDocumentCache entryForKey:key]; - // Make sure we don't apply an old document version to the remote cache, though we - // make an exception for SnapshotVersion::None() which can happen for manufactured - // events (e.g. in the case of a limbo document resolution failing). - if (!existingDoc || SnapshotVersion{doc.version} == SnapshotVersion::None() || - SnapshotVersion{doc.version} >= SnapshotVersion{existingDoc.version}) { + + // If a document update isn't authoritative, make sure we don't apply an old document version + // to the remote cache. We make an exception for SnapshotVersion.MIN which can happen for + // manufactured events (e.g. in the case of a limbo document resolution failing). + if (!existingDoc || doc.version == SnapshotVersion::None() || + authoritativeUpdates.contains(doc.key) || doc.version >= existingDoc.version) { [self.remoteDocumentCache addEntry:doc]; } else { LOG_DEBUG(