diff options
author | Gil <mcg@google.com> | 2018-05-05 08:10:51 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-05-05 08:10:51 -0700 |
commit | a080e481b5e6fcbc2b645920051cf20fc8cad7a7 (patch) | |
tree | 5d5ce3d39be316ac82a2fd34c92b05fc9e67f83a /Firestore/Source/Core | |
parent | d158e420c6fa04997ee3d2a6c44fd53a52883d81 (diff) |
Port FSTDocumentKeySet to C++ DocumentKeySet (#1229)
* Define a Comparator for DocumentKey
* Automated migration from FSTDocumentKeySet to DocumentKeySet
* Manual fixups for DocumentKeySet
* Delete FSTDocumentKeySet
Diffstat (limited to 'Firestore/Source/Core')
-rw-r--r-- | Firestore/Source/Core/FSTFirestoreClient.mm | 4 | ||||
-rw-r--r-- | Firestore/Source/Core/FSTSyncEngine.mm | 21 | ||||
-rw-r--r-- | Firestore/Source/Core/FSTTransaction.mm | 11 | ||||
-rw-r--r-- | Firestore/Source/Core/FSTView.h | 11 | ||||
-rw-r--r-- | Firestore/Source/Core/FSTView.mm | 100 |
5 files changed, 79 insertions, 68 deletions
diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm index f1b4ffd..658cf57 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.mm +++ b/Firestore/Source/Core/FSTFirestoreClient.mm @@ -56,6 +56,7 @@ using firebase::firestore::auth::CredentialsProvider; using firebase::firestore::auth::User; using firebase::firestore::core::DatabaseInfo; using firebase::firestore::model::DatabaseId; +using firebase::firestore::model::DocumentKeySet; NS_ASSUME_NONNULL_BEGIN @@ -312,9 +313,8 @@ NS_ASSUME_NONNULL_BEGIN NSError *_Nullable error))completion { [self.workerDispatchQueue dispatchAsync:^{ FSTDocumentDictionary *docs = [self.localStore executeQuery:query.query]; - FSTDocumentKeySet *remoteKeys = [FSTDocumentKeySet keySet]; - FSTView *view = [[FSTView alloc] initWithQuery:query.query remoteDocuments:remoteKeys]; + FSTView *view = [[FSTView alloc] initWithQuery:query.query remoteDocuments:DocumentKeySet{}]; FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:docs]; FSTViewChange *viewChange = [view applyChangesToDocuments:viewDocChanges]; FSTAssert(viewChange.limboChanges.count == 0, diff --git a/Firestore/Source/Core/FSTSyncEngine.mm b/Firestore/Source/Core/FSTSyncEngine.mm index 8ec98e8..ed97d6c 100644 --- a/Firestore/Source/Core/FSTSyncEngine.mm +++ b/Firestore/Source/Core/FSTSyncEngine.mm @@ -21,6 +21,7 @@ #include <map> #include <set> #include <unordered_map> +#include <utility> #import "FIRFirestoreErrors.h" #import "Firestore/Source/Core/FSTQuery.h" @@ -52,6 +53,7 @@ using firebase::firestore::core::TargetIdGenerator; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::SnapshotVersion; using firebase::firestore::model::TargetId; +using firebase::firestore::model::DocumentKeySet; NS_ASSUME_NONNULL_BEGIN @@ -185,9 +187,9 @@ static const FSTListenSequenceNumber kIrrelevantSequenceNumber = -1; FSTQueryData *queryData = [self.localStore allocateQuery:query]; FSTDocumentDictionary *docs = [self.localStore executeQuery:query]; - FSTDocumentKeySet *remoteKeys = [self.localStore remoteDocumentKeysForTarget:queryData.targetID]; + DocumentKeySet remoteKeys = [self.localStore remoteDocumentKeysForTarget:queryData.targetID]; - FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:remoteKeys]; + FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:std::move(remoteKeys)]; FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:docs]; FSTViewChange *viewChange = [view applyChangesToDocuments:viewDocChanges]; FSTAssert(viewChange.limboChanges.count == 0, @@ -347,13 +349,14 @@ static const FSTListenSequenceNumber kIrrelevantSequenceNumber = -1; [NSMutableDictionary dictionary]; FSTDeletedDocument *doc = [FSTDeletedDocument documentWithKey:limboKey version:SnapshotVersion::None()]; - FSTDocumentKeySet *limboDocuments = [[FSTDocumentKeySet keySet] setByAddingObject:doc.key]; - FSTRemoteEvent *event = [[FSTRemoteEvent alloc] initWithSnapshotVersion:SnapshotVersion::None() - targetChanges:targetChanges - documentUpdates:{ - { limboKey, doc } - } - limboDocuments:limboDocuments]; + DocumentKeySet limboDocuments = DocumentKeySet{doc.key}; + FSTRemoteEvent *event = + [[FSTRemoteEvent alloc] initWithSnapshotVersion:SnapshotVersion::None() + targetChanges:targetChanges + documentUpdates:{ + { limboKey, doc } + } + limboDocuments:std::move(limboDocuments)]; [self applyRemoteEvent:event]; } else { FSTQueryView *queryView = self.queryViewsByTarget[@(targetID)]; diff --git a/Firestore/Source/Core/FSTTransaction.mm b/Firestore/Source/Core/FSTTransaction.mm index 57dcc48..5c36b20 100644 --- a/Firestore/Source/Core/FSTTransaction.mm +++ b/Firestore/Source/Core/FSTTransaction.mm @@ -24,19 +24,20 @@ #import "FIRFirestoreErrors.h" #import "Firestore/Source/API/FSTUserDataConverter.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKeySet.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Remote/FSTDatastore.h" #import "Firestore/Source/Util/FSTAssert.h" #import "Firestore/Source/Util/FSTUsageValidation.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/document_key_set.h" #include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" using firebase::firestore::model::DocumentKey; using firebase::firestore::model::Precondition; using firebase::firestore::model::SnapshotVersion; +using firebase::firestore::model::DocumentKeySet; NS_ASSUME_NONNULL_BEGIN @@ -216,15 +217,15 @@ NS_ASSUME_NONNULL_BEGIN } // Make a list of read documents that haven't been written. - FSTDocumentKeySet *unwritten = [FSTDocumentKeySet keySet]; + DocumentKeySet unwritten; for (const auto &kv : _readVersions) { - unwritten = [unwritten setByAddingObject:kv.first]; + unwritten = unwritten.insert(kv.first); }; // For each mutation, note that the doc was written. for (FSTMutation *mutation in self.mutations) { - unwritten = [unwritten setByRemovingObject:mutation.key]; + unwritten = unwritten.erase(mutation.key); } - if (unwritten.count) { + if (!unwritten.empty()) { // TODO(klimt): This is a temporary restriction, until "verify" is supported on the backend. completion([NSError errorWithDomain:FIRFirestoreErrorDomain diff --git a/Firestore/Source/Core/FSTView.h b/Firestore/Source/Core/FSTView.h index 431b863..fc6cead 100644 --- a/Firestore/Source/Core/FSTView.h +++ b/Firestore/Source/Core/FSTView.h @@ -18,9 +18,9 @@ #import "Firestore/Source/Core/FSTTypes.h" #import "Firestore/Source/Model/FSTDocumentDictionary.h" -#import "Firestore/Source/Model/FSTDocumentKeySet.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/document_key_set.h" @class FSTDocumentSet; @class FSTDocumentViewChangeSet; @@ -38,6 +38,8 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)init NS_UNAVAILABLE; +- (const firebase::firestore::model::DocumentKeySet &)mutatedKeys; + /** The new set of docs that should be in the view. */ @property(nonatomic, strong, readonly) FSTDocumentSet *documentSet; @@ -50,8 +52,6 @@ NS_ASSUME_NONNULL_BEGIN */ @property(nonatomic, assign, readonly) BOOL needsRefill; -@property(nonatomic, strong, readonly) FSTDocumentKeySet *mutatedKeys; - @end #pragma mark - FSTLimboDocumentChange @@ -97,7 +97,8 @@ typedef NS_ENUM(NSInteger, FSTLimboDocumentChangeType) { - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithQuery:(FSTQuery *)query - remoteDocuments:(FSTDocumentKeySet *)remoteDocuments NS_DESIGNATED_INITIALIZER; + remoteDocuments:(firebase::firestore::model::DocumentKeySet)remoteDocuments + NS_DESIGNATED_INITIALIZER; /** * Iterates over a set of doc changes, applies the query limit, and computes what the new results @@ -152,7 +153,7 @@ typedef NS_ENUM(NSInteger, FSTLimboDocumentChangeType) { * The set of remote documents that the server has told us belongs to the target associated with * this view. */ -@property(nonatomic, strong, readonly) FSTDocumentKeySet *syncedDocuments; +- (const firebase::firestore::model::DocumentKeySet &)syncedDocuments; @end diff --git a/Firestore/Source/Core/FSTView.mm b/Firestore/Source/Core/FSTView.mm index d87951a..d254a82 100644 --- a/Firestore/Source/Core/FSTView.mm +++ b/Firestore/Source/Core/FSTView.mm @@ -29,6 +29,7 @@ #include "Firestore/core/src/firebase/firestore/model/document_key.h" using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; NS_ASSUME_NONNULL_BEGIN @@ -40,26 +41,32 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithDocumentSet:(FSTDocumentSet *)documentSet changeSet:(FSTDocumentViewChangeSet *)changeSet needsRefill:(BOOL)needsRefill - mutatedKeys:(FSTDocumentKeySet *)mutatedKeys NS_DESIGNATED_INITIALIZER; + mutatedKeys:(DocumentKeySet)mutatedKeys NS_DESIGNATED_INITIALIZER; @end -@implementation FSTViewDocumentChanges +@implementation FSTViewDocumentChanges { + DocumentKeySet _mutatedKeys; +} - (instancetype)initWithDocumentSet:(FSTDocumentSet *)documentSet changeSet:(FSTDocumentViewChangeSet *)changeSet needsRefill:(BOOL)needsRefill - mutatedKeys:(FSTDocumentKeySet *)mutatedKeys { + mutatedKeys:(DocumentKeySet)mutatedKeys { self = [super init]; if (self) { _documentSet = documentSet; _changeSet = changeSet; _needsRefill = needsRefill; - _mutatedKeys = mutatedKeys; + _mutatedKeys = std::move(mutatedKeys); } return self; } +- (const DocumentKeySet &)mutatedKeys { + return _mutatedKeys; +} + @end #pragma mark - FSTLimboDocumentChange @@ -165,32 +172,33 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang @property(nonatomic, strong) FSTDocumentSet *documentSet; -/** Documents included in the remote target. */ -@property(nonatomic, strong) FSTDocumentKeySet *syncedDocuments; - -/** Documents in the view but not in the remote target */ -@property(nonatomic, strong) FSTDocumentKeySet *limboDocuments; +@end -/** Document Keys that have local changes. */ -@property(nonatomic, strong) FSTDocumentKeySet *mutatedKeys; +@implementation FSTView { + /** Documents included in the remote target. */ + DocumentKeySet _syncedDocuments; -@end + /** Documents in the view but not in the remote target */ + DocumentKeySet _limboDocuments; -@implementation FSTView + /** Document Keys that have local changes. */ + DocumentKeySet _mutatedKeys; +} -- (instancetype)initWithQuery:(FSTQuery *)query - remoteDocuments:(nonnull FSTDocumentKeySet *)remoteDocuments { +- (instancetype)initWithQuery:(FSTQuery *)query remoteDocuments:(DocumentKeySet)remoteDocuments { self = [super init]; if (self) { _query = query; _documentSet = [FSTDocumentSet documentSetWithComparator:query.comparator]; - _syncedDocuments = remoteDocuments; - _limboDocuments = [FSTDocumentKeySet keySet]; - _mutatedKeys = [FSTDocumentKeySet keySet]; + _syncedDocuments = std::move(remoteDocuments); } return self; } +- (const DocumentKeySet &)syncedDocuments { + return _syncedDocuments; +} + - (FSTViewDocumentChanges *)computeChangesWithDocuments:(FSTMaybeDocumentDictionary *)docChanges { return [self computeChangesWithDocuments:docChanges previousChanges:nil]; } @@ -202,8 +210,8 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang previousChanges ? previousChanges.changeSet : [FSTDocumentViewChangeSet changeSet]; FSTDocumentSet *oldDocumentSet = previousChanges ? previousChanges.documentSet : self.documentSet; - __block FSTDocumentKeySet *newMutatedKeys = - previousChanges ? previousChanges.mutatedKeys : self.mutatedKeys; + __block DocumentKeySet newMutatedKeys = + previousChanges ? previousChanges.mutatedKeys : _mutatedKeys; __block FSTDocumentSet *newDocumentSet = oldDocumentSet; __block BOOL needsRefill = NO; @@ -236,13 +244,13 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang if (newDoc) { newDocumentSet = [newDocumentSet documentSetByAddingDocument:newDoc]; if (newDoc.hasLocalMutations) { - newMutatedKeys = [newMutatedKeys setByAddingObject:key]; + newMutatedKeys = newMutatedKeys.insert(key); } else { - newMutatedKeys = [newMutatedKeys setByRemovingObject:key]; + newMutatedKeys = newMutatedKeys.erase(key); } } else { newDocumentSet = [newDocumentSet documentSetByRemovingKey:key]; - newMutatedKeys = [newMutatedKeys setByRemovingObject:key]; + newMutatedKeys = newMutatedKeys.erase(key); } // Calculate change @@ -311,7 +319,7 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang FSTDocumentSet *oldDocuments = self.documentSet; self.documentSet = docChanges.documentSet; - self.mutatedKeys = docChanges.mutatedKeys; + _mutatedKeys = docChanges.mutatedKeys; // Sort changes based on type and query comparator. NSArray<FSTDocumentViewChange *> *changes = [docChanges.changeSet changes]; @@ -325,7 +333,7 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang }]; [self applyTargetChange:targetChange]; NSArray<FSTLimboDocumentChange *> *limboChanges = [self updateLimboDocuments]; - BOOL synced = self.limboDocuments.count == 0 && self.isCurrent; + BOOL synced = _limboDocuments.empty() && self.isCurrent; FSTSyncState newSyncState = synced ? FSTSyncStateSynced : FSTSyncStateLocal; BOOL syncStateChanged = newSyncState != self.syncState; self.syncState = newSyncState; @@ -340,7 +348,7 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang oldDocuments:oldDocuments documentChanges:changes fromCache:newSyncState == FSTSyncStateLocal - hasPendingWrites:!docChanges.mutatedKeys.isEmpty + hasPendingWrites:!docChanges.mutatedKeys.empty() syncStateChanged:syncStateChanged]; return [FSTViewChange changeWithSnapshot:snapshot limboChanges:limboChanges]; @@ -358,7 +366,7 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang initWithDocumentSet:self.documentSet changeSet:[FSTDocumentViewChangeSet changeSet] needsRefill:NO - mutatedKeys:self.mutatedKeys]]; + mutatedKeys:_mutatedKeys]]; } else { // No effect, just return a no-op FSTViewChange. return [[FSTViewChange alloc] initWithSnapshot:nil limboChanges:@[]]; @@ -370,7 +378,7 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang /** Returns whether the doc for the given key should be in limbo. */ - (BOOL)shouldBeLimboDocumentKey:(const DocumentKey &)key { // If the remote end says it's part of this query, it's not in limbo. - if ([self.syncedDocuments containsObject:key]) { + if (_syncedDocuments.contains(key)) { return NO; } // The local store doesn't think it's a result, so it shouldn't be in limbo. @@ -395,16 +403,14 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang if (targetChange) { FSTTargetMapping *targetMapping = targetChange.mapping; if ([targetMapping isKindOfClass:[FSTResetMapping class]]) { - self.syncedDocuments = ((FSTResetMapping *)targetMapping).documents; + _syncedDocuments = ((FSTResetMapping *)targetMapping).documents; } else if ([targetMapping isKindOfClass:[FSTUpdateMapping class]]) { - [((FSTUpdateMapping *)targetMapping).addedDocuments - enumerateObjectsUsingBlock:^(FSTDocumentKey *key, BOOL *stop) { - self.syncedDocuments = [self.syncedDocuments setByAddingObject:key]; - }]; - [((FSTUpdateMapping *)targetMapping).removedDocuments - enumerateObjectsUsingBlock:^(FSTDocumentKey *key, BOOL *stop) { - self.syncedDocuments = [self.syncedDocuments setByRemovingObject:key]; - }]; + for (const DocumentKey &key : ((FSTUpdateMapping *)targetMapping).addedDocuments) { + _syncedDocuments = _syncedDocuments.insert(key); + } + for (const DocumentKey &key : ((FSTUpdateMapping *)targetMapping).removedDocuments) { + _syncedDocuments = _syncedDocuments.erase(key); + } } switch (targetChange.currentStatusUpdate) { @@ -428,29 +434,29 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang } // TODO(klimt): Do this incrementally so that it's not quadratic when updating many documents. - FSTDocumentKeySet *oldLimboDocuments = self.limboDocuments; - self.limboDocuments = [FSTDocumentKeySet keySet]; + DocumentKeySet oldLimboDocuments = std::move(_limboDocuments); + _limboDocuments = DocumentKeySet{}; for (FSTDocument *doc in self.documentSet.documentEnumerator) { if ([self shouldBeLimboDocumentKey:doc.key]) { - self.limboDocuments = [self.limboDocuments setByAddingObject:doc.key]; + _limboDocuments = _limboDocuments.insert(doc.key); } } // Diff the new limbo docs with the old limbo docs. NSMutableArray<FSTLimboDocumentChange *> *changes = - [NSMutableArray arrayWithCapacity:(oldLimboDocuments.count + self.limboDocuments.count)]; - [oldLimboDocuments enumerateObjectsUsingBlock:^(FSTDocumentKey *key, BOOL *stop) { - if (![self.limboDocuments containsObject:key]) { + [NSMutableArray arrayWithCapacity:(oldLimboDocuments.size() + _limboDocuments.size())]; + for (const DocumentKey &key : oldLimboDocuments) { + if (!_limboDocuments.contains(key)) { [changes addObject:[FSTLimboDocumentChange changeWithType:FSTLimboDocumentChangeTypeRemoved key:key]]; } - }]; - [self.limboDocuments enumerateObjectsUsingBlock:^(FSTDocumentKey *key, BOOL *stop) { - if (![oldLimboDocuments containsObject:key]) { + } + for (const DocumentKey &key : _limboDocuments) { + if (!oldLimboDocuments.contains(key)) { [changes addObject:[FSTLimboDocumentChange changeWithType:FSTLimboDocumentChangeTypeAdded key:key]]; } - }]; + } return changes; } |