Bump sequence numbers when receiving a new resume token (#1214)

* Bump sequence number on resume token refresh

* Style

* Fix comment formatting

6 files changed, 31 insertions, 15 deletions

diff --git a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm
index 6c7fb3d..4175118 100644
--- a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm
+++ b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm
@@ -808,6 +808,7 @@ FSTDocumentVersionDictionary *FSTVersionDictionary(FSTMutation *mutation,
 
   FSTQuery *query = FSTTestQuery("foo/bar");
   FSTQueryData *queryData = [self.localStore allocateQuery:query];
+  FSTListenSequenceNumber initialSequenceNumber = queryData.sequenceNumber;
   FSTBoxedTargetID *targetID = @(queryData.targetID);
   NSData *resumeToken = FSTTestResumeTokenFromSnapshotVersion(1000);
 
@@ -834,6 +835,10 @@ FSTDocumentVersionDictionary *FSTVersionDictionary(FSTMutation *mutation,
   // Should come back with the same resume token
   FSTQueryData *queryData2 = [self.localStore allocateQuery:query];
   XCTAssertEqualObjects(queryData2.resumeToken, resumeToken);
+
+  // The sequence number should have been bumped when we saved the new resume token.
+  FSTListenSequenceNumber newSequenceNumber = queryData2.sequenceNumber;
+  XCTAssertGreaterThan(newSequenceNumber, initialSequenceNumber);
 }
 
 - (void)testRemoteDocumentKeysForTarget {
diff --git a/Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm b/Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm
index 263ca22..dd34556 100644
--- a/Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm
+++ b/Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm
@@ -121,7 +121,8 @@ NS_ASSUME_NONNULL_BEGIN
   self.datastore.watchStreamRequestCount += 1;
   // Snapshot version is ignored on the wire
   FSTQueryData *sentQueryData = [query queryDataByReplacingSnapshotVersion:SnapshotVersion::None()
-                                                               resumeToken:query.resumeToken];
+                                                               resumeToken:query.resumeToken
+                                                            sequenceNumber:query.sequenceNumber];
   self.activeTargets[@(query.targetID)] = sentQueryData;
 }
 
diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm
index 3a324cd..92462f0 100644
--- a/Firestore/Source/Local/FSTLocalStore.mm
+++ b/Firestore/Source/Local/FSTLocalStore.mm
@@ -262,6 +262,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (FSTMaybeDocumentDictionary *)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent {
   return self.persistence.run("Apply remote event", [&]() -> FSTMaybeDocumentDictionary * {
+    FSTListenSequenceNumber sequenceNumber = [self.listenSequence next];
     id<FSTQueryCache> queryCache = self.queryCache;
 
     [remoteEvent.targetChanges enumerateKeysAndObjectsUsingBlock:^(
@@ -274,6 +275,18 @@ NS_ASSUME_NONNULL_BEGIN
         return;
       }
 
+      // Update the resume token if the change includes one. Don't clear any preexisting value.
+      // Bump the sequence number as well, so that documents being removed now are ordered later
+      // than documents that were previously removed from this target.
+      NSData *resumeToken = change.resumeToken;
+      if (resumeToken.length > 0) {
+        queryData = [queryData queryDataByReplacingSnapshotVersion:change.snapshotVersion
+                                                       resumeToken:resumeToken
+                                                    sequenceNumber:sequenceNumber];
+        self.targetIDs[targetIDNumber] = queryData;
+        [self.queryCache updateQueryData:queryData];
+      }
+
       FSTTargetMapping *mapping = change.mapping;
       if (mapping) {
         // First make sure that all references are deleted.
@@ -291,15 +304,6 @@ NS_ASSUME_NONNULL_BEGIN
           FSTFail(@"Unknown mapping type: %@", mapping);
         }
       }
-
-      // Update the resume token if the change includes one. Don't clear any preexisting value.
-      NSData *resumeToken = change.resumeToken;
-      if (resumeToken.length > 0) {
-        queryData = [queryData queryDataByReplacingSnapshotVersion:change.snapshotVersion
-                                                       resumeToken:resumeToken];
-        self.targetIDs[targetIDNumber] = queryData;
-        [self.queryCache updateQueryData:queryData];
-      }
     }];
 
     // TODO(klimt): This could probably be an NSMutableDictionary.
diff --git a/Firestore/Source/Local/FSTQueryData.h b/Firestore/Source/Local/FSTQueryData.h
index d2dacd6..bde0a15 100644
--- a/Firestore/Source/Local/FSTQueryData.h
+++ b/Firestore/Source/Local/FSTQueryData.h
@@ -54,10 +54,14 @@ typedef NS_ENUM(NSInteger, FSTQueryPurpose) {
 
 - (instancetype)init NS_UNAVAILABLE;
 
-/** Creates a new query data instance with an updated snapshot version and resume token. */
+/**
+ * Creates a new query data instance with an updated snapshot version, resume token, and sequence
+ * number.
+ */
 - (instancetype)queryDataByReplacingSnapshotVersion:
                     (firebase::firestore::model::SnapshotVersion)snapshotVersion
-                                        resumeToken:(NSData *)resumeToken;
+                                        resumeToken:(NSData *)resumeToken
+                                     sequenceNumber:(FSTListenSequenceNumber)sequenceNumber;
 
 /** The latest snapshot version seen for this target. */
 - (const firebase::firestore::model::SnapshotVersion &)snapshotVersion;
diff --git a/Firestore/Source/Local/FSTQueryData.mm b/Firestore/Source/Local/FSTQueryData.mm
index e352101..5087dfc 100644
--- a/Firestore/Source/Local/FSTQueryData.mm
+++ b/Firestore/Source/Local/FSTQueryData.mm
@@ -95,10 +95,11 @@ NS_ASSUME_NONNULL_BEGIN
 }
 
 - (instancetype)queryDataByReplacingSnapshotVersion:(SnapshotVersion)snapshotVersion
-                                        resumeToken:(NSData *)resumeToken {
+                                        resumeToken:(NSData *)resumeToken
+                                     sequenceNumber:(FSTListenSequenceNumber)sequenceNumber {
   return [[FSTQueryData alloc] initWithQuery:self.query
                                     targetID:self.targetID
-                        listenSequenceNumber:self.sequenceNumber
+                        listenSequenceNumber:sequenceNumber
                                      purpose:self.purpose
                              snapshotVersion:std::move(snapshotVersion)
                                  resumeToken:resumeToken];
diff --git a/Firestore/Source/Remote/FSTRemoteStore.mm b/Firestore/Source/Remote/FSTRemoteStore.mm
index c1ec9ff..d15d2e5 100644
--- a/Firestore/Source/Remote/FSTRemoteStore.mm
+++ b/Firestore/Source/Remote/FSTRemoteStore.mm
@@ -448,7 +448,8 @@ static const int kMaxPendingWrites = 10;
       if (queryData) {
         self->_listenTargets[target] =
             [queryData queryDataByReplacingSnapshotVersion:change.snapshotVersion
-                                               resumeToken:resumeToken];
+                                               resumeToken:resumeToken
+                                            sequenceNumber:queryData.sequenceNumber];
       }
     }
   }];