Skip to content

Migrate FSTListenSequenceNumber to C++ ListenSequenceNumber #1703

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

Merged
merged 12 commits into from
Aug 16, 2018
22 changes: 11 additions & 11 deletions Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
using firebase::firestore::model::DocumentKey;
using firebase::firestore::model::DocumentKeyHash;
using firebase::firestore::model::DocumentKeySet;
using firebase::firestore::model::ListenSequenceNumber;
using firebase::firestore::model::Precondition;
using firebase::firestore::model::TargetId;

Expand All @@ -57,7 +58,7 @@ @implementation FSTLRUGarbageCollectorTests {
id<FSTRemoteDocumentCache> _documentCache;
id<FSTMutationQueue> _mutationQueue;
FSTLRUGarbageCollector *_gc;
FSTListenSequenceNumber _initialSequenceNumber;
ListenSequenceNumber _initialSequenceNumber;
User _user;
}

Expand All @@ -82,7 +83,7 @@ - (void)newTestResources {
_queryCache = [_persistence queryCache];
_documentCache = [_persistence remoteDocumentCache];
_mutationQueue = [_persistence mutationQueueForUser:_user];
_initialSequenceNumber = _persistence.run("start querycache", [&]() -> FSTListenSequenceNumber {
_initialSequenceNumber = _persistence.run("start querycache", [&]() -> ListenSequenceNumber {
[_mutationQueue start];
_gc = ((id<FSTLRUDelegate>)_persistence.referenceDelegate).gc;
return _persistence.currentSequenceNumber;
Expand All @@ -95,18 +96,17 @@ - (void)newTestResources {

#pragma mark - helpers

- (FSTListenSequenceNumber)sequenceNumberForQueryCount:(int)queryCount {
return _persistence.run("gc", [&]() -> FSTListenSequenceNumber {
return [_gc sequenceNumberForQueryCount:queryCount];
});
- (ListenSequenceNumber)sequenceNumberForQueryCount:(int)queryCount {
return _persistence.run(
"gc", [&]() -> ListenSequenceNumber { return [_gc sequenceNumberForQueryCount:queryCount]; });
}

- (int)queryCountForPercentile:(int)percentile {
return _persistence.run("query count",
[&]() -> int { return [_gc queryCountForPercentile:percentile]; });
}

- (int)removeQueriesThroughSequenceNumber:(FSTListenSequenceNumber)sequenceNumber
- (int)removeQueriesThroughSequenceNumber:(ListenSequenceNumber)sequenceNumber
liveQueries:(NSDictionary<NSNumber *, FSTQueryData *> *)liveQueries {
return _persistence.run("gc", [&]() -> int {
return [_gc removeQueriesUpThroughSequenceNumber:sequenceNumber liveQueries:liveQueries];
Expand All @@ -115,15 +115,15 @@ - (int)removeQueriesThroughSequenceNumber:(FSTListenSequenceNumber)sequenceNumbe

// Removes documents that are not part of a target or a mutation and have a sequence number
// less than or equal to the given sequence number.
- (int)removeOrphanedDocumentsThroughSequenceNumber:(FSTListenSequenceNumber)sequenceNumber {
- (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)sequenceNumber {
return _persistence.run("gc", [&]() -> int {
return [_gc removeOrphanedDocumentsThroughSequenceNumber:sequenceNumber];
});
}

- (FSTQueryData *)nextTestQuery {
TargetId targetID = ++_previousTargetID;
FSTListenSequenceNumber listenSequenceNumber = _persistence.currentSequenceNumber;
ListenSequenceNumber listenSequenceNumber = _persistence.currentSequenceNumber;
FSTQuery *query = FSTTestQuery(absl::StrCat("path", targetID));
return [[FSTQueryData alloc] initWithQuery:query
targetID:targetID
Expand Down Expand Up @@ -586,8 +586,8 @@ - (void)testRemoveTargetsThenGC {
// Add a couple docs from the newest target to the oldest (preserves them past the point where
// newest was removed)
// upperBound is the sequence number right before middleTarget is updated, then removed.
FSTListenSequenceNumber upperBound = _persistence.run(
"Add a couple docs from the newest target to the oldest", [&]() -> FSTListenSequenceNumber {
ListenSequenceNumber upperBound = _persistence.run(
"Add a couple docs from the newest target to the oldest", [&]() -> ListenSequenceNumber {
[self updateTargetInTransaction:oldestTarget];
for (const DocumentKey &docKey : newestDocsToAddToOldest) {
[self addDocument:docKey toTarget:oldestTarget.targetID];
Expand Down
3 changes: 2 additions & 1 deletion Firestore/Example/Tests/Local/FSTLevelDBQueryCacheTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

using firebase::Timestamp;
using firebase::firestore::model::DatabaseId;
using firebase::firestore::model::ListenSequenceNumber;
using firebase::firestore::model::ResourcePath;
using firebase::firestore::model::SnapshotVersion;
using firebase::firestore::model::TargetId;
Expand Down Expand Up @@ -75,7 +76,7 @@ - (void)testMetadataPersistedAcrossRestarts {
SnapshotVersion versionZero;
XCTAssertEqual(versionZero, queryCache.lastRemoteSnapshotVersion);

FSTListenSequenceNumber minimumSequenceNumber = 1234;
ListenSequenceNumber minimumSequenceNumber = 1234;
TargetId lastTargetId = 5;
SnapshotVersion lastVersion(Timestamp(1, 2));

Expand Down
7 changes: 4 additions & 3 deletions Firestore/Example/Tests/Local/FSTLocalStoreTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@

namespace testutil = firebase::firestore::testutil;
using firebase::firestore::auth::User;
using firebase::firestore::model::SnapshotVersion;
using firebase::firestore::model::DocumentKeySet;
using firebase::firestore::model::ListenSequenceNumber;
using firebase::firestore::model::SnapshotVersion;
using firebase::firestore::model::TargetId;

NS_ASSUME_NONNULL_BEGIN
Expand Down Expand Up @@ -817,7 +818,7 @@ - (void)testPersistsResumeTokens {

FSTQuery *query = FSTTestQuery("foo/bar");
FSTQueryData *queryData = [self.localStore allocateQuery:query];
FSTListenSequenceNumber initialSequenceNumber = queryData.sequenceNumber;
ListenSequenceNumber initialSequenceNumber = queryData.sequenceNumber;
FSTBoxedTargetID *targetID = @(queryData.targetID);
NSData *resumeToken = FSTTestResumeTokenFromSnapshotVersion(1000);

Expand All @@ -844,7 +845,7 @@ - (void)testPersistsResumeTokens {
XCTAssertEqualObjects(queryData2.resumeToken, resumeToken);

// The sequence number should have been bumped when we saved the new resume token.
FSTListenSequenceNumber newSequenceNumber = queryData2.sequenceNumber;
ListenSequenceNumber newSequenceNumber = queryData2.sequenceNumber;
XCTAssertGreaterThan(newSequenceNumber, initialSequenceNumber);
}

Expand Down
5 changes: 3 additions & 2 deletions Firestore/Example/Tests/Local/FSTQueryCacheTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@
namespace testutil = firebase::firestore::testutil;
using firebase::firestore::model::DocumentKey;
using firebase::firestore::model::DocumentKeySet;
using firebase::firestore::model::ListenSequenceNumber;
using firebase::firestore::model::SnapshotVersion;
using firebase::firestore::model::TargetId;

NS_ASSUME_NONNULL_BEGIN

@implementation FSTQueryCacheTests {
FSTQuery *_queryRooms;
FSTListenSequenceNumber _previousSequenceNumber;
ListenSequenceNumber _previousSequenceNumber;
TargetId _previousTargetID;
FSTTestSnapshotVersion _previousSnapshotVersion;
}
Expand Down Expand Up @@ -365,7 +366,7 @@ - (FSTQueryData *)queryDataWithQuery:(FSTQuery *)query {

- (FSTQueryData *)queryDataWithQuery:(FSTQuery *)query
targetID:(TargetId)targetID
listenSequenceNumber:(FSTListenSequenceNumber)sequenceNumber
listenSequenceNumber:(ListenSequenceNumber)sequenceNumber
version:(FSTTestSnapshotVersion)version {
NSData *resumeToken = FSTTestResumeTokenFromSnapshotVersion(version);
return [[FSTQueryData alloc] initWithQuery:query
Expand Down
9 changes: 5 additions & 4 deletions Firestore/Source/Core/FSTListenSequence.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

#import <Foundation/Foundation.h>

#import "Firestore/Source/Core/FSTTypes.h"
#include "Firestore/core/src/firebase/firestore/model/types.h"

NS_ASSUME_NONNULL_BEGIN

Expand All @@ -26,12 +26,13 @@ NS_ASSUME_NONNULL_BEGIN
*/
@interface FSTListenSequence : NSObject

- (instancetype)initStartingAfter:(FSTListenSequenceNumber)after NS_DESIGNATED_INITIALIZER;
- (instancetype)initStartingAfter:(firebase::firestore::model::ListenSequenceNumber)after
NS_DESIGNATED_INITIALIZER;

- (id)init NS_UNAVAILABLE;

- (FSTListenSequenceNumber)next;
- (firebase::firestore::model::ListenSequenceNumber)next;

@end

NS_ASSUME_NONNULL_END
NS_ASSUME_NONNULL_END
10 changes: 6 additions & 4 deletions Firestore/Source/Core/FSTListenSequence.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@

#import "Firestore/Source/Core/FSTListenSequence.h"

using firebase::firestore::model::ListenSequenceNumber;

NS_ASSUME_NONNULL_BEGIN

#pragma mark - FSTListenSequence

@interface FSTListenSequence () {
FSTListenSequenceNumber _previousSequenceNumber;
ListenSequenceNumber _previousSequenceNumber;
}

@end
Expand All @@ -30,7 +32,7 @@ @implementation FSTListenSequence

#pragma mark - Constructors

- (instancetype)initStartingAfter:(FSTListenSequenceNumber)after {
- (instancetype)initStartingAfter:(ListenSequenceNumber)after {
self = [super init];
if (self) {
_previousSequenceNumber = after;
Expand All @@ -40,11 +42,11 @@ - (instancetype)initStartingAfter:(FSTListenSequenceNumber)after {

#pragma mark - Public methods

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

@end

NS_ASSUME_NONNULL_END
NS_ASSUME_NONNULL_END
3 changes: 2 additions & 1 deletion Firestore/Source/Core/FSTSyncEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,15 @@
using firebase::firestore::model::BatchId;
using firebase::firestore::model::DocumentKey;
using firebase::firestore::model::DocumentKeySet;
using firebase::firestore::model::ListenSequenceNumber;
using firebase::firestore::model::SnapshotVersion;
using firebase::firestore::model::TargetId;

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;
static const ListenSequenceNumber kIrrelevantSequenceNumber = -1;

#pragma mark - FSTQueryView

Expand Down
2 changes: 0 additions & 2 deletions Firestore/Source/Core/FSTTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ NS_ASSUME_NONNULL_BEGIN
@class FSTMaybeDocument;
@class FSTTransaction;

typedef int64_t FSTListenSequenceNumber;

/**
* FSTVoidBlock is a block that's called when a specific event happens but that otherwise has
* no information associated with it.
Expand Down
25 changes: 16 additions & 9 deletions Firestore/Source/Local/FSTLRUGarbageCollector.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@

#import "Firestore/Source/Local/FSTQueryData.h"
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
#include "Firestore/core/src/firebase/firestore/model/types.h"

@protocol FSTQueryCache;

@class FSTLRUGarbageCollector;

extern const FSTListenSequenceNumber kFSTListenSequenceNumberInvalid;
extern const firebase::firestore::model::ListenSequenceNumber kFSTListenSequenceNumberInvalid;

/**
* Persistence layers intending to use LRU Garbage collection should implement this protocol. This
Expand All @@ -40,21 +41,24 @@ extern const FSTListenSequenceNumber kFSTListenSequenceNumberInvalid;
/**
* Enumerates all of the outstanding mutations.
*/
- (void)enumerateMutationsUsingBlock:(void (^)(const firebase::firestore::model::DocumentKey &key,
FSTListenSequenceNumber sequenceNumber,
BOOL *stop))block;
- (void)enumerateMutationsUsingBlock:
(void (^)(const firebase::firestore::model::DocumentKey &key,
firebase::firestore::model::ListenSequenceNumber sequenceNumber,
BOOL *stop))block;

/**
* Removes all unreferenced documents from the cache that have a sequence number less than or equal
* to the given sequence number. Returns the number of documents removed.
*/
- (int)removeOrphanedDocumentsThroughSequenceNumber:(FSTListenSequenceNumber)sequenceNumber;
- (int)removeOrphanedDocumentsThroughSequenceNumber:
(firebase::firestore::model::ListenSequenceNumber)sequenceNumber;

/**
* Removes all targets that are not currently being listened to and have a sequence number less than
* or equal to the given sequence number. Returns the number of targets removed.
*/
- (int)removeTargetsThroughSequenceNumber:(FSTListenSequenceNumber)sequenceNumber
- (int)removeTargetsThroughSequenceNumber:
(firebase::firestore::model::ListenSequenceNumber)sequenceNumber
liveQueries:(NSDictionary<NSNumber *, FSTQueryData *> *)liveQueries;

/** Access to the underlying LRU Garbage collector instance. */
Expand All @@ -81,19 +85,22 @@ extern const FSTListenSequenceNumber kFSTListenSequenceNumberInvalid;
/**
* Given a number of queries n, return the nth sequence number in the cache.
*/
- (FSTListenSequenceNumber)sequenceNumberForQueryCount:(NSUInteger)queryCount;
- (firebase::firestore::model::ListenSequenceNumber)sequenceNumberForQueryCount:
(NSUInteger)queryCount;

/**
* Removes queries that are not currently live (as indicated by presence in the liveQueries map) and
* have a sequence number less than or equal to the given sequence number.
*/
- (int)removeQueriesUpThroughSequenceNumber:(FSTListenSequenceNumber)sequenceNumber
- (int)removeQueriesUpThroughSequenceNumber:
(firebase::firestore::model::ListenSequenceNumber)sequenceNumber
liveQueries:(NSDictionary<NSNumber *, FSTQueryData *> *)liveQueries;

/**
* Removes all unreferenced documents from the cache that have a sequence number less than or equal
* to the given sequence number. Returns the number of documents removed.
*/
- (int)removeOrphanedDocumentsThroughSequenceNumber:(FSTListenSequenceNumber)sequenceNumber;
- (int)removeOrphanedDocumentsThroughSequenceNumber:
(firebase::firestore::model::ListenSequenceNumber)sequenceNumber;

@end
21 changes: 11 additions & 10 deletions Firestore/Source/Local/FSTLRUGarbageCollector.mm
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@
#include "Firestore/core/src/firebase/firestore/model/document_key.h"

using firebase::firestore::model::DocumentKey;
using firebase::firestore::model::ListenSequenceNumber;

const FSTListenSequenceNumber kFSTListenSequenceNumberInvalid = -1;
const ListenSequenceNumber kFSTListenSequenceNumberInvalid = -1;

/**
* RollingSequenceNumberBuffer tracks the nth sequence number in a series. Sequence numbers may be
Expand All @@ -33,26 +34,26 @@
class RollingSequenceNumberBuffer {
public:
explicit RollingSequenceNumberBuffer(size_t max_elements)
: max_elements_(max_elements), queue_(std::priority_queue<FSTListenSequenceNumber>()) {
: max_elements_(max_elements), queue_(std::priority_queue<ListenSequenceNumber>()) {
}

RollingSequenceNumberBuffer(const RollingSequenceNumberBuffer &other) = delete;

RollingSequenceNumberBuffer &operator=(const RollingSequenceNumberBuffer &other) = delete;

void AddElement(FSTListenSequenceNumber sequence_number) {
void AddElement(ListenSequenceNumber sequence_number) {
if (queue_.size() < max_elements_) {
queue_.push(sequence_number);
} else {
FSTListenSequenceNumber highestValue = queue_.top();
ListenSequenceNumber highestValue = queue_.top();
if (sequence_number < highestValue) {
queue_.pop();
queue_.push(sequence_number);
}
}
}

FSTListenSequenceNumber max_value() const {
ListenSequenceNumber max_value() const {
return queue_.top();
}

Expand All @@ -61,7 +62,7 @@ size_t size() const {
}

private:
std::priority_queue<FSTListenSequenceNumber> queue_;
std::priority_queue<ListenSequenceNumber> queue_;
const size_t max_elements_;
};

Expand Down Expand Up @@ -91,7 +92,7 @@ - (int)queryCountForPercentile:(NSUInteger)percentile {
return setSize;
}

- (FSTListenSequenceNumber)sequenceNumberForQueryCount:(NSUInteger)queryCount {
- (ListenSequenceNumber)sequenceNumberForQueryCount:(NSUInteger)queryCount {
if (queryCount == 0) {
return kFSTListenSequenceNumberInvalid;
}
Expand All @@ -102,19 +103,19 @@ - (FSTListenSequenceNumber)sequenceNumberForQueryCount:(NSUInteger)queryCount {
ptr_to_buffer->AddElement(queryData.sequenceNumber);
}];
[_delegate enumerateMutationsUsingBlock:^(const DocumentKey &docKey,
FSTListenSequenceNumber sequenceNumber, BOOL *stop) {
ListenSequenceNumber sequenceNumber, BOOL *stop) {
ptr_to_buffer->AddElement(sequenceNumber);
}];
return buffer.max_value();
}

- (int)removeQueriesUpThroughSequenceNumber:(FSTListenSequenceNumber)sequenceNumber
- (int)removeQueriesUpThroughSequenceNumber:(ListenSequenceNumber)sequenceNumber
liveQueries:
(NSDictionary<NSNumber *, FSTQueryData *> *)liveQueries {
return [_delegate removeTargetsThroughSequenceNumber:sequenceNumber liveQueries:liveQueries];
}

- (int)removeOrphanedDocumentsThroughSequenceNumber:(FSTListenSequenceNumber)sequenceNumber {
- (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)sequenceNumber {
return [_delegate removeOrphanedDocumentsThroughSequenceNumber:sequenceNumber];
}

Expand Down
Loading