From 8311c6432ecff78bedd13e27f64d241659324786 Mon Sep 17 00:00:00 2001 From: zxu Date: Tue, 6 Mar 2018 14:28:04 -0500 Subject: port paths to FSTDocumentKey (#877) * replace path with C++ implementation in FSTDocumentKey.{h,mm} only * address changes * address changes --- .../Example/Tests/Model/FSTDocumentKeyTests.mm | 21 +++++---- Firestore/Example/Tests/Util/FSTHelpers.mm | 6 ++- Firestore/Source/API/FIRCollectionReference.mm | 7 +-- Firestore/Source/API/FIRDocumentReference.mm | 23 +++++++--- Firestore/Source/API/FIRDocumentSnapshot.mm | 2 +- Firestore/Source/API/FIRQuery.mm | 9 ++-- Firestore/Source/Core/FSTQuery.mm | 18 +++----- Firestore/Source/Core/FSTSyncEngine.mm | 2 +- Firestore/Source/Local/FSTLevelDBKey.mm | 25 ++++++---- Firestore/Source/Local/FSTLevelDBMutationQueue.mm | 12 +++-- Firestore/Source/Local/FSTLevelDBQueryCache.mm | 4 +- .../Source/Local/FSTLevelDBRemoteDocumentCache.mm | 2 +- Firestore/Source/Local/FSTLocalDocumentsView.mm | 5 +- Firestore/Source/Local/FSTMemoryMutationQueue.mm | 18 +++++--- .../Source/Local/FSTMemoryRemoteDocumentCache.mm | 5 +- Firestore/Source/Local/FSTReferenceSet.mm | 4 +- Firestore/Source/Model/FSTDocument.mm | 4 +- Firestore/Source/Model/FSTDocumentKey.h | 15 +++--- Firestore/Source/Model/FSTDocumentKey.mm | 53 +++++++++++++++------- Firestore/Source/Remote/FSTRemoteStore.mm | 3 +- Firestore/Source/Remote/FSTSerializerBeta.mm | 15 ++++-- .../src/firebase/firestore/model/resource_path.h | 5 +- .../test/firebase/firestore/testutil/testutil.h | 5 ++ 23 files changed, 152 insertions(+), 111 deletions(-) (limited to 'Firestore') diff --git a/Firestore/Example/Tests/Model/FSTDocumentKeyTests.mm b/Firestore/Example/Tests/Model/FSTDocumentKeyTests.mm index d66ee73..5992b42 100644 --- a/Firestore/Example/Tests/Model/FSTDocumentKeyTests.mm +++ b/Firestore/Example/Tests/Model/FSTDocumentKeyTests.mm @@ -18,7 +18,9 @@ #import -#import "Firestore/Source/Model/FSTPath.h" +#include "Firestore/core/src/firebase/firestore/model/resource_path.h" + +using firebase::firestore::model::ResourcePath; NS_ASSUME_NONNULL_BEGIN @@ -28,23 +30,22 @@ NS_ASSUME_NONNULL_BEGIN @implementation FSTDocumentKeyTests - (void)testConstructor { - FSTResourcePath *path = - [FSTResourcePath pathWithSegments:@[ @"rooms", @"firestore", @"messages", @"1" ]]; + 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" ]]; + 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" ]]; + 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]); diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index 9b05604..00464bc 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -43,8 +43,10 @@ #include "Firestore/core/src/firebase/firestore/model/database_id.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" +#include "Firestore/core/test/firebase/firestore/testutil/testutil.h" namespace util = firebase::firestore::util; +namespace testutil = firebase::firestore::testutil; using firebase::firestore::model::DatabaseId; NS_ASSUME_NONNULL_BEGIN @@ -266,7 +268,7 @@ FSTPatchMutation *FSTTestPatchMutation(NSString *path, } }]; - FSTDocumentKey *key = [FSTDocumentKey keyWithPath:FSTTestPath(path)]; + FSTDocumentKey *key = [FSTDocumentKey keyWithPath:testutil::Resource([path UTF8String])]; FSTFieldMask *mask = [[FSTFieldMask alloc] initWithFields:merge ? updateMask : fieldMaskPaths]; return [[FSTPatchMutation alloc] initWithKey:key fieldMask:mask @@ -277,7 +279,7 @@ FSTPatchMutation *FSTTestPatchMutation(NSString *path, // For now this only creates TransformMutations with server timestamps. FSTTransformMutation *FSTTestTransformMutation(NSString *path, NSArray *serverTimestampFields) { - FSTDocumentKey *key = [FSTDocumentKey keyWithPath:FSTTestPath(path)]; + FSTDocumentKey *key = [FSTDocumentKey keyWithPath:testutil::Resource([path UTF8String])]; NSMutableArray *fieldTransforms = [NSMutableArray array]; for (NSString *field in serverTimestampFields) { FSTFieldPath *fieldPath = FSTTestFieldPath(field); diff --git a/Firestore/Source/API/FIRCollectionReference.mm b/Firestore/Source/API/FIRCollectionReference.mm index cb7b61a..9560d13 100644 --- a/Firestore/Source/API/FIRCollectionReference.mm +++ b/Firestore/Source/API/FIRCollectionReference.mm @@ -101,8 +101,7 @@ NS_ASSUME_NONNULL_BEGIN if (parentPath.empty()) { return nil; } else { - FSTDocumentKey *key = - [FSTDocumentKey keyWithPath:[FSTResourcePath resourcePathWithCPPResourcePath:parentPath]]; + FSTDocumentKey *key = [FSTDocumentKey keyWithPath:parentPath]; return [FIRDocumentReference referenceWithKey:key firestore:self.firestore]; } } @@ -135,9 +134,7 @@ NS_ASSUME_NONNULL_BEGIN } - (FIRDocumentReference *)documentWithAutoID { - const ResourcePath path = self.query.path.Append(CreateAutoId()); - FSTDocumentKey *key = - [FSTDocumentKey keyWithPath:[FSTResourcePath resourcePathWithCPPResourcePath:path]]; + FSTDocumentKey *key = [FSTDocumentKey keyWithPath:self.query.path.Append(CreateAutoId())]; return [FIRDocumentReference referenceWithKey:key firestore:self.firestore]; } diff --git a/Firestore/Source/API/FIRDocumentReference.mm b/Firestore/Source/API/FIRDocumentReference.mm index b1a5d49..cc14af5 100644 --- a/Firestore/Source/API/FIRDocumentReference.mm +++ b/Firestore/Source/API/FIRDocumentReference.mm @@ -39,6 +39,12 @@ #import "Firestore/Source/Util/FSTAsyncQueryListener.h" #import "Firestore/Source/Util/FSTUsageValidation.h" +#include "Firestore/core/src/firebase/firestore/model/resource_path.h" +#include "Firestore/core/src/firebase/firestore/util/string_apple.h" + +namespace util = firebase::firestore::util; +using firebase::firestore::model::ResourcePath; + NS_ASSUME_NONNULL_BEGIN #pragma mark - FIRDocumentListenOptions @@ -93,7 +99,8 @@ NS_ASSUME_NONNULL_BEGIN path.canonicalString, path.length); } return - [FIRDocumentReference referenceWithKey:[FSTDocumentKey keyWithPath:path] firestore:firestore]; + [FIRDocumentReference referenceWithKey:[FSTDocumentKey keyWithPath:[path toCPPResourcePath]] + firestore:firestore]; } + (instancetype)referenceWithKey:(FSTDocumentKey *)key firestore:(FIRFirestore *)firestore { @@ -136,16 +143,17 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - Public Methods - (NSString *)documentID { - return [self.key.path lastSegment]; + return util::WrapNSString(self.key.path.last_segment()); } - (FIRCollectionReference *)parent { - FSTResourcePath *parentPath = [self.key.path pathByRemovingLastSegment]; - return [FIRCollectionReference referenceWithPath:parentPath firestore:self.firestore]; + return [FIRCollectionReference + referenceWithPath:[FSTResourcePath resourcePathWithCPPResourcePath:self.key.path.PopLast()] + firestore:self.firestore]; } - (NSString *)path { - return [self.key.path canonicalString]; + return util::WrapNSString(self.key.path.CanonicalString()); } - (FIRCollectionReference *)collectionWithPath:(NSString *)collectionPath { @@ -153,7 +161,8 @@ NS_ASSUME_NONNULL_BEGIN FSTThrowInvalidArgument(@"Collection path cannot be nil."); } FSTResourcePath *subPath = [FSTResourcePath pathWithString:collectionPath]; - FSTResourcePath *path = [self.key.path pathByAppendingPath:subPath]; + FSTResourcePath *path = [FSTResourcePath + resourcePathWithCPPResourcePath:self.key.path.Append([subPath toCPPResourcePath])]; return [FIRCollectionReference referenceWithPath:path firestore:self.firestore]; } @@ -266,7 +275,7 @@ NS_ASSUME_NONNULL_BEGIN addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions listener:(FIRDocumentSnapshotBlock)listener { FIRFirestore *firestore = self.firestore; - FSTQuery *query = [FSTQuery queryWithPath:[self.key.path toCPPResourcePath]]; + FSTQuery *query = [FSTQuery queryWithPath:self.key.path]; FSTDocumentKey *key = self.key; FSTViewSnapshotHandler snapshotHandler = ^(FSTViewSnapshot *snapshot, NSError *error) { diff --git a/Firestore/Source/API/FIRDocumentSnapshot.mm b/Firestore/Source/API/FIRDocumentSnapshot.mm index ddf4cca..8e731da 100644 --- a/Firestore/Source/API/FIRDocumentSnapshot.mm +++ b/Firestore/Source/API/FIRDocumentSnapshot.mm @@ -122,7 +122,7 @@ NS_ASSUME_NONNULL_BEGIN } - (NSString *)documentID { - return [self.internalKey.path lastSegment]; + return util::WrapNSString(self.internalKey.path.last_segment()); } - (FIRSnapshotMetadata *)metadata { diff --git a/Firestore/Source/API/FIRQuery.mm b/Firestore/Source/API/FIRQuery.mm index 45ee482..c277561 100644 --- a/Firestore/Source/API/FIRQuery.mm +++ b/Firestore/Source/API/FIRQuery.mm @@ -469,8 +469,7 @@ addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions @"Invalid query. When querying by document ID you must provide " "a valid document ID, but it was an empty string."); } - FSTResourcePath *path = [[FSTResourcePath resourcePathWithCPPResourcePath:self.query.path] - pathByAppendingSegment:documentKey]; + ResourcePath path = self.query.path.Append([documentKey UTF8String]); fieldValue = [FSTReferenceValue referenceValue:[FSTDocumentKey keyWithPath:path] databaseID:self.firestore.databaseID]; } else if ([value isKindOfClass:[FIRDocumentReference class]]) { @@ -621,10 +620,8 @@ addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions FSTThrowInvalidUsage(@"InvalidQueryException", @"Invalid query. Document ID '%@' contains a slash.", documentID); } - FSTDocumentKey *key = [FSTDocumentKey - keyWithPath:[FSTResourcePath - resourcePathWithCPPResourcePath:self.query.path.Append( - [documentID UTF8String])]]; + FSTDocumentKey *key = + [FSTDocumentKey keyWithPath:self.query.path.Append([documentID UTF8String])]; [components addObject:[FSTReferenceValue referenceValue:key databaseID:self.firestore.databaseID]]; } else { diff --git a/Firestore/Source/Core/FSTQuery.mm b/Firestore/Source/Core/FSTQuery.mm index 7314ce9..c10b94d 100644 --- a/Firestore/Source/Core/FSTQuery.mm +++ b/Firestore/Source/Core/FSTQuery.mm @@ -620,8 +620,7 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe } - (instancetype)queryByAddingFilter:(id)filter { - FSTAssert(![FSTDocumentKey isDocumentKey:[FSTResourcePath resourcePathWithCPPResourcePath:_path]], - @"No filtering allowed for document query"); + FSTAssert(![FSTDocumentKey isDocumentKey:_path], @"No filtering allowed for document query"); const FieldPath *newInequalityField = nullptr; if ([filter isKindOfClass:[FSTRelationFilter class]] && @@ -642,8 +641,7 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe } - (instancetype)queryByAddingSortOrder:(FSTSortOrder *)sortOrder { - FSTAssert(![FSTDocumentKey isDocumentKey:[FSTResourcePath resourcePathWithCPPResourcePath:_path]], - @"No ordering is allowed for a document query."); + FSTAssert(![FSTDocumentKey isDocumentKey:_path], @"No ordering is allowed for a document query."); // TODO(klimt): Validate that the same key isn't added twice. return [[FSTQuery alloc] initWithPath:self.path @@ -682,8 +680,7 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe } - (BOOL)isDocumentQuery { - return [FSTDocumentKey isDocumentKey:[FSTResourcePath resourcePathWithCPPResourcePath:_path]] && - self.filters.count == 0; + return [FSTDocumentKey isDocumentKey:_path] && self.filters.count == 0; } - (BOOL)matchesDocument:(FSTDocument *)document { @@ -777,14 +774,13 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe /* Returns YES if the document matches the path for the receiver. */ - (BOOL)pathMatchesDocument:(FSTDocument *)document { - FSTResourcePath *documentPath = document.key.path; - if ([FSTDocumentKey isDocumentKey:[FSTResourcePath resourcePathWithCPPResourcePath:_path]]) { + const ResourcePath &documentPath = document.key.path; + if ([FSTDocumentKey isDocumentKey:_path]) { // Exact match for document queries. - return self.path == [documentPath toCPPResourcePath]; + return self.path == documentPath; } else { // Shallow ancestor queries by default. - return self.path.IsPrefixOf([documentPath toCPPResourcePath]) && - _path.size() == documentPath.length - 1; + return self.path.IsPrefixOf(documentPath) && _path.size() == documentPath.size() - 1; } } diff --git a/Firestore/Source/Core/FSTSyncEngine.mm b/Firestore/Source/Core/FSTSyncEngine.mm index 88be23c..a857f5a 100644 --- a/Firestore/Source/Core/FSTSyncEngine.mm +++ b/Firestore/Source/Core/FSTSyncEngine.mm @@ -497,7 +497,7 @@ static const FSTListenSequenceNumber kIrrelevantSequenceNumber = -1; if (!self.limboTargetsByKey[key]) { FSTLog(@"New document in limbo: %@", key); FSTTargetID limboTargetID = _targetIdGenerator.NextId(); - FSTQuery *query = [FSTQuery queryWithPath:[key.path toCPPResourcePath]]; + FSTQuery *query = [FSTQuery queryWithPath:key.path]; FSTQueryData *queryData = [[FSTQueryData alloc] initWithQuery:query targetID:limboTargetID listenSequenceNumber:kIrrelevantSequenceNumber diff --git a/Firestore/Source/Local/FSTLevelDBKey.mm b/Firestore/Source/Local/FSTLevelDBKey.mm index 41aea39..bd7b44a 100644 --- a/Firestore/Source/Local/FSTLevelDBKey.mm +++ b/Firestore/Source/Local/FSTLevelDBKey.mm @@ -17,12 +17,18 @@ #import "Firestore/Source/Local/FSTLevelDBKey.h" #include +#include +#include #import "Firestore/Source/Model/FSTDocumentKey.h" #import "Firestore/Source/Model/FSTPath.h" +#include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/util/ordered_code.h" +namespace util = firebase::firestore::util; +using firebase::firestore::model::ResourcePath; + NS_ASSUME_NONNULL_BEGIN using firebase::firestore::util::OrderedCode; @@ -271,7 +277,7 @@ BOOL ReadDocumentKey(Slice *contents, FSTDocumentKey *__strong *result) { Slice completeSegments = *contents; std::string segment; - NSMutableArray *pathSegments = [NSMutableArray array]; + std::vector path_segments{}; for (;;) { // Advance a temporary slice to avoid advancing contents into the next key component which may // not be a path segment. @@ -283,15 +289,14 @@ BOOL ReadDocumentKey(Slice *contents, FSTDocumentKey *__strong *result) { return NO; } - NSString *pathSegment = [[NSString alloc] initWithUTF8String:segment.c_str()]; - [pathSegments addObject:pathSegment]; + path_segments.push_back(std::move(segment)); segment.clear(); completeSegments = leveldb::Slice(readPosition.data(), readPosition.size()); } - FSTResourcePath *path = [FSTResourcePath pathWithSegments:pathSegments]; - if (path.length > 0 && [FSTDocumentKey isDocumentKey:path]) { + ResourcePath path{std::move(path_segments)}; + if (path.size() > 0 && [FSTDocumentKey isDocumentKey:path]) { *contents = completeSegments; *result = [FSTDocumentKey keyWithPath:path]; return YES; @@ -391,7 +396,7 @@ NSString *InvalidKey(const Slice &key) { if (!ReadDocumentKey(&tmp, &documentKey)) { break; } - [description appendFormat:@" key=%@", [documentKey.path description]]; + [description appendFormat:@" key=%s", documentKey.path.CanonicalString().c_str()]; } else if (label == FSTComponentLabelTableName) { std::string table; @@ -531,7 +536,7 @@ NSString *InvalidKey(const Slice &key) { std::string result; WriteTableName(&result, kDocumentMutationsTable); WriteUserID(&result, userID); - WriteResourcePath(&result, documentKey.path); + WriteResourcePath(&result, [FSTResourcePath resourcePathWithCPPResourcePath:documentKey.path]); WriteBatchID(&result, batchID); WriteTerminator(&result); return result; @@ -685,7 +690,7 @@ NSString *InvalidKey(const Slice &key) { std::string result; WriteTableName(&result, kTargetDocumentsTable); WriteTargetID(&result, targetID); - WriteResourcePath(&result, documentKey.path); + WriteResourcePath(&result, [FSTResourcePath resourcePathWithCPPResourcePath:documentKey.path]); WriteTerminator(&result); return result; } @@ -719,7 +724,7 @@ NSString *InvalidKey(const Slice &key) { + (std::string)keyWithDocumentKey:(FSTDocumentKey *)documentKey targetID:(FSTTargetID)targetID { std::string result; WriteTableName(&result, kDocumentTargetsTable); - WriteResourcePath(&result, documentKey.path); + WriteResourcePath(&result, [FSTResourcePath resourcePathWithCPPResourcePath:documentKey.path]); WriteTargetID(&result, targetID); WriteTerminator(&result); return result; @@ -754,7 +759,7 @@ NSString *InvalidKey(const Slice &key) { + (std::string)keyWithDocumentKey:(FSTDocumentKey *)key { std::string result; WriteTableName(&result, kRemoteDocumentsTable); - WriteResourcePath(&result, key.path); + WriteResourcePath(&result, [FSTResourcePath resourcePathWithCPPResourcePath:key.path]); WriteTerminator(&result); return result; } diff --git a/Firestore/Source/Local/FSTLevelDBMutationQueue.mm b/Firestore/Source/Local/FSTLevelDBMutationQueue.mm index 9041ddc..1f6484d 100644 --- a/Firestore/Source/Local/FSTLevelDBMutationQueue.mm +++ b/Firestore/Source/Local/FSTLevelDBMutationQueue.mm @@ -375,8 +375,9 @@ static ReadOptions StandardReadOptions() { NSString *userID = self.userID; // Scan the document-mutation index starting with a prefix starting with the given documentKey. - std::string indexPrefix = - [FSTLevelDBDocumentMutationKey keyPrefixWithUserID:self.userID resourcePath:documentKey.path]; + std::string indexPrefix = [FSTLevelDBDocumentMutationKey + keyPrefixWithUserID:self.userID + resourcePath:[FSTResourcePath resourcePathWithCPPResourcePath:documentKey.path]]; std::unique_ptr indexIterator(_db->NewIterator(StandardReadOptions())); indexIterator->Seek(indexPrefix); @@ -467,7 +468,7 @@ static ReadOptions StandardReadOptions() { // Rows with document keys more than one segment longer than the query path can't be matches. // For example, a query on 'rooms' can't match the document /rooms/abc/messages/xyx. // TODO(mcg): we'll need a different scanner when we implement ancestor queries. - if (rowKey.documentKey.path.length != immediateChildrenPathLength) { + if (rowKey.documentKey.path.size() != immediateChildrenPathLength) { continue; } @@ -614,8 +615,9 @@ static ReadOptions StandardReadOptions() { #pragma mark - FSTGarbageSource implementation - (BOOL)containsKey:(FSTDocumentKey *)documentKey { - std::string indexPrefix = - [FSTLevelDBDocumentMutationKey keyPrefixWithUserID:self.userID resourcePath:documentKey.path]; + std::string indexPrefix = [FSTLevelDBDocumentMutationKey + keyPrefixWithUserID:self.userID + resourcePath:[FSTResourcePath resourcePathWithCPPResourcePath:documentKey.path]]; std::unique_ptr indexIterator(_db->NewIterator(StandardReadOptions())); indexIterator->Seek(indexPrefix); diff --git a/Firestore/Source/Local/FSTLevelDBQueryCache.mm b/Firestore/Source/Local/FSTLevelDBQueryCache.mm index fe1bf19..dcbcee1 100644 --- a/Firestore/Source/Local/FSTLevelDBQueryCache.mm +++ b/Firestore/Source/Local/FSTLevelDBQueryCache.mm @@ -27,6 +27,7 @@ #import "Firestore/Source/Local/FSTQueryData.h" #import "Firestore/Source/Local/FSTWriteGroup.h" #import "Firestore/Source/Model/FSTDocumentKey.h" +#import "Firestore/Source/Model/FSTPath.h" #import "Firestore/Source/Util/FSTAssert.h" NS_ASSUME_NONNULL_BEGIN @@ -339,7 +340,8 @@ using leveldb::WriteOptions; #pragma mark - FSTGarbageSource implementation - (BOOL)containsKey:(FSTDocumentKey *)key { - std::string indexPrefix = [FSTLevelDBDocumentTargetKey keyPrefixWithResourcePath:key.path]; + std::string indexPrefix = [FSTLevelDBDocumentTargetKey + keyPrefixWithResourcePath:[FSTResourcePath resourcePathWithCPPResourcePath:key.path]]; std::unique_ptr indexIterator(_db->NewIterator([FSTLevelDB standardReadOptions])); indexIterator->Seek(indexPrefix); diff --git a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm index 17ecb53..423912f 100644 --- a/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm +++ b/Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm @@ -113,7 +113,7 @@ static ReadOptions StandardReadOptions() { for (; it->Valid() && [currentKey decodeKey:it->key()]; it->Next()) { FSTMaybeDocument *maybeDoc = [self decodedMaybeDocument:it->value() withKey:currentKey.documentKey]; - if (!query.path.IsPrefixOf([maybeDoc.key.path toCPPResourcePath])) { + if (!query.path.IsPrefixOf(maybeDoc.key.path)) { break; } else if ([maybeDoc isKindOfClass:[FSTDocument class]]) { results = [results dictionaryBySettingObject:(FSTDocument *)maybeDoc forKey:maybeDoc.key]; diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index 4bcdf47..0059e28 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -75,7 +75,7 @@ NS_ASSUME_NONNULL_BEGIN } - (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query { - if ([FSTDocumentKey isDocumentKey:[FSTResourcePath resourcePathWithCPPResourcePath:query.path]]) { + if ([FSTDocumentKey isDocumentKey:query.path]) { return [self documentsMatchingDocumentQuery:[FSTResourcePath resourcePathWithCPPResourcePath:query.path]]; } else { @@ -86,7 +86,8 @@ NS_ASSUME_NONNULL_BEGIN - (FSTDocumentDictionary *)documentsMatchingDocumentQuery:(FSTResourcePath *)docPath { FSTDocumentDictionary *result = [FSTDocumentDictionary documentDictionary]; // Just do a simple document lookup. - FSTMaybeDocument *doc = [self documentForKey:[FSTDocumentKey keyWithPath:docPath]]; + FSTMaybeDocument *doc = + [self documentForKey:[FSTDocumentKey keyWithPath:[docPath toCPPResourcePath]]]; if ([doc isKindOfClass:[FSTDocument class]]) { result = [result dictionaryBySettingObject:(FSTDocument *)doc forKey:doc.key]; } diff --git a/Firestore/Source/Local/FSTMemoryMutationQueue.mm b/Firestore/Source/Local/FSTMemoryMutationQueue.mm index bf4f600..5b2fca6 100644 --- a/Firestore/Source/Local/FSTMemoryMutationQueue.mm +++ b/Firestore/Source/Local/FSTMemoryMutationQueue.mm @@ -24,6 +24,10 @@ #import "Firestore/Source/Model/FSTPath.h" #import "Firestore/Source/Util/FSTAssert.h" +#include "Firestore/core/src/firebase/firestore/model/resource_path.h" + +using firebase::firestore::model::ResourcePath; + NS_ASSUME_NONNULL_BEGIN static const NSComparator NumberComparator = ^NSComparisonResult(NSNumber *left, NSNumber *right) { @@ -248,15 +252,15 @@ static const NSComparator NumberComparator = ^NSComparisonResult(NSNumber *left, - (NSArray *)allMutationBatchesAffectingQuery:(FSTQuery *)query { // Use the query path as a prefix for testing if a document matches the query. - FSTResourcePath *prefix = [FSTResourcePath resourcePathWithCPPResourcePath:query.path]; - int immediateChildrenPathLength = prefix.length + 1; + const ResourcePath &prefix = query.path; + size_t immediateChildrenPathLength = prefix.size() + 1; // Construct a document reference for actually scanning the index. Unlike the prefix, the document // key in this reference must have an even number of segments. The empty segment can be used as // a suffix of the query path because it precedes all other segments in an ordered traversal. - FSTResourcePath *startPath = [FSTResourcePath resourcePathWithCPPResourcePath:query.path]; + ResourcePath startPath = query.path; if (![FSTDocumentKey isDocumentKey:startPath]) { - startPath = [startPath pathByAppendingSegment:@""]; + startPath = startPath.Append(""); } FSTDocumentReference *start = [[FSTDocumentReference alloc] initWithKey:[FSTDocumentKey keyWithPath:startPath] ID:0]; @@ -265,8 +269,8 @@ static const NSComparator NumberComparator = ^NSComparisonResult(NSNumber *left, __block FSTImmutableSortedSet *uniqueBatchIDs = [FSTImmutableSortedSet setWithComparator:NumberComparator]; FSTDocumentReferenceBlock block = ^(FSTDocumentReference *reference, BOOL *stop) { - FSTResourcePath *rowKeyPath = reference.key.path; - if (![prefix isPrefixOfPath:rowKeyPath]) { + const ResourcePath &rowKeyPath = reference.key.path; + if (!prefix.IsPrefixOf(rowKeyPath)) { *stop = YES; return; } @@ -274,7 +278,7 @@ static const NSComparator NumberComparator = ^NSComparisonResult(NSNumber *left, // Rows with document keys more than one segment longer than the query path can't be matches. // For example, a query on 'rooms' can't match the document /rooms/abc/messages/xyx. // TODO(mcg): we'll need a different scanner when we implement ancestor queries. - if (rowKeyPath.length != immediateChildrenPathLength) { + if (rowKeyPath.size() != immediateChildrenPathLength) { return; } diff --git a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm index 7d5e1e2..b0d6807 100644 --- a/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm +++ b/Firestore/Source/Local/FSTMemoryRemoteDocumentCache.mm @@ -60,11 +60,10 @@ NS_ASSUME_NONNULL_BEGIN // 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:[FSTResourcePath resourcePathWithCPPResourcePath:query.path.Append("")]]; + FSTDocumentKey *prefix = [FSTDocumentKey keyWithPath:query.path.Append("")]; NSEnumerator *enumerator = [self.docs keyEnumeratorFrom:prefix]; for (FSTDocumentKey *key in enumerator) { - if (!query.path.IsPrefixOf([key.path toCPPResourcePath])) { + if (!query.path.IsPrefixOf(key.path)) { break; } FSTMaybeDocument *maybeDoc = self.docs[key]; diff --git a/Firestore/Source/Local/FSTReferenceSet.mm b/Firestore/Source/Local/FSTReferenceSet.mm index 2acd64b..15bb033 100644 --- a/Firestore/Source/Local/FSTReferenceSet.mm +++ b/Firestore/Source/Local/FSTReferenceSet.mm @@ -82,7 +82,7 @@ NS_ASSUME_NONNULL_BEGIN } - (void)removeReferencesForID:(int)ID { - FSTDocumentKey *emptyKey = [FSTDocumentKey keyWithSegments:@[]]; + FSTDocumentKey *emptyKey = [FSTDocumentKey keyWithSegments:{}]; FSTDocumentReference *start = [[FSTDocumentReference alloc] initWithKey:emptyKey ID:ID]; FSTDocumentReference *end = [[FSTDocumentReference alloc] initWithKey:emptyKey ID:(ID + 1)]; @@ -106,7 +106,7 @@ NS_ASSUME_NONNULL_BEGIN } - (FSTDocumentKeySet *)referencedKeysForID:(int)ID { - FSTDocumentKey *emptyKey = [FSTDocumentKey keyWithSegments:@[]]; + FSTDocumentKey *emptyKey = [FSTDocumentKey keyWithSegments:{}]; FSTDocumentReference *start = [[FSTDocumentReference alloc] initWithKey:emptyKey ID:ID]; FSTDocumentReference *end = [[FSTDocumentReference alloc] initWithKey:emptyKey ID:(ID + 1)]; diff --git a/Firestore/Source/Model/FSTDocument.mm b/Firestore/Source/Model/FSTDocument.mm index bf416e7..58d3629 100644 --- a/Firestore/Source/Model/FSTDocument.mm +++ b/Firestore/Source/Model/FSTDocument.mm @@ -94,8 +94,8 @@ NS_ASSUME_NONNULL_BEGIN } - (NSString *)description { - return [NSString stringWithFormat:@"", - self.key.path, self.version, + return [NSString stringWithFormat:@"", + self.key.path.CanonicalString().c_str(), self.version, self.localMutations ? @"YES" : @"NO", self.data]; } diff --git a/Firestore/Source/Model/FSTDocumentKey.h b/Firestore/Source/Model/FSTDocumentKey.h index 2af1c9a..dbcff2c 100644 --- a/Firestore/Source/Model/FSTDocumentKey.h +++ b/Firestore/Source/Model/FSTDocumentKey.h @@ -16,7 +16,9 @@ #import -@class FSTResourcePath; +#include + +#include "Firestore/core/src/firebase/firestore/model/resource_path.h" NS_ASSUME_NONNULL_BEGIN @@ -29,16 +31,14 @@ NS_ASSUME_NONNULL_BEGIN * @param path The path to the document. * @return A new instance of FSTDocumentKey. */ -+ (instancetype)keyWithPath:(FSTResourcePath *)path; - ++ (instancetype)keyWithPath:(firebase::firestore::model::ResourcePath)path; /** * 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:(NSArray *)segments; - ++ (instancetype)keyWithSegments:(std::initializer_list)segments; /** * Creates and returns a new document key from the given resource path string. * @@ -48,13 +48,12 @@ NS_ASSUME_NONNULL_BEGIN + (instancetype)keyWithPathString:(NSString *)resourcePath; /** Returns true iff the given path is a path to a document. */ -+ (BOOL)isDocumentKey:(FSTResourcePath *)path; - ++ (BOOL)isDocumentKey:(const firebase::firestore::model::ResourcePath &)path; - (BOOL)isEqualToKey:(FSTDocumentKey *)other; - (NSComparisonResult)compare:(FSTDocumentKey *)other; /** The path to the document. */ -@property(strong, nonatomic, readonly) FSTResourcePath *path; +- (const firebase::firestore::model::ResourcePath &)path; @end diff --git a/Firestore/Source/Model/FSTDocumentKey.mm b/Firestore/Source/Model/FSTDocumentKey.mm index a382a55..269748f 100644 --- a/Firestore/Source/Model/FSTDocumentKey.mm +++ b/Firestore/Source/Model/FSTDocumentKey.mm @@ -16,35 +16,43 @@ #import "Firestore/Source/Model/FSTDocumentKey.h" +#include + #import "Firestore/Source/Core/FSTFirestoreClient.h" -#import "Firestore/Source/Model/FSTPath.h" #import "Firestore/Source/Util/FSTAssert.h" +#include "Firestore/core/src/firebase/firestore/model/resource_path.h" +#include "Firestore/core/src/firebase/firestore/util/string_apple.h" + +namespace util = firebase::firestore::util; +using firebase::firestore::model::ResourcePath; + NS_ASSUME_NONNULL_BEGIN -@interface FSTDocumentKey () -/** The path to the document. */ -@property(strong, nonatomic, readwrite) FSTResourcePath *path; +@interface FSTDocumentKey () { + /** The path to the document. */ + ResourcePath _path; +} @end @implementation FSTDocumentKey -+ (instancetype)keyWithPath:(FSTResourcePath *)path { - return [[FSTDocumentKey alloc] initWithPath:path]; ++ (instancetype)keyWithPath:(ResourcePath)path { + return [[FSTDocumentKey alloc] initWithPath:std::move(path)]; } -+ (instancetype)keyWithSegments:(NSArray *)segments { - return [FSTDocumentKey keyWithPath:[FSTResourcePath pathWithSegments:segments]]; ++ (instancetype)keyWithSegments:(std::initializer_list)segments { + return [FSTDocumentKey keyWithPath:ResourcePath(segments)]; } + (instancetype)keyWithPathString:(NSString *)resourcePath { - NSArray *segments = [resourcePath componentsSeparatedByString:@"/"]; - return [FSTDocumentKey keyWithSegments:segments]; + return [FSTDocumentKey keyWithPath:ResourcePath::FromString(util::MakeStringView(resourcePath))]; } /** Designated initializer. */ -- (instancetype)initWithPath:(FSTResourcePath *)path { - FSTAssert([FSTDocumentKey isDocumentKey:path], @"invalid document key path: %@", path); +- (instancetype)initWithPath:(ResourcePath)path { + FSTAssert([FSTDocumentKey isDocumentKey:path], @"invalid document key path: %s", + path.CanonicalString().c_str()); if (self = [super init]) { _path = path; @@ -63,11 +71,11 @@ NS_ASSUME_NONNULL_BEGIN } - (NSUInteger)hash { - return self.path.hash; + return _path.Hash(); } - (NSString *)description { - return [NSString stringWithFormat:@"", self.path]; + return [NSString stringWithFormat:@"", _path.CanonicalString().c_str()]; } /** Implements NSCopying without actually copying because FSTDocumentKeys are immutable. */ @@ -89,15 +97,26 @@ NS_ASSUME_NONNULL_BEGIN }; } -+ (BOOL)isDocumentKey:(FSTResourcePath *)path { - return path.length % 2 == 0; ++ (BOOL)isDocumentKey:(const ResourcePath &)path { + return path.size() % 2 == 0; +} + +- (const firebase::firestore::model::ResourcePath &)path { + return _path; } @end const NSComparator FSTDocumentKeyComparator = ^NSComparisonResult(FSTDocumentKey *key1, FSTDocumentKey *key2) { - return [key1.path compare:key2.path]; + if (key1.path < key2.path) { + return NSOrderedAscending; + } else if (key1.path > key2.path) { + return NSOrderedDescending; + } else { + return NSOrderedSame; + } + }; NSString *const kDocumentKeyPath = @"__name__"; diff --git a/Firestore/Source/Remote/FSTRemoteStore.mm b/Firestore/Source/Remote/FSTRemoteStore.mm index 081b90e..2c8e84a 100644 --- a/Firestore/Source/Remote/FSTRemoteStore.mm +++ b/Firestore/Source/Remote/FSTRemoteStore.mm @@ -384,8 +384,7 @@ static const int kMaxPendingWrites = 10; // updates. Without applying a 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:[FSTResourcePath resourcePathWithCPPResourcePath:query.path]]; + FSTDocumentKey *key = [FSTDocumentKey keyWithPath:query.path]; FSTDeletedDocument *deletedDoc = [FSTDeletedDocument documentWithKey:key version:snapshotVersion]; [remoteEvent addDocumentUpdate:deletedDoc]; diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 1dc7c79..0fd7d33 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -104,7 +104,9 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - FSTDocumentKey <=> Key proto - (NSString *)encodedDocumentKey:(FSTDocumentKey *)key { - return [self encodedResourcePathForDatabaseID:self.databaseID path:key.path]; + return [self + encodedResourcePathForDatabaseID:self.databaseID + path:[FSTResourcePath resourcePathWithCPPResourcePath:key.path]]; } - (FSTDocumentKey *)decodedDocumentKey:(NSString *)name { @@ -115,7 +117,8 @@ NS_ASSUME_NONNULL_BEGIN FSTAssert([[path segmentAtIndex:3] isEqualToString:util::WrapNSStringNoCopy(self.databaseID->database_id())], @"Tried to deserialize key from different datbase."); - return [FSTDocumentKey keyWithPath:[self localResourcePathForQualifiedResourcePath:path]]; + return [FSTDocumentKey + keyWithPath:[[self localResourcePathForQualifiedResourcePath:path] toCPPResourcePath]]; } - (NSString *)encodedResourcePathForDatabaseID:(const DatabaseId *)databaseID @@ -313,7 +316,9 @@ NS_ASSUME_NONNULL_BEGIN util::WrapNSStringNoCopy(databaseID->project_id()), util::WrapNSStringNoCopy(databaseID->database_id())); GCFSValue *result = [GCFSValue message]; - result.referenceValue = [self encodedResourcePathForDatabaseID:databaseID path:key.path]; + result.referenceValue = [self + encodedResourcePathForDatabaseID:databaseID + path:[FSTResourcePath resourcePathWithCPPResourcePath:key.path]]; return result; } @@ -321,8 +326,8 @@ NS_ASSUME_NONNULL_BEGIN FSTResourcePath *path = [self decodedResourcePathWithDatabaseID:resourceName]; NSString *project = [path segmentAtIndex:1]; NSString *database = [path segmentAtIndex:3]; - FSTDocumentKey *key = - [FSTDocumentKey keyWithPath:[self localResourcePathForQualifiedResourcePath:path]]; + FSTDocumentKey *key = [FSTDocumentKey + keyWithPath:[[self localResourcePathForQualifiedResourcePath:path] toCPPResourcePath]]; const DatabaseId database_id(util::MakeStringView(project), util::MakeStringView(database)); FSTAssert(database_id == *self.databaseID, @"Database %@:%@ cannot encode reference from %@:%@", diff --git a/Firestore/core/src/firebase/firestore/model/resource_path.h b/Firestore/core/src/firebase/firestore/model/resource_path.h index 53c1951..6ff1b68 100644 --- a/Firestore/core/src/firebase/firestore/model/resource_path.h +++ b/Firestore/core/src/firebase/firestore/model/resource_path.h @@ -41,6 +41,8 @@ class ResourcePath : public impl::BasePath { } ResourcePath(std::initializer_list list) : BasePath{list} { } + explicit ResourcePath(SegmentsT&& segments) : BasePath{std::move(segments)} { + } /** * Creates and returns a new path from the given resource-path string, where * the path segments are separated by a slash "/". @@ -70,9 +72,6 @@ class ResourcePath : public impl::BasePath { } private: - explicit ResourcePath(SegmentsT&& segments) : BasePath{std::move(segments)} { - } - // So that methods of base can construct ResourcePath using the private // constructor. friend class BasePath; diff --git a/Firestore/core/test/firebase/firestore/testutil/testutil.h b/Firestore/core/test/firebase/firestore/testutil/testutil.h index 7e4f313..094efc5 100644 --- a/Firestore/core/test/firebase/firestore/testutil/testutil.h +++ b/Firestore/core/test/firebase/firestore/testutil/testutil.h @@ -18,6 +18,7 @@ #define FIRESTORE_CORE_TEST_FIREBASE_FIRESTORE_TESTUTIL_TESTUTIL_H_ #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "absl/strings/string_view.h" namespace firebase { @@ -30,6 +31,10 @@ inline model::FieldPath Field(absl::string_view field) { return model::FieldPath::FromServerFormat(field); } +inline model::ResourcePath Resource(absl::string_view field) { + return model::ResourcePath::FromString(field); +} + // Add a non-inline function to make this library buildable. // TODO(zxu123): remove once there is non-inline function. void dummy(); -- cgit v1.2.3