Filter out document updates from target association changes (#1122)

* Filter out document updates from target association changes

* Move remote-event-modifying methods onto remote event

* Style

2 files changed, 52 insertions, 0 deletions

diff --git a/Firestore/Source/Remote/FSTRemoteEvent.h b/Firestore/Source/Remote/FSTRemoteEvent.h
index dd45117..0f6b6c7 100644
--- a/Firestore/Source/Remote/FSTRemoteEvent.h
+++ b/Firestore/Source/Remote/FSTRemoteEvent.h
@@ -172,6 +172,17 @@ eventWithSnapshotVersion:(FSTSnapshotVersion *)snapshotVersion
 /** Handles an existence filter mismatch */
 - (void)handleExistenceFilterMismatchForTargetID:(FSTBoxedTargetID *)targetID;
 
+- (void)synthesizeDeleteForLimboTargetChange:(FSTTargetChange *)targetChange
+                                         key:(const firebase::firestore::model::DocumentKey &)key;
+
+/**
+ * Strips out mapping changes that aren't actually changes. That is, if the document already
+ * existed in the target, and is being added in the target, and this is not a reset, we can
+ * skip doing the work to associate the document with the target because it has already been done.
+ */
+- (void)filterUpdatesFromTargetChange:(FSTTargetChange *)targetChange
+                    existingDocuments:(FSTDocumentKeySet *)existingDocuments;
+
 @end
 
 #pragma mark - FSTWatchChangeAggregator
diff --git a/Firestore/Source/Remote/FSTRemoteEvent.mm b/Firestore/Source/Remote/FSTRemoteEvent.mm
index a3c85df..30aa0d3 100644
--- a/Firestore/Source/Remote/FSTRemoteEvent.mm
+++ b/Firestore/Source/Remote/FSTRemoteEvent.mm
@@ -281,6 +281,47 @@ eventWithSnapshotVersion:(FSTSnapshotVersion *)snapshotVersion
   return self;
 }
 
+- (void)filterUpdatesFromTargetChange:(FSTTargetChange *)targetChange
+                    existingDocuments:(FSTDocumentKeySet *)existingDocuments {
+  if ([targetChange.mapping isKindOfClass:[FSTUpdateMapping class]]) {
+    FSTUpdateMapping *update = (FSTUpdateMapping *)targetChange.mapping;
+    FSTDocumentKeySet *added = update.addedDocuments;
+    __block FSTDocumentKeySet *result = added;
+    [added enumerateObjectsUsingBlock:^(FSTDocumentKey *docKey, BOOL *stop) {
+      if ([existingDocuments containsObject:docKey]) {
+        result = [result setByRemovingObject:docKey];
+      }
+    }];
+    update.addedDocuments = result;
+  }
+}
+
+- (void)synthesizeDeleteForLimboTargetChange:(FSTTargetChange *)targetChange
+                                         key:(const DocumentKey &)key {
+  if (targetChange.currentStatusUpdate == FSTCurrentStatusUpdateMarkCurrent &&
+      _documentUpdates.find(key) == _documentUpdates.end()) {
+    // When listening to a query the server responds with a snapshot containing documents
+    // matching the query and a current marker telling us we're now in sync. It's possible for
+    // these to arrive as separate remote events or as a single remote event. For a document
+    // query, there will be no documents sent in the response if the document doesn't exist.
+    //
+    // If the snapshot arrives separately from the current marker, we handle it normally and
+    // updateTrackedLimboDocumentsWithChanges:targetID: will resolve the limbo status of the
+    // document, removing it from limboDocumentRefs. This works because clients only initiate
+    // limbo resolution when a target is current and because all current targets are always at a
+    // consistent snapshot.
+    //
+    // However, if the document doesn't exist and the current marker arrives, the document is
+    // not present in the snapshot and our normal view handling would consider the document to
+    // remain in limbo indefinitely because there are no updates to the document. To avoid this,
+    // we specially handle this just this case here: synthesizing a delete.
+    //
+    // TODO(dimond): Ideally we would have an explicit lookup query instead resulting in an
+    // explicit delete message and we could remove this special logic.
+    _documentUpdates[key] = [FSTDeletedDocument documentWithKey:key version:_snapshotVersion];
+  }
+}
+
 - (NSDictionary<FSTBoxedTargetID *, FSTTargetChange *> *)targetChanges {
   return static_cast<NSDictionary<FSTBoxedTargetID *, FSTTargetChange *> *>(_targetChanges);
 }