From 66f2ce4c39051bcffaae4054702c1d87a97c8973 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 3 May 2018 12:11:39 -0700 Subject: Bump sequence numbers when receiving a new resume token (#1214) * Bump sequence number on resume token refresh * Style * Fix comment formatting --- .../Example/Tests/Local/FSTLocalStoreTests.mm | 5 +++++ .../Example/Tests/SpecTests/FSTMockDatastore.mm | 3 ++- Firestore/Source/Local/FSTLocalStore.mm | 22 +++++++++++++--------- Firestore/Source/Local/FSTQueryData.h | 8 ++++++-- Firestore/Source/Local/FSTQueryData.mm | 5 +++-- Firestore/Source/Remote/FSTRemoteStore.mm | 3 ++- 6 files changed, 31 insertions(+), 15 deletions(-) (limited to 'Firestore') 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 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]; } } }]; -- cgit v1.2.3