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

Listen sequence numbers #675

Merged
merged 5 commits into from
Jan 17, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Firestore/Example/Tests/Local/FSTLocalSerializerTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ - (void)testEncodesQueryData {

FSTQueryData *queryData = [[FSTQueryData alloc] initWithQuery:query
targetID:targetID
listenSequenceNumber:10
purpose:FSTQueryPurposeListen
snapshotVersion:version
resumeToken:resumeToken];
Expand All @@ -166,6 +167,7 @@ - (void)testEncodesQueryData {

FSTPBTarget *expected = [FSTPBTarget message];
expected.targetId = targetID;
expected.lastListenSequenceNumber = 10;
expected.snapshotVersion.nanos = 1039000;
expected.resumeToken = [resumeToken copy];
expected.query.parent = queryTarget.parent;
Expand Down
73 changes: 63 additions & 10 deletions Firestore/Example/Tests/Local/FSTQueryCacheTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ - (void)testReadQueryNotInCache {
- (void)testSetAndReadAQuery {
if ([self isTestBaseClass]) return;

FSTQueryData *queryData = [self queryDataWithQuery:_queryRooms targetID:1 version:1];
FSTQueryData *queryData =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like most of these tests don't care about the targetID, the listenSequenceNumber, or the version. Could you add a queryDataWithQuery: overload that defaults the targetID, listenSequenceNumber, and version for tests that don't care to cut down on the noise in here?

Alternatively, since it seems like the listenSequenceNumber-testing methods don't actually use this helper maybe we can have queryDataWithQuery just default the listen sequence number?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a helper that just takes a query, and transitioned most of the helper calls over to it.

[self queryDataWithQuery:_queryRooms targetID:1 listenSequenceNumber:1 version:1];
[self addQueryData:queryData];

FSTQueryData *result = [self.queryCache queryDataForQuery:_queryRooms];
Expand All @@ -74,14 +75,14 @@ - (void)testCanonicalIDCollision {
FSTQuery *q2 = [FSTTestQuery(@"a") queryByAddingFilter:FSTTestFilter(@"foo", @"==", @"1")];
XCTAssertEqualObjects(q1.canonicalID, q2.canonicalID);

FSTQueryData *data1 = [self queryDataWithQuery:q1 targetID:1 version:1];
FSTQueryData *data1 = [self queryDataWithQuery:q1 targetID:1 listenSequenceNumber:1 version:1];
[self addQueryData:data1];

// Using the other query should not return the query cache entry despite equal canonicalIDs.
XCTAssertNil([self.queryCache queryDataForQuery:q2]);
XCTAssertEqualObjects([self.queryCache queryDataForQuery:q1], data1);

FSTQueryData *data2 = [self queryDataWithQuery:q2 targetID:2 version:1];
FSTQueryData *data2 = [self queryDataWithQuery:q2 targetID:2 listenSequenceNumber:2 version:1];
[self addQueryData:data2];

XCTAssertEqualObjects([self.queryCache queryDataForQuery:q1], data1);
Expand All @@ -99,10 +100,12 @@ - (void)testCanonicalIDCollision {
- (void)testSetQueryToNewValue {
if ([self isTestBaseClass]) return;

FSTQueryData *queryData1 = [self queryDataWithQuery:_queryRooms targetID:1 version:1];
FSTQueryData *queryData1 =
[self queryDataWithQuery:_queryRooms targetID:1 listenSequenceNumber:1 version:1];
[self addQueryData:queryData1];

FSTQueryData *queryData2 = [self queryDataWithQuery:_queryRooms targetID:1 version:2];
FSTQueryData *queryData2 =
[self queryDataWithQuery:_queryRooms targetID:1 listenSequenceNumber:1 version:2];
[self addQueryData:queryData2];

FSTQueryData *result = [self.queryCache queryDataForQuery:_queryRooms];
Expand All @@ -115,7 +118,8 @@ - (void)testSetQueryToNewValue {
- (void)testRemoveQuery {
if ([self isTestBaseClass]) return;

FSTQueryData *queryData1 = [self queryDataWithQuery:_queryRooms targetID:1 version:1];
FSTQueryData *queryData1 =
[self queryDataWithQuery:_queryRooms targetID:1 listenSequenceNumber:1 version:1];
[self addQueryData:queryData1];

[self removeQueryData:queryData1];
Expand All @@ -127,7 +131,8 @@ - (void)testRemoveQuery {
- (void)testRemoveNonExistentQuery {
if ([self isTestBaseClass]) return;

FSTQueryData *queryData = [self queryDataWithQuery:_queryRooms targetID:1 version:1];
FSTQueryData *queryData =
[self queryDataWithQuery:_queryRooms targetID:1 listenSequenceNumber:1 version:1];

// no-op, but make sure it doesn't throw.
XCTAssertNoThrow([self removeQueryData:queryData]);
Expand All @@ -136,7 +141,8 @@ - (void)testRemoveNonExistentQuery {
- (void)testRemoveQueryRemovesMatchingKeysToo {
if ([self isTestBaseClass]) return;

FSTQueryData *rooms = [self queryDataWithQuery:_queryRooms targetID:1 version:1];
FSTQueryData *rooms =
[self queryDataWithQuery:_queryRooms targetID:1 listenSequenceNumber:1 version:1];
[self addQueryData:rooms];

FSTDocumentKey *key1 = FSTTestDocKey(@"rooms/foo");
Expand Down Expand Up @@ -204,14 +210,16 @@ - (void)testRemoveEmitsGarbageEvents {
[garbageCollector addGarbageSource:self.queryCache];
FSTAssertEqualSets([garbageCollector collectGarbage], @[]);

FSTQueryData *rooms = [self queryDataWithQuery:FSTTestQuery(@"rooms") targetID:1 version:1];
FSTQueryData *rooms =
[self queryDataWithQuery:FSTTestQuery(@"rooms") targetID:1 listenSequenceNumber:1 version:1];
FSTDocumentKey *room1 = FSTTestDocKey(@"rooms/bar");
FSTDocumentKey *room2 = FSTTestDocKey(@"rooms/foo");
[self addQueryData:rooms];
[self addMatchingKey:room1 forTargetID:rooms.targetID];
[self addMatchingKey:room2 forTargetID:rooms.targetID];

FSTQueryData *halls = [self queryDataWithQuery:FSTTestQuery(@"halls") targetID:2 version:1];
FSTQueryData *halls =
[self queryDataWithQuery:FSTTestQuery(@"halls") targetID:2 listenSequenceNumber:2 version:1];
FSTDocumentKey *hall1 = FSTTestDocKey(@"halls/bar");
FSTDocumentKey *hall2 = FSTTestDocKey(@"halls/foo");
[self addQueryData:halls];
Expand Down Expand Up @@ -249,13 +257,54 @@ - (void)testMatchingKeysForTargetID {
FSTAssertEqualSets([self.queryCache matchingKeysForTargetID:2], (@[ key1, key3 ]));
}

- (void)testHighestListenSequenceNumber {
if ([self isTestBaseClass]) return;

FSTQueryData *query1 = [[FSTQueryData alloc] initWithQuery:FSTTestQuery(@"rooms")
targetID:1
listenSequenceNumber:10
purpose:FSTQueryPurposeListen];
[self addQueryData:query1];
FSTQueryData *query2 = [[FSTQueryData alloc] initWithQuery:FSTTestQuery(@"halls")
targetID:2
listenSequenceNumber:20
purpose:FSTQueryPurposeListen];
[self addQueryData:query2];
XCTAssertEqual([self.queryCache highestListenSequenceNumber], 20);

// TargetIDs never come down.
[self removeQueryData:query2];
XCTAssertEqual([self.queryCache highestListenSequenceNumber], 20);

// A query with an empty result set still counts.
FSTQueryData *query3 = [[FSTQueryData alloc] initWithQuery:FSTTestQuery(@"garages")
targetID:42
listenSequenceNumber:100
purpose:FSTQueryPurposeListen];
[self addQueryData:query3];
XCTAssertEqual([self.queryCache highestListenSequenceNumber], 100);

[self removeQueryData:query1];
XCTAssertEqual([self.queryCache highestListenSequenceNumber], 100);

[self removeQueryData:query3];
XCTAssertEqual([self.queryCache highestListenSequenceNumber], 100);

// Verify that the highestTargetID even survives restarts.
[self.queryCache shutdown];
self.queryCache = [self.persistence queryCache];
[self.queryCache start];
XCTAssertEqual([self.queryCache highestListenSequenceNumber], 100);
}

- (void)testHighestTargetID {
if ([self isTestBaseClass]) return;

XCTAssertEqual([self.queryCache highestTargetID], 0);

FSTQueryData *query1 = [[FSTQueryData alloc] initWithQuery:FSTTestQuery(@"rooms")
targetID:1
listenSequenceNumber:10
purpose:FSTQueryPurposeListen];
FSTDocumentKey *key1 = FSTTestDocKey(@"rooms/bar");
FSTDocumentKey *key2 = FSTTestDocKey(@"rooms/foo");
Expand All @@ -265,6 +314,7 @@ - (void)testHighestTargetID {

FSTQueryData *query2 = [[FSTQueryData alloc] initWithQuery:FSTTestQuery(@"halls")
targetID:2
listenSequenceNumber:20
purpose:FSTQueryPurposeListen];
FSTDocumentKey *key3 = FSTTestDocKey(@"halls/foo");
[self addQueryData:query2];
Expand All @@ -278,6 +328,7 @@ - (void)testHighestTargetID {
// A query with an empty result set still counts.
FSTQueryData *query3 = [[FSTQueryData alloc] initWithQuery:FSTTestQuery(@"garages")
targetID:42
listenSequenceNumber:100
purpose:FSTQueryPurposeListen];
[self addQueryData:query3];
XCTAssertEqual([self.queryCache highestTargetID], 42);
Expand Down Expand Up @@ -321,10 +372,12 @@ - (void)testLastRemoteSnapshotVersion {
*/
- (FSTQueryData *)queryDataWithQuery:(FSTQuery *)query
targetID:(FSTTargetID)targetID
listenSequenceNumber:(FSTListenSequenceNumber)sequenceNumber
version:(FSTTestSnapshotVersion)version {
NSData *resumeToken = FSTTestResumeTokenFromSnapshotVersion(version);
return [[FSTQueryData alloc] initWithQuery:query
targetID:targetID
listenSequenceNumber:sequenceNumber
purpose:FSTQueryPurposeListen
snapshotVersion:FSTTestVersion(version)
resumeToken:resumeToken];
Expand Down
15 changes: 11 additions & 4 deletions Firestore/Example/Tests/Remote/FSTSerializerBetaTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -396,20 +396,25 @@ - (void)testRoundTripSpecialFieldNames {

- (void)testEncodesListenRequestLabels {
FSTQuery *query = FSTTestQuery(@"collection/key");
FSTQueryData *queryData =
[[FSTQueryData alloc] initWithQuery:query targetID:2 purpose:FSTQueryPurposeListen];
FSTQueryData *queryData = [[FSTQueryData alloc] initWithQuery:query
targetID:2
listenSequenceNumber:2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making this a different value than the targetID would help catch cases where we propagate the wrong property via the serializer.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

purpose:FSTQueryPurposeListen];

NSDictionary<NSString *, NSString *> *result =
[self.serializer encodedListenRequestLabelsForQueryData:queryData];
XCTAssertNil(result);

queryData =
[[FSTQueryData alloc] initWithQuery:query targetID:2 purpose:FSTQueryPurposeLimboResolution];
queryData = [[FSTQueryData alloc] initWithQuery:query
targetID:2
listenSequenceNumber:2
purpose:FSTQueryPurposeLimboResolution];
result = [self.serializer encodedListenRequestLabelsForQueryData:queryData];
XCTAssertEqualObjects(result, @{@"goog-listen-tags" : @"limbo-document"});

queryData = [[FSTQueryData alloc] initWithQuery:query
targetID:2
listenSequenceNumber:2
purpose:FSTQueryPurposeExistenceFilterMismatch];
result = [self.serializer encodedListenRequestLabelsForQueryData:queryData];
XCTAssertEqualObjects(result, @{@"goog-listen-tags" : @"existence-filter-mismatch"});
Expand Down Expand Up @@ -627,6 +632,7 @@ - (void)testEncodesResumeTokens {
FSTQuery *q = FSTTestQuery(@"docs");
FSTQueryData *model = [[FSTQueryData alloc] initWithQuery:q
targetID:1
listenSequenceNumber:0
purpose:FSTQueryPurposeListen
snapshotVersion:[FSTSnapshotVersion noVersion]
resumeToken:FSTTestData(1, 2, 3, -1)];
Expand All @@ -647,6 +653,7 @@ - (void)testEncodesResumeTokens {
- (FSTQueryData *)queryDataForQuery:(FSTQuery *)query {
return [[FSTQueryData alloc] initWithQuery:query
targetID:1
listenSequenceNumber:0
purpose:FSTQueryPurposeListen
snapshotVersion:[FSTSnapshotVersion noVersion]
resumeToken:[NSData data]];
Expand Down
1 change: 1 addition & 0 deletions Firestore/Example/Tests/SpecTests/FSTSpecTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ - (void)validateStateExpectations:(nullable NSDictionary *)expected {
expectedActiveTargets[@(targetID)] =
[[FSTQueryData alloc] initWithQuery:query
targetID:targetID
listenSequenceNumber:0
purpose:FSTQueryPurposeListen
snapshotVersion:[FSTSnapshotVersion noVersion]
resumeToken:resumeToken];
Expand Down
37 changes: 37 additions & 0 deletions Firestore/Source/Core/FSTListenSequence.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright 2017 Google
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's 2018 :-)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you say so.

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import <Foundation/Foundation.h>

#import "FSTTypes.h"

NS_ASSUME_NONNULL_BEGIN

/**
* FSTListenSequence is a monotonic sequence. It is initialized with a minimum value to
* exceed. All subsequent calls to next will return increasing values.
*/
@interface FSTListenSequence : NSObject

- (instancetype)initStartingAfter:(FSTListenSequenceNumber)after NS_DESIGNATED_INITIALIZER;

- (id)init __attribute__((unavailable("Use initStartingAfter:")));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just NS_UNAVAILABLE is sufficient to mark the default initializer as not to be used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


- (FSTListenSequenceNumber)next;

@end

NS_ASSUME_NONNULL_END
34 changes: 34 additions & 0 deletions Firestore/Source/Core/FSTListenSequence.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#import "FSTListenSequence.h"

NS_ASSUME_NONNULL_BEGIN

#pragma mark - FSTListenSequence

@interface FSTListenSequence () {
FSTListenSequenceNumber _previousSequenceNumber;
}

@end

@implementation FSTListenSequence

#pragma mark - Constructors

- (instancetype)initStartingAfter:(FSTListenSequenceNumber)after {
self = [super init];
if (self) {
_previousSequenceNumber = after;
}
return self;
}

#pragma mark - Public methods

- (FSTListenSequenceNumber)next {
_previousSequenceNumber++;
return _previousSequenceNumber;
}

@end

NS_ASSUME_NONNULL_END
5 changes: 5 additions & 0 deletions Firestore/Source/Core/FSTSyncEngine.m
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@

NS_ASSUME_NONNULL_BEGIN

// Limbo documents don't use persistence, and are eagerly GC'd. So, listens for them don't need
// real sequence numbers.
static const FSTListenSequenceNumber kIrrelevantSequenceNumber = -1;

#pragma mark - FSTQueryView

/**
Expand Down Expand Up @@ -490,6 +494,7 @@ - (void)trackLimboChange:(FSTLimboDocumentChange *)limboChange {
FSTQuery *query = [FSTQuery queryWithPath:key.path];
FSTQueryData *queryData = [[FSTQueryData alloc] initWithQuery:query
targetID:limboTargetID
listenSequenceNumber:kIrrelevantSequenceNumber
purpose:FSTQueryPurposeLimboResolution];
self.limboKeysByTarget[@(limboTargetID)] = key;
[self.remoteStore listenToTargetWithQueryData:queryData];
Expand Down
2 changes: 2 additions & 0 deletions Firestore/Source/Core/FSTTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ typedef int32_t FSTBatchID;

typedef int32_t FSTTargetID;

typedef int64_t FSTListenSequenceNumber;

typedef NSNumber FSTBoxedTargetID;

/**
Expand Down
15 changes: 14 additions & 1 deletion Firestore/Source/Local/FSTLevelDBQueryCache.mm
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ - (FSTTargetID)highestTargetID {
return self.metadata.highestTargetId;
}

- (FSTListenSequenceNumber)highestListenSequenceNumber {
return self.metadata.highestListenSequenceNumber;
}

- (FSTSnapshotVersion *)lastRemoteSnapshotVersion {
return _lastRemoteSnapshotVersion;
}
Expand All @@ -116,7 +120,6 @@ - (void)shutdown {
}

- (void)addQueryData:(FSTQueryData *)queryData group:(FSTWriteGroup *)group {
// TODO(mcg): actually populate listen sequence number
FSTTargetID targetID = queryData.targetID;
std::string key = [FSTLevelDBTargetKey keyWithTargetID:targetID];
[group setMessage:[self.serializer encodedQueryData:queryData] forKey:key];
Expand All @@ -127,9 +130,19 @@ - (void)addQueryData:(FSTQueryData *)queryData group:(FSTWriteGroup *)group {
std::string emptyBuffer;
[group setData:emptyBuffer forKey:indexKey];

BOOL saveMetadata = NO;
FSTPBTargetGlobal *metadata = self.metadata;
if (targetID > metadata.highestTargetId) {
metadata.highestTargetId = targetID;
saveMetadata = YES;
}

if (queryData.sequenceNumber > metadata.highestListenSequenceNumber) {
metadata.highestListenSequenceNumber = queryData.sequenceNumber;
saveMetadata = YES;
}

if (saveMetadata) {
[group setMessage:metadata forKey:[FSTLevelDBTargetGlobalKey key]];
}
}
Expand Down
Loading