From abbd403cc917b596bf6c7823317f6497794fb746 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 2 Dec 2018 17:20:40 -0500 Subject: [PATCH 01/16] Initial --- .../Benchmarks/FSTLevelDBBenchmarkTests.mm | 18 ++-- .../Firestore.xcodeproj/project.pbxproj | 8 +- Firestore/Example/Tests/API/FSTAPIHelpers.mm | 1 - Firestore/Example/Tests/Core/FSTQueryTests.mm | 1 - Firestore/Example/Tests/Core/FSTViewTests.mm | 1 - .../Tests/Integration/FSTDatastoreTests.mm | 5 +- .../Tests/Local/FSTLevelDBMigrationsTests.mm | 4 +- .../Tests/Local/FSTLocalSerializerTests.mm | 1 - .../Example/Tests/Local/FSTLocalStoreTests.mm | 91 ++++++++++--------- .../Tests/Local/FSTReferenceSetTests.mm | 13 ++- .../Local/FSTRemoteDocumentCacheTests.mm | 39 ++++++-- .../Tests/Model/FSTDocumentKeyTests.mm | 40 ++------ .../Example/Tests/Model/FSTDocumentTests.mm | 2 +- .../Tests/Remote/FSTRemoteEventTests.mm | 1 - .../Example/Tests/SpecTests/FSTSpecTests.mm | 17 ++-- .../Tests/SpecTests/FSTSyncEngineTestDriver.h | 10 +- .../SpecTests/FSTSyncEngineTestDriver.mm | 13 ++- Firestore/Example/Tests/Util/FSTHelpers.h | 29 +----- Firestore/Example/Tests/Util/FSTHelpers.mm | 14 +-- Firestore/Source/API/FIRDocumentSnapshot.mm | 4 +- Firestore/Source/Core/FSTFirestoreClient.mm | 3 +- Firestore/Source/Core/FSTQuery.mm | 6 +- Firestore/Source/Core/FSTSyncEngine.mm | 23 +++-- Firestore/Source/Core/FSTView.h | 13 ++- Firestore/Source/Core/FSTView.mm | 25 ++--- Firestore/Source/Core/FSTViewSnapshot.mm | 66 +++++++------- .../Source/Local/FSTDocumentReference.mm | 4 +- .../Local/FSTLevelDBRemoteDocumentCache.mm | 9 +- .../Source/Local/FSTLocalDocumentsView.h | 7 +- .../Source/Local/FSTLocalDocumentsView.mm | 50 +++++----- Firestore/Source/Local/FSTLocalStore.h | 16 ++-- Firestore/Source/Local/FSTLocalStore.mm | 30 +++--- Firestore/Source/Local/FSTLocalWriteResult.h | 8 +- Firestore/Source/Local/FSTLocalWriteResult.mm | 21 +++-- .../Source/Local/FSTMemoryMutationQueue.mm | 4 +- .../Local/FSTMemoryRemoteDocumentCache.h | 2 - .../Local/FSTMemoryRemoteDocumentCache.mm | 74 +++++++-------- Firestore/Source/Local/FSTPersistence.h | 1 - .../Source/Local/FSTRemoteDocumentCache.h | 8 +- Firestore/Source/Model/FSTDocument.mm | 2 +- .../Source/Model/FSTDocumentDictionary.h | 44 --------- .../Source/Model/FSTDocumentDictionary.mm | 42 --------- Firestore/Source/Model/FSTDocumentKey.h | 36 +------- Firestore/Source/Model/FSTDocumentKey.mm | 51 +---------- Firestore/Source/Model/FSTDocumentSet.h | 9 +- Firestore/Source/Model/FSTDocumentSet.mm | 62 ++++++------- Firestore/Source/Model/FSTFieldValue.mm | 5 +- Firestore/Source/Remote/FSTRemoteEvent.h | 2 - Firestore/Source/Remote/FSTRemoteEvent.mm | 8 +- Firestore/Source/Remote/FSTSerializerBeta.mm | 3 +- Firestore/Source/Remote/FSTWatchChange.mm | 2 +- .../firebase/firestore/model/document_key.h | 20 +++- .../firebase/firestore/model/document_map.h | 44 +++++++++ 53 files changed, 463 insertions(+), 549 deletions(-) delete mode 100644 Firestore/Source/Model/FSTDocumentDictionary.h delete mode 100644 Firestore/Source/Model/FSTDocumentDictionary.mm create mode 100644 Firestore/core/src/firebase/firestore/model/document_map.h diff --git a/Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm b/Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm index 7fbf78ccd73..0dde7a42839 100644 --- a/Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm +++ b/Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm @@ -22,19 +22,23 @@ #import "Firestore/Source/Local/FSTLevelDB.h" #import "Firestore/Source/Local/FSTLocalSerializer.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Remote/FSTSerializerBeta.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_key.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h" +#include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/types.h" +#include "Firestore/core/src/firebase/firestore/util/string_format.h" NS_ASSUME_NONNULL_BEGIN +using firebase::firestore::local::LevelDbRemoteDocumentKey; using firebase::firestore::local::LevelDbTargetDocumentKey; using firebase::firestore::local::LevelDbTransaction; using firebase::firestore::model::DatabaseId; +using firebase::firestore::model::DocumentKey; using firebase::firestore::model::TargetId; +using firebase::firestore::util::StringFormat; namespace { @@ -100,9 +104,8 @@ void FillDB() { LevelDbTransaction txn(db_.ptr, "benchmark"); for (int i = 0; i < numDocuments_; i++) { - FSTDocumentKey *docKey = - [FSTDocumentKey keyWithPathString:[NSString stringWithFormat:@"docs/doc_%i", i]]; - std::string docKeyString = [FSTLevelDBRemoteDocumentKey keyWithDocumentKey:docKey]; + auto docKey = DocumentKey::FromPathString(StringFormat(docs / doc_ % i, i)); + std::string docKeyString = LevelDbRemoteDocumentKey::Key(docKey); txn.Put(docKeyString, DocumentData()); WriteIndex(txn, docKey); } @@ -112,7 +115,7 @@ void FillDB() { } protected: - void WriteIndex(LevelDbTransaction &txn, FSTDocumentKey *docKey) { + void WriteIndex(LevelDbTransaction &txn, const DocumentKey &docKey) { // Arbitrary target ID TargetId targetID = 1; txn.Put(LevelDbDocumentTargetKey::Key(docKey, targetID), emptyBuffer_); @@ -136,10 +139,9 @@ void WriteIndex(LevelDbTransaction &txn, FSTDocumentKey *docKey) { for (const auto &_ : state) { LevelDbTransaction txn(db_.ptr, "benchmark"); for (int i = 0; i < docsToUpdate; i++) { - FSTDocumentKey *docKey = - [FSTDocumentKey keyWithPathString:[NSString stringWithFormat:@"docs/doc_%i", i]]; + auto docKey = DocumentKey::FromPathString(StringFormat(docs / doc_ % i, i)); if (writeIndexes) WriteIndex(txn, docKey); - std::string docKeyString = [FSTLevelDBRemoteDocumentKey keyWithDocumentKey:docKey]; + std::string docKeyString = LevelDbRemoteDocumentKey::Key(docKey); txn.Put(docKeyString, documentUpdate); } txn.Commit(); diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index b228b8707a1..edbdbad2c49 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -1526,11 +1526,11 @@ ); inputPaths = ( "${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS-resources.sh", - "${PODS_CONFIGURATION_BUILD_DIR}/FirebaseFirestore/gRPCCertificates.bundle", + "${PODS_CONFIGURATION_BUILD_DIR}/FirebaseFirestore/gRPCCertificates-Firestore.bundle", ); name = "[CP] Copy Pods Resources"; outputPaths = ( - "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/gRPCCertificates.bundle", + "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/gRPCCertificates-Firestore.bundle", ); runOnlyForDeploymentPostprocessing = 0; shellPath = /bin/sh; @@ -1785,11 +1785,11 @@ ); inputPaths = ( "${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS-resources.sh", - "${PODS_CONFIGURATION_BUILD_DIR}/FirebaseFirestore/gRPCCertificates.bundle", + "${PODS_CONFIGURATION_BUILD_DIR}/FirebaseFirestore/gRPCCertificates-Firestore.bundle", ); name = "[CP] Copy Pods Resources"; outputPaths = ( - "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/gRPCCertificates.bundle", + "${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/gRPCCertificates-Firestore.bundle", ); runOnlyForDeploymentPostprocessing = 0; shellPath = /bin/sh; diff --git a/Firestore/Example/Tests/API/FSTAPIHelpers.mm b/Firestore/Example/Tests/API/FSTAPIHelpers.mm index 5e5c41314de..8e783c364b2 100644 --- a/Firestore/Example/Tests/API/FSTAPIHelpers.mm +++ b/Firestore/Example/Tests/API/FSTAPIHelpers.mm @@ -31,7 +31,6 @@ #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Core/FSTViewSnapshot.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Model/FSTDocumentSet.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" diff --git a/Firestore/Example/Tests/Core/FSTQueryTests.mm b/Firestore/Example/Tests/Core/FSTQueryTests.mm index 9ad357eb981..7e08d34581d 100644 --- a/Firestore/Example/Tests/Core/FSTQueryTests.mm +++ b/Firestore/Example/Tests/Core/FSTQueryTests.mm @@ -20,7 +20,6 @@ #import "Firestore/Source/API/FIRFirestore+Internal.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Example/Tests/Util/FSTHelpers.h" diff --git a/Firestore/Example/Tests/Core/FSTViewTests.mm b/Firestore/Example/Tests/Core/FSTViewTests.mm index 9ca3cea1a75..770028ffaeb 100644 --- a/Firestore/Example/Tests/Core/FSTViewTests.mm +++ b/Firestore/Example/Tests/Core/FSTViewTests.mm @@ -22,7 +22,6 @@ #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Core/FSTViewSnapshot.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Model/FSTDocumentSet.h" #import "Firestore/Source/Model/FSTFieldValue.h" #import "Firestore/Source/Remote/FSTRemoteEvent.h" diff --git a/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm b/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm index b0140bb6b76..dbcb66d14c0 100644 --- a/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm +++ b/Firestore/Example/Tests/Integration/FSTDatastoreTests.mm @@ -26,7 +26,6 @@ #import "Firestore/Source/Core/FSTFirestoreClient.h" #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTQueryData.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Model/FSTFieldValue.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Model/FSTMutationBatch.h" @@ -39,6 +38,7 @@ #include "Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h" #include "Firestore/core/src/firebase/firestore/core/database_info.h" #include "Firestore/core/src/firebase/firestore/model/database_id.h" +#include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/precondition.h" #include "Firestore/core/src/firebase/firestore/util/async_queue.h" #include "Firestore/core/src/firebase/firestore/util/executor_libdispatch.h" @@ -51,6 +51,7 @@ using firebase::firestore::core::DatabaseInfo; using firebase::firestore::model::BatchId; using firebase::firestore::model::DatabaseId; +using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::Precondition; using firebase::firestore::model::TargetId; @@ -243,7 +244,7 @@ - (void)awaitExpectations { - (FSTSetMutation *)setMutation { return [[FSTSetMutation alloc] - initWithKey:[FSTDocumentKey keyWithPathString:@"rooms/eros"] + initWithKey:DocumentKey::FromPathString("rooms/eros") value:[[FSTObjectValue alloc] initWithDictionary:@{@"name" : [FSTStringValue stringValue:@"Eros"]}] precondition:Precondition::None()]; diff --git a/Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm b/Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm index d1cd59ba6a3..6b92bb769b0 100644 --- a/Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm +++ b/Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm @@ -130,8 +130,8 @@ - (void)testDropsTheQueryCache { BatchId batchID = 1; TargetId targetID = 2; - FSTDocumentKey *key1 = Key("documents/1"); - FSTDocumentKey *key2 = Key("documents/2"); + DocumentKey key1 = Key("documents/1"); + DocumentKey key2 = Key("documents/2"); std::string targetKeys[] = { LevelDbTargetKey::Key(targetID), diff --git a/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm b/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm index 284bd2d12af..ca868f19397 100644 --- a/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm +++ b/Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm @@ -31,7 +31,6 @@ #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTQueryData.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Model/FSTFieldValue.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Model/FSTMutationBatch.h" diff --git a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm index e5399fb8e31..34b372fb17b 100644 --- a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm +++ b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm @@ -25,7 +25,6 @@ #import "Firestore/Source/Local/FSTQueryCache.h" #import "Firestore/Source/Local/FSTQueryData.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Model/FSTDocumentSet.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Model/FSTMutationBatch.h" @@ -40,15 +39,26 @@ #import "Firestore/third_party/Immutable/Tests/FSTImmutableSortedSet+Testing.h" #include "Firestore/core/src/firebase/firestore/auth/user.h" +#include "Firestore/core/src/firebase/firestore/model/document_map.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" namespace testutil = firebase::firestore::testutil; using firebase::firestore::auth::User; +using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::ListenSequenceNumber; +using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::SnapshotVersion; using firebase::firestore::model::TargetId; +static NSArray *docMapToArray(const MaybeDocumentMap &docs) { + NSMutableArray *result = [NSMutableArray array]; + for (const auto &kv : docs) { + [result addObject:kv.second]; + } + return result; +} + NS_ASSUME_NONNULL_BEGIN @interface FSTLocalStoreTests () @@ -57,12 +67,13 @@ @interface FSTLocalStoreTests () @property(nonatomic, strong, readwrite) FSTLocalStore *localStore; @property(nonatomic, strong, readonly) NSMutableArray *batches; -@property(nonatomic, strong, readwrite, nullable) FSTMaybeDocumentDictionary *lastChanges; @property(nonatomic, assign, readwrite) TargetId lastTargetID; @end -@implementation FSTLocalStoreTests +@implementation FSTLocalStoreTests { + MaybeDocumentMap _lastChanges; +} - (void)setUp { [super setUp]; @@ -78,7 +89,6 @@ - (void)setUp { [self.localStore start]; _batches = [NSMutableArray array]; - _lastChanges = nil; _lastTargetID = 0; } @@ -115,11 +125,11 @@ - (void)writeMutations:(NSArray *)mutations { [self.batches addObject:[[FSTMutationBatch alloc] initWithBatchID:result.batchID localWriteTime:[FIRTimestamp timestamp] mutations:mutations]]; - self.lastChanges = result.changes; + _lastChanges = result.changes; } - (void)applyRemoteEvent:(FSTRemoteEvent *)event { - self.lastChanges = [self.localStore applyRemoteEvent:event]; + _lastChanges = [self.localStore applyRemoteEvent:event]; } - (void)notifyLocalViewChanges:(FSTLocalViewChanges *)changes { @@ -137,13 +147,13 @@ - (void)acknowledgeMutationWithVersion:(FSTTestSnapshotVersion)documentVersion { commitVersion:version mutationResults:@[ mutationResult ] streamToken:nil]; - self.lastChanges = [self.localStore acknowledgeBatchWithResult:result]; + _lastChanges = [self.localStore acknowledgeBatchWithResult:result]; } - (void)rejectMutation { FSTMutationBatch *batch = [self.batches firstObject]; [self.batches removeObjectAtIndex:0]; - self.lastChanges = [self.localStore rejectBatchID:batch.batchID]; + _lastChanges = [self.localStore rejectBatchID:batch.batchID]; } - (TargetId)allocateQuery:(FSTQuery *)query { @@ -159,37 +169,34 @@ - (TargetId)allocateQuery:(FSTQuery *)query { } while (0) /** Asserts that a the lastChanges contain the docs in the given array. */ -#define FSTAssertChanged(documents) \ - XCTAssertNotNil(self.lastChanges); \ - do { \ - FSTMaybeDocumentDictionary *actual = self.lastChanges; \ - NSArray *expected = (documents); \ - XCTAssertEqual(actual.count, expected.count); \ - NSEnumerator *enumerator = expected.objectEnumerator; \ - [actual enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey * key, FSTMaybeDocument * value, \ - BOOL * stop) { \ - XCTAssertEqualObjects(value, [enumerator nextObject]); \ - }]; \ - self.lastChanges = nil; \ +#define FSTAssertChanged(documents) \ + do { \ + NSArray *expected = (documents); \ + XCTAssertEqual(_lastChanges.size(), expected.count); \ + NSEnumerator *enumerator = expected.objectEnumerator; \ + for (const auto &kv : _lastChanges) { \ + FSTMaybeDocument *value = kv.second; \ + XCTAssertEqualObjects(value, [enumerator nextObject]); \ + } \ + _lastChanges = MaybeDocumentMap{}; \ } while (0) /** Asserts that the given keys were removed. */ -#define FSTAssertRemoved(keyPaths) \ - XCTAssertNotNil(self.lastChanges); \ - do { \ - FSTMaybeDocumentDictionary *actual = self.lastChanges; \ - XCTAssertEqual(actual.count, keyPaths.count); \ - NSEnumerator *keyPathEnumerator = keyPaths.objectEnumerator; \ - [actual enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey * actualKey, \ - FSTMaybeDocument * value, BOOL * stop) { \ - FSTDocumentKey *expectedKey = FSTTestDocKey([keyPathEnumerator nextObject]); \ - XCTAssertEqualObjects(actualKey, expectedKey); \ - XCTAssertTrue([value isKindOfClass:[FSTDeletedDocument class]]); \ - }]; \ - self.lastChanges = nil; \ +#define FSTAssertRemoved(keyPaths) \ + do { \ + XCTAssertEqual(_lastChanges.size(), keyPaths.count); \ + NSEnumerator *keyPathEnumerator = keyPaths.objectEnumerator; \ + for (const auto &kv : _lastChanges) { \ + const DocumentKey &actualKey = kv.first; \ + FSTMaybeDocument *value = kv.second; \ + DocumentKey expectedKey = FSTTestDocKey([keyPathEnumerator nextObject]); \ + XCTAssertEqual(actualKey, expectedKey); \ + XCTAssertTrue([value isKindOfClass:[FSTDeletedDocument class]]); \ + } \ + _lastChanges = MaybeDocumentMap{}; \ } while (0) -/** Asserts that the given local store contains the given document. */ +/** Asserts that the given local store contains the given document.*/ #define FSTAssertContains(document) \ do { \ FSTMaybeDocument *expected = (document); \ @@ -200,7 +207,7 @@ - (TargetId)allocateQuery:(FSTQuery *)query { /** Asserts that the given local store does not contain the given document. */ #define FSTAssertNotContains(keyPathString) \ do { \ - FSTDocumentKey *key = FSTTestDocKey(keyPathString); \ + DocumentKey key = FSTTestDocKey(keyPathString); \ FSTMaybeDocument *actual = [self.localStore readDocument:key]; \ XCTAssertNil(actual); \ } while (0) @@ -834,9 +841,9 @@ - (void)testCanExecuteDocumentQueries { FSTTestSetMutation(@"foo/bar/Foo/Bar", @{@"Foo" : @"Bar"}) ]]; FSTQuery *query = FSTTestQuery("foo/bar"); - FSTDocumentDictionary *docs = [self.localStore executeQuery:query]; - XCTAssertEqualObjects([docs values], @[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, - FSTDocumentStateLocalMutations) ]); + MaybeDocumentMap docs = [self.localStore executeQuery:query]; + XCTAssertEqualObjects(docMapToArray(docs), @[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, + FSTDocumentStateLocalMutations) ]); } - (void)testCanExecuteCollectionQueries { @@ -850,9 +857,9 @@ - (void)testCanExecuteCollectionQueries { FSTTestSetMutation(@"fooo/blah", @{@"fooo" : @"blah"}) ]]; FSTQuery *query = FSTTestQuery("foo"); - FSTDocumentDictionary *docs = [self.localStore executeQuery:query]; + MaybeDocumentMap docs = [self.localStore executeQuery:query]; XCTAssertEqualObjects( - [docs values], (@[ + docMapToArray(docs), (@[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, FSTDocumentStateLocalMutations), FSTTestDoc("foo/baz", 0, @{@"foo" : @"baz"}, FSTDocumentStateLocalMutations) ])); @@ -874,8 +881,8 @@ - (void)testCanExecuteMixedCollectionQueries { [self.localStore locallyWriteMutations:@[ FSTTestSetMutation(@"foo/bonk", @{@"a" : @"b"}) ]]; - FSTDocumentDictionary *docs = [self.localStore executeQuery:query]; - XCTAssertEqualObjects([docs values], (@[ + MaybeDocumentMap docs = [self.localStore executeQuery:query]; + XCTAssertEqualObjects(docMapToArray(docs), (@[ FSTTestDoc("foo/bar", 20, @{@"a" : @"b"}, FSTDocumentStateSynced), FSTTestDoc("foo/baz", 10, @{@"a" : @"b"}, FSTDocumentStateSynced), FSTTestDoc("foo/bonk", 0, @{@"a" : @"b"}, FSTDocumentStateLocalMutations) diff --git a/Firestore/Example/Tests/Local/FSTReferenceSetTests.mm b/Firestore/Example/Tests/Local/FSTReferenceSetTests.mm index 802117a9eb7..c4e4f7cdbb5 100644 --- a/Firestore/Example/Tests/Local/FSTReferenceSetTests.mm +++ b/Firestore/Example/Tests/Local/FSTReferenceSetTests.mm @@ -19,7 +19,10 @@ #import #import "Firestore/Example/Tests/Util/FSTHelpers.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" + +#include "Firestore/core/src/firebase/firestore/model/document_key.h" + +using firebase::firestore::model::DocumentKey; NS_ASSUME_NONNULL_BEGIN @@ -29,7 +32,7 @@ @interface FSTReferenceSetTests : XCTestCase @implementation FSTReferenceSetTests - (void)testAddOrRemoveReferences { - FSTDocumentKey *key = FSTTestDocKey(@"foo/bar"); + DocumentKey key = FSTTestDocKey(@"foo/bar"); FSTReferenceSet *referenceSet = [[FSTReferenceSet alloc] init]; XCTAssertTrue([referenceSet isEmpty]); @@ -54,9 +57,9 @@ - (void)testAddOrRemoveReferences { } - (void)testRemoveAllReferencesForTargetID { - FSTDocumentKey *key1 = FSTTestDocKey(@"foo/bar"); - FSTDocumentKey *key2 = FSTTestDocKey(@"foo/baz"); - FSTDocumentKey *key3 = FSTTestDocKey(@"foo/blah"); + DocumentKey key1 = FSTTestDocKey(@"foo/bar"); + DocumentKey key2 = FSTTestDocKey(@"foo/baz"); + DocumentKey key3 = FSTTestDocKey(@"foo/blah"); FSTReferenceSet *referenceSet = [[FSTReferenceSet alloc] init]; [referenceSet addReferenceToKey:key1 forID:1]; diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm index 4714aa4c8dd..84d6028c44a 100644 --- a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm @@ -19,17 +19,22 @@ #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTPersistence.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Model/FSTDocumentSet.h" #import "Firestore/Example/Tests/Util/FSTHelpers.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/document_map.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" #include "absl/strings/string_view.h" namespace testutil = firebase::firestore::testutil; namespace util = firebase::firestore::util; +using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::MaybeDocumentMap; NS_ASSUME_NONNULL_BEGIN @@ -137,15 +142,13 @@ - (void)testDocumentsMatchingQuery { [self setTestDocumentAtPath:"c/1"]; FSTQuery *query = FSTTestQuery("b"); - FSTDocumentDictionary *results = [self.remoteDocumentCache documentsMatchingQuery:query]; - NSArray *expected = @[ - FSTTestDoc("b/1", kVersion, _kDocData, FSTDocumentStateSynced), - FSTTestDoc("b/2", kVersion, _kDocData, FSTDocumentStateSynced) - ]; - XCTAssertEqual([results count], [expected count]); - for (FSTDocument *doc in expected) { - XCTAssertEqualObjects([results objectForKey:doc.key], doc); - } + MaybeDocumentMap results = [self.remoteDocumentCache documentsMatchingQuery:query]; + [self expectMap:results + hasDocsInArray:@[ + FSTTestDoc("b/1", kVersion, _kDocData, FSTDocumentStateSynced), + FSTTestDoc("b/2", kVersion, _kDocData, FSTDocumentStateSynced) + ] + exactly:YES]; }); } @@ -158,6 +161,22 @@ - (FSTDocument *)setTestDocumentAtPath:(const absl::string_view)path { return doc; } +- (void)expectMap:(const MaybeDocumentMap &)map + hasDocsInArray:(NSArray *)expected + exactly:(BOOL)exactly { + if (exactly) { + XCTAssertEqual(map.size(), [expected count]); + } + for (FSTDocument *doc in expected) { + FSTDocument *actual = nil; + auto found = map.find(doc.key); + if (found != map.end()) { + actual = static_cast(found->second); + } + XCTAssertEqualObjects(actual, doc); + } +} + @end NS_ASSUME_NONNULL_END diff --git a/Firestore/Example/Tests/Model/FSTDocumentKeyTests.mm b/Firestore/Example/Tests/Model/FSTDocumentKeyTests.mm index 5e465f796f1..013f008e491 100644 --- a/Firestore/Example/Tests/Model/FSTDocumentKeyTests.mm +++ b/Firestore/Example/Tests/Model/FSTDocumentKeyTests.mm @@ -20,7 +20,9 @@ #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" +#include "Firestore/core/test/firebase/firestore/testutil/testutil.h" +using firebase::firestore::testutil::Key; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::ResourcePath; @@ -31,40 +33,12 @@ @interface FSTDocumentKeyTests : XCTestCase @implementation FSTDocumentKeyTests -- (void)testConstructor { - ResourcePath path{"rooms", "firestore", "messages", "1"}; - FSTDocumentKey *key = [FSTDocumentKey keyWithPath:path]; - XCTAssertEqual(path, key.path); -} - - (void)testComparison { - FSTDocumentKey *key1 = [FSTDocumentKey keyWithSegments:{"a", "b", "c", "d"}]; - FSTDocumentKey *key2 = [FSTDocumentKey keyWithSegments:{"a", "b", "c", "d"}]; - FSTDocumentKey *key3 = [FSTDocumentKey keyWithSegments:{"x", "y", "z", "w"}]; - XCTAssertTrue([key1 isEqualToKey:key2]); - XCTAssertFalse([key1 isEqualToKey:key3]); - - FSTDocumentKey *empty = [FSTDocumentKey keyWithSegments:{}]; - FSTDocumentKey *a = [FSTDocumentKey keyWithSegments:{"a", "a"}]; - FSTDocumentKey *b = [FSTDocumentKey keyWithSegments:{"b", "b"}]; - FSTDocumentKey *ab = [FSTDocumentKey keyWithSegments:{"a", "a", "b", "b"}]; - - XCTAssertEqual(NSOrderedAscending, [empty compare:a]); - XCTAssertEqual(NSOrderedAscending, [a compare:b]); - XCTAssertEqual(NSOrderedAscending, [a compare:ab]); - - XCTAssertEqual(NSOrderedDescending, [a compare:empty]); - XCTAssertEqual(NSOrderedDescending, [b compare:a]); - XCTAssertEqual(NSOrderedDescending, [ab compare:a]); -} - -- (void)testConverter { - const ResourcePath path{"rooms", "firestore", "messages", "1"}; - FSTDocumentKey *objcKey = [FSTDocumentKey keyWithPath:path]; - XCTAssertEqualObjects(objcKey, (FSTDocumentKey *)(DocumentKey{objcKey})); - - DocumentKey cpp_key{path}; - XCTAssertEqual(cpp_key, DocumentKey{(FSTDocumentKey *)(cpp_key)}); + FSTDocumentKey *key1 = [FSTDocumentKey keyWithDocumentKey:Key("a/b/c/d")]; + FSTDocumentKey *key2 = [FSTDocumentKey keyWithDocumentKey:Key("a/b/c/d")]; + FSTDocumentKey *key3 = [FSTDocumentKey keyWithDocumentKey:Key("x/y/z/w")]; + XCTAssertTrue([key1 isEqual:key2]); + XCTAssertFalse([key1 isEqual:key3]); } @end diff --git a/Firestore/Example/Tests/Model/FSTDocumentTests.mm b/Firestore/Example/Tests/Model/FSTDocumentTests.mm index 7867396c3a1..be919d8b08a 100644 --- a/Firestore/Example/Tests/Model/FSTDocumentTests.mm +++ b/Firestore/Example/Tests/Model/FSTDocumentTests.mm @@ -43,7 +43,7 @@ - (void)testConstructor { FSTDocument *doc = [FSTDocument documentWithData:data key:key version:version state:FSTDocumentStateSynced]; - XCTAssertEqualObjects(doc.key, FSTTestDocKey(@"messages/first")); + XCTAssertEqual(doc.key, FSTTestDocKey(@"messages/first")); XCTAssertEqual(doc.version, version); XCTAssertEqualObjects(doc.data, data); XCTAssertEqual(doc.hasLocalMutations, NO); diff --git a/Firestore/Example/Tests/Remote/FSTRemoteEventTests.mm b/Firestore/Example/Tests/Remote/FSTRemoteEventTests.mm index 948cafb8fd6..1e71ba8d144 100644 --- a/Firestore/Example/Tests/Remote/FSTRemoteEventTests.mm +++ b/Firestore/Example/Tests/Remote/FSTRemoteEventTests.mm @@ -21,7 +21,6 @@ #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTQueryData.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Remote/FSTExistenceFilter.h" #import "Firestore/Source/Remote/FSTWatchChange.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" diff --git a/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm b/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm index a95d5c3e84d..acbb31c6b41 100644 --- a/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm +++ b/Firestore/Example/Tests/SpecTests/FSTSpecTests.mm @@ -26,7 +26,6 @@ #import "Firestore/Source/Local/FSTPersistence.h" #import "Firestore/Source/Local/FSTQueryData.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Model/FSTFieldValue.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Remote/FSTExistenceFilter.h" @@ -39,6 +38,7 @@ #include "Firestore/core/src/firebase/firestore/auth/user.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/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/util/async_queue.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" @@ -50,6 +50,7 @@ namespace util = firebase::firestore::util; using firebase::firestore::auth::User; using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::SnapshotVersion; using firebase::firestore::model::TargetId; using firebase::firestore::util::TimerId; @@ -272,7 +273,7 @@ - (void)doWatchEntity:(NSDictionary *)watchEntity { } } else if (watchEntity[@"doc"]) { NSDictionary *docSpec = watchEntity[@"doc"]; - FSTDocumentKey *key = FSTTestDocKey(docSpec[@"key"]); + DocumentKey key = FSTTestDocKey(docSpec[@"key"]); FSTObjectValue *_Nullable value = [docSpec[@"value"] isKindOfClass:[NSNull class]] ? nil : FSTTestObjectValue(docSpec[@"value"]); @@ -291,7 +292,7 @@ - (void)doWatchEntity:(NSDictionary *)watchEntity { document:doc]; [self.driver receiveWatchChange:change snapshotVersion:SnapshotVersion::None()]; } else if (watchEntity[@"key"]) { - FSTDocumentKey *docKey = FSTTestDocKey(watchEntity[@"key"]); + DocumentKey docKey = FSTTestDocKey(watchEntity[@"key"]); FSTWatchChange *change = [[FSTDocumentWatchChange alloc] initWithUpdatedTargetIDs:@[] removedTargetIDs:watchEntity[@"removedTargets"] @@ -574,13 +575,13 @@ - (void)validateStateExpectations:(nullable NSDictionary *)expected { [expected[@"watchStreamRequestCount"] intValue]); } if (expected[@"limboDocs"]) { - NSMutableSet *expectedLimboDocuments = [NSMutableSet set]; + DocumentKeySet expectedLimboDocuments; NSArray *docNames = expected[@"limboDocs"]; for (NSString *name in docNames) { - [expectedLimboDocuments addObject:FSTTestDocKey(name)]; + expectedLimboDocuments = expectedLimboDocuments.insert(FSTTestDocKey(name)); } // Update the expected limbo documents - self.driver.expectedLimboDocuments = expectedLimboDocuments; + [self.driver setExpectedLimboDocuments:std::move(expectedLimboDocuments)]; } if (expected[@"activeTargets"]) { NSMutableDictionary *expectedActiveTargets = [NSMutableDictionary dictionary]; @@ -638,9 +639,9 @@ - (void)validateLimboDocuments { @"Found limbo doc without an expected active target"); } - for (FSTDocumentKey *expectedLimboDoc in self.driver.expectedLimboDocuments) { + for (const DocumentKey &expectedLimboDoc : self.driver.expectedLimboDocuments) { XCTAssert(actualLimboDocs.find(expectedLimboDoc) != actualLimboDocs.end(), - @"Expected doc to be in limbo, but was not: %@", expectedLimboDoc); + @"Expected doc to be in limbo, but was not: %s", expectedLimboDoc.ToString().c_str()); actualLimboDocs.erase(expectedLimboDoc); } XCTAssertTrue(actualLimboDocs.empty(), "%lu Unexpected docs in limbo, the first one is <%s, %d>", diff --git a/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.h b/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.h index 548947aa23e..4ba4fa7192c 100644 --- a/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.h +++ b/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.h @@ -23,6 +23,7 @@ #include "Firestore/core/src/firebase/firestore/auth/user.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/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/model/types.h" #include "Firestore/core/src/firebase/firestore/util/async_queue.h" @@ -265,6 +266,12 @@ typedef std::unordered_map) currentLimboDocuments; +/** The expected set of documents in limbo. */ +- (const firebase::firestore::model::DocumentKeySet &)expectedLimboDocuments; + +/** Sets the expected set of documents in limbo. */ +- (void)setExpectedLimboDocuments:(firebase::firestore::model::DocumentKeySet)docs; + /** * The writes that have been sent to the FSTSyncEngine via writeUserMutation: but not yet * acknowledged by calling receiveWriteAck/Error:. They are tracked per-user. @@ -286,9 +293,6 @@ typedef std::unordered_map *expectedLimboDocuments; - /** The set of active targets as observed on the watch stream. */ @property(nonatomic, strong, readonly) NSDictionary *activeTargets; diff --git a/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm b/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm index c6c63fe6f00..70d3307290c 100644 --- a/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm +++ b/Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm @@ -21,6 +21,7 @@ #include #include #include +#include #import "Firestore/Source/Core/FSTEventManager.h" #import "Firestore/Source/Core/FSTQuery.h" @@ -51,6 +52,7 @@ using firebase::firestore::core::DatabaseInfo; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::OnlineState; using firebase::firestore::model::SnapshotVersion; using firebase::firestore::model::TargetId; @@ -108,6 +110,7 @@ @implementation FSTSyncEngineTestDriver { // ivar is declared as mutable. std::unordered_map *, HashUser> _outstandingWrites; + DocumentKeySet _expectedLimboDocuments; DatabaseInfo _databaseInfo; User _currentUser; @@ -164,8 +167,6 @@ - (instancetype)initWithPersistence:(id)persistence _queryListeners = [NSMutableDictionary dictionary]; - _expectedLimboDocuments = [NSSet set]; - _expectedActiveTargets = [NSDictionary dictionary]; _currentUser = initialUser; @@ -181,6 +182,14 @@ - (instancetype)initWithPersistence:(id)persistence return _outstandingWrites; } +- (const DocumentKeySet &)expectedLimboDocuments { + return _expectedLimboDocuments; +} + +- (void)setExpectedLimboDocuments:(DocumentKeySet)docs { + _expectedLimboDocuments = std::move(docs); +} + - (void)drainQueue { _workerQueue->EnqueueBlocking([] {}); } diff --git a/Firestore/Example/Tests/Util/FSTHelpers.h b/Firestore/Example/Tests/Util/FSTHelpers.h index faeafe8a352..679b59196f9 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.h +++ b/Firestore/Example/Tests/Util/FSTHelpers.h @@ -20,9 +20,9 @@ #include #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentDictionary.h" #import "Firestore/Source/Remote/FSTRemoteEvent.h" +#include "Firestore/core/src/firebase/firestore/model/document_map.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" #include "Firestore/core/src/firebase/firestore/model/field_value.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" @@ -52,30 +52,11 @@ NS_ASSUME_NONNULL_BEGIN -#if __cplusplus -extern "C" { -#endif - #define FSTAssertIsKindOfClass(value, classType) \ do { \ XCTAssertEqualObjects([value class], [classType class]); \ } while (0); -/** - * Asserts that the given NSSet of FSTDocumentKeys contains exactly the given expected keys. - * This is a macro instead of a method so that the failure shows up on the right line. - * - * @param actualSet An NSSet of FSTDocumentKeys. - * @param expectedArray A sorted array of keys that actualSet must be equal to (after converting - * to an array and sorting). - */ -#define FSTAssertEqualSets(actualSet, expectedArray) \ - do { \ - NSArray *actual = [(actualSet)allObjects]; \ - actual = [actual sortedArrayUsingSelector:@selector(compare:)]; \ - XCTAssertEqualObjects(actual, (expectedArray)); \ - } while (0) - /** * Takes an array of "equality group" arrays and asserts that the compare: selector returns the * same as compare: on the indexes of the "equality groups" (NSOrderedSame for items in the same @@ -220,7 +201,7 @@ FSTFieldValue *FSTTestFieldValue(id _Nullable value); FSTObjectValue *FSTTestObjectValue(NSDictionary *data); /** A convenience method for creating document keys for tests. */ -FSTDocumentKey *FSTTestDocKey(NSString *path); +firebase::firestore::model::DocumentKey FSTTestDocKey(NSString *path); /** Allow tests to just use an int literal for versions. */ typedef int64_t FSTTestSnapshotVersion; @@ -292,7 +273,7 @@ FSTTransformMutation *FSTTestTransformMutation(NSString *path, NSDictionary *docs); +firebase::firestore::model::MaybeDocumentMap FSTTestDocUpdates(NSArray *docs); /** Creates a remote event that inserts a new document. */ FSTRemoteEvent *FSTTestAddedRemoteEvent(FSTMaybeDocument *doc, NSArray *addedToTargets); @@ -329,8 +310,4 @@ FSTTargetChange *FSTTestTargetChange(firebase::firestore::model::DocumentKeySet /** Creates a resume token to match the given snapshot version. */ NSData *_Nullable FSTTestResumeTokenFromSnapshotVersion(FSTTestSnapshotVersion watchSnapshot); -#if __cplusplus -} // extern "C" -#endif - NS_ASSUME_NONNULL_END diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index 965dc395aef..b9e6ca08050 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -34,7 +34,6 @@ #import "Firestore/Source/Local/FSTLocalViewChanges.h" #import "Firestore/Source/Local/FSTQueryData.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Model/FSTDocumentSet.h" #import "Firestore/Source/Model/FSTFieldValue.h" #import "Firestore/Source/Model/FSTMutation.h" @@ -64,6 +63,7 @@ using firebase::firestore::model::FieldPath; using firebase::firestore::model::FieldTransform; using firebase::firestore::model::FieldValue; +using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::Precondition; using firebase::firestore::model::ResourcePath; using firebase::firestore::model::ServerTimestampTransform; @@ -146,8 +146,8 @@ return (FSTObjectValue *)wrapped; } -FSTDocumentKey *FSTTestDocKey(NSString *path) { - return [FSTDocumentKey keyWithPathString:path]; +DocumentKey FSTTestDocKey(NSString *path) { + return DocumentKey::FromPathString(util::MakeString(path)); } FSTDocument *FSTTestDoc(const absl::string_view path, @@ -271,7 +271,7 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) { } FSTTransformMutation *FSTTestTransformMutation(NSString *path, NSDictionary *data) { - FSTDocumentKey *key = [FSTDocumentKey keyWithPath:testutil::Resource(util::MakeString(path))]; + DocumentKey key{testutil::Resource(util::MakeString(path))}; FSTUserDataConverter *converter = FSTTestUserDataConverter(); ParsedUpdateData result = [converter parsedUpdateData:data]; HARD_ASSERT(result.data().value.count == 0, @@ -284,10 +284,10 @@ NSComparator FSTTestDocComparator(const absl::string_view fieldPath) { [[FSTDeleteMutation alloc] initWithKey:FSTTestDocKey(path) precondition:Precondition::None()]; } -FSTMaybeDocumentDictionary *FSTTestDocUpdates(NSArray *docs) { - FSTMaybeDocumentDictionary *updates = [FSTMaybeDocumentDictionary maybeDocumentDictionary]; +MaybeDocumentMap FSTTestDocUpdates(NSArray *docs) { + MaybeDocumentMap updates; for (FSTMaybeDocument *doc in docs) { - updates = [updates dictionaryBySettingObject:doc forKey:doc.key]; + updates = updates.insert(doc.key, doc); } return updates; } diff --git a/Firestore/Source/API/FIRDocumentSnapshot.mm b/Firestore/Source/API/FIRDocumentSnapshot.mm index 042c07161ed..18f8747b330 100644 --- a/Firestore/Source/API/FIRDocumentSnapshot.mm +++ b/Firestore/Source/API/FIRDocumentSnapshot.mm @@ -228,8 +228,8 @@ - (id)convertedValue:(FSTFieldValue *)value options:(FSTFieldValueOptions *)opti refDatabase->database_id().c_str(), database->project_id().c_str(), database->database_id().c_str()); } - return [FIRDocumentReference referenceWithKey:[ref valueWithOptions:options] - firestore:self.firestore]; + DocumentKey key = [[ref valueWithOptions:options] key]; + return [FIRDocumentReference referenceWithKey:key firestore:self.firestore]; } else { return [value valueWithOptions:options]; } diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm index e82cae926b7..a1a512ff811 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.mm +++ b/Firestore/Source/Core/FSTFirestoreClient.mm @@ -56,6 +56,7 @@ using firebase::firestore::core::DatabaseInfo; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::OnlineState; using firebase::firestore::util::Path; using firebase::firestore::util::Status; @@ -321,7 +322,7 @@ - (void)getDocumentsFromLocalCache:(FIRQuery *)query completion:(void (^)(FIRQuerySnapshot *_Nullable query, NSError *_Nullable error))completion { _workerQueue->Enqueue([self, query, completion] { - FSTDocumentDictionary *docs = [self.localStore executeQuery:query.query]; + MaybeDocumentMap docs = [self.localStore executeQuery:query.query]; FSTView *view = [[FSTView alloc] initWithQuery:query.query remoteDocuments:DocumentKeySet{}]; FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:docs]; diff --git a/Firestore/Source/Core/FSTQuery.mm b/Firestore/Source/Core/FSTQuery.mm index e3b67217d62..f5f3e971652 100644 --- a/Firestore/Source/Core/FSTQuery.mm +++ b/Firestore/Source/Core/FSTQuery.mm @@ -188,7 +188,7 @@ - (BOOL)matchesDocument:(FSTDocument *)document { HARD_ASSERT(self.filterOperator != FSTRelationFilterOperatorArrayContains, "arrayContains queries don't make sense on document keys."); FSTReferenceValue *refValue = (FSTReferenceValue *)self.value; - NSComparisonResult comparison = FSTDocumentKeyComparator(document.key, refValue.value); + NSComparisonResult comparison = CompareKeys(document.key, refValue.value.key); return [self matchesComparison:comparison]; } else { return [self matchesValue:[document fieldForPath:self.field]]; @@ -381,7 +381,7 @@ - (instancetype)initWithFieldPath:(FieldPath)fieldPath ascending:(BOOL)ascending - (NSComparisonResult)compareDocument:(FSTDocument *)document1 toDocument:(FSTDocument *)document2 { NSComparisonResult result; if (_field == FieldPath::KeyFieldPath()) { - result = FSTDocumentKeyComparator(document1.key, document2.key); + result = CompareKeys(document1.key, document2.key); } else { FSTFieldValue *value1 = [document1 fieldForPath:self.field]; FSTFieldValue *value2 = [document2 fieldForPath:self.field]; @@ -475,7 +475,7 @@ - (BOOL)sortsBeforeDocument:(FSTDocument *)document HARD_ASSERT([fieldValue isKindOfClass:[FSTReferenceValue class]], "FSTBound has a non-key value where the key path is being used %s", fieldValue); FSTReferenceValue *refValue = (FSTReferenceValue *)fieldValue; - comparison = [refValue.value compare:document.key]; + comparison = CompareKeys(refValue.value.key, document.key); } else { FSTFieldValue *docValue = [document fieldForPath:sortOrderComponent.field]; HARD_ASSERT(docValue != nil, diff --git a/Firestore/Source/Core/FSTSyncEngine.mm b/Firestore/Source/Core/FSTSyncEngine.mm index c8b1d7fd4c2..451656e8113 100644 --- a/Firestore/Source/Core/FSTSyncEngine.mm +++ b/Firestore/Source/Core/FSTSyncEngine.mm @@ -39,6 +39,7 @@ #include "Firestore/core/src/firebase/firestore/auth/user.h" #include "Firestore/core/src/firebase/firestore/core/target_id_generator.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/document_map.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "Firestore/core/src/firebase/firestore/util/log.h" @@ -49,6 +50,8 @@ using firebase::firestore::model::BatchId; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::DocumentMap; +using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::ListenSequenceNumber; using firebase::firestore::model::OnlineState; using firebase::firestore::model::SnapshotVersion; @@ -212,7 +215,7 @@ - (TargetId)listenToQuery:(FSTQuery *)query { } - (FSTViewSnapshot *)initializeViewAndComputeSnapshotForQueryData:(FSTQueryData *)queryData { - FSTDocumentDictionary *docs = [self.localStore executeQuery:queryData.query]; + MaybeDocumentMap docs = [self.localStore executeQuery:queryData.query]; DocumentKeySet remoteKeys = [self.localStore remoteDocumentKeysForTarget:queryData.targetID]; FSTView *view = @@ -349,7 +352,7 @@ - (void)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent { } } - FSTMaybeDocumentDictionary *changes = [self.localStore applyRemoteEvent:remoteEvent]; + MaybeDocumentMap changes = [self.localStore applyRemoteEvent:remoteEvent]; [self emitNewSnapshotsAndNotifyLocalStoreWithChanges:changes remoteEvent:remoteEvent]; } @@ -419,17 +422,17 @@ - (void)applySuccessfulWriteWithResult:(FSTMutationBatchResult *)batchResult { // consistently happen before listen events. [self processUserCallbacksForBatchID:batchResult.batch.batchID error:nil]; - FSTMaybeDocumentDictionary *changes = [self.localStore acknowledgeBatchWithResult:batchResult]; + MaybeDocumentMap changes = [self.localStore acknowledgeBatchWithResult:batchResult]; [self emitNewSnapshotsAndNotifyLocalStoreWithChanges:changes remoteEvent:nil]; } - (void)rejectFailedWriteWithBatchID:(BatchId)batchID error:(NSError *)error { [self assertDelegateExistsForSelector:_cmd]; - FSTMaybeDocumentDictionary *changes = [self.localStore rejectBatchID:batchID]; + MaybeDocumentMap changes = [self.localStore rejectBatchID:batchID]; - if (!changes.isEmpty && [self errorIsInteresting:error]) { - LOG_WARN("Write at %s failed: %s", changes.minKey.path.CanonicalString(), - error.localizedDescription); + if (!changes.empty() && [self errorIsInteresting:error]) { + const DocumentKey &minKey = changes.min()->first; + LOG_WARN("Write at %s failed: %s", minKey.ToString(), error.localizedDescription); } // The local store may or may not be able to apply the write result and raise events immediately @@ -478,7 +481,7 @@ - (void)removeAndCleanupQuery:(FSTQueryView *)queryView { /** * Computes a new snapshot from the changes and calls the registered callback with the new snapshot. */ -- (void)emitNewSnapshotsAndNotifyLocalStoreWithChanges:(FSTMaybeDocumentDictionary *)changes +- (void)emitNewSnapshotsAndNotifyLocalStoreWithChanges:(const MaybeDocumentMap &)changes remoteEvent:(FSTRemoteEvent *_Nullable)remoteEvent { NSMutableArray *newSnapshots = [NSMutableArray array]; NSMutableArray *documentChangesInAllViews = [NSMutableArray array]; @@ -491,7 +494,7 @@ - (void)emitNewSnapshotsAndNotifyLocalStoreWithChanges:(FSTMaybeDocumentDictiona // The query has a limit and some docs were removed/updated, so we need to re-run the // query against the local store to make sure we didn't lose any good docs that had been // past the limit. - FSTDocumentDictionary *docs = [self.localStore executeQuery:queryView.query]; + MaybeDocumentMap docs = [self.localStore executeQuery:queryView.query]; viewDocChanges = [view computeChangesWithDocuments:docs previousChanges:viewDocChanges]; } @@ -587,7 +590,7 @@ - (void)credentialDidChangeWithUser:(const firebase::firestore::auth::User &)use if (userChanged) { // Notify local store and emit any resulting events from swapping out the mutation queue. - FSTMaybeDocumentDictionary *changes = [self.localStore userDidChange:user]; + MaybeDocumentMap changes = [self.localStore userDidChange:user]; [self emitNewSnapshotsAndNotifyLocalStoreWithChanges:changes remoteEvent:nil]; } diff --git a/Firestore/Source/Core/FSTView.h b/Firestore/Source/Core/FSTView.h index 1c7ab374a63..1a25cd4ea7d 100644 --- a/Firestore/Source/Core/FSTView.h +++ b/Firestore/Source/Core/FSTView.h @@ -16,15 +16,13 @@ #import -#import "Firestore/Source/Model/FSTDocumentDictionary.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/document_map.h" #include "Firestore/core/src/firebase/firestore/model/types.h" @class FSTDocumentSet; @class FSTDocumentViewChangeSet; -@class FSTMaybeDocument; @class FSTQuery; @class FSTTargetChange; @class FSTViewSnapshot; @@ -108,7 +106,8 @@ typedef NS_ENUM(NSInteger, FSTLimboDocumentChangeType) { * @param docChanges The doc changes to apply to this view. * @return a new set of docs, changes, and refill flag. */ -- (FSTViewDocumentChanges *)computeChangesWithDocuments:(FSTMaybeDocumentDictionary *)docChanges; +- (FSTViewDocumentChanges *)computeChangesWithDocuments: + (const firebase::firestore::model::MaybeDocumentMap &)docChanges; /** * Iterates over a set of doc changes, applies the query limit, and computes what the new results @@ -120,9 +119,9 @@ typedef NS_ENUM(NSInteger, FSTLimboDocumentChangeType) { * and changes instead of the current view. * @return a new set of docs, changes, and refill flag. */ -- (FSTViewDocumentChanges *)computeChangesWithDocuments:(FSTMaybeDocumentDictionary *)docChanges - previousChanges: - (nullable FSTViewDocumentChanges *)previousChanges; +- (FSTViewDocumentChanges *) + computeChangesWithDocuments:(const firebase::firestore::model::MaybeDocumentMap &)docChanges + previousChanges:(nullable FSTViewDocumentChanges *)previousChanges; /** * Updates the view with the given ViewDocumentChanges. diff --git a/Firestore/Source/Core/FSTView.mm b/Firestore/Source/Core/FSTView.mm index acd4114c567..2dc95d57eb7 100644 --- a/Firestore/Source/Core/FSTView.mm +++ b/Firestore/Source/Core/FSTView.mm @@ -30,6 +30,7 @@ using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::OnlineState; NS_ASSUME_NONNULL_BEGIN @@ -200,22 +201,21 @@ - (instancetype)initWithQuery:(FSTQuery *)query remoteDocuments:(DocumentKeySet) return _syncedDocuments; } -- (FSTViewDocumentChanges *)computeChangesWithDocuments:(FSTMaybeDocumentDictionary *)docChanges { +- (FSTViewDocumentChanges *)computeChangesWithDocuments:(const MaybeDocumentMap &)docChanges { return [self computeChangesWithDocuments:docChanges previousChanges:nil]; } -- (FSTViewDocumentChanges *)computeChangesWithDocuments:(FSTMaybeDocumentDictionary *)docChanges +- (FSTViewDocumentChanges *)computeChangesWithDocuments:(const MaybeDocumentMap &)docChanges previousChanges: (nullable FSTViewDocumentChanges *)previousChanges { FSTDocumentViewChangeSet *changeSet = previousChanges ? previousChanges.changeSet : [FSTDocumentViewChangeSet changeSet]; FSTDocumentSet *oldDocumentSet = previousChanges ? previousChanges.documentSet : self.documentSet; - __block DocumentKeySet newMutatedKeys = - previousChanges ? previousChanges.mutatedKeys : _mutatedKeys; - __block DocumentKeySet oldMutatedKeys = _mutatedKeys; - __block FSTDocumentSet *newDocumentSet = oldDocumentSet; - __block BOOL needsRefill = NO; + DocumentKeySet newMutatedKeys = previousChanges ? previousChanges.mutatedKeys : _mutatedKeys; + DocumentKeySet oldMutatedKeys = _mutatedKeys; + FSTDocumentSet *newDocumentSet = oldDocumentSet; + BOOL needsRefill = NO; // Track the last doc in a (full) limit. This is necessary, because some update (a delete, or an // update moving a doc past the old limit) might mean there is some other document in the local @@ -229,15 +229,17 @@ - (FSTViewDocumentChanges *)computeChangesWithDocuments:(FSTMaybeDocumentDiction (self.query.limit && oldDocumentSet.count == self.query.limit) ? oldDocumentSet.lastDocument : nil; - [docChanges enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey *key, - FSTMaybeDocument *maybeNewDoc, BOOL *stop) { + for (const auto &kv : docChanges) { + const DocumentKey &key = kv.first; + FSTMaybeDocument *maybeNewDoc = kv.second; + FSTDocument *_Nullable oldDoc = [oldDocumentSet documentForKey:key]; FSTDocument *_Nullable newDoc = nil; if ([maybeNewDoc isKindOfClass:[FSTDocument class]]) { newDoc = (FSTDocument *)maybeNewDoc; } if (newDoc) { - HARD_ASSERT([key isEqual:newDoc.key], "Mismatching key in document changes: %s != %s", key, + HARD_ASSERT(key == newDoc.key, "Mismatching key in document changes: %s != %s", key, newDoc.key.ToString()); if (![self.query matchesDocument:newDoc]) { newDoc = nil; @@ -307,7 +309,8 @@ - (FSTViewDocumentChanges *)computeChangesWithDocuments:(FSTMaybeDocumentDiction newMutatedKeys = newMutatedKeys.erase(key); } } - }]; + } + if (self.query.limit) { for (long i = newDocumentSet.count - self.query.limit; i > 0; --i) { FSTDocument *oldDoc = [newDocumentSet lastDocument]; diff --git a/Firestore/Source/Core/FSTViewSnapshot.mm b/Firestore/Source/Core/FSTViewSnapshot.mm index abfbd5f20b1..655d8459546 100644 --- a/Firestore/Source/Core/FSTViewSnapshot.mm +++ b/Firestore/Source/Core/FSTViewSnapshot.mm @@ -16,15 +16,24 @@ #import "Firestore/Source/Core/FSTViewSnapshot.h" +#include +#include + #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Model/FSTDocument.h" #import "Firestore/Source/Model/FSTDocumentSet.h" -#import "Firestore/third_party/Immutable/FSTImmutableSortedDictionary.h" +#include "Firestore/core/src/firebase/firestore/immutable/sorted_map.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" +#include "Firestore/core/src/firebase/firestore/util/string_apple.h" +#include "Firestore/core/src/firebase/firestore/util/string_format.h" +#include "absl/strings/str_join.h" +using firebase::firestore::immutable::SortedMap; using firebase::firestore::model::DocumentKey; +using firebase::firestore::util::WrapNSString; +using firebase::firestore::util::StringFormat; NS_ASSUME_NONNULL_BEGIN @@ -74,78 +83,71 @@ - (NSString *)description { #pragma mark - FSTDocumentViewChangeSet -@interface FSTDocumentViewChangeSet () - -/** The set of all changes tracked so far, with redundant changes merged. */ -@property(nonatomic, strong) - FSTImmutableSortedDictionary *changeMap; - -@end - -@implementation FSTDocumentViewChangeSet +@implementation FSTDocumentViewChangeSet { + /** The set of all changes tracked so far, with redundant changes merged. */ + SortedMap _changeMap; +} + (instancetype)changeSet { return [[FSTDocumentViewChangeSet alloc] init]; } -- (instancetype)init { - self = [super init]; - if (self) { - _changeMap = [FSTImmutableSortedDictionary dictionaryWithComparator:FSTDocumentKeyComparator]; - } - return self; -} - - (NSString *)description { - return [self.changeMap description]; + std::string result = absl::StrJoin( + _changeMap, ",", + [](std::string *out, const std::pair &kv) { + out->append(StringFormat("%s: %s", kv.first, kv.second)); + }); + return WrapNSString(std::string{"{"} + result + "}"); } - (void)addChange:(FSTDocumentViewChange *)change { const DocumentKey &key = change.document.key; - FSTDocumentViewChange *oldChange = [self.changeMap objectForKey:key]; - if (!oldChange) { - self.changeMap = [self.changeMap dictionaryBySettingObject:change forKey:key]; + auto oldChangeIter = _changeMap.find(key); + if (oldChangeIter == _changeMap.end()) { + _changeMap = _changeMap.insert(key, change); return; } + FSTDocumentViewChange *oldChange = oldChangeIter->second; // Merge the new change with the existing change. if (change.type != FSTDocumentViewChangeTypeAdded && oldChange.type == FSTDocumentViewChangeTypeMetadata) { - self.changeMap = [self.changeMap dictionaryBySettingObject:change forKey:key]; + _changeMap = _changeMap.insert(key, change); } else if (change.type == FSTDocumentViewChangeTypeMetadata && oldChange.type != FSTDocumentViewChangeTypeRemoved) { FSTDocumentViewChange *newChange = [FSTDocumentViewChange changeWithDocument:change.document type:oldChange.type]; - self.changeMap = [self.changeMap dictionaryBySettingObject:newChange forKey:key]; + _changeMap = _changeMap.insert(key, newChange); } else if (change.type == FSTDocumentViewChangeTypeModified && oldChange.type == FSTDocumentViewChangeTypeModified) { FSTDocumentViewChange *newChange = [FSTDocumentViewChange changeWithDocument:change.document type:FSTDocumentViewChangeTypeModified]; - self.changeMap = [self.changeMap dictionaryBySettingObject:newChange forKey:key]; + _changeMap = _changeMap.insert(key, newChange); } else if (change.type == FSTDocumentViewChangeTypeModified && oldChange.type == FSTDocumentViewChangeTypeAdded) { FSTDocumentViewChange *newChange = [FSTDocumentViewChange changeWithDocument:change.document type:FSTDocumentViewChangeTypeAdded]; - self.changeMap = [self.changeMap dictionaryBySettingObject:newChange forKey:key]; + _changeMap = _changeMap.insert(key, newChange); } else if (change.type == FSTDocumentViewChangeTypeRemoved && oldChange.type == FSTDocumentViewChangeTypeAdded) { - self.changeMap = [self.changeMap dictionaryByRemovingObjectForKey:key]; + _changeMap = _changeMap.erase(key); } else if (change.type == FSTDocumentViewChangeTypeRemoved && oldChange.type == FSTDocumentViewChangeTypeModified) { FSTDocumentViewChange *newChange = [FSTDocumentViewChange changeWithDocument:oldChange.document type:FSTDocumentViewChangeTypeRemoved]; - self.changeMap = [self.changeMap dictionaryBySettingObject:newChange forKey:key]; + _changeMap = _changeMap.insert(key, newChange); } else if (change.type == FSTDocumentViewChangeTypeAdded && oldChange.type == FSTDocumentViewChangeTypeRemoved) { FSTDocumentViewChange *newChange = [FSTDocumentViewChange changeWithDocument:change.document type:FSTDocumentViewChangeTypeModified]; - self.changeMap = [self.changeMap dictionaryBySettingObject:newChange forKey:key]; + _changeMap = _changeMap.insert(key, newChange); } else { // This includes these cases, which don't make sense: // Added -> Added @@ -160,10 +162,10 @@ - (void)addChange:(FSTDocumentViewChange *)change { - (NSArray *)changes { NSMutableArray *changes = [NSMutableArray array]; - [self.changeMap enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey *key, - FSTDocumentViewChange *change, BOOL *stop) { + for (const auto &kv : _changeMap) { + FSTDocumentViewChange *change = kv.second; [changes addObject:change]; - }]; + } return changes; } diff --git a/Firestore/Source/Local/FSTDocumentReference.mm b/Firestore/Source/Local/FSTDocumentReference.mm index 215801d511c..bdba50b2f4e 100644 --- a/Firestore/Source/Local/FSTDocumentReference.mm +++ b/Firestore/Source/Local/FSTDocumentReference.mm @@ -75,7 +75,7 @@ - (id)copyWithZone:(nullable NSZone *)zone { /** Sorts document references by key then ID. */ const NSComparator FSTDocumentReferenceComparatorByKey = ^NSComparisonResult(FSTDocumentReference *left, FSTDocumentReference *right) { - NSComparisonResult result = FSTDocumentKeyComparator(left.key, right.key); + NSComparisonResult result = CompareKeys(left.key, right.key); if (result != NSOrderedSame) { return result; } @@ -89,7 +89,7 @@ - (id)copyWithZone:(nullable NSZone *)zone { if (result != NSOrderedSame) { return result; } - return FSTDocumentKeyComparator(left.key, right.key); + return CompareKeys(left.key, right.key); }; NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm index 6258ff419ff..af81b3decd3 100644 --- a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm +++ b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm @@ -23,7 +23,6 @@ #import "Firestore/Source/Local/FSTLevelDB.h" #import "Firestore/Source/Local/FSTLocalSerializer.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentDictionary.h" #import "Firestore/Source/Model/FSTDocumentSet.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_key.h" @@ -38,6 +37,8 @@ using firebase::firestore::local::LevelDbRemoteDocumentKey; using firebase::firestore::local::LevelDbTransaction; using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::MaybeDocumentMap; using leveldb::DB; using leveldb::Status; @@ -83,8 +84,8 @@ - (nullable FSTMaybeDocument *)entryForKey:(const DocumentKey &)documentKey { } } -- (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query { - FSTDocumentDictionary *results = [FSTDocumentDictionary documentDictionary]; +- (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { + MaybeDocumentMap results; // Documents are ordered by key, so we can use a prefix scan to narrow down // the documents we need to match the query against. @@ -99,7 +100,7 @@ - (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query { if (!query.path.IsPrefixOf(maybeDoc.key.path())) { break; } else if ([maybeDoc isKindOfClass:[FSTDocument class]]) { - results = [results dictionaryBySettingObject:(FSTDocument *)maybeDoc forKey:maybeDoc.key]; + results = results.insert(maybeDoc.key, maybeDoc); } } diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.h b/Firestore/Source/Local/FSTLocalDocumentsView.h index bb5bb2215c3..2b54471f9db 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.h +++ b/Firestore/Source/Local/FSTLocalDocumentsView.h @@ -16,10 +16,9 @@ #import -#import "Firestore/Source/Model/FSTDocumentDictionary.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/document_map.h" @class FSTMaybeDocument; @class FSTQuery; @@ -53,11 +52,11 @@ NS_ASSUME_NONNULL_BEGIN * If we don't have cached state for a document in `keys`, a FSTDeletedDocument will be stored * for that key in the resulting set. */ -- (FSTMaybeDocumentDictionary *)documentsForKeys: +- (firebase::firestore::model::MaybeDocumentMap)documentsForKeys: (const firebase::firestore::model::DocumentKeySet &)keys; /** Performs a query against the local view of all documents. */ -- (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query; +- (firebase::firestore::model::MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query; @end diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index f25dc6142cd..ec7ff97cf18 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -20,19 +20,20 @@ #import "Firestore/Source/Local/FSTMutationQueue.h" #import "Firestore/Source/Local/FSTRemoteDocumentCache.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentDictionary.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Model/FSTMutationBatch.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/document_map.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::ResourcePath; using firebase::firestore::model::SnapshotVersion; -using firebase::firestore::model::DocumentKeySet; NS_ASSUME_NONNULL_BEGIN @@ -78,8 +79,8 @@ - (nullable FSTMaybeDocument *)documentForKey:(const DocumentKey &)key return document; } -- (FSTMaybeDocumentDictionary *)documentsForKeys:(const DocumentKeySet &)keys { - FSTMaybeDocumentDictionary *results = [FSTMaybeDocumentDictionary maybeDocumentDictionary]; +- (MaybeDocumentMap)documentsForKeys:(const DocumentKeySet &)keys { + MaybeDocumentMap results; NSArray *batches = [self.mutationQueue allMutationBatchesAffectingDocumentKeys:keys]; for (const DocumentKey &key : keys) { @@ -91,13 +92,13 @@ - (FSTMaybeDocumentDictionary *)documentsForKeys:(const DocumentKeySet &)keys { version:SnapshotVersion::None() hasCommittedMutations:NO]; } - results = [results dictionaryBySettingObject:maybeDoc forKey:key]; + results = results.insert(key, maybeDoc); } return results; } -- (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query { +- (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { if (DocumentKey::IsDocumentKey(query.path)) { return [self documentsMatchingDocumentQuery:query.path]; } else { @@ -105,18 +106,18 @@ - (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query { } } -- (FSTDocumentDictionary *)documentsMatchingDocumentQuery:(const ResourcePath &)docPath { - FSTDocumentDictionary *result = [FSTDocumentDictionary documentDictionary]; +- (MaybeDocumentMap)documentsMatchingDocumentQuery:(const ResourcePath &)docPath { + MaybeDocumentMap result; // Just do a simple document lookup. FSTMaybeDocument *doc = [self documentForKey:DocumentKey{docPath}]; if ([doc isKindOfClass:[FSTDocument class]]) { - result = [result dictionaryBySettingObject:(FSTDocument *)doc forKey:doc.key]; + result = result.insert(doc.key, doc); } return result; } -- (FSTDocumentDictionary *)documentsMatchingCollectionQuery:(FSTQuery *)query { - __block FSTDocumentDictionary *results = [self.remoteDocumentCache documentsMatchingQuery:query]; +- (MaybeDocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { + MaybeDocumentMap results = [self.remoteDocumentCache documentsMatchingQuery:query]; // Get locally persisted mutation batches. NSArray *matchingBatches = [self.mutationQueue allMutationBatchesAffectingQuery:query]; @@ -128,17 +129,21 @@ - (FSTDocumentDictionary *)documentsMatchingCollectionQuery:(FSTQuery *)query { continue; } - FSTDocumentKey *key = static_cast(mutation.key); + const DocumentKey key = mutation.key; // baseDoc may be nil for the documents that weren't yet written to the backend. - FSTMaybeDocument *baseDoc = results[key]; + FSTMaybeDocument *baseDoc = nil; + auto found = results.find(key); + if (found != results.end()) { + baseDoc = found->second; + } FSTMaybeDocument *mutatedDoc = [mutation applyToLocalDocument:baseDoc baseDocument:baseDoc localWriteTime:batch.localWriteTime]; if ([mutatedDoc isKindOfClass:[FSTDocument class]]) { - results = [results dictionaryBySettingObject:(FSTDocument *)mutatedDoc forKey:key]; + results = results.insert(key, mutatedDoc); } else { - results = [results dictionaryByRemovingObjectForKey:key]; + results = results.erase(key); } } } @@ -146,13 +151,14 @@ - (FSTDocumentDictionary *)documentsMatchingCollectionQuery:(FSTQuery *)query { // Finally, filter out any documents that don't actually match the query. Note that the extra // reference here prevents ARC from deallocating the initial unfiltered results while we're // enumerating them. - FSTDocumentDictionary *unfiltered = results; - [unfiltered - enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey *key, FSTDocument *doc, BOOL *stop) { - if (![query matchesDocument:doc]) { - results = [results dictionaryByRemovingObjectForKey:key]; - } - }]; + MaybeDocumentMap unfiltered = results; + for (const auto &kv : unfiltered) { + const DocumentKey& key = kv.first; + FSTDocument *doc = static_cast(kv.second); + if (![query matchesDocument:doc]) { + results = results.erase(key); + } + } return results; } diff --git a/Firestore/Source/Local/FSTLocalStore.h b/Firestore/Source/Local/FSTLocalStore.h index 6978afa1250..9e8f6b3b81c 100644 --- a/Firestore/Source/Local/FSTLocalStore.h +++ b/Firestore/Source/Local/FSTLocalStore.h @@ -16,11 +16,10 @@ #import -#import "Firestore/Source/Model/FSTDocumentDictionary.h" - #include "Firestore/core/src/firebase/firestore/auth/user.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/document_map.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/model/types.h" @@ -89,7 +88,8 @@ NS_ASSUME_NONNULL_BEGIN * In response the local store switches the mutation queue to the new user and returns any * resulting document changes. */ -- (FSTMaybeDocumentDictionary *)userDidChange:(const firebase::firestore::auth::User &)user; +- (firebase::firestore::model::MaybeDocumentMap)userDidChange: + (const firebase::firestore::auth::User &)user; /** Accepts locally generated Mutations and commits them to storage. */ - (FSTLocalWriteResult *)locallyWriteMutations:(NSArray *)mutations; @@ -110,7 +110,8 @@ NS_ASSUME_NONNULL_BEGIN * * @return The resulting (modified) documents. */ -- (FSTMaybeDocumentDictionary *)acknowledgeBatchWithResult:(FSTMutationBatchResult *)batchResult; +- (firebase::firestore::model::MaybeDocumentMap)acknowledgeBatchWithResult: + (FSTMutationBatchResult *)batchResult; /** * Removes mutations from the MutationQueue for the specified batch. LocalDocuments will be @@ -118,7 +119,8 @@ NS_ASSUME_NONNULL_BEGIN * * @return The resulting (modified) documents. */ -- (FSTMaybeDocumentDictionary *)rejectBatchID:(firebase::firestore::model::BatchId)batchID; +- (firebase::firestore::model::MaybeDocumentMap)rejectBatchID: + (firebase::firestore::model::BatchId)batchID; /** Returns the last recorded stream token for the current user. */ - (nullable NSData *)lastStreamToken; @@ -143,7 +145,7 @@ NS_ASSUME_NONNULL_BEGIN * * LocalDocuments are re-calculated if there are remaining mutations in the queue. */ -- (FSTMaybeDocumentDictionary *)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent; +- (firebase::firestore::model::MaybeDocumentMap)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent; /** * Returns the keys of the documents that are associated with the given targetID in the remote @@ -162,7 +164,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)releaseQuery:(FSTQuery *)query; /** Runs @a query against all the documents in the local store and returns the results. */ -- (FSTDocumentDictionary *)executeQuery:(FSTQuery *)query; +- (firebase::firestore::model::MaybeDocumentMap)executeQuery:(FSTQuery *)query; /** Notify the local store of the changed views to locally pin / unpin documents. */ - (void)notifyLocalViewChanges:(NSArray *)viewChanges; diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm index 4031013fcdd..53e5aca7e0d 100644 --- a/Firestore/Source/Local/FSTLocalStore.mm +++ b/Firestore/Source/Local/FSTLocalStore.mm @@ -17,6 +17,7 @@ #import "Firestore/Source/Local/FSTLocalStore.h" #include +#include #import "FIRTimestamp.h" #import "Firestore/Source/Core/FSTListenSequence.h" @@ -31,14 +32,13 @@ #import "Firestore/Source/Local/FSTReferenceSet.h" #import "Firestore/Source/Local/FSTRemoteDocumentCache.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentDictionary.h" #import "Firestore/Source/Model/FSTMutation.h" #import "Firestore/Source/Model/FSTMutationBatch.h" #import "Firestore/Source/Remote/FSTRemoteEvent.h" #include "Firestore/core/src/firebase/firestore/auth/user.h" #include "Firestore/core/src/firebase/firestore/core/target_id_generator.h" -#include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/immutable/sorted_set.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "Firestore/core/src/firebase/firestore/util/log.h" @@ -48,7 +48,9 @@ using firebase::firestore::model::BatchId; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::DocumentMap; using firebase::firestore::model::DocumentVersionMap; +using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::ListenSequenceNumber; using firebase::firestore::model::SnapshotVersion; using firebase::firestore::model::TargetId; @@ -122,7 +124,7 @@ - (void)startMutationQueue { self.persistence.run("Start MutationQueue", [&]() { [self.mutationQueue start]; }); } -- (FSTMaybeDocumentDictionary *)userDidChange:(const User &)user { +- (MaybeDocumentMap)userDidChange:(const User &)user { // Swap out the mutation queue, grabbing the pending mutation batches before and after. NSArray *oldBatches = self.persistence.run( "OldBatches", @@ -132,7 +134,7 @@ - (FSTMaybeDocumentDictionary *)userDidChange:(const User &)user { [self startMutationQueue]; - return self.persistence.run("NewBatches", [&]() -> FSTMaybeDocumentDictionary * { + return self.persistence.run("NewBatches", [&]() -> MaybeDocumentMap { NSArray *newBatches = [self.mutationQueue allMutationBatches]; // Recreate our LocalDocumentsView using the new MutationQueue. @@ -161,13 +163,13 @@ - (FSTLocalWriteResult *)locallyWriteMutations:(NSArray *)mutatio FSTMutationBatch *batch = [self.mutationQueue addMutationBatchWithWriteTime:localWriteTime mutations:mutations]; DocumentKeySet keys = [batch keys]; - FSTMaybeDocumentDictionary *changedDocuments = [self.localDocuments documentsForKeys:keys]; - return [FSTLocalWriteResult resultForBatchID:batch.batchID changes:changedDocuments]; + MaybeDocumentMap changedDocuments = [self.localDocuments documentsForKeys:keys]; + return [FSTLocalWriteResult resultForBatchID:batch.batchID changes:std::move(changedDocuments)]; }); } -- (FSTMaybeDocumentDictionary *)acknowledgeBatchWithResult:(FSTMutationBatchResult *)batchResult { - return self.persistence.run("Acknowledge batch", [&]() -> FSTMaybeDocumentDictionary * { +- (MaybeDocumentMap)acknowledgeBatchWithResult:(FSTMutationBatchResult *)batchResult { + return self.persistence.run("Acknowledge batch", [&]() -> MaybeDocumentMap { id mutationQueue = self.mutationQueue; FSTMutationBatch *batch = batchResult.batch; @@ -179,8 +181,8 @@ - (FSTMaybeDocumentDictionary *)acknowledgeBatchWithResult:(FSTMutationBatchResu }); } -- (FSTMaybeDocumentDictionary *)rejectBatchID:(BatchId)batchID { - return self.persistence.run("Reject batch", [&]() -> FSTMaybeDocumentDictionary * { +- (MaybeDocumentMap)rejectBatchID:(BatchId)batchID { + return self.persistence.run("Reject batch", [&]() -> MaybeDocumentMap { FSTMutationBatch *toReject = [self.mutationQueue lookupMutationBatch:batchID]; HARD_ASSERT(toReject, "Attempt to reject nonexistent batch!"); @@ -207,8 +209,8 @@ - (void)setLastStreamToken:(nullable NSData *)streamToken { return [self.queryCache lastRemoteSnapshotVersion]; } -- (FSTMaybeDocumentDictionary *)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent { - return self.persistence.run("Apply remote event", [&]() -> FSTMaybeDocumentDictionary * { +- (MaybeDocumentMap)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent { + return self.persistence.run("Apply remote event", [&]() -> MaybeDocumentMap { // TODO(gsoltis): move the sequence number into the reference delegate. ListenSequenceNumber sequenceNumber = self.persistence.currentSequenceNumber; id queryCache = self.queryCache; @@ -421,8 +423,8 @@ - (void)releaseQuery:(FSTQuery *)query { }); } -- (FSTDocumentDictionary *)executeQuery:(FSTQuery *)query { - return self.persistence.run("ExecuteQuery", [&]() -> FSTDocumentDictionary * { +- (MaybeDocumentMap)executeQuery:(FSTQuery *)query { + return self.persistence.run("ExecuteQuery", [&]() -> MaybeDocumentMap { return [self.localDocuments documentsMatchingQuery:query]; }); } diff --git a/Firestore/Source/Local/FSTLocalWriteResult.h b/Firestore/Source/Local/FSTLocalWriteResult.h index 8b38ea25884..e4b208d834d 100644 --- a/Firestore/Source/Local/FSTLocalWriteResult.h +++ b/Firestore/Source/Local/FSTLocalWriteResult.h @@ -16,8 +16,7 @@ #import -#import "Firestore/Source/Model/FSTDocumentDictionary.h" - +#include "Firestore/core/src/firebase/firestore/model/document_map.h" #include "Firestore/core/src/firebase/firestore/model/types.h" NS_ASSUME_NONNULL_BEGIN @@ -26,12 +25,13 @@ NS_ASSUME_NONNULL_BEGIN @interface FSTLocalWriteResult : NSObject + (instancetype)resultForBatchID:(firebase::firestore::model::BatchId)batchID - changes:(FSTMaybeDocumentDictionary *)changes; + changes:(firebase::firestore::model::MaybeDocumentMap &&)changes; - (id)init __attribute__((unavailable("Use resultForBatchID:changes:"))); +- (const firebase::firestore::model::MaybeDocumentMap &)changes; + @property(nonatomic, assign, readonly) firebase::firestore::model::BatchId batchID; -@property(nonatomic, strong, readonly) FSTMaybeDocumentDictionary *changes; @end diff --git a/Firestore/Source/Local/FSTLocalWriteResult.mm b/Firestore/Source/Local/FSTLocalWriteResult.mm index 2f19ff587b8..fcad635248b 100644 --- a/Firestore/Source/Local/FSTLocalWriteResult.mm +++ b/Firestore/Source/Local/FSTLocalWriteResult.mm @@ -16,26 +16,35 @@ #import "Firestore/Source/Local/FSTLocalWriteResult.h" +#include + using firebase::firestore::model::BatchId; +using firebase::firestore::model::MaybeDocumentMap; NS_ASSUME_NONNULL_BEGIN @interface FSTLocalWriteResult () - (instancetype)initWithBatchID:(BatchId)batchID - changes:(FSTMaybeDocumentDictionary *)changes NS_DESIGNATED_INITIALIZER; + changes:(MaybeDocumentMap &&)changes NS_DESIGNATED_INITIALIZER; @end -@implementation FSTLocalWriteResult +@implementation FSTLocalWriteResult { + MaybeDocumentMap _changes; +} + +- (const MaybeDocumentMap &)changes { + return _changes; +} -+ (instancetype)resultForBatchID:(BatchId)batchID changes:(FSTMaybeDocumentDictionary *)changes { - return [[FSTLocalWriteResult alloc] initWithBatchID:batchID changes:changes]; ++ (instancetype)resultForBatchID:(BatchId)batchID changes:(MaybeDocumentMap &&)changes { + return [[FSTLocalWriteResult alloc] initWithBatchID:batchID changes:std::move(changes)]; } -- (instancetype)initWithBatchID:(BatchId)batchID changes:(FSTMaybeDocumentDictionary *)changes { +- (instancetype)initWithBatchID:(BatchId)batchID changes:(MaybeDocumentMap &&)changes { self = [super init]; if (self) { _batchID = batchID; - _changes = changes; + _changes = std::move(changes); } return self; } diff --git a/Firestore/Source/Local/FSTMemoryMutationQueue.mm b/Firestore/Source/Local/FSTMemoryMutationQueue.mm index 3109bdfa6cf..2b937fc1137 100644 --- a/Firestore/Source/Local/FSTMemoryMutationQueue.mm +++ b/Firestore/Source/Local/FSTMemoryMutationQueue.mm @@ -208,7 +208,7 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(BatchId)batchID { NSMutableArray *result = [NSMutableArray array]; FSTDocumentReferenceBlock block = ^(FSTDocumentReference *reference, BOOL *stop) { - if (![documentKey isEqualToKey:reference.key]) { + if (documentKey != reference.key) { *stop = YES; return; } @@ -230,7 +230,7 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(BatchId)batchID { FSTDocumentReference *start = [[FSTDocumentReference alloc] initWithKey:key ID:0]; FSTDocumentReferenceBlock block = ^(FSTDocumentReference *reference, BOOL *stop) { - if (![key isEqualToKey:reference.key]) { + if (key != reference.key) { *stop = YES; return; } diff --git a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h index 71bce81eb55..453fcce7845 100644 --- a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h +++ b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.h @@ -29,8 +29,6 @@ NS_ASSUME_NONNULL_BEGIN @interface FSTMemoryRemoteDocumentCache : NSObject -- (instancetype)init NS_DESIGNATED_INITIALIZER; - - (std::vector) removeOrphanedDocuments:(FSTMemoryLRUReferenceDelegate *)referenceDelegate throughSequenceNumber:(firebase::firestore::model::ListenSequenceNumber)upperBound; diff --git a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm index 889268822f5..614c06c97fb 100644 --- a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm +++ b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm @@ -21,12 +21,14 @@ #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTMemoryPersistence.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentDictionary.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/document_map.h" using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::ListenSequenceNumber; +using firebase::firestore::model::MaybeDocumentMap; NS_ASSUME_NONNULL_BEGIN @@ -35,9 +37,9 @@ * document key in memory. This is only an estimate and includes the size * of the segments of the path, but not any object overhead or path separators. */ -static size_t FSTDocumentKeyByteSize(FSTDocumentKey *key) { +static size_t FSTDocumentKeyByteSize(const DocumentKey &key) { size_t count = 0; - for (const auto &segment : key.path) { + for (const auto &segment : key.path()) { count += segment.size(); } return count; @@ -45,50 +47,48 @@ static size_t FSTDocumentKeyByteSize(FSTDocumentKey *key) { @interface FSTMemoryRemoteDocumentCache () -/** Underlying cache of documents. */ -@property(nonatomic, strong) FSTMaybeDocumentDictionary *docs; - @end -@implementation FSTMemoryRemoteDocumentCache - -- (instancetype)init { - if (self = [super init]) { - _docs = [FSTMaybeDocumentDictionary maybeDocumentDictionary]; - } - return self; +@implementation FSTMemoryRemoteDocumentCache { + /** Underlying cache of documents. */ + MaybeDocumentMap _docs; } - (void)addEntry:(FSTMaybeDocument *)document { - self.docs = [self.docs dictionaryBySettingObject:document forKey:document.key]; + self->_docs = self->_docs.insert(document.key, document); } - (void)removeEntryForKey:(const DocumentKey &)key { - self.docs = [self.docs dictionaryByRemovingObjectForKey:key]; + self->_docs = self->_docs.erase(key); } - (nullable FSTMaybeDocument *)entryForKey:(const DocumentKey &)key { - return self.docs[static_cast(key)]; + auto found = self->_docs.find(key); + return found != self->_docs.end() ? found->second : nil; } -- (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query { - FSTDocumentDictionary *result = [FSTDocumentDictionary documentDictionary]; +- (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { + MaybeDocumentMap result; // Documents are ordered by key, so we can use a prefix scan to narrow down the documents // we need to match the query against. - FSTDocumentKey *prefix = [FSTDocumentKey keyWithPath:query.path.Append("")]; - NSEnumerator *enumerator = [self.docs keyEnumeratorFrom:prefix]; - for (FSTDocumentKey *key in enumerator) { - if (!query.path.IsPrefixOf(key.path)) { + DocumentKey prefix{query.path.Append("")}; + for (auto it = self->_docs.lower_bound(prefix); it != self->_docs.end(); ++it) { + const DocumentKey &key = it->first; + if (!query.path.IsPrefixOf(key.path())) { break; } - FSTMaybeDocument *maybeDoc = self.docs[key]; + FSTMaybeDocument *maybeDoc = nil; + auto found = self->_docs.find(key); + if (found != self->_docs.end()) { + maybeDoc = found->second; + } if (![maybeDoc isKindOfClass:[FSTDocument class]]) { continue; } - FSTDocument *doc = (FSTDocument *)maybeDoc; + FSTDocument *doc = static_cast(maybeDoc); if ([query matchesDocument:doc]) { - result = [result dictionaryBySettingObject:doc forKey:doc.key]; + result = result.insert(key, doc); } } @@ -99,24 +99,26 @@ - (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query { (FSTMemoryLRUReferenceDelegate *)referenceDelegate throughSequenceNumber:(ListenSequenceNumber)upperBound { std::vector removed; - FSTMaybeDocumentDictionary *updatedDocs = self.docs; - for (FSTDocumentKey *docKey in [self.docs keyEnumerator]) { + MaybeDocumentMap updatedDocs = self->_docs; + for (const auto &kv : self->_docs) { + const DocumentKey &docKey = kv.first; if (![referenceDelegate isPinnedAtSequenceNumber:upperBound document:docKey]) { - updatedDocs = [updatedDocs dictionaryByRemovingObjectForKey:docKey]; - removed.push_back(DocumentKey{docKey}); + updatedDocs = updatedDocs.erase(docKey); + removed.push_back(docKey); } } - self.docs = updatedDocs; + self->_docs = updatedDocs; return removed; } - (size_t)byteSizeWithSerializer:(FSTLocalSerializer *)serializer { - __block size_t count = 0; - [self.docs - enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey *key, FSTMaybeDocument *doc, BOOL *stop) { - count += FSTDocumentKeyByteSize(key); - count += [[[serializer encodedMaybeDocument:doc] data] length]; - }]; + size_t count = 0; + for (const auto &kv : self->_docs) { + const DocumentKey &key = kv.first; + FSTMaybeDocument *doc = kv.second; + count += FSTDocumentKeyByteSize(key); + count += [[[serializer encodedMaybeDocument:doc] data] length]; + } return count; } diff --git a/Firestore/Source/Local/FSTPersistence.h b/Firestore/Source/Local/FSTPersistence.h index 088d43d465a..29607875d4b 100644 --- a/Firestore/Source/Local/FSTPersistence.h +++ b/Firestore/Source/Local/FSTPersistence.h @@ -22,7 +22,6 @@ #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "Firestore/core/src/firebase/firestore/util/status.h" -@class FSTDocumentKey; @class FSTQueryData; @class FSTReferenceSet; @protocol FSTMutationQueue; diff --git a/Firestore/Source/Local/FSTRemoteDocumentCache.h b/Firestore/Source/Local/FSTRemoteDocumentCache.h index 2c6d8721c4c..86a3fe0dacc 100644 --- a/Firestore/Source/Local/FSTRemoteDocumentCache.h +++ b/Firestore/Source/Local/FSTRemoteDocumentCache.h @@ -16,9 +16,9 @@ #import -#import "Firestore/Source/Model/FSTDocumentDictionary.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/document_map.h" @class FSTMaybeDocument; @class FSTQuery; @@ -28,7 +28,7 @@ NS_ASSUME_NONNULL_BEGIN /** * Represents cached documents received from the remote backend. * - * The cache is keyed by FSTDocumentKey and entries in the cache are FSTMaybeDocument instances, + * The cache is keyed by DocumentKey and entries in the cache are FSTMaybeDocument instances, * meaning we can cache both FSTDocument instances (an actual document with data) as well as * FSTDeletedDocument instances (indicating that the document is known to not exist). */ @@ -67,7 +67,7 @@ NS_ASSUME_NONNULL_BEGIN * @param query The query to match documents against. * @return The set of matching documents. */ -- (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query; +- (firebase::firestore::model::MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query; @end diff --git a/Firestore/Source/Model/FSTDocument.mm b/Firestore/Source/Model/FSTDocument.mm index 88c37e99681..49ce4511693 100644 --- a/Firestore/Source/Model/FSTDocument.mm +++ b/Firestore/Source/Model/FSTDocument.mm @@ -238,7 +238,7 @@ - (NSString *)description { const NSComparator FSTDocumentComparatorByKey = ^NSComparisonResult(FSTMaybeDocument *doc1, FSTMaybeDocument *doc2) { - return [doc1.key compare:doc2.key]; + return CompareKeys(doc1.key, doc2.key); }; NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Model/FSTDocumentDictionary.h b/Firestore/Source/Model/FSTDocumentDictionary.h deleted file mode 100644 index f4e4ba1198b..00000000000 --- a/Firestore/Source/Model/FSTDocumentDictionary.h +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2017 Google - * - * 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 - -#import "Firestore/third_party/Immutable/FSTImmutableSortedDictionary.h" - -@class FSTDocument; -@class FSTDocumentKey; -@class FSTMaybeDocument; - -NS_ASSUME_NONNULL_BEGIN - -/** Convenience type for a map of keys to MaybeDocuments, since they are so common. */ -typedef FSTImmutableSortedDictionary - FSTMaybeDocumentDictionary; - -/** Convenience type for a map of keys to Documents, since they are so common. */ -typedef FSTImmutableSortedDictionary FSTDocumentDictionary; - -@interface FSTImmutableSortedDictionary (FSTDocumentDictionary) - -/** Returns a new set using the DocumentKeyComparator. */ -+ (FSTMaybeDocumentDictionary *)maybeDocumentDictionary; - -/** Returns a new set using the DocumentKeyComparator. */ -+ (FSTDocumentDictionary *)documentDictionary; - -@end - -NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Model/FSTDocumentDictionary.mm b/Firestore/Source/Model/FSTDocumentDictionary.mm deleted file mode 100644 index 362af548b23..00000000000 --- a/Firestore/Source/Model/FSTDocumentDictionary.mm +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2017 Google - * - * 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 "Firestore/Source/Model/FSTDocumentDictionary.h" - -#import "Firestore/Source/Model/FSTDocumentKey.h" - -NS_ASSUME_NONNULL_BEGIN - -@implementation FSTImmutableSortedDictionary (FSTMaybeDocumentDictionary) - -+ (instancetype)maybeDocumentDictionary { - // Immutable dictionaries are contravariant in their value type, so just return a - // FSTDocumentDictionary here. - return [FSTDocumentDictionary documentDictionary]; -} - -+ (instancetype)documentDictionary { - static FSTDocumentDictionary *singleton; - static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ - singleton = [FSTDocumentDictionary dictionaryWithComparator:FSTDocumentKeyComparator]; - }); - return singleton; -} - -@end - -NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Model/FSTDocumentKey.h b/Firestore/Source/Model/FSTDocumentKey.h index ebc88c975be..80e6cc9f945 100644 --- a/Firestore/Source/Model/FSTDocumentKey.h +++ b/Firestore/Source/Model/FSTDocumentKey.h @@ -32,45 +32,19 @@ class DocumentKey; NS_ASSUME_NONNULL_BEGIN -/** FSTDocumentKey represents the location of a document in the Firestore database. */ -@interface FSTDocumentKey : NSObject - /** - * Creates and returns a new document key with the given path. - * - * @param path The path to the document. - * @return A new instance of FSTDocumentKey. + * `FSTDocumentKey` is a thin wrapper over `DocumentKey`, necessary until full migration is + * possible. Use the underlying `DocumentKey` for any operations. */ -+ (instancetype)keyWithPath:(firebase::firestore::model::ResourcePath)path; +@interface FSTDocumentKey : NSObject + (instancetype)keyWithDocumentKey:(const firebase::firestore::model::DocumentKey &)documentKey; -/** - * Creates and returns a new document key with a path with the given segments. - * - * @param segments The segments of the path to the document. - * @return A new instance of FSTDocumentKey. - */ -+ (instancetype)keyWithSegments:(std::initializer_list)segments; -/** - * Creates and returns a new document key from the given resource path string. - * - * @param resourcePath The slash-separated segments of the resource's path. - * @return A new instance of FSTDocumentKey. - */ -+ (instancetype)keyWithPathString:(NSString *)resourcePath; - -/** Returns true iff the given path is a path to a document. */ -+ (BOOL)isDocumentKey:(const firebase::firestore::model::ResourcePath &)path; -- (BOOL)isEqualToKey:(FSTDocumentKey *)other; -- (NSComparisonResult)compare:(FSTDocumentKey *)other; -/** The path to the document. */ -- (const firebase::firestore::model::ResourcePath &)path; +/** Gets the underlying C++ representation. */ +- (const firebase::firestore::model::DocumentKey &)key; @end -extern const NSComparator FSTDocumentKeyComparator; - /** The field path string that represents the document's key. */ extern NSString *const kDocumentKeyPath; diff --git a/Firestore/Source/Model/FSTDocumentKey.mm b/Firestore/Source/Model/FSTDocumentKey.mm index 841da067469..5fc78c1d067 100644 --- a/Firestore/Source/Model/FSTDocumentKey.mm +++ b/Firestore/Source/Model/FSTDocumentKey.mm @@ -41,22 +41,10 @@ @interface FSTDocumentKey () { @implementation FSTDocumentKey -+ (instancetype)keyWithPath:(ResourcePath)path { - return [[FSTDocumentKey alloc] initWithDocumentKey:DocumentKey{path}]; -} - + (instancetype)keyWithDocumentKey:(const firebase::firestore::model::DocumentKey &)documentKey { return [[FSTDocumentKey alloc] initWithDocumentKey:documentKey]; } -+ (instancetype)keyWithSegments:(std::initializer_list)segments { - return [FSTDocumentKey keyWithPath:ResourcePath(segments)]; -} - -+ (instancetype)keyWithPathString:(NSString *)resourcePath { - return [FSTDocumentKey keyWithPath:ResourcePath::FromString(util::MakeString(resourcePath))]; -} - /** Designated initializer. */ - (instancetype)initWithDocumentKey:(const DocumentKey &)key { if (self = [super init]) { @@ -65,6 +53,10 @@ - (instancetype)initWithDocumentKey:(const DocumentKey &)key { return self; } +- (const DocumentKey &)key { + return _delegate; +} + - (BOOL)isEqual:(id)object { if (self == object) { return YES; @@ -72,7 +64,7 @@ - (BOOL)isEqual:(id)object { if (![object isKindOfClass:[FSTDocumentKey class]]) { return NO; } - return [self isEqualToKey:(FSTDocumentKey *)object]; + return _delegate == static_cast(object)->_delegate; } - (NSUInteger)hash { @@ -88,41 +80,8 @@ - (id)copyWithZone:(NSZone *_Nullable)zone { return self; } -- (BOOL)isEqualToKey:(FSTDocumentKey *)other { - return FSTDocumentKeyComparator(self, other) == NSOrderedSame; -} - -- (NSComparisonResult)compare:(FSTDocumentKey *)other { - return FSTDocumentKeyComparator(self, other); -} - -+ (NSComparator)comparator { - return ^NSComparisonResult(id obj1, id obj2) { - return [obj1 compare:obj2]; - }; -} - -+ (BOOL)isDocumentKey:(const ResourcePath &)path { - return DocumentKey::IsDocumentKey(path); -} - -- (const ResourcePath &)path { - return _delegate.path(); -} - @end -const NSComparator FSTDocumentKeyComparator = - ^NSComparisonResult(FSTDocumentKey *key1, FSTDocumentKey *key2) { - if (key1.path < key2.path) { - return NSOrderedAscending; - } else if (key1.path > key2.path) { - return NSOrderedDescending; - } else { - return NSOrderedSame; - } - }; - NSString *const kDocumentKeyPath = @"__name__"; NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Model/FSTDocumentSet.h b/Firestore/Source/Model/FSTDocumentSet.h index b5521e73a2f..2087a3da59a 100644 --- a/Firestore/Source/Model/FSTDocumentSet.h +++ b/Firestore/Source/Model/FSTDocumentSet.h @@ -16,9 +16,8 @@ #import -#import "Firestore/Source/Model/FSTDocumentDictionary.h" - #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/document_map.h" @class FSTDocument; @@ -71,10 +70,10 @@ NS_ASSUME_NONNULL_BEGIN - (NSArray *)arrayValue; /** - * Returns the documents as a FSTMaybeDocumentDictionary. This is O(1) as this leverages our - * internal representation. + * Returns the documents as a `DocumentMap`. This is O(1) as this leverages + * our internal representation. */ -- (FSTMaybeDocumentDictionary *)dictionaryValue; +- (const firebase::firestore::model::DocumentMap &)mapValue; /** Returns a new FSTDocumentSet that contains the given document. */ - (instancetype)documentSetByAddingDocument:(FSTDocument *_Nullable)document; diff --git a/Firestore/Source/Model/FSTDocumentSet.mm b/Firestore/Source/Model/FSTDocumentSet.mm index 2f0b42b748f..c96a7e8e94a 100644 --- a/Firestore/Source/Model/FSTDocumentSet.mm +++ b/Firestore/Source/Model/FSTDocumentSet.mm @@ -14,24 +14,20 @@ * limitations under the License. */ +#include + #import "Firestore/Source/Model/FSTDocumentSet.h" #import "Firestore/Source/Model/FSTDocument.h" -#import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/third_party/Immutable/FSTImmutableSortedSet.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +using firebase::firestore::model::DocumentMap; using firebase::firestore::model::DocumentKey; NS_ASSUME_NONNULL_BEGIN -/** - * The type of the index of the documents in an FSTDocumentSet. - * @see FSTDocumentSet#index - */ -typedef FSTImmutableSortedDictionary IndexType; - /** * The type of the main collection of documents in an FSTDocumentSet. * @see FSTDocumentSet#sortedSet @@ -40,14 +36,8 @@ @interface FSTDocumentSet () -- (instancetype)initWithIndex:(IndexType *)index set:(SetType *)sortedSet NS_DESIGNATED_INITIALIZER; - -/** - * An index of the documents in the FSTDocumentSet, indexed by document key. The index - * exists to guarantee the uniqueness of document keys in the set and to allow lookup and removal - * of documents by key. - */ -@property(nonatomic, strong, readonly) IndexType *index; +- (instancetype)initWithIndex:(DocumentMap &&)index + set:(SetType *)sortedSet NS_DESIGNATED_INITIALIZER; /** * The main collection of documents in the FSTDocumentSet. The documents are ordered by a @@ -57,19 +47,24 @@ - (instancetype)initWithIndex:(IndexType *)index set:(SetType *)sortedSet NS_DES @property(nonatomic, strong, readonly) SetType *sortedSet; @end -@implementation FSTDocumentSet +@implementation FSTDocumentSet { + /** + * An index of the documents in the FSTDocumentSet, indexed by document key. The index + * exists to guarantee the uniqueness of document keys in the set and to allow lookup and removal + * of documents by key. + */ + DocumentMap _index; +} + (instancetype)documentSetWithComparator:(NSComparator)comparator { - IndexType *index = - [FSTImmutableSortedDictionary dictionaryWithComparator:FSTDocumentKeyComparator]; SetType *set = [FSTImmutableSortedSet setWithComparator:comparator]; - return [[FSTDocumentSet alloc] initWithIndex:index set:set]; + return [[FSTDocumentSet alloc] initWithIndex:DocumentMap {} set:set]; } -- (instancetype)initWithIndex:(IndexType *)index set:(SetType *)sortedSet { +- (instancetype)initWithIndex:(DocumentMap &&)index set:(SetType *)sortedSet { self = [super init]; if (self) { - _index = index; + _index = std::move(index); _sortedSet = sortedSet; } return self; @@ -116,19 +111,20 @@ - (NSString *)description { } - (NSUInteger)count { - return [self.index count]; + return self->_index.size(); } - (BOOL)isEmpty { - return [self.index isEmpty]; + return self->_index.empty(); } - (BOOL)containsKey:(const DocumentKey &)key { - return [self.index objectForKey:(FSTDocumentKey *)key] != nil; + return self->_index.find(key) != self->_index.end(); } - (FSTDocument *_Nullable)documentForKey:(const DocumentKey &)key { - return [self.index objectForKey:(FSTDocumentKey *)key]; + auto found = self->_index.find(key); + return found != self->_index.end() ? found->second : nil; } - (FSTDocument *_Nullable)firstDocument { @@ -140,7 +136,7 @@ - (FSTDocument *_Nullable)lastDocument { } - (NSUInteger)indexOfKey:(const DocumentKey &)key { - FSTDocument *doc = [self.index objectForKey:(FSTDocumentKey *)key]; + FSTDocument *doc = [self documentForKey:key]; return doc ? [self.sortedSet indexOfObject:doc] : NSNotFound; } @@ -156,8 +152,8 @@ - (NSArray *)arrayValue { return result; } -- (FSTMaybeDocumentDictionary *)dictionaryValue { - return self.index; +- (const DocumentMap &)mapValue { + return self->_index; } - (instancetype)documentSetByAddingDocument:(FSTDocument *_Nullable)document { @@ -170,20 +166,20 @@ - (instancetype)documentSetByAddingDocument:(FSTDocument *_Nullable)document { // accumulating values that aren't in the index. FSTDocumentSet *removed = [self documentSetByRemovingKey:document.key]; - IndexType *index = [removed.index dictionaryBySettingObject:document forKey:document.key]; + DocumentMap index = removed->_index.insert(document.key, document); SetType *set = [removed.sortedSet setByAddingObject:document]; - return [[FSTDocumentSet alloc] initWithIndex:index set:set]; + return [[FSTDocumentSet alloc] initWithIndex:std::move(index) set:set]; } - (instancetype)documentSetByRemovingKey:(const DocumentKey &)key { - FSTDocument *doc = [self.index objectForKey:(FSTDocumentKey *)key]; + FSTDocument *doc = [self documentForKey:key]; if (!doc) { return self; } - IndexType *index = [self.index dictionaryByRemovingObjectForKey:key]; + DocumentMap index = self->_index.erase(key); SetType *set = [self.sortedSet setByRemovingObject:doc]; - return [[FSTDocumentSet alloc] initWithIndex:index set:set]; + return [[FSTDocumentSet alloc] initWithIndex:std::move(index) set:set]; } @end diff --git a/Firestore/Source/Model/FSTFieldValue.mm b/Firestore/Source/Model/FSTFieldValue.mm index b721a73d326..6a790102934 100644 --- a/Firestore/Source/Model/FSTFieldValue.mm +++ b/Firestore/Source/Model/FSTFieldValue.mm @@ -24,6 +24,7 @@ #import "Firestore/Source/Util/FSTClasses.h" #include "Firestore/core/src/firebase/firestore/model/database_id.h" +#include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" #include "Firestore/core/src/firebase/firestore/util/comparison.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" @@ -678,7 +679,7 @@ - (BOOL)isEqual:(id)other { } FSTReferenceValue *otherRef = (FSTReferenceValue *)other; - return [self.key isEqualToKey:otherRef.key] && *self.databaseID == *otherRef.databaseID; + return self.key.key == otherRef.key.key && *self.databaseID == *otherRef.databaseID; } - (NSUInteger)hash { @@ -696,7 +697,7 @@ - (NSComparisonResult)compare:(FSTFieldValue *)other { return cmp; } cmp = WrapCompare(self.databaseID->database_id(), ref.databaseID->database_id()); - return cmp != NSOrderedSame ? cmp : [self.key compare:ref.key]; + return cmp != NSOrderedSame ? cmp : CompareKeys(self.key.key, ref.key.key); } else { return [self defaultCompare:other]; } diff --git a/Firestore/Source/Remote/FSTRemoteEvent.h b/Firestore/Source/Remote/FSTRemoteEvent.h index 46ec954c395..e3a96e4df45 100644 --- a/Firestore/Source/Remote/FSTRemoteEvent.h +++ b/Firestore/Source/Remote/FSTRemoteEvent.h @@ -21,8 +21,6 @@ #include #include -#import "Firestore/Source/Model/FSTDocumentDictionary.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/snapshot_version.h" diff --git a/Firestore/Source/Remote/FSTRemoteEvent.mm b/Firestore/Source/Remote/FSTRemoteEvent.mm index 3b64fabb3f0..2c5fe95fdc6 100644 --- a/Firestore/Source/Remote/FSTRemoteEvent.mm +++ b/Firestore/Source/Remote/FSTRemoteEvent.mm @@ -426,7 +426,7 @@ - (void)handleExistenceFilter:(FSTExistenceFilterWatchChange *)existenceFilter { // does not exist and apply a deleted document to our updates. Without applying this deleted // document there might be another query that will raise this document as part of a snapshot // until it is resolved, essentially exposing inconsistency between queries. - FSTDocumentKey *key = [FSTDocumentKey keyWithPath:query.path]; + DocumentKey key{query.path}; [self removeDocument:[FSTDeletedDocument documentWithKey:key version:SnapshotVersion::None() hasCommittedMutations:NO] @@ -470,7 +470,7 @@ - (void)resetTarget:(TargetId)targetID { // of the initial snapshot if Watch does not resend these documents. DocumentKeySet existingKeys = [_targetMetadataProvider remoteKeysForTarget:@(targetID)]; - for (FSTDocumentKey *key : existingKeys) { + for (const DocumentKey &key : existingKeys) { [self removeDocument:nil withKey:key fromTarget:targetID]; } } @@ -527,7 +527,7 @@ - (void)removeDocument:(FSTMaybeDocument *_Nullable)document /** * Returns whether the LocalStore considers the document to be part of the specified target. */ -- (BOOL)containsDocument:(FSTDocumentKey *)key inTarget:(TargetId)targetID { +- (BOOL)containsDocument:(const DocumentKey &)key inTarget:(TargetId)targetID { const DocumentKeySet &existingKeys = [_targetMetadataProvider remoteKeysForTarget:@(targetID)]; return existingKeys.contains(key); } @@ -573,7 +573,7 @@ - (FSTRemoteEvent *)remoteEventAtSnapshotVersion:(const SnapshotVersion &)snapsh // our local cache, we synthesize a document delete if we have not previously received the // document. This resolves the limbo state of the document, removing it from // limboDocumentRefs. - FSTDocumentKey *key = [FSTDocumentKey keyWithPath:queryData.query.path]; + DocumentKey key{queryData.query.path}; if (_pendingDocumentUpdates.find(key) == _pendingDocumentUpdates.end() && ![self containsDocument:key inTarget:targetID]) { [self removeDocument:[FSTDeletedDocument documentWithKey:key diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 2e52c3302cb..4657454b0df 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -216,7 +216,8 @@ - (GCFSValue *)encodedFieldValue:(FSTFieldValue *)fieldValue { } else if (fieldClass == [FSTReferenceValue class]) { FSTReferenceValue *ref = (FSTReferenceValue *)fieldValue; - return [self encodedReferenceValueForDatabaseID:[ref databaseID] key:[ref value]]; + DocumentKey key = [[ref value] key]; + return [self encodedReferenceValueForDatabaseID:[ref databaseID] key:key]; } else if (fieldClass == [FSTObjectValue class]) { GCFSValue *result = [GCFSValue message]; diff --git a/Firestore/Source/Remote/FSTWatchChange.mm b/Firestore/Source/Remote/FSTWatchChange.mm index 2c668cb093b..1387b02270e 100644 --- a/Firestore/Source/Remote/FSTWatchChange.mm +++ b/Firestore/Source/Remote/FSTWatchChange.mm @@ -64,7 +64,7 @@ - (BOOL)isEqual:(id)other { FSTDocumentWatchChange *otherChange = (FSTDocumentWatchChange *)other; return [_updatedTargetIDs isEqual:otherChange.updatedTargetIDs] && [_removedTargetIDs isEqual:otherChange.removedTargetIDs] && - [_documentKey isEqual:otherChange.documentKey] && + _documentKey == otherChange.documentKey && (_document == otherChange.document || [_document isEqual:otherChange.document]); } diff --git a/Firestore/core/src/firebase/firestore/model/document_key.h b/Firestore/core/src/firebase/firestore/model/document_key.h index 19671f9b853..a795e3ea90a 100644 --- a/Firestore/core/src/firebase/firestore/model/document_key.h +++ b/Firestore/core/src/firebase/firestore/model/document_key.h @@ -51,10 +51,6 @@ class DocumentKey { explicit DocumentKey(ResourcePath&& path); #if defined(__OBJC__) - DocumentKey(FSTDocumentKey* key) // NOLINT(runtime/explicit) - : path_(std::make_shared(key.path)) { - } - operator FSTDocumentKey*() const { return [FSTDocumentKey keyWithDocumentKey:*this]; } @@ -62,7 +58,7 @@ class DocumentKey { NSUInteger Hash() const { return util::Hash(ToString()); } -#endif +#endif // defined(__OBJC__) std::string ToString() const { return path().CanonicalString(); @@ -125,6 +121,20 @@ struct DocumentKeyHash { } }; +#if defined(__OBJC__) +inline NSComparisonResult CompareKeys(const DocumentKey& lhs, + const DocumentKey& rhs) { + if (lhs < rhs) { + return NSOrderedAscending; + } + if (lhs > rhs) { + return NSOrderedDescending; + } + return NSOrderedSame; +} + +#endif // defined(__OBJC__) + } // namespace model namespace util { diff --git a/Firestore/core/src/firebase/firestore/model/document_map.h b/Firestore/core/src/firebase/firestore/model/document_map.h new file mode 100644 index 00000000000..0b5a421a06f --- /dev/null +++ b/Firestore/core/src/firebase/firestore/model/document_map.h @@ -0,0 +1,44 @@ +/* + * Copyright 2018 Google + * + * 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. + */ + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_DOCUMENT_MAP_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_DOCUMENT_MAP_H_ + +#import "Firestore/Source/Model/FSTDocument.h" + +#include "Firestore/core/src/firebase/firestore/immutable/sorted_map.h" +#include "Firestore/core/src/firebase/firestore/model/document_key.h" + +namespace firebase { +namespace firestore { +namespace model { + +/** + * Convenience type for a map of keys to MaybeDocuments, since they are so + * common. + */ +using MaybeDocumentMap = immutable::SortedMap; + +/** + * Convenience type for a map of keys to Documents, since they are so common. + */ +using DocumentMap = immutable::SortedMap; + +} // namespace model +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_DOCUMENT_MAP_H_ From 560ef0f6da65ab02198a460a2f5c908924a8cac7 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 2 Dec 2018 17:22:20 -0500 Subject: [PATCH 02/16] style.sh --- Firestore/Source/Local/FSTLocalDocumentsView.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index ec7ff97cf18..09b6d1e2a40 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -153,8 +153,8 @@ - (MaybeDocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { // enumerating them. MaybeDocumentMap unfiltered = results; for (const auto &kv : unfiltered) { - const DocumentKey& key = kv.first; - FSTDocument *doc = static_cast(kv.second); + const DocumentKey &key = kv.first; + FSTDocument *doc = static_cast(kv.second); if (![query matchesDocument:doc]) { results = results.erase(key); } From afca380131557eaa2e58f7756524b52a8968a1b5 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 2 Dec 2018 17:58:17 -0500 Subject: [PATCH 03/16] Small fixes --- .../Benchmarks/FSTLevelDBBenchmarkTests.mm | 4 ++-- .../Example/Tests/Local/FSTLocalStoreTests.mm | 2 +- .../Source/Local/FSTLocalDocumentsView.mm | 2 +- .../Local/FSTMemoryRemoteDocumentCache.mm | 22 +++++++++---------- Firestore/Source/Model/FSTDocumentSet.mm | 14 ++++++------ 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm b/Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm index 0dde7a42839..d7be928c9d6 100644 --- a/Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm +++ b/Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm @@ -104,7 +104,7 @@ void FillDB() { LevelDbTransaction txn(db_.ptr, "benchmark"); for (int i = 0; i < numDocuments_; i++) { - auto docKey = DocumentKey::FromPathString(StringFormat(docs / doc_ % i, i)); + auto docKey = DocumentKey::FromPathString(StringFormat("docs/doc_%i", i)); std::string docKeyString = LevelDbRemoteDocumentKey::Key(docKey); txn.Put(docKeyString, DocumentData()); WriteIndex(txn, docKey); @@ -139,7 +139,7 @@ void WriteIndex(LevelDbTransaction &txn, const DocumentKey &docKey) { for (const auto &_ : state) { LevelDbTransaction txn(db_.ptr, "benchmark"); for (int i = 0; i < docsToUpdate; i++) { - auto docKey = DocumentKey::FromPathString(StringFormat(docs / doc_ % i, i)); + auto docKey = DocumentKey::FromPathString(StringFormat("docs/doc_%i", i)); if (writeIndexes) WriteIndex(txn, docKey); std::string docKeyString = LevelDbRemoteDocumentKey::Key(docKey); txn.Put(docKeyString, documentUpdate); diff --git a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm index 34b372fb17b..3790a6c9d68 100644 --- a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm +++ b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm @@ -196,7 +196,7 @@ - (TargetId)allocateQuery:(FSTQuery *)query { _lastChanges = MaybeDocumentMap{}; \ } while (0) -/** Asserts that the given local store contains the given document.*/ +/** Asserts that the given local store contains the given document. */ #define FSTAssertContains(document) \ do { \ FSTMaybeDocument *expected = (document); \ diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index 09b6d1e2a40..d86f9229efb 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -129,7 +129,7 @@ - (MaybeDocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { continue; } - const DocumentKey key = mutation.key; + const DocumentKey& key = mutation.key; // baseDoc may be nil for the documents that weren't yet written to the backend. FSTMaybeDocument *baseDoc = nil; auto found = results.find(key); diff --git a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm index f6d4fcc1915..ff00e05f02d 100644 --- a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm +++ b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm @@ -55,16 +55,16 @@ @implementation FSTMemoryRemoteDocumentCache { } - (void)addEntry:(FSTMaybeDocument *)document { - self->_docs = self->_docs.insert(document.key, document); + _docs = _docs.insert(document.key, document); } - (void)removeEntryForKey:(const DocumentKey &)key { - self->_docs = self->_docs.erase(key); + _docs = _docs.erase(key); } - (nullable FSTMaybeDocument *)entryForKey:(const DocumentKey &)key { - auto found = self->_docs.find(key); - return found != self->_docs.end() ? found->second : nil; + auto found = _docs.find(key); + return found != _docs.end() ? found->second : nil; } - (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { @@ -73,14 +73,14 @@ - (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { // Documents are ordered by key, so we can use a prefix scan to narrow down the documents // we need to match the query against. DocumentKey prefix{query.path.Append("")}; - for (auto it = self->_docs.lower_bound(prefix); it != self->_docs.end(); ++it) { + for (auto it = _docs.lower_bound(prefix); it != _docs.end(); ++it) { const DocumentKey &key = it->first; if (!query.path.IsPrefixOf(key.path())) { break; } FSTMaybeDocument *maybeDoc = nil; - auto found = self->_docs.find(key); - if (found != self->_docs.end()) { + auto found = _docs.find(key); + if (found != _docs.end()) { maybeDoc = found->second; } if (![maybeDoc isKindOfClass:[FSTDocument class]]) { @@ -99,21 +99,21 @@ - (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { (FSTMemoryLRUReferenceDelegate *)referenceDelegate throughSequenceNumber:(ListenSequenceNumber)upperBound { std::vector removed; - MaybeDocumentMap updatedDocs = self->_docs; - for (const auto &kv : self->_docs) { + MaybeDocumentMap updatedDocs = _docs; + for (const auto &kv : _docs) { const DocumentKey &docKey = kv.first; if (![referenceDelegate isPinnedAtSequenceNumber:upperBound document:docKey]) { updatedDocs = updatedDocs.erase(docKey); removed.push_back(docKey); } } - self->_docs = updatedDocs; + _docs = updatedDocs; return removed; } - (size_t)byteSizeWithSerializer:(FSTLocalSerializer *)serializer { size_t count = 0; - for (const auto &kv : self->_docs) { + for (const auto &kv : _docs) { const DocumentKey &key = kv.first; FSTMaybeDocument *doc = kv.second; count += FSTDocumentKeyByteSize(key); diff --git a/Firestore/Source/Model/FSTDocumentSet.mm b/Firestore/Source/Model/FSTDocumentSet.mm index c96a7e8e94a..65625ae8004 100644 --- a/Firestore/Source/Model/FSTDocumentSet.mm +++ b/Firestore/Source/Model/FSTDocumentSet.mm @@ -111,20 +111,20 @@ - (NSString *)description { } - (NSUInteger)count { - return self->_index.size(); + return _index.size(); } - (BOOL)isEmpty { - return self->_index.empty(); + return _index.empty(); } - (BOOL)containsKey:(const DocumentKey &)key { - return self->_index.find(key) != self->_index.end(); + return _index.find(key) != _index.end(); } - (FSTDocument *_Nullable)documentForKey:(const DocumentKey &)key { - auto found = self->_index.find(key); - return found != self->_index.end() ? found->second : nil; + auto found = _index.find(key); + return found != _index.end() ? found->second : nil; } - (FSTDocument *_Nullable)firstDocument { @@ -153,7 +153,7 @@ - (NSArray *)arrayValue { } - (const DocumentMap &)mapValue { - return self->_index; + return _index; } - (instancetype)documentSetByAddingDocument:(FSTDocument *_Nullable)document { @@ -177,7 +177,7 @@ - (instancetype)documentSetByRemovingKey:(const DocumentKey &)key { return self; } - DocumentMap index = self->_index.erase(key); + DocumentMap index = _index.erase(key); SetType *set = [self.sortedSet setByRemovingObject:doc]; return [[FSTDocumentSet alloc] initWithIndex:std::move(index) set:set]; } From 96eba87ff481958bdc0792288575478023df10a9 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 2 Dec 2018 18:31:48 -0500 Subject: [PATCH 04/16] style.sh --- Firestore/Source/Local/FSTLocalDocumentsView.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index d86f9229efb..6cfe9fe3f3b 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -129,7 +129,7 @@ - (MaybeDocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { continue; } - const DocumentKey& key = mutation.key; + const DocumentKey &key = mutation.key; // baseDoc may be nil for the documents that weren't yet written to the backend. FSTMaybeDocument *baseDoc = nil; auto found = results.find(key); From 83ea0a064bb83058297f2ccead7ef51409909388 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Wed, 5 Dec 2018 16:13:32 -0500 Subject: [PATCH 05/16] Review feedback --- Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm index 84d6028c44a..0bf2153c286 100644 --- a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm @@ -168,10 +168,10 @@ - (void)expectMap:(const MaybeDocumentMap &)map XCTAssertEqual(map.size(), [expected count]); } for (FSTDocument *doc in expected) { - FSTDocument *actual = nil; + FSTMaybeDocument *actual = nil; auto found = map.find(doc.key); if (found != map.end()) { - actual = static_cast(found->second); + actual = found->second; } XCTAssertEqualObjects(actual, doc); } From 99ca6796f255ac73d4d68bba1eb6832b76788f61 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 6 Dec 2018 14:49:22 -0500 Subject: [PATCH 06/16] Review feedback --- Firestore/Example/Tests/Core/FSTViewTests.mm | 104 ++++++++++-------- .../Example/Tests/Local/FSTLocalStoreTests.mm | 11 +- .../Local/FSTRemoteDocumentCacheTests.mm | 7 +- Firestore/Example/Tests/Util/FSTHelpers.mm | 7 +- Firestore/Source/Core/FSTFirestoreClient.mm | 3 +- Firestore/Source/Core/FSTSyncEngine.mm | 6 +- Firestore/Source/Core/FSTView.h | 10 +- Firestore/Source/Core/FSTView.mm | 81 +++++++++----- .../Local/FSTLevelDBRemoteDocumentCache.mm | 7 +- .../Source/Local/FSTLocalDocumentsView.h | 2 +- .../Source/Local/FSTLocalDocumentsView.mm | 19 ++-- Firestore/Source/Local/FSTLocalStore.h | 2 +- Firestore/Source/Local/FSTLocalStore.mm | 4 +- .../Local/FSTMemoryRemoteDocumentCache.mm | 5 +- .../Source/Local/FSTRemoteDocumentCache.h | 2 +- 15 files changed, 159 insertions(+), 111 deletions(-) diff --git a/Firestore/Example/Tests/Core/FSTViewTests.mm b/Firestore/Example/Tests/Core/FSTViewTests.mm index 770028ffaeb..a0919aac157 100644 --- a/Firestore/Example/Tests/Core/FSTViewTests.mm +++ b/Firestore/Example/Tests/Core/FSTViewTests.mm @@ -276,11 +276,12 @@ - (void)testDoesntReportChangesForDocumentBeyondLimitOfQuery { // doc4 will be added + removed = nothing doc2 = FSTTestDoc("rooms/eros/messages/2", 1, @{@"num" : @5}, FSTDocumentStateSynced); FSTViewDocumentChanges *viewDocChanges = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc2, doc3, doc4 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc2, doc3, doc4 ])]; XCTAssertTrue(viewDocChanges.needsRefill); // Verify that all the docs still match. - viewDocChanges = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4 ]) - previousChanges:viewDocChanges]; + viewDocChanges = + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4 ]) + previousChanges:viewDocChanges]; FSTViewSnapshot *snapshot = [view applyChangesToDocuments:viewDocChanges targetChange:FSTTestTargetChangeAckDocuments( @@ -310,33 +311,35 @@ - (void)testKeepsTrackOfLimboDocuments { FSTDocument *doc3 = FSTTestDoc("rooms/eros/messages/2", 0, @{}, FSTDocumentStateSynced); FSTViewChange *change = [view - applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1 ])]]; + applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1 ])]]; XCTAssertEqualObjects(change.limboChanges, @[]); - change = [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[])] - targetChange:FSTTestTargetChangeMarkCurrent()]; + change = + [view applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[])] + targetChange:FSTTestTargetChangeMarkCurrent()]; XCTAssertEqualObjects( change.limboChanges, @[ [FSTLimboDocumentChange changeWithType:FSTLimboDocumentChangeTypeAdded key:doc1.key] ]); - change = [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[])] - targetChange:FSTTestTargetChangeAckDocuments({doc1.key})]; + change = + [view applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[])] + targetChange:FSTTestTargetChangeAckDocuments({doc1.key})]; XCTAssertEqualObjects( change.limboChanges, @[ [FSTLimboDocumentChange changeWithType:FSTLimboDocumentChangeTypeRemoved key:doc1.key] ]); - change = - [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc2 ])] - targetChange:FSTTestTargetChangeAckDocuments({doc2.key})]; + change = [view + applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc2 ])] + targetChange:FSTTestTargetChangeAckDocuments({doc2.key})]; XCTAssertEqualObjects(change.limboChanges, @[]); change = [view - applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc3 ])]]; + applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc3 ])]]; XCTAssertEqualObjects( change.limboChanges, @[ [FSTLimboDocumentChange changeWithType:FSTLimboDocumentChangeTypeAdded key:doc3.key] ]); - change = [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[ + change = [view applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc("rooms/eros/messages/2", 1, NO) ])]]; // remove XCTAssertEqualObjects( @@ -355,7 +358,7 @@ - (void)testResumingQueryCreatesNoLimbos { FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{doc1.key, doc2.key}]; - FSTViewDocumentChanges *changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[])]; + FSTViewDocumentChanges *changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[])]; FSTViewChange *change = [view applyChangesToDocuments:changes targetChange:FSTTestTargetChangeMarkCurrent()]; XCTAssertEqualObjects(change.limboChanges, @[]); @@ -376,20 +379,21 @@ - (void)testReturnsNeedsRefillOnDeleteInLimitQuery { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); [view applyChangesToDocuments:changes]; // Remove one of the docs. - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( - "rooms/eros/messages/0", 0, NO) ])]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( + "rooms/eros/messages/0", 0, NO) ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc2 ]]; XCTAssertTrue(changes.needsRefill); XCTAssertEqual(1, [changes.changeSet changes].count); // Refill it with just the one doc remaining. - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc2 ]) previousChanges:changes]; + changes = + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc2 ]) previousChanges:changes]; [self assertDocSet:changes.documentSet containsDocs:@[ doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(1, [changes.changeSet changes].count); @@ -412,7 +416,7 @@ - (void)testReturnsNeedsRefillOnReorderInLimitQuery { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); @@ -420,13 +424,13 @@ - (void)testReturnsNeedsRefillOnReorderInLimitQuery { // Move one of the docs. doc2 = FSTTestDoc("rooms/eros/messages/1", 1, @{@"order" : @2000}, FSTDocumentStateSynced); - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc2 ])]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc2 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertTrue(changes.needsRefill); XCTAssertEqual(1, [changes.changeSet changes].count); // Refill it with all three current docs. - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3 ]) - previousChanges:changes]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3 ]) + previousChanges:changes]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc3 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); @@ -453,7 +457,7 @@ - (void)testDoesntNeedRefillOnReorderWithinLimit { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4, doc5 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4, doc5 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2, doc3 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(3, [changes.changeSet changes].count); @@ -461,7 +465,7 @@ - (void)testDoesntNeedRefillOnReorderWithinLimit { // Move one of the docs. doc1 = FSTTestDoc("rooms/eros/messages/0", 1, @{@"order" : @3}, FSTDocumentStateSynced); - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1 ])]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc2, doc3, doc1 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(1, [changes.changeSet changes].count); @@ -488,7 +492,7 @@ - (void)testDoesntNeedRefillOnReorderAfterLimitQuery { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4, doc5 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4, doc5 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2, doc3 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(3, [changes.changeSet changes].count); @@ -496,7 +500,7 @@ - (void)testDoesntNeedRefillOnReorderAfterLimitQuery { // Move one of the docs. doc4 = FSTTestDoc("rooms/eros/messages/3", 1, @{@"order" : @6}, FSTDocumentStateSynced); - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc4 ])]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc4 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2, doc3 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(0, [changes.changeSet changes].count); @@ -511,7 +515,7 @@ - (void)testDoesntNeedRefillForAdditionAfterTheLimit { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); @@ -519,7 +523,7 @@ - (void)testDoesntNeedRefillForAdditionAfterTheLimit { // Add a doc that is past the limit. FSTDocument *doc3 = FSTTestDoc("rooms/eros/messages/2", 1, @{}, FSTDocumentStateSynced); - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc3 ])]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc3 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(0, [changes.changeSet changes].count); @@ -533,15 +537,15 @@ - (void)testDoesntNeedRefillForDeletionsWhenNotNearTheLimit { FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}]; FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); [view applyChangesToDocuments:changes]; // Remove one of the docs. - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( - "rooms/eros/messages/1", 0, NO) ])]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( + "rooms/eros/messages/1", 0, NO) ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(1, [changes.changeSet changes].count); @@ -556,15 +560,15 @@ - (void)testHandlesApplyingIrrelevantDocs { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); [view applyChangesToDocuments:changes]; // Remove a doc that isn't even in the results. - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( - "rooms/eros/messages/2", 0, NO) ])]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( + "rooms/eros/messages/2", 0, NO) ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(0, [changes.changeSet changes].count); @@ -579,12 +583,12 @@ - (void)testComputesMutatedKeys { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [view applyChangesToDocuments:changes]; XCTAssertEqual(changes.mutatedKeys, DocumentKeySet{}); FSTDocument *doc3 = FSTTestDoc("rooms/eros/messages/2", 0, @{}, FSTDocumentStateLocalMutations); - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc3 ])]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc3 ])]; XCTAssertEqual(changes.mutatedKeys, DocumentKeySet{doc3.key}); } @@ -596,12 +600,12 @@ - (void)testRemovesKeysFromMutatedKeysWhenNewDocHasNoLocalChanges { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [view applyChangesToDocuments:changes]; XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key})); FSTDocument *doc2Prime = FSTTestDoc("rooms/eros/messages/1", 0, @{}, FSTDocumentStateSynced); - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc2Prime ])]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc2Prime ])]; [view applyChangesToDocuments:changes]; XCTAssertEqual(changes.mutatedKeys, DocumentKeySet{}); } @@ -614,12 +618,12 @@ - (void)testRemembersLocalMutationsFromPreviousSnapshot { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [view applyChangesToDocuments:changes]; XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key})); FSTDocument *doc3 = FSTTestDoc("rooms/eros/messages/2", 0, @{}, FSTDocumentStateSynced); - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc3 ])]; + changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc3 ])]; [view applyChangesToDocuments:changes]; XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key})); } @@ -632,11 +636,12 @@ - (void)testRemembersLocalMutationsFromPreviousCallToComputeChangesWithDocuments // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key})); FSTDocument *doc3 = FSTTestDoc("rooms/eros/messages/2", 0, @{}, FSTDocumentStateSynced); - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc3 ]) previousChanges:changes]; + changes = + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc3 ]) previousChanges:changes]; XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key})); } @@ -644,7 +649,8 @@ - (void)testRaisesHasPendingWritesForPendingMutationsInInitialSnapshot { FSTQuery *query = [self queryForMessages]; FSTDocument *doc1 = FSTTestDoc("rooms/eros/messages/1", 0, @{}, FSTDocumentStateLocalMutations); FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}]; - FSTViewDocumentChanges *changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1 ])]; + FSTViewDocumentChanges *changes = + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1 ])]; FSTViewChange *viewChange = [view applyChangesToDocuments:changes]; XCTAssertTrue(viewChange.snapshot.hasPendingWrites); } @@ -654,7 +660,8 @@ - (void)testDoesntRaiseHasPendingWritesForCommittedMutationsInInitialSnapshot { FSTDocument *doc1 = FSTTestDoc("rooms/eros/messages/1", 0, @{}, FSTDocumentStateCommittedMutations); FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}]; - FSTViewDocumentChanges *changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1 ])]; + FSTViewDocumentChanges *changes = + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1 ])]; FSTViewChange *viewChange = [view applyChangesToDocuments:changes]; XCTAssertFalse(viewChange.snapshot.hasPendingWrites); } @@ -679,7 +686,7 @@ - (void)testSuppressesWriteAcknowledgementIfWatchHasNotCaughtUp { @{@"time" : @3}, FSTDocumentStateSynced); FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}]; FSTViewDocumentChanges *changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; FSTViewChange *viewChange = [view applyChangesToDocuments:changes]; XCTAssertEqualObjects( @@ -689,7 +696,8 @@ - (void)testSuppressesWriteAcknowledgementIfWatchHasNotCaughtUp { ]), viewChange.snapshot.documentChanges); - changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1Committed, doc2Modified ])]; + changes = + [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1Committed, doc2Modified ])]; viewChange = [view applyChangesToDocuments:changes]; // The 'doc1Committed' update is suppressed XCTAssertEqualObjects( @@ -697,8 +705,8 @@ - (void)testSuppressesWriteAcknowledgementIfWatchHasNotCaughtUp { type:FSTDocumentViewChangeTypeModified] ]), viewChange.snapshot.documentChanges); - changes = - [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1Acknowledged, doc2Acknowledged ])]; + changes = [view + computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1Acknowledged, doc2Acknowledged ])]; viewChange = [view applyChangesToDocuments:changes]; XCTAssertEqualObjects( (@[ diff --git a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm index 3790a6c9d68..40f25bdaa35 100644 --- a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm +++ b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm @@ -47,12 +47,13 @@ using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::ListenSequenceNumber; +using firebase::firestore::model::DocumentMap; using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::SnapshotVersion; using firebase::firestore::model::TargetId; -static NSArray *docMapToArray(const MaybeDocumentMap &docs) { - NSMutableArray *result = [NSMutableArray array]; +static NSArray *docMapToArray(const DocumentMap &docs) { + NSMutableArray *result = [NSMutableArray array]; for (const auto &kv : docs) { [result addObject:kv.second]; } @@ -841,7 +842,7 @@ - (void)testCanExecuteDocumentQueries { FSTTestSetMutation(@"foo/bar/Foo/Bar", @{@"Foo" : @"Bar"}) ]]; FSTQuery *query = FSTTestQuery("foo/bar"); - MaybeDocumentMap docs = [self.localStore executeQuery:query]; + DocumentMap docs = [self.localStore executeQuery:query]; XCTAssertEqualObjects(docMapToArray(docs), @[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, FSTDocumentStateLocalMutations) ]); } @@ -857,7 +858,7 @@ - (void)testCanExecuteCollectionQueries { FSTTestSetMutation(@"fooo/blah", @{@"fooo" : @"blah"}) ]]; FSTQuery *query = FSTTestQuery("foo"); - MaybeDocumentMap docs = [self.localStore executeQuery:query]; + DocumentMap docs = [self.localStore executeQuery:query]; XCTAssertEqualObjects( docMapToArray(docs), (@[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, FSTDocumentStateLocalMutations), @@ -881,7 +882,7 @@ - (void)testCanExecuteMixedCollectionQueries { [self.localStore locallyWriteMutations:@[ FSTTestSetMutation(@"foo/bonk", @{@"a" : @"b"}) ]]; - MaybeDocumentMap docs = [self.localStore executeQuery:query]; + DocumentMap docs = [self.localStore executeQuery:query]; XCTAssertEqualObjects(docMapToArray(docs), (@[ FSTTestDoc("foo/bar", 20, @{@"a" : @"b"}, FSTDocumentStateSynced), FSTTestDoc("foo/baz", 10, @{@"a" : @"b"}, FSTDocumentStateSynced), diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm index 0bf2153c286..c42a713e358 100644 --- a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm @@ -34,6 +34,7 @@ namespace util = firebase::firestore::util; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::DocumentMap; using firebase::firestore::model::MaybeDocumentMap; NS_ASSUME_NONNULL_BEGIN @@ -142,7 +143,7 @@ - (void)testDocumentsMatchingQuery { [self setTestDocumentAtPath:"c/1"]; FSTQuery *query = FSTTestQuery("b"); - MaybeDocumentMap results = [self.remoteDocumentCache documentsMatchingQuery:query]; + DocumentMap results = [self.remoteDocumentCache documentsMatchingQuery:query]; [self expectMap:results hasDocsInArray:@[ FSTTestDoc("b/1", kVersion, _kDocData, FSTDocumentStateSynced), @@ -161,14 +162,14 @@ - (FSTDocument *)setTestDocumentAtPath:(const absl::string_view)path { return doc; } -- (void)expectMap:(const MaybeDocumentMap &)map +- (void)expectMap:(const DocumentMap &)map hasDocsInArray:(NSArray *)expected exactly:(BOOL)exactly { if (exactly) { XCTAssertEqual(map.size(), [expected count]); } for (FSTDocument *doc in expected) { - FSTMaybeDocument *actual = nil; + FSTDocument *actual = nil; auto found = map.find(doc.key); if (found != map.end()) { actual = found->second; diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index b9e6ca08050..0e3767da029 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -295,9 +295,10 @@ MaybeDocumentMap FSTTestDocUpdates(NSArray *docs) { FSTViewSnapshot *_Nullable FSTTestApplyChanges(FSTView *view, NSArray *docs, FSTTargetChange *_Nullable targetChange) { - return [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(docs)] - targetChange:targetChange] - .snapshot; + return + [view applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(docs)] + targetChange:targetChange] + .snapshot; } @implementation FSTTestTargetMetadataProvider { diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm index 9afa096a7cc..44de14b388c 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.mm +++ b/Firestore/Source/Core/FSTFirestoreClient.mm @@ -60,6 +60,7 @@ using firebase::firestore::local::LruParams; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::DocumentMap; using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::OnlineState; using firebase::firestore::util::Path; @@ -363,7 +364,7 @@ - (void)getDocumentsFromLocalCache:(FIRQuery *)query completion:(void (^)(FIRQuerySnapshot *_Nullable query, NSError *_Nullable error))completion { _workerQueue->Enqueue([self, query, completion] { - MaybeDocumentMap docs = [self.localStore executeQuery:query.query]; + DocumentMap docs = [self.localStore executeQuery:query.query]; FSTView *view = [[FSTView alloc] initWithQuery:query.query remoteDocuments:DocumentKeySet{}]; FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:docs]; diff --git a/Firestore/Source/Core/FSTSyncEngine.mm b/Firestore/Source/Core/FSTSyncEngine.mm index 451656e8113..5f287a247fc 100644 --- a/Firestore/Source/Core/FSTSyncEngine.mm +++ b/Firestore/Source/Core/FSTSyncEngine.mm @@ -215,7 +215,7 @@ - (TargetId)listenToQuery:(FSTQuery *)query { } - (FSTViewSnapshot *)initializeViewAndComputeSnapshotForQueryData:(FSTQueryData *)queryData { - MaybeDocumentMap docs = [self.localStore executeQuery:queryData.query]; + DocumentMap docs = [self.localStore executeQuery:queryData.query]; DocumentKeySet remoteKeys = [self.localStore remoteDocumentKeysForTarget:queryData.targetID]; FSTView *view = @@ -489,12 +489,12 @@ - (void)emitNewSnapshotsAndNotifyLocalStoreWithChanges:(const MaybeDocumentMap & [self.queryViewsByQuery enumerateKeysAndObjectsUsingBlock:^(FSTQuery *query, FSTQueryView *queryView, BOOL *stop) { FSTView *view = queryView.view; - FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:changes]; + FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithMaybeDocuments:changes]; if (viewDocChanges.needsRefill) { // The query has a limit and some docs were removed/updated, so we need to re-run the // query against the local store to make sure we didn't lose any good docs that had been // past the limit. - MaybeDocumentMap docs = [self.localStore executeQuery:queryView.query]; + DocumentMap docs = [self.localStore executeQuery:queryView.query]; viewDocChanges = [view computeChangesWithDocuments:docs previousChanges:viewDocChanges]; } diff --git a/Firestore/Source/Core/FSTView.h b/Firestore/Source/Core/FSTView.h index 1a25cd4ea7d..4ffc9452c01 100644 --- a/Firestore/Source/Core/FSTView.h +++ b/Firestore/Source/Core/FSTView.h @@ -107,6 +107,9 @@ typedef NS_ENUM(NSInteger, FSTLimboDocumentChangeType) { * @return a new set of docs, changes, and refill flag. */ - (FSTViewDocumentChanges *)computeChangesWithDocuments: + (const firebase::firestore::model::DocumentMap &)docChanges; + +- (FSTViewDocumentChanges *)computeChangesWithMaybeDocuments: (const firebase::firestore::model::MaybeDocumentMap &)docChanges; /** @@ -120,9 +123,14 @@ typedef NS_ENUM(NSInteger, FSTLimboDocumentChangeType) { * @return a new set of docs, changes, and refill flag. */ - (FSTViewDocumentChanges *) - computeChangesWithDocuments:(const firebase::firestore::model::MaybeDocumentMap &)docChanges + computeChangesWithDocuments:(const firebase::firestore::model::DocumentMap &)docChanges previousChanges:(nullable FSTViewDocumentChanges *)previousChanges; +- (FSTViewDocumentChanges *)computeChangesWithMaybeDocuments: + (const firebase::firestore::model::MaybeDocumentMap &)docChanges + previousChanges: + (nullable FSTViewDocumentChanges *)previousChanges; + /** * Updates the view with the given ViewDocumentChanges. * diff --git a/Firestore/Source/Core/FSTView.mm b/Firestore/Source/Core/FSTView.mm index 2dc95d57eb7..ef389d0ad3f 100644 --- a/Firestore/Source/Core/FSTView.mm +++ b/Firestore/Source/Core/FSTView.mm @@ -30,6 +30,7 @@ using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::DocumentMap; using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::OnlineState; @@ -201,19 +202,38 @@ - (instancetype)initWithQuery:(FSTQuery *)query remoteDocuments:(DocumentKeySet) return _syncedDocuments; } -- (FSTViewDocumentChanges *)computeChangesWithDocuments:(const MaybeDocumentMap &)docChanges { - return [self computeChangesWithDocuments:docChanges previousChanges:nil]; +FSTDocument *GetFSTDocumentOrNil(FSTMaybeDocument *maybeDoc) { + if ([maybeDoc isKindOfClass:[FSTDocument class]]) { + return static_cast(maybeDoc); + } + return nil; } -- (FSTViewDocumentChanges *)computeChangesWithDocuments:(const MaybeDocumentMap &)docChanges - previousChanges: - (nullable FSTViewDocumentChanges *)previousChanges { +FSTDocument *GetFSTDocumentOrNil(FSTDocument *doc) { + return doc; +} + +bool ShouldWaitForSyncedDocument(FSTDocument *newDoc, FSTDocument *oldDoc) { + // We suppress the initial change event for documents that were modified as part of a write + // acknowledgment (e.g. when the value of a server transform is applied) as Watch will send us + // the same document again. By suppressing the event, we only raise two user visible events (one + // with `hasPendingWrites` and the final state of the document) instead of three (one with + // `hasPendingWrites`, the modified document with `hasPendingWrites` and the final state of the + // document). + return (oldDoc.hasLocalMutations && newDoc.hasCommittedMutations && !newDoc.hasLocalMutations); +} + +template +FSTViewDocumentChanges *ComputeChanges(FSTView *view, + const DocumentKeySet &mutatedKeys, + const MapT &docChanges, + FSTViewDocumentChanges *previousChanges) { FSTDocumentViewChangeSet *changeSet = previousChanges ? previousChanges.changeSet : [FSTDocumentViewChangeSet changeSet]; - FSTDocumentSet *oldDocumentSet = previousChanges ? previousChanges.documentSet : self.documentSet; + FSTDocumentSet *oldDocumentSet = previousChanges ? previousChanges.documentSet : view.documentSet; - DocumentKeySet newMutatedKeys = previousChanges ? previousChanges.mutatedKeys : _mutatedKeys; - DocumentKeySet oldMutatedKeys = _mutatedKeys; + DocumentKeySet newMutatedKeys = previousChanges ? previousChanges.mutatedKeys : mutatedKeys; + DocumentKeySet oldMutatedKeys = mutatedKeys; FSTDocumentSet *newDocumentSet = oldDocumentSet; BOOL needsRefill = NO; @@ -226,22 +246,17 @@ - (FSTViewDocumentChanges *)computeChangesWithDocuments:(const MaybeDocumentMap // Note that this should never get used in a refill (when previousChanges is set), because there // will only be adds -- no deletes or updates. FSTDocument *_Nullable lastDocInLimit = - (self.query.limit && oldDocumentSet.count == self.query.limit) ? oldDocumentSet.lastDocument + (view.query.limit && oldDocumentSet.count == view.query.limit) ? oldDocumentSet.lastDocument : nil; for (const auto &kv : docChanges) { const DocumentKey &key = kv.first; - FSTMaybeDocument *maybeNewDoc = kv.second; - FSTDocument *_Nullable oldDoc = [oldDocumentSet documentForKey:key]; - FSTDocument *_Nullable newDoc = nil; - if ([maybeNewDoc isKindOfClass:[FSTDocument class]]) { - newDoc = (FSTDocument *)maybeNewDoc; - } + FSTDocument *_Nullable newDoc = GetFSTDocumentOrNil(kv.second); if (newDoc) { HARD_ASSERT(key == newDoc.key, "Mismatching key in document changes: %s != %s", key, newDoc.key.ToString()); - if (![self.query matchesDocument:newDoc]) { + if (![view.query matchesDocument:newDoc]) { newDoc = nil; } } @@ -259,13 +274,13 @@ - (FSTViewDocumentChanges *)computeChangesWithDocuments:(const MaybeDocumentMap if (oldDoc && newDoc) { BOOL docsEqual = [oldDoc.data isEqual:newDoc.data]; if (!docsEqual) { - if (![self shouldWaitForSyncedDocument:newDoc oldDocument:oldDoc]) { + if (!ShouldWaitForSyncedDocument(newDoc, oldDoc)) { [changeSet addChange:[FSTDocumentViewChange changeWithDocument:newDoc type:FSTDocumentViewChangeTypeModified]]; changeApplied = YES; - if (lastDocInLimit && self.query.comparator(newDoc, lastDocInLimit) > 0) { + if (lastDocInLimit && view.query.comparator(newDoc, lastDocInLimit) > 0) { // This doc moved from inside the limit to after the limit. That means there may be // some doc in the local cache that's actually less than this one. needsRefill = YES; @@ -311,8 +326,8 @@ - (FSTViewDocumentChanges *)computeChangesWithDocuments:(const MaybeDocumentMap } } - if (self.query.limit) { - for (long i = newDocumentSet.count - self.query.limit; i > 0; --i) { + if (view.query.limit) { + for (long i = newDocumentSet.count - view.query.limit; i > 0; --i) { FSTDocument *oldDoc = [newDocumentSet lastDocument]; newDocumentSet = [newDocumentSet documentSetByRemovingKey:oldDoc.key]; newMutatedKeys = newMutatedKeys.erase(oldDoc.key); @@ -331,14 +346,24 @@ - (FSTViewDocumentChanges *)computeChangesWithDocuments:(const MaybeDocumentMap mutatedKeys:newMutatedKeys]; } -- (BOOL)shouldWaitForSyncedDocument:(FSTDocument *)newDoc oldDocument:(FSTDocument *)oldDoc { - // We suppress the initial change event for documents that were modified as part of a write - // acknowledgment (e.g. when the value of a server transform is applied) as Watch will send us - // the same document again. By suppressing the event, we only raise two user visible events (one - // with `hasPendingWrites` and the final state of the document) instead of three (one with - // `hasPendingWrites`, the modified document with `hasPendingWrites` and the final state of the - // document). - return (oldDoc.hasLocalMutations && newDoc.hasCommittedMutations && !newDoc.hasLocalMutations); +- (FSTViewDocumentChanges *)computeChangesWithDocuments:(const DocumentMap &)docChanges { + return [self computeChangesWithDocuments:docChanges previousChanges:nil]; +} + +- (FSTViewDocumentChanges *)computeChangesWithMaybeDocuments:(const MaybeDocumentMap &)docChanges { + return [self computeChangesWithMaybeDocuments:docChanges previousChanges:nil]; +} + +- (FSTViewDocumentChanges *)computeChangesWithMaybeDocuments:(const MaybeDocumentMap &)docChanges + previousChanges:(nullable FSTViewDocumentChanges *) + previousChanges { + return ComputeChanges(self, _mutatedKeys, docChanges, previousChanges); +} + +- (FSTViewDocumentChanges *)computeChangesWithDocuments:(const DocumentMap &)docChanges + previousChanges: + (nullable FSTViewDocumentChanges *)previousChanges { + return ComputeChanges(self, _mutatedKeys, docChanges, previousChanges); } - (FSTViewChange *)applyChangesToDocuments:(FSTViewDocumentChanges *)docChanges { diff --git a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm index af81b3decd3..5dcaadd4c71 100644 --- a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm +++ b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm @@ -38,6 +38,7 @@ using firebase::firestore::local::LevelDbTransaction; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::DocumentMap; using firebase::firestore::model::MaybeDocumentMap; using leveldb::DB; using leveldb::Status; @@ -84,8 +85,8 @@ - (nullable FSTMaybeDocument *)entryForKey:(const DocumentKey &)documentKey { } } -- (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { - MaybeDocumentMap results; +- (DocumentMap)documentsMatchingQuery:(FSTQuery *)query { + DocumentMap results; // Documents are ordered by key, so we can use a prefix scan to narrow down // the documents we need to match the query against. @@ -100,7 +101,7 @@ - (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { if (!query.path.IsPrefixOf(maybeDoc.key.path())) { break; } else if ([maybeDoc isKindOfClass:[FSTDocument class]]) { - results = results.insert(maybeDoc.key, maybeDoc); + results = results.insert(maybeDoc.key, static_cast(maybeDoc)); } } diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.h b/Firestore/Source/Local/FSTLocalDocumentsView.h index 2b54471f9db..d87dc837e70 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.h +++ b/Firestore/Source/Local/FSTLocalDocumentsView.h @@ -56,7 +56,7 @@ NS_ASSUME_NONNULL_BEGIN (const firebase::firestore::model::DocumentKeySet &)keys; /** Performs a query against the local view of all documents. */ -- (firebase::firestore::model::MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query; +- (firebase::firestore::model::DocumentMap)documentsMatchingQuery:(FSTQuery *)query; @end diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index 6cfe9fe3f3b..65798bf687e 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -31,6 +31,7 @@ using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::model::DocumentMap; using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::ResourcePath; using firebase::firestore::model::SnapshotVersion; @@ -98,7 +99,7 @@ - (MaybeDocumentMap)documentsForKeys:(const DocumentKeySet &)keys { return results; } -- (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { +- (DocumentMap)documentsMatchingQuery:(FSTQuery *)query { if (DocumentKey::IsDocumentKey(query.path)) { return [self documentsMatchingDocumentQuery:query.path]; } else { @@ -106,18 +107,18 @@ - (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { } } -- (MaybeDocumentMap)documentsMatchingDocumentQuery:(const ResourcePath &)docPath { - MaybeDocumentMap result; +- (DocumentMap)documentsMatchingDocumentQuery:(const ResourcePath &)docPath { + DocumentMap result; // Just do a simple document lookup. FSTMaybeDocument *doc = [self documentForKey:DocumentKey{docPath}]; if ([doc isKindOfClass:[FSTDocument class]]) { - result = result.insert(doc.key, doc); + result = result.insert(doc.key, static_cast(doc)); } return result; } -- (MaybeDocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { - MaybeDocumentMap results = [self.remoteDocumentCache documentsMatchingQuery:query]; +- (DocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { + DocumentMap results = [self.remoteDocumentCache documentsMatchingQuery:query]; // Get locally persisted mutation batches. NSArray *matchingBatches = [self.mutationQueue allMutationBatchesAffectingQuery:query]; @@ -141,7 +142,7 @@ - (MaybeDocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { localWriteTime:batch.localWriteTime]; if ([mutatedDoc isKindOfClass:[FSTDocument class]]) { - results = results.insert(key, mutatedDoc); + results = results.insert(key, static_cast(mutatedDoc)); } else { results = results.erase(key); } @@ -151,10 +152,10 @@ - (MaybeDocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { // Finally, filter out any documents that don't actually match the query. Note that the extra // reference here prevents ARC from deallocating the initial unfiltered results while we're // enumerating them. - MaybeDocumentMap unfiltered = results; + DocumentMap unfiltered = results; for (const auto &kv : unfiltered) { const DocumentKey &key = kv.first; - FSTDocument *doc = static_cast(kv.second); + FSTDocument *doc = kv.second; if (![query matchesDocument:doc]) { results = results.erase(key); } diff --git a/Firestore/Source/Local/FSTLocalStore.h b/Firestore/Source/Local/FSTLocalStore.h index eb74aa3673b..60f8d2ee4a7 100644 --- a/Firestore/Source/Local/FSTLocalStore.h +++ b/Firestore/Source/Local/FSTLocalStore.h @@ -166,7 +166,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)releaseQuery:(FSTQuery *)query; /** Runs @a query against all the documents in the local store and returns the results. */ -- (firebase::firestore::model::MaybeDocumentMap)executeQuery:(FSTQuery *)query; +- (firebase::firestore::model::DocumentMap)executeQuery:(FSTQuery *)query; /** Notify the local store of the changed views to locally pin / unpin documents. */ - (void)notifyLocalViewChanges:(NSArray *)viewChanges; diff --git a/Firestore/Source/Local/FSTLocalStore.mm b/Firestore/Source/Local/FSTLocalStore.mm index edb7d839c99..4626a3a297a 100644 --- a/Firestore/Source/Local/FSTLocalStore.mm +++ b/Firestore/Source/Local/FSTLocalStore.mm @@ -425,8 +425,8 @@ - (void)releaseQuery:(FSTQuery *)query { }); } -- (MaybeDocumentMap)executeQuery:(FSTQuery *)query { - return self.persistence.run("ExecuteQuery", [&]() -> MaybeDocumentMap { +- (DocumentMap)executeQuery:(FSTQuery *)query { + return self.persistence.run("ExecuteQuery", [&]() -> DocumentMap { return [self.localDocuments documentsMatchingQuery:query]; }); } diff --git a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm index ff00e05f02d..ae4912cef35 100644 --- a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm +++ b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm @@ -28,6 +28,7 @@ using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::ListenSequenceNumber; +using firebase::firestore::model::DocumentMap; using firebase::firestore::model::MaybeDocumentMap; NS_ASSUME_NONNULL_BEGIN @@ -67,8 +68,8 @@ - (nullable FSTMaybeDocument *)entryForKey:(const DocumentKey &)key { return found != _docs.end() ? found->second : nil; } -- (MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query { - MaybeDocumentMap result; +- (DocumentMap)documentsMatchingQuery:(FSTQuery *)query { + DocumentMap result; // Documents are ordered by key, so we can use a prefix scan to narrow down the documents // we need to match the query against. diff --git a/Firestore/Source/Local/FSTRemoteDocumentCache.h b/Firestore/Source/Local/FSTRemoteDocumentCache.h index 86a3fe0dacc..46fb5955a36 100644 --- a/Firestore/Source/Local/FSTRemoteDocumentCache.h +++ b/Firestore/Source/Local/FSTRemoteDocumentCache.h @@ -67,7 +67,7 @@ NS_ASSUME_NONNULL_BEGIN * @param query The query to match documents against. * @return The set of matching documents. */ -- (firebase::firestore::model::MaybeDocumentMap)documentsMatchingQuery:(FSTQuery *)query; +- (firebase::firestore::model::DocumentMap)documentsMatchingQuery:(FSTQuery *)query; @end From 595e2c163d7fb5b417db69a43724c78184e855c4 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 6 Dec 2018 15:01:11 -0500 Subject: [PATCH 07/16] Comments --- .../xcschemes/Firestore_IntegrationTests_iOS.xcscheme | 2 ++ Firestore/Source/Core/FSTView.h | 4 ++++ Firestore/Source/Core/FSTView.mm | 4 +++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme b/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme index 4e6f82845e2..2d22568f77b 100644 --- a/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme +++ b/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme @@ -26,6 +26,8 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" + enableAddressSanitizer = "YES" + enableASanStackUseAfterReturn = "YES" shouldUseLaunchSchemeArgsEnv = "YES"> (maybeDoc); } return nil; } - FSTDocument *GetFSTDocumentOrNil(FSTDocument *doc) { return doc; } @@ -223,6 +223,8 @@ bool ShouldWaitForSyncedDocument(FSTDocument *newDoc, FSTDocument *oldDoc) { return (oldDoc.hasLocalMutations && newDoc.hasCommittedMutations && !newDoc.hasLocalMutations); } +// Shared implementation of the `computeChanges` methods; the customization point is in +// `GetFSTDocumentOrNil` function. template FSTViewDocumentChanges *ComputeChanges(FSTView *view, const DocumentKeySet &mutatedKeys, From 229515f3b23a7031f789cd2ac1c7703499db26e8 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 6 Dec 2018 18:12:10 -0500 Subject: [PATCH 08/16] Undo --- .../xcschemes/Firestore_IntegrationTests_iOS.xcscheme | 2 -- 1 file changed, 2 deletions(-) diff --git a/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme b/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme index 2d22568f77b..4e6f82845e2 100644 --- a/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme +++ b/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme @@ -26,8 +26,6 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" - enableAddressSanitizer = "YES" - enableASanStackUseAfterReturn = "YES" shouldUseLaunchSchemeArgsEnv = "YES"> Date: Sun, 9 Dec 2018 19:27:43 -0800 Subject: [PATCH 09/16] DocumentMap take 2 wip --- Firestore/Source/Core/FSTFirestoreClient.mm | 2 +- Firestore/Source/Core/FSTSyncEngine.mm | 4 +- .../firebase/firestore/model/document_map.h | 73 ++++++++++++++++++- 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm index 44de14b388c..7b172d475c4 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.mm +++ b/Firestore/Source/Core/FSTFirestoreClient.mm @@ -367,7 +367,7 @@ - (void)getDocumentsFromLocalCache:(FIRQuery *)query DocumentMap docs = [self.localStore executeQuery:query.query]; FSTView *view = [[FSTView alloc] initWithQuery:query.query remoteDocuments:DocumentKeySet{}]; - FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:docs]; + FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:docs.underlying_map()]; FSTViewChange *viewChange = [view applyChangesToDocuments:viewDocChanges]; HARD_ASSERT(viewChange.limboChanges.count == 0, "View returned limbo documents during local-only query execution."); diff --git a/Firestore/Source/Core/FSTSyncEngine.mm b/Firestore/Source/Core/FSTSyncEngine.mm index 5f287a247fc..39972c86730 100644 --- a/Firestore/Source/Core/FSTSyncEngine.mm +++ b/Firestore/Source/Core/FSTSyncEngine.mm @@ -220,7 +220,7 @@ - (FSTViewSnapshot *)initializeViewAndComputeSnapshotForQueryData:(FSTQueryData FSTView *view = [[FSTView alloc] initWithQuery:queryData.query remoteDocuments:std::move(remoteKeys)]; - FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:docs]; + FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:docs.underlying_map()]; FSTViewChange *viewChange = [view applyChangesToDocuments:viewDocChanges]; HARD_ASSERT(viewChange.limboChanges.count == 0, "View returned limbo docs before target ack from the server."); @@ -495,7 +495,7 @@ - (void)emitNewSnapshotsAndNotifyLocalStoreWithChanges:(const MaybeDocumentMap & // query against the local store to make sure we didn't lose any good docs that had been // past the limit. DocumentMap docs = [self.localStore executeQuery:queryView.query]; - viewDocChanges = [view computeChangesWithDocuments:docs previousChanges:viewDocChanges]; + viewDocChanges = [view computeChangesWithDocuments:docs.underlying_map() previousChanges:viewDocChanges]; } FSTTargetChange *_Nullable targetChange = nil; diff --git a/Firestore/core/src/firebase/firestore/model/document_map.h b/Firestore/core/src/firebase/firestore/model/document_map.h index 0b5a421a06f..e1c7a27f3c9 100644 --- a/Firestore/core/src/firebase/firestore/model/document_map.h +++ b/Firestore/core/src/firebase/firestore/model/document_map.h @@ -19,9 +19,14 @@ #import "Firestore/Source/Model/FSTDocument.h" +#include + #include "Firestore/core/src/firebase/firestore/immutable/sorted_map.h" +#include "Firestore/core/src/firebase/firestore/immutable/sorted_map_iterator.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "absl/base/attributes.h" + namespace firebase { namespace firestore { namespace model { @@ -34,8 +39,74 @@ using MaybeDocumentMap = immutable::SortedMap; /** * Convenience type for a map of keys to Documents, since they are so common. + * + * PORTING NOTE: OBC */ -using DocumentMap = immutable::SortedMap; +class DocumentMap { + public: + class const_iterator { + public: + using iterator_category = MaybeDocumentMap::const_iterator::iterator_category; + using value_type = FSTDocument*; + using pointer = const value_type*; + using reference = const value_type&; + using difference_type = MaybeDocumentMap::const_iterator::difference_type; + + const_iterator() = default; + + pointer get() const { return static_cast(iter_.get()); } + reference operator*() const { return static_cast(*iter_); } + pointer operator->() const { return static_cast(iter_.operator->()); } + + const_iterator& operator++() { + ++iter_; + return *this; + } + const_iterator operator++(int /*unused*/) { + const_iterator old_value = *this; + ++iter_; + return old_value; + } + + friend bool operator==(const const_iterator& a, + const const_iterator& b) { return a.iter_ == b.iter_; } + bool operator!=(const const_iterator& b) const { return a.iter_ != b.iter_; } + + private: + explicit const_iterator(MaybeDocumentMap::const_iterator&& iter) : iter_{iter} {} + + MaybeDocumentMap::const_iterator iter_; + }; + + DocumentMap() = default; + + const_iterator begin() const { + return const_iterator{map_.begin()}; + } + const_iterator end() const { + return const_iterator{map_.end()}; + } + + const_iterator find(const DocumentKey& key) const { + return const_iterator{map_.find(key)}; + } + + ABSL_MUST_USE_RESULT DocumentMap insert(const DocumentKey& key, FSTDocument* value) const { + return DocumentMap{map_.insert(key, value)}; + } + + ABSL_MUST_USE_RESULT DocumentMap erase(const DocumentKey& key) const { + return DocumentMap{map_.erase(key)}; + } + + const MaybeDocumentMap& underlying_map() const { return map_; } + + private: + explicit DocumentMap(MaybeDocumentMap&& map) : map_{std::move(map)} {} + + MaybeDocumentMap map_; +}; + } // namespace model } // namespace firestore From 40a03d807f8736b57f5106e696d90d69f88cb0dd Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 9 Dec 2018 20:07:01 -0800 Subject: [PATCH 10/16] DocumentMap take 2 compiles --- Firestore/Example/Tests/Util/FSTHelpers.mm | 2 +- Firestore/Source/Core/FSTSyncEngine.mm | 2 +- Firestore/Source/Core/FSTView.h | 14 +- Firestore/Source/Core/FSTView.mm | 83 ++++------- .../firebase/firestore/model/document_map.h | 134 +++++++++++------- 5 files changed, 113 insertions(+), 122 deletions(-) diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index 0e3767da029..e05c0e930c9 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -296,7 +296,7 @@ MaybeDocumentMap FSTTestDocUpdates(NSArray *docs) { NSArray *docs, FSTTargetChange *_Nullable targetChange) { return - [view applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(docs)] + [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(docs)] targetChange:targetChange] .snapshot; } diff --git a/Firestore/Source/Core/FSTSyncEngine.mm b/Firestore/Source/Core/FSTSyncEngine.mm index 39972c86730..0ee2d3738fe 100644 --- a/Firestore/Source/Core/FSTSyncEngine.mm +++ b/Firestore/Source/Core/FSTSyncEngine.mm @@ -489,7 +489,7 @@ - (void)emitNewSnapshotsAndNotifyLocalStoreWithChanges:(const MaybeDocumentMap & [self.queryViewsByQuery enumerateKeysAndObjectsUsingBlock:^(FSTQuery *query, FSTQueryView *queryView, BOOL *stop) { FSTView *view = queryView.view; - FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithMaybeDocuments:changes]; + FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:changes]; if (viewDocChanges.needsRefill) { // The query has a limit and some docs were removed/updated, so we need to re-run the // query against the local store to make sure we didn't lose any good docs that had been diff --git a/Firestore/Source/Core/FSTView.h b/Firestore/Source/Core/FSTView.h index d1d34d8000f..1a25cd4ea7d 100644 --- a/Firestore/Source/Core/FSTView.h +++ b/Firestore/Source/Core/FSTView.h @@ -107,12 +107,6 @@ typedef NS_ENUM(NSInteger, FSTLimboDocumentChangeType) { * @return a new set of docs, changes, and refill flag. */ - (FSTViewDocumentChanges *)computeChangesWithDocuments: - (const firebase::firestore::model::DocumentMap &)docChanges; - -/** Like `computeChangesWithDocuments:`, but accepts a map of `MaybeDocument`s. */ -// PORTING NOTE: in C++, a container of `Derived` does not convert to a container of `Base`, -// necessitating two overloads. -- (FSTViewDocumentChanges *)computeChangesWithMaybeDocuments: (const firebase::firestore::model::MaybeDocumentMap &)docChanges; /** @@ -126,15 +120,9 @@ typedef NS_ENUM(NSInteger, FSTLimboDocumentChangeType) { * @return a new set of docs, changes, and refill flag. */ - (FSTViewDocumentChanges *) - computeChangesWithDocuments:(const firebase::firestore::model::DocumentMap &)docChanges + computeChangesWithDocuments:(const firebase::firestore::model::MaybeDocumentMap &)docChanges previousChanges:(nullable FSTViewDocumentChanges *)previousChanges; -/** Like `computeChangesWithDocuments:`, but accepts a map of `MaybeDocument`s. */ -- (FSTViewDocumentChanges *)computeChangesWithMaybeDocuments: - (const firebase::firestore::model::MaybeDocumentMap &)docChanges - previousChanges: - (nullable FSTViewDocumentChanges *)previousChanges; - /** * Updates the view with the given ViewDocumentChanges. * diff --git a/Firestore/Source/Core/FSTView.mm b/Firestore/Source/Core/FSTView.mm index 07488499f39..2dc95d57eb7 100644 --- a/Firestore/Source/Core/FSTView.mm +++ b/Firestore/Source/Core/FSTView.mm @@ -30,7 +30,6 @@ using firebase::firestore::model::DocumentKey; using firebase::firestore::model::DocumentKeySet; -using firebase::firestore::model::DocumentMap; using firebase::firestore::model::MaybeDocumentMap; using firebase::firestore::model::OnlineState; @@ -202,40 +201,19 @@ - (instancetype)initWithQuery:(FSTQuery *)query remoteDocuments:(DocumentKeySet) return _syncedDocuments; } -// These two overloads allow skipping a cast when dealing with a map of `FSTDocument`s. -FSTDocument *GetFSTDocumentOrNil(FSTMaybeDocument *maybeDoc) { - if ([maybeDoc isKindOfClass:[FSTDocument class]]) { - return static_cast(maybeDoc); - } - return nil; -} -FSTDocument *GetFSTDocumentOrNil(FSTDocument *doc) { - return doc; -} - -bool ShouldWaitForSyncedDocument(FSTDocument *newDoc, FSTDocument *oldDoc) { - // We suppress the initial change event for documents that were modified as part of a write - // acknowledgment (e.g. when the value of a server transform is applied) as Watch will send us - // the same document again. By suppressing the event, we only raise two user visible events (one - // with `hasPendingWrites` and the final state of the document) instead of three (one with - // `hasPendingWrites`, the modified document with `hasPendingWrites` and the final state of the - // document). - return (oldDoc.hasLocalMutations && newDoc.hasCommittedMutations && !newDoc.hasLocalMutations); +- (FSTViewDocumentChanges *)computeChangesWithDocuments:(const MaybeDocumentMap &)docChanges { + return [self computeChangesWithDocuments:docChanges previousChanges:nil]; } -// Shared implementation of the `computeChanges` methods; the customization point is in -// `GetFSTDocumentOrNil` function. -template -FSTViewDocumentChanges *ComputeChanges(FSTView *view, - const DocumentKeySet &mutatedKeys, - const MapT &docChanges, - FSTViewDocumentChanges *previousChanges) { +- (FSTViewDocumentChanges *)computeChangesWithDocuments:(const MaybeDocumentMap &)docChanges + previousChanges: + (nullable FSTViewDocumentChanges *)previousChanges { FSTDocumentViewChangeSet *changeSet = previousChanges ? previousChanges.changeSet : [FSTDocumentViewChangeSet changeSet]; - FSTDocumentSet *oldDocumentSet = previousChanges ? previousChanges.documentSet : view.documentSet; + FSTDocumentSet *oldDocumentSet = previousChanges ? previousChanges.documentSet : self.documentSet; - DocumentKeySet newMutatedKeys = previousChanges ? previousChanges.mutatedKeys : mutatedKeys; - DocumentKeySet oldMutatedKeys = mutatedKeys; + DocumentKeySet newMutatedKeys = previousChanges ? previousChanges.mutatedKeys : _mutatedKeys; + DocumentKeySet oldMutatedKeys = _mutatedKeys; FSTDocumentSet *newDocumentSet = oldDocumentSet; BOOL needsRefill = NO; @@ -248,17 +226,22 @@ bool ShouldWaitForSyncedDocument(FSTDocument *newDoc, FSTDocument *oldDoc) { // Note that this should never get used in a refill (when previousChanges is set), because there // will only be adds -- no deletes or updates. FSTDocument *_Nullable lastDocInLimit = - (view.query.limit && oldDocumentSet.count == view.query.limit) ? oldDocumentSet.lastDocument + (self.query.limit && oldDocumentSet.count == self.query.limit) ? oldDocumentSet.lastDocument : nil; for (const auto &kv : docChanges) { const DocumentKey &key = kv.first; + FSTMaybeDocument *maybeNewDoc = kv.second; + FSTDocument *_Nullable oldDoc = [oldDocumentSet documentForKey:key]; - FSTDocument *_Nullable newDoc = GetFSTDocumentOrNil(kv.second); + FSTDocument *_Nullable newDoc = nil; + if ([maybeNewDoc isKindOfClass:[FSTDocument class]]) { + newDoc = (FSTDocument *)maybeNewDoc; + } if (newDoc) { HARD_ASSERT(key == newDoc.key, "Mismatching key in document changes: %s != %s", key, newDoc.key.ToString()); - if (![view.query matchesDocument:newDoc]) { + if (![self.query matchesDocument:newDoc]) { newDoc = nil; } } @@ -276,13 +259,13 @@ bool ShouldWaitForSyncedDocument(FSTDocument *newDoc, FSTDocument *oldDoc) { if (oldDoc && newDoc) { BOOL docsEqual = [oldDoc.data isEqual:newDoc.data]; if (!docsEqual) { - if (!ShouldWaitForSyncedDocument(newDoc, oldDoc)) { + if (![self shouldWaitForSyncedDocument:newDoc oldDocument:oldDoc]) { [changeSet addChange:[FSTDocumentViewChange changeWithDocument:newDoc type:FSTDocumentViewChangeTypeModified]]; changeApplied = YES; - if (lastDocInLimit && view.query.comparator(newDoc, lastDocInLimit) > 0) { + if (lastDocInLimit && self.query.comparator(newDoc, lastDocInLimit) > 0) { // This doc moved from inside the limit to after the limit. That means there may be // some doc in the local cache that's actually less than this one. needsRefill = YES; @@ -328,8 +311,8 @@ bool ShouldWaitForSyncedDocument(FSTDocument *newDoc, FSTDocument *oldDoc) { } } - if (view.query.limit) { - for (long i = newDocumentSet.count - view.query.limit; i > 0; --i) { + if (self.query.limit) { + for (long i = newDocumentSet.count - self.query.limit; i > 0; --i) { FSTDocument *oldDoc = [newDocumentSet lastDocument]; newDocumentSet = [newDocumentSet documentSetByRemovingKey:oldDoc.key]; newMutatedKeys = newMutatedKeys.erase(oldDoc.key); @@ -348,24 +331,14 @@ bool ShouldWaitForSyncedDocument(FSTDocument *newDoc, FSTDocument *oldDoc) { mutatedKeys:newMutatedKeys]; } -- (FSTViewDocumentChanges *)computeChangesWithDocuments:(const DocumentMap &)docChanges { - return [self computeChangesWithDocuments:docChanges previousChanges:nil]; -} - -- (FSTViewDocumentChanges *)computeChangesWithMaybeDocuments:(const MaybeDocumentMap &)docChanges { - return [self computeChangesWithMaybeDocuments:docChanges previousChanges:nil]; -} - -- (FSTViewDocumentChanges *)computeChangesWithMaybeDocuments:(const MaybeDocumentMap &)docChanges - previousChanges:(nullable FSTViewDocumentChanges *) - previousChanges { - return ComputeChanges(self, _mutatedKeys, docChanges, previousChanges); -} - -- (FSTViewDocumentChanges *)computeChangesWithDocuments:(const DocumentMap &)docChanges - previousChanges: - (nullable FSTViewDocumentChanges *)previousChanges { - return ComputeChanges(self, _mutatedKeys, docChanges, previousChanges); +- (BOOL)shouldWaitForSyncedDocument:(FSTDocument *)newDoc oldDocument:(FSTDocument *)oldDoc { + // We suppress the initial change event for documents that were modified as part of a write + // acknowledgment (e.g. when the value of a server transform is applied) as Watch will send us + // the same document again. By suppressing the event, we only raise two user visible events (one + // with `hasPendingWrites` and the final state of the document) instead of three (one with + // `hasPendingWrites`, the modified document with `hasPendingWrites` and the final state of the + // document). + return (oldDoc.hasLocalMutations && newDoc.hasCommittedMutations && !newDoc.hasLocalMutations); } - (FSTViewChange *)applyChangesToDocuments:(FSTViewDocumentChanges *)docChanges { diff --git a/Firestore/core/src/firebase/firestore/model/document_map.h b/Firestore/core/src/firebase/firestore/model/document_map.h index e1c7a27f3c9..6412f920034 100644 --- a/Firestore/core/src/firebase/firestore/model/document_map.h +++ b/Firestore/core/src/firebase/firestore/model/document_map.h @@ -43,70 +43,100 @@ using MaybeDocumentMap = immutable::SortedMap; * PORTING NOTE: OBC */ class DocumentMap { - public: - class const_iterator { - public: - using iterator_category = MaybeDocumentMap::const_iterator::iterator_category; - using value_type = FSTDocument*; - using pointer = const value_type*; - using reference = const value_type&; - using difference_type = MaybeDocumentMap::const_iterator::difference_type; - - const_iterator() = default; - - pointer get() const { return static_cast(iter_.get()); } - reference operator*() const { return static_cast(*iter_); } - pointer operator->() const { return static_cast(iter_.operator->()); } - - const_iterator& operator++() { - ++iter_; - return *this; - } - const_iterator operator++(int /*unused*/) { - const_iterator old_value = *this; - ++iter_; - return old_value; - } - - friend bool operator==(const const_iterator& a, - const const_iterator& b) { return a.iter_ == b.iter_; } - bool operator!=(const const_iterator& b) const { return a.iter_ != b.iter_; } - - private: - explicit const_iterator(MaybeDocumentMap::const_iterator&& iter) : iter_{iter} {} - - MaybeDocumentMap::const_iterator iter_; - }; - - DocumentMap() = default; - - const_iterator begin() const { - return const_iterator{map_.begin()}; + public: + class const_iterator { + public: + using iterator_category = + MaybeDocumentMap::const_iterator::iterator_category; + using value_type = std::pair; + using pointer = const value_type*; + using reference = const value_type&; + using difference_type = MaybeDocumentMap::const_iterator::difference_type; + + const_iterator() = default; + + pointer get() const { + UpdateCurrentValue(); + return ¤t_value_; } - const_iterator end() const { - return const_iterator{map_.end()}; + reference operator*() const { + UpdateCurrentValue(); + return current_value_; + } + pointer operator->() const { + return get(); } - const_iterator find(const DocumentKey& key) const { - return const_iterator{map_.find(key)}; + const_iterator& operator++() { + ++iter_; + return *this; + } + const_iterator operator++(int /*unused*/) { + const_iterator old_value = *this; + ++iter_; + return old_value; } - ABSL_MUST_USE_RESULT DocumentMap insert(const DocumentKey& key, FSTDocument* value) const { - return DocumentMap{map_.insert(key, value)}; + friend bool operator==(const const_iterator& a, const const_iterator& b) { + return a.iter_ == b.iter_; } + friend bool operator!=(const const_iterator& a, const const_iterator& b) { + return a.iter_ != b.iter_; + } + + private: + friend class DocumentMap; - ABSL_MUST_USE_RESULT DocumentMap erase(const DocumentKey& key) const { - return DocumentMap{map_.erase(key)}; + explicit const_iterator(MaybeDocumentMap::const_iterator&& iter) + : iter_{iter} { } - const MaybeDocumentMap& underlying_map() const { return map_; } + void UpdateCurrentValue() const { + const std::pair& underlying_value = *iter_; + current_value_ = + value_type{underlying_value.first, + static_cast(underlying_value.second)}; + } - private: - explicit DocumentMap(MaybeDocumentMap&& map) : map_{std::move(map)} {} + MaybeDocumentMap::const_iterator iter_; + mutable value_type current_value_; + }; - MaybeDocumentMap map_; -}; + DocumentMap() = default; + + const_iterator begin() const { + return const_iterator{map_.begin()}; + } + const_iterator end() const { + return const_iterator{map_.end()}; + } + const_iterator find(const DocumentKey& key) const { + return const_iterator{map_.find(key)}; + } + + ABSL_MUST_USE_RESULT DocumentMap insert(const DocumentKey& key, + FSTDocument* value) const { + return DocumentMap{map_.insert(key, value)}; + } + + ABSL_MUST_USE_RESULT DocumentMap erase(const DocumentKey& key) const { + return DocumentMap{map_.erase(key)}; + } + + bool empty() const { return map_.empty(); } + MaybeDocumentMap::size_type size() const { return map_.size(); } + + const MaybeDocumentMap& underlying_map() const { + return map_; + } + + private: + explicit DocumentMap(MaybeDocumentMap&& map) : map_{std::move(map)} { + } + + MaybeDocumentMap map_; +}; } // namespace model } // namespace firestore From cca3650b3530a0a82943a7a355131d3de534f6c1 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 9 Dec 2018 20:11:41 -0800 Subject: [PATCH 11/16] style.sh --- .../tvOSSample/AuthViewController.swift | 1 + .../tvOSSample/EmailLoginViewController.swift | 1 + Firestore/Example/Tests/Util/FSTHelpers.mm | 7 +++---- Firestore/Source/Core/FSTFirestoreClient.mm | 3 ++- Firestore/Source/Core/FSTSyncEngine.mm | 3 ++- .../src/firebase/firestore/model/document_map.h | 15 ++++++++++----- 6 files changed, 19 insertions(+), 11 deletions(-) diff --git a/Example/tvOSSample/tvOSSample/AuthViewController.swift b/Example/tvOSSample/tvOSSample/AuthViewController.swift index 2492618f928..56276ed353b 100644 --- a/Example/tvOSSample/tvOSSample/AuthViewController.swift +++ b/Example/tvOSSample/tvOSSample/AuthViewController.swift @@ -16,6 +16,7 @@ import UIKit import FirebaseAuth class AuthViewController: UIViewController { + // MARK: - User Interface /// A stackview containing all of the buttons to providers (Email, OAuth, etc). diff --git a/Example/tvOSSample/tvOSSample/EmailLoginViewController.swift b/Example/tvOSSample/tvOSSample/EmailLoginViewController.swift index 38d5425a49e..cabb8c97e63 100644 --- a/Example/tvOSSample/tvOSSample/EmailLoginViewController.swift +++ b/Example/tvOSSample/tvOSSample/EmailLoginViewController.swift @@ -21,6 +21,7 @@ protocol EmailLoginDelegate { } class EmailLoginViewController: UIViewController { + // MARK: - Public Properties var delegate: EmailLoginDelegate? diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index e05c0e930c9..b9e6ca08050 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -295,10 +295,9 @@ MaybeDocumentMap FSTTestDocUpdates(NSArray *docs) { FSTViewSnapshot *_Nullable FSTTestApplyChanges(FSTView *view, NSArray *docs, FSTTargetChange *_Nullable targetChange) { - return - [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(docs)] - targetChange:targetChange] - .snapshot; + return [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(docs)] + targetChange:targetChange] + .snapshot; } @implementation FSTTestTargetMetadataProvider { diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm index 7b172d475c4..acb16e3644c 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.mm +++ b/Firestore/Source/Core/FSTFirestoreClient.mm @@ -367,7 +367,8 @@ - (void)getDocumentsFromLocalCache:(FIRQuery *)query DocumentMap docs = [self.localStore executeQuery:query.query]; FSTView *view = [[FSTView alloc] initWithQuery:query.query remoteDocuments:DocumentKeySet{}]; - FSTViewDocumentChanges *viewDocChanges = [view computeChangesWithDocuments:docs.underlying_map()]; + FSTViewDocumentChanges *viewDocChanges = + [view computeChangesWithDocuments:docs.underlying_map()]; FSTViewChange *viewChange = [view applyChangesToDocuments:viewDocChanges]; HARD_ASSERT(viewChange.limboChanges.count == 0, "View returned limbo documents during local-only query execution."); diff --git a/Firestore/Source/Core/FSTSyncEngine.mm b/Firestore/Source/Core/FSTSyncEngine.mm index 0ee2d3738fe..69e2e6b2e87 100644 --- a/Firestore/Source/Core/FSTSyncEngine.mm +++ b/Firestore/Source/Core/FSTSyncEngine.mm @@ -495,7 +495,8 @@ - (void)emitNewSnapshotsAndNotifyLocalStoreWithChanges:(const MaybeDocumentMap & // query against the local store to make sure we didn't lose any good docs that had been // past the limit. DocumentMap docs = [self.localStore executeQuery:queryView.query]; - viewDocChanges = [view computeChangesWithDocuments:docs.underlying_map() previousChanges:viewDocChanges]; + viewDocChanges = [view computeChangesWithDocuments:docs.underlying_map() + previousChanges:viewDocChanges]; } FSTTargetChange *_Nullable targetChange = nil; diff --git a/Firestore/core/src/firebase/firestore/model/document_map.h b/Firestore/core/src/firebase/firestore/model/document_map.h index 6412f920034..4b8e0ee26af 100644 --- a/Firestore/core/src/firebase/firestore/model/document_map.h +++ b/Firestore/core/src/firebase/firestore/model/document_map.h @@ -17,10 +17,10 @@ #ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_DOCUMENT_MAP_H_ #define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_DOCUMENT_MAP_H_ -#import "Firestore/Source/Model/FSTDocument.h" - #include +#import "Firestore/Source/Model/FSTDocument.h" + #include "Firestore/core/src/firebase/firestore/immutable/sorted_map.h" #include "Firestore/core/src/firebase/firestore/immutable/sorted_map_iterator.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" @@ -92,7 +92,8 @@ class DocumentMap { } void UpdateCurrentValue() const { - const std::pair& underlying_value = *iter_; + const std::pair& underlying_value = + *iter_; current_value_ = value_type{underlying_value.first, static_cast(underlying_value.second)}; @@ -124,8 +125,12 @@ class DocumentMap { return DocumentMap{map_.erase(key)}; } - bool empty() const { return map_.empty(); } - MaybeDocumentMap::size_type size() const { return map_.size(); } + bool empty() const { + return map_.empty(); + } + MaybeDocumentMap::size_type size() const { + return map_.size(); + } const MaybeDocumentMap& underlying_map() const { return map_; From 5a2cd655e6d071c8d86e6e53a2242652ab46bb95 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 9 Dec 2018 20:39:33 -0800 Subject: [PATCH 12/16] Fix unit tests --- Firestore/Example/Tests/Core/FSTViewTests.mm | 104 +++++++++---------- 1 file changed, 48 insertions(+), 56 deletions(-) diff --git a/Firestore/Example/Tests/Core/FSTViewTests.mm b/Firestore/Example/Tests/Core/FSTViewTests.mm index a0919aac157..770028ffaeb 100644 --- a/Firestore/Example/Tests/Core/FSTViewTests.mm +++ b/Firestore/Example/Tests/Core/FSTViewTests.mm @@ -276,12 +276,11 @@ - (void)testDoesntReportChangesForDocumentBeyondLimitOfQuery { // doc4 will be added + removed = nothing doc2 = FSTTestDoc("rooms/eros/messages/2", 1, @{@"num" : @5}, FSTDocumentStateSynced); FSTViewDocumentChanges *viewDocChanges = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc2, doc3, doc4 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc2, doc3, doc4 ])]; XCTAssertTrue(viewDocChanges.needsRefill); // Verify that all the docs still match. - viewDocChanges = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4 ]) - previousChanges:viewDocChanges]; + viewDocChanges = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4 ]) + previousChanges:viewDocChanges]; FSTViewSnapshot *snapshot = [view applyChangesToDocuments:viewDocChanges targetChange:FSTTestTargetChangeAckDocuments( @@ -311,35 +310,33 @@ - (void)testKeepsTrackOfLimboDocuments { FSTDocument *doc3 = FSTTestDoc("rooms/eros/messages/2", 0, @{}, FSTDocumentStateSynced); FSTViewChange *change = [view - applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1 ])]]; + applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1 ])]]; XCTAssertEqualObjects(change.limboChanges, @[]); - change = - [view applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[])] - targetChange:FSTTestTargetChangeMarkCurrent()]; + change = [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[])] + targetChange:FSTTestTargetChangeMarkCurrent()]; XCTAssertEqualObjects( change.limboChanges, @[ [FSTLimboDocumentChange changeWithType:FSTLimboDocumentChangeTypeAdded key:doc1.key] ]); - change = - [view applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[])] - targetChange:FSTTestTargetChangeAckDocuments({doc1.key})]; + change = [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[])] + targetChange:FSTTestTargetChangeAckDocuments({doc1.key})]; XCTAssertEqualObjects( change.limboChanges, @[ [FSTLimboDocumentChange changeWithType:FSTLimboDocumentChangeTypeRemoved key:doc1.key] ]); - change = [view - applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc2 ])] - targetChange:FSTTestTargetChangeAckDocuments({doc2.key})]; + change = + [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc2 ])] + targetChange:FSTTestTargetChangeAckDocuments({doc2.key})]; XCTAssertEqualObjects(change.limboChanges, @[]); change = [view - applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc3 ])]]; + applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc3 ])]]; XCTAssertEqualObjects( change.limboChanges, @[ [FSTLimboDocumentChange changeWithType:FSTLimboDocumentChangeTypeAdded key:doc3.key] ]); - change = [view applyChangesToDocuments:[view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ + change = [view applyChangesToDocuments:[view computeChangesWithDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc("rooms/eros/messages/2", 1, NO) ])]]; // remove XCTAssertEqualObjects( @@ -358,7 +355,7 @@ - (void)testResumingQueryCreatesNoLimbos { FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{doc1.key, doc2.key}]; - FSTViewDocumentChanges *changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[])]; + FSTViewDocumentChanges *changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[])]; FSTViewChange *change = [view applyChangesToDocuments:changes targetChange:FSTTestTargetChangeMarkCurrent()]; XCTAssertEqualObjects(change.limboChanges, @[]); @@ -379,21 +376,20 @@ - (void)testReturnsNeedsRefillOnDeleteInLimitQuery { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); [view applyChangesToDocuments:changes]; // Remove one of the docs. - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( - "rooms/eros/messages/0", 0, NO) ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( + "rooms/eros/messages/0", 0, NO) ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc2 ]]; XCTAssertTrue(changes.needsRefill); XCTAssertEqual(1, [changes.changeSet changes].count); // Refill it with just the one doc remaining. - changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc2 ]) previousChanges:changes]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc2 ]) previousChanges:changes]; [self assertDocSet:changes.documentSet containsDocs:@[ doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(1, [changes.changeSet changes].count); @@ -416,7 +412,7 @@ - (void)testReturnsNeedsRefillOnReorderInLimitQuery { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); @@ -424,13 +420,13 @@ - (void)testReturnsNeedsRefillOnReorderInLimitQuery { // Move one of the docs. doc2 = FSTTestDoc("rooms/eros/messages/1", 1, @{@"order" : @2000}, FSTDocumentStateSynced); - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc2 ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc2 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertTrue(changes.needsRefill); XCTAssertEqual(1, [changes.changeSet changes].count); // Refill it with all three current docs. - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3 ]) - previousChanges:changes]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3 ]) + previousChanges:changes]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc3 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); @@ -457,7 +453,7 @@ - (void)testDoesntNeedRefillOnReorderWithinLimit { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4, doc5 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4, doc5 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2, doc3 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(3, [changes.changeSet changes].count); @@ -465,7 +461,7 @@ - (void)testDoesntNeedRefillOnReorderWithinLimit { // Move one of the docs. doc1 = FSTTestDoc("rooms/eros/messages/0", 1, @{@"order" : @3}, FSTDocumentStateSynced); - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1 ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc2, doc3, doc1 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(1, [changes.changeSet changes].count); @@ -492,7 +488,7 @@ - (void)testDoesntNeedRefillOnReorderAfterLimitQuery { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4, doc5 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2, doc3, doc4, doc5 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2, doc3 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(3, [changes.changeSet changes].count); @@ -500,7 +496,7 @@ - (void)testDoesntNeedRefillOnReorderAfterLimitQuery { // Move one of the docs. doc4 = FSTTestDoc("rooms/eros/messages/3", 1, @{@"order" : @6}, FSTDocumentStateSynced); - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc4 ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc4 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2, doc3 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(0, [changes.changeSet changes].count); @@ -515,7 +511,7 @@ - (void)testDoesntNeedRefillForAdditionAfterTheLimit { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); @@ -523,7 +519,7 @@ - (void)testDoesntNeedRefillForAdditionAfterTheLimit { // Add a doc that is past the limit. FSTDocument *doc3 = FSTTestDoc("rooms/eros/messages/2", 1, @{}, FSTDocumentStateSynced); - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc3 ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc3 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(0, [changes.changeSet changes].count); @@ -537,15 +533,15 @@ - (void)testDoesntNeedRefillForDeletionsWhenNotNearTheLimit { FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}]; FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); [view applyChangesToDocuments:changes]; // Remove one of the docs. - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( - "rooms/eros/messages/1", 0, NO) ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( + "rooms/eros/messages/1", 0, NO) ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(1, [changes.changeSet changes].count); @@ -560,15 +556,15 @@ - (void)testHandlesApplyingIrrelevantDocs { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(2, [changes.changeSet changes].count); [view applyChangesToDocuments:changes]; // Remove a doc that isn't even in the results. - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( - "rooms/eros/messages/2", 0, NO) ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ FSTTestDeletedDoc( + "rooms/eros/messages/2", 0, NO) ])]; [self assertDocSet:changes.documentSet containsDocs:@[ doc1, doc2 ]]; XCTAssertFalse(changes.needsRefill); XCTAssertEqual(0, [changes.changeSet changes].count); @@ -583,12 +579,12 @@ - (void)testComputesMutatedKeys { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [view applyChangesToDocuments:changes]; XCTAssertEqual(changes.mutatedKeys, DocumentKeySet{}); FSTDocument *doc3 = FSTTestDoc("rooms/eros/messages/2", 0, @{}, FSTDocumentStateLocalMutations); - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc3 ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc3 ])]; XCTAssertEqual(changes.mutatedKeys, DocumentKeySet{doc3.key}); } @@ -600,12 +596,12 @@ - (void)testRemovesKeysFromMutatedKeysWhenNewDocHasNoLocalChanges { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [view applyChangesToDocuments:changes]; XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key})); FSTDocument *doc2Prime = FSTTestDoc("rooms/eros/messages/1", 0, @{}, FSTDocumentStateSynced); - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc2Prime ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc2Prime ])]; [view applyChangesToDocuments:changes]; XCTAssertEqual(changes.mutatedKeys, DocumentKeySet{}); } @@ -618,12 +614,12 @@ - (void)testRemembersLocalMutationsFromPreviousSnapshot { // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; [view applyChangesToDocuments:changes]; XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key})); FSTDocument *doc3 = FSTTestDoc("rooms/eros/messages/2", 0, @{}, FSTDocumentStateSynced); - changes = [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc3 ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc3 ])]; [view applyChangesToDocuments:changes]; XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key})); } @@ -636,12 +632,11 @@ - (void)testRemembersLocalMutationsFromPreviousCallToComputeChangesWithDocuments // Start with a full view. FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key})); FSTDocument *doc3 = FSTTestDoc("rooms/eros/messages/2", 0, @{}, FSTDocumentStateSynced); - changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc3 ]) previousChanges:changes]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc3 ]) previousChanges:changes]; XCTAssertEqual(changes.mutatedKeys, (DocumentKeySet{doc2.key})); } @@ -649,8 +644,7 @@ - (void)testRaisesHasPendingWritesForPendingMutationsInInitialSnapshot { FSTQuery *query = [self queryForMessages]; FSTDocument *doc1 = FSTTestDoc("rooms/eros/messages/1", 0, @{}, FSTDocumentStateLocalMutations); FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}]; - FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1 ])]; + FSTViewDocumentChanges *changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1 ])]; FSTViewChange *viewChange = [view applyChangesToDocuments:changes]; XCTAssertTrue(viewChange.snapshot.hasPendingWrites); } @@ -660,8 +654,7 @@ - (void)testDoesntRaiseHasPendingWritesForCommittedMutationsInInitialSnapshot { FSTDocument *doc1 = FSTTestDoc("rooms/eros/messages/1", 0, @{}, FSTDocumentStateCommittedMutations); FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}]; - FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1 ])]; + FSTViewDocumentChanges *changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1 ])]; FSTViewChange *viewChange = [view applyChangesToDocuments:changes]; XCTAssertFalse(viewChange.snapshot.hasPendingWrites); } @@ -686,7 +679,7 @@ - (void)testSuppressesWriteAcknowledgementIfWatchHasNotCaughtUp { @{@"time" : @3}, FSTDocumentStateSynced); FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}]; FSTViewDocumentChanges *changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1, doc2 ])]; FSTViewChange *viewChange = [view applyChangesToDocuments:changes]; XCTAssertEqualObjects( @@ -696,8 +689,7 @@ - (void)testSuppressesWriteAcknowledgementIfWatchHasNotCaughtUp { ]), viewChange.snapshot.documentChanges); - changes = - [view computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1Committed, doc2Modified ])]; + changes = [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1Committed, doc2Modified ])]; viewChange = [view applyChangesToDocuments:changes]; // The 'doc1Committed' update is suppressed XCTAssertEqualObjects( @@ -705,8 +697,8 @@ - (void)testSuppressesWriteAcknowledgementIfWatchHasNotCaughtUp { type:FSTDocumentViewChangeTypeModified] ]), viewChange.snapshot.documentChanges); - changes = [view - computeChangesWithMaybeDocuments:FSTTestDocUpdates(@[ doc1Acknowledged, doc2Acknowledged ])]; + changes = + [view computeChangesWithDocuments:FSTTestDocUpdates(@[ doc1Acknowledged, doc2Acknowledged ])]; viewChange = [view applyChangesToDocuments:changes]; XCTAssertEqualObjects( (@[ From 1c1a1023747bbc07b175ec1821ee0b23d293623c Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 9 Dec 2018 20:56:32 -0800 Subject: [PATCH 13/16] Comments --- .../firebase/firestore/model/document_map.h | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/Firestore/core/src/firebase/firestore/model/document_map.h b/Firestore/core/src/firebase/firestore/model/document_map.h index 4b8e0ee26af..9fff6caa85a 100644 --- a/Firestore/core/src/firebase/firestore/model/document_map.h +++ b/Firestore/core/src/firebase/firestore/model/document_map.h @@ -40,10 +40,23 @@ using MaybeDocumentMap = immutable::SortedMap; /** * Convenience type for a map of keys to Documents, since they are so common. * - * PORTING NOTE: OBC + * PORTING NOTE: unlike other platforms, in C++ `Foo` cannot be + * converted to `Foo`; consequently, if `DocumentMap` were simply an + * alias similar to `MaybeDocumentMap`, it couldn't be passed to functions + * expecting `MaybeDocumentMap`. + * + * To work around this, in C++ `DocumentMap` is a simple wrapper over + * a `MaybeDocumentMap` that forwards all functions to the underlying map but + * with added type safety (it only accepts `FSTDocument`s, not + * `FSTMaybeDocument`s). Use `DocumentMap` in functions creating and/or + * returning maps that only contain `FSTDocument`s; when the `DocumentMap` needs + * to be passed to a function accepting a `MaybeDocumentMap`, use + * `underlying_map` function to get (read-only) access to the representation. */ class DocumentMap { public: + // Wraps `MaybeDocumentMap::const_iterator`, providing necessary conversions + // from `FSTMaybeDocument*` to `FSTDocument*`. class const_iterator { public: using iterator_category = @@ -91,6 +104,19 @@ class DocumentMap { : iter_{iter} { } + // Iterator cannot use the value of the underlying iterator because the + // types are unrelated and one cannot be cast to the other. Also, the value + // cannot be created on the fly, otherwise `get` and `operator->` would + // return a pointer to a stale temporary. As a result, the value has be + // stored as a data member. + // + // The problem is when to update the value. Value cannot be updated in + // constructor or during iteration, because the underlying iterator might be + // one-past-end at that point. Consequently, value has to be updated upon + // access. It's a tradeoff whether to only update the value if it changed + // or unconditionally. Here it is done unconditionally, on the assumption + // that each particular value usually doesn't get more than one or two + // accesses. void UpdateCurrentValue() const { const std::pair& underlying_value = *iter_; @@ -100,6 +126,9 @@ class DocumentMap { } MaybeDocumentMap::const_iterator iter_; + // To mimic the underlying iterator, functions returning the value or + // a pointer to value are const, but they still have to update the + // `current_value_`. mutable value_type current_value_; }; @@ -132,6 +161,7 @@ class DocumentMap { return map_.size(); } + /** Use this function to "convert" `DocumentMap` to a `MaybeDocumentMap`. */ const MaybeDocumentMap& underlying_map() const { return map_; } From 5cdd10ef1928f4559d1f40332a6be9efd865b315 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 9 Dec 2018 21:14:47 -0800 Subject: [PATCH 14/16] No iterator? --- .../Example/Tests/Local/FSTLocalStoreTests.mm | 4 +- .../Local/FSTRemoteDocumentCacheTests.mm | 6 +- .../Source/Local/FSTLocalDocumentsView.mm | 8 +- Firestore/Source/Model/FSTDocumentSet.mm | 6 +- .../firebase/firestore/model/document_map.h | 101 +++--------------- 5 files changed, 24 insertions(+), 101 deletions(-) diff --git a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm index 40f25bdaa35..5cff444f6bf 100644 --- a/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm +++ b/Firestore/Example/Tests/Local/FSTLocalStoreTests.mm @@ -54,8 +54,8 @@ static NSArray *docMapToArray(const DocumentMap &docs) { NSMutableArray *result = [NSMutableArray array]; - for (const auto &kv : docs) { - [result addObject:kv.second]; + for (const auto &kv : docs.underlying_map()) { + [result addObject:static_cast(kv.second)]; } return result; } diff --git a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm index c42a713e358..f7cf4bdb8ea 100644 --- a/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm +++ b/Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.mm @@ -170,9 +170,9 @@ - (void)expectMap:(const DocumentMap &)map } for (FSTDocument *doc in expected) { FSTDocument *actual = nil; - auto found = map.find(doc.key); - if (found != map.end()) { - actual = found->second; + auto found = map.underlying_map().find(doc.key); + if (found != map.underlying_map().end()) { + actual = static_cast(found->second); } XCTAssertEqualObjects(actual, doc); } diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index 65798bf687e..3a067225bf2 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -133,8 +133,8 @@ - (DocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { const DocumentKey &key = mutation.key; // baseDoc may be nil for the documents that weren't yet written to the backend. FSTMaybeDocument *baseDoc = nil; - auto found = results.find(key); - if (found != results.end()) { + auto found = results.underlying_map().find(key); + if (found != results.underlying_map().end()) { baseDoc = found->second; } FSTMaybeDocument *mutatedDoc = [mutation applyToLocalDocument:baseDoc @@ -153,9 +153,9 @@ - (DocumentMap)documentsMatchingCollectionQuery:(FSTQuery *)query { // reference here prevents ARC from deallocating the initial unfiltered results while we're // enumerating them. DocumentMap unfiltered = results; - for (const auto &kv : unfiltered) { + for (const auto &kv : unfiltered.underlying_map()) { const DocumentKey &key = kv.first; - FSTDocument *doc = kv.second; + FSTDocument *doc = static_cast(kv.second); if (![query matchesDocument:doc]) { results = results.erase(key); } diff --git a/Firestore/Source/Model/FSTDocumentSet.mm b/Firestore/Source/Model/FSTDocumentSet.mm index 65625ae8004..c87e2039e54 100644 --- a/Firestore/Source/Model/FSTDocumentSet.mm +++ b/Firestore/Source/Model/FSTDocumentSet.mm @@ -119,12 +119,12 @@ - (BOOL)isEmpty { } - (BOOL)containsKey:(const DocumentKey &)key { - return _index.find(key) != _index.end(); + return _index.underlying_map().find(key) != _index.underlying_map().end(); } - (FSTDocument *_Nullable)documentForKey:(const DocumentKey &)key { - auto found = _index.find(key); - return found != _index.end() ? found->second : nil; + auto found = _index.underlying_map().find(key); + return found != _index.underlying_map().end() ? static_cast(found->second) : nil; } - (FSTDocument *_Nullable)firstDocument { diff --git a/Firestore/core/src/firebase/firestore/model/document_map.h b/Firestore/core/src/firebase/firestore/model/document_map.h index 9fff6caa85a..59ae69f41b5 100644 --- a/Firestore/core/src/firebase/firestore/model/document_map.h +++ b/Firestore/core/src/firebase/firestore/model/document_map.h @@ -22,7 +22,6 @@ #import "Firestore/Source/Model/FSTDocument.h" #include "Firestore/core/src/firebase/firestore/immutable/sorted_map.h" -#include "Firestore/core/src/firebase/firestore/immutable/sorted_map_iterator.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "absl/base/attributes.h" @@ -52,99 +51,12 @@ using MaybeDocumentMap = immutable::SortedMap; * returning maps that only contain `FSTDocument`s; when the `DocumentMap` needs * to be passed to a function accepting a `MaybeDocumentMap`, use * `underlying_map` function to get (read-only) access to the representation. + * Also use `underlying_map` for iterating and searching. */ class DocumentMap { public: - // Wraps `MaybeDocumentMap::const_iterator`, providing necessary conversions - // from `FSTMaybeDocument*` to `FSTDocument*`. - class const_iterator { - public: - using iterator_category = - MaybeDocumentMap::const_iterator::iterator_category; - using value_type = std::pair; - using pointer = const value_type*; - using reference = const value_type&; - using difference_type = MaybeDocumentMap::const_iterator::difference_type; - - const_iterator() = default; - - pointer get() const { - UpdateCurrentValue(); - return ¤t_value_; - } - reference operator*() const { - UpdateCurrentValue(); - return current_value_; - } - pointer operator->() const { - return get(); - } - - const_iterator& operator++() { - ++iter_; - return *this; - } - const_iterator operator++(int /*unused*/) { - const_iterator old_value = *this; - ++iter_; - return old_value; - } - - friend bool operator==(const const_iterator& a, const const_iterator& b) { - return a.iter_ == b.iter_; - } - friend bool operator!=(const const_iterator& a, const const_iterator& b) { - return a.iter_ != b.iter_; - } - - private: - friend class DocumentMap; - - explicit const_iterator(MaybeDocumentMap::const_iterator&& iter) - : iter_{iter} { - } - - // Iterator cannot use the value of the underlying iterator because the - // types are unrelated and one cannot be cast to the other. Also, the value - // cannot be created on the fly, otherwise `get` and `operator->` would - // return a pointer to a stale temporary. As a result, the value has be - // stored as a data member. - // - // The problem is when to update the value. Value cannot be updated in - // constructor or during iteration, because the underlying iterator might be - // one-past-end at that point. Consequently, value has to be updated upon - // access. It's a tradeoff whether to only update the value if it changed - // or unconditionally. Here it is done unconditionally, on the assumption - // that each particular value usually doesn't get more than one or two - // accesses. - void UpdateCurrentValue() const { - const std::pair& underlying_value = - *iter_; - current_value_ = - value_type{underlying_value.first, - static_cast(underlying_value.second)}; - } - - MaybeDocumentMap::const_iterator iter_; - // To mimic the underlying iterator, functions returning the value or - // a pointer to value are const, but they still have to update the - // `current_value_`. - mutable value_type current_value_; - }; - DocumentMap() = default; - const_iterator begin() const { - return const_iterator{map_.begin()}; - } - const_iterator end() const { - return const_iterator{map_.end()}; - } - - const_iterator find(const DocumentKey& key) const { - return const_iterator{map_.find(key)}; - } - ABSL_MUST_USE_RESULT DocumentMap insert(const DocumentKey& key, FSTDocument* value) const { return DocumentMap{map_.insert(key, value)}; @@ -173,6 +85,17 @@ class DocumentMap { MaybeDocumentMap map_; }; +inline FSTDocument* GetFSTDocumentOrNil(FSTMaybeDocument* maybeDoc) { + if ([maybeDoc isKindOfClass:[FSTDocument class]]) { + return static_cast(maybeDoc); + } + return nil; +} + +inline FSTDocument* GetFSTDocumentOrNil(FSTDocument* doc) { + return doc; +} + } // namespace model } // namespace firestore } // namespace firebase From ab023b047d5dcd1eb47c48ba9ebaed41cd3b8a95 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 9 Dec 2018 21:17:16 -0800 Subject: [PATCH 15/16] Add to CMake --- Firestore/core/src/firebase/firestore/model/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt index 4a230eefe52..cd8d7876f60 100644 --- a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt @@ -22,6 +22,7 @@ cc_library( document.h document_key.cc document_key.h + document_map.h field_mask.h field_path.cc field_path.h @@ -45,6 +46,7 @@ cc_library( DEPENDS absl_optional absl_strings + firebase_firestore_immutable firebase_firestore_util firebase_firestore_types ) From 9f069f28bdd01d3f79678652f03afef0f69cf55e Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 10 Dec 2018 09:51:18 -0800 Subject: [PATCH 16/16] Revert accidentally formatted files --- Example/tvOSSample/tvOSSample/AuthViewController.swift | 1 - Example/tvOSSample/tvOSSample/EmailLoginViewController.swift | 1 - 2 files changed, 2 deletions(-) diff --git a/Example/tvOSSample/tvOSSample/AuthViewController.swift b/Example/tvOSSample/tvOSSample/AuthViewController.swift index 56276ed353b..2492618f928 100644 --- a/Example/tvOSSample/tvOSSample/AuthViewController.swift +++ b/Example/tvOSSample/tvOSSample/AuthViewController.swift @@ -16,7 +16,6 @@ import UIKit import FirebaseAuth class AuthViewController: UIViewController { - // MARK: - User Interface /// A stackview containing all of the buttons to providers (Email, OAuth, etc). diff --git a/Example/tvOSSample/tvOSSample/EmailLoginViewController.swift b/Example/tvOSSample/tvOSSample/EmailLoginViewController.swift index cabb8c97e63..38d5425a49e 100644 --- a/Example/tvOSSample/tvOSSample/EmailLoginViewController.swift +++ b/Example/tvOSSample/tvOSSample/EmailLoginViewController.swift @@ -21,7 +21,6 @@ protocol EmailLoginDelegate { } class EmailLoginViewController: UIViewController { - // MARK: - Public Properties var delegate: EmailLoginDelegate?