Skip to content

Commit

Permalink
port paths to FSTDocumentKey (firebase#877)
Browse files Browse the repository at this point in the history
* replace path with C++ implementation in FSTDocumentKey.{h,mm} only

* address changes

* address changes
  • Loading branch information
zxu123 authored Mar 6, 2018
1 parent 34ebf10 commit 8311c64
Show file tree
Hide file tree
Showing 23 changed files with 152 additions and 111 deletions.
21 changes: 11 additions & 10 deletions Firestore/Example/Tests/Model/FSTDocumentKeyTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@

#import <XCTest/XCTest.h>

#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

Expand All @@ -28,23 +30,22 @@ @interface FSTDocumentKeyTests : XCTestCase
@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]);
Expand Down
6 changes: 4 additions & 2 deletions Firestore/Example/Tests/Util/FSTHelpers.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -266,7 +268,7 @@ NSComparator FSTTestDocComparator(NSString *fieldPath) {
}
}];

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
Expand All @@ -277,7 +279,7 @@ NSComparator FSTTestDocComparator(NSString *fieldPath) {
// For now this only creates TransformMutations with server timestamps.
FSTTransformMutation *FSTTestTransformMutation(NSString *path,
NSArray<NSString *> *serverTimestampFields) {
FSTDocumentKey *key = [FSTDocumentKey keyWithPath:FSTTestPath(path)];
FSTDocumentKey *key = [FSTDocumentKey keyWithPath:testutil::Resource([path UTF8String])];
NSMutableArray<FSTFieldTransform *> *fieldTransforms = [NSMutableArray array];
for (NSString *field in serverTimestampFields) {
FSTFieldPath *fieldPath = FSTTestFieldPath(field);
Expand Down
7 changes: 2 additions & 5 deletions Firestore/Source/API/FIRCollectionReference.mm
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ - (FIRDocumentReference *_Nullable)parent {
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];
}
}
Expand Down Expand Up @@ -135,9 +134,7 @@ - (FIRDocumentReference *)addDocumentWithData:(NSDictionary<NSString *, id> *)da
}

- (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];
}

Expand Down
23 changes: 16 additions & 7 deletions Firestore/Source/API/FIRDocumentReference.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -93,7 +99,8 @@ + (instancetype)referenceWithPath:(FSTResourcePath *)path firestore:(FIRFirestor
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 {
Expand Down Expand Up @@ -136,24 +143,26 @@ - (NSUInteger)hash {
#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 {
if (!collectionPath) {
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];
}

Expand Down Expand Up @@ -266,7 +275,7 @@ - (void)getDocumentWithCompletion:(void (^)(FIRDocumentSnapshot *_Nullable docum
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) {
Expand Down
2 changes: 1 addition & 1 deletion Firestore/Source/API/FIRDocumentSnapshot.mm
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ - (FIRDocumentReference *)reference {
}

- (NSString *)documentID {
return [self.internalKey.path lastSegment];
return util::WrapNSString(self.internalKey.path.last_segment());
}

- (FIRSnapshotMetadata *)metadata {
Expand Down
9 changes: 3 additions & 6 deletions Firestore/Source/API/FIRQuery.mm
Original file line number Diff line number Diff line change
Expand Up @@ -469,8 +469,7 @@ - (FIRQuery *)queryWithFilterOperator:(FSTRelationFilterOperator)filterOperator
@"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]]) {
Expand Down Expand Up @@ -621,10 +620,8 @@ - (FSTBound *)boundFromFieldValues:(NSArray<id> *)fieldValues isBefore:(BOOL)isB
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 {
Expand Down
18 changes: 7 additions & 11 deletions Firestore/Source/Core/FSTQuery.mm
Original file line number Diff line number Diff line change
Expand Up @@ -620,8 +620,7 @@ - (NSArray *)sortOrders {
}

- (instancetype)queryByAddingFilter:(id<FSTFilter>)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]] &&
Expand All @@ -642,8 +641,7 @@ - (instancetype)queryByAddingFilter:(id<FSTFilter>)filter {
}

- (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
Expand Down Expand Up @@ -682,8 +680,7 @@ - (instancetype)queryByAddingEndAt:(FSTBound *)bound {
}

- (BOOL)isDocumentQuery {
return [FSTDocumentKey isDocumentKey:[FSTResourcePath resourcePathWithCPPResourcePath:_path]] &&
self.filters.count == 0;
return [FSTDocumentKey isDocumentKey:_path] && self.filters.count == 0;
}

- (BOOL)matchesDocument:(FSTDocument *)document {
Expand Down Expand Up @@ -777,14 +774,13 @@ - (BOOL)isEqualToQuery:(FSTQuery *)other {

/* 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;
}
}

Expand Down
2 changes: 1 addition & 1 deletion Firestore/Source/Core/FSTSyncEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ - (void)trackLimboChange:(FSTLimboDocumentChange *)limboChange {
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
Expand Down
25 changes: 15 additions & 10 deletions Firestore/Source/Local/FSTLevelDBKey.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,18 @@
#import "Firestore/Source/Local/FSTLevelDBKey.h"

#include <string>
#include <utility>
#include <vector>

#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;
Expand Down Expand Up @@ -271,7 +277,7 @@ BOOL ReadDocumentKey(Slice *contents, FSTDocumentKey *__strong *result) {
Slice completeSegments = *contents;

std::string segment;
NSMutableArray<NSString *> *pathSegments = [NSMutableArray array];
std::vector<std::string> path_segments{};
for (;;) {
// Advance a temporary slice to avoid advancing contents into the next key component which may
// not be a path segment.
Expand All @@ -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;
Expand Down Expand Up @@ -391,7 +396,7 @@ + (NSString *)descriptionForKey:(StringView)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;
Expand Down Expand Up @@ -531,7 +536,7 @@ @implementation FSTLevelDBDocumentMutationKey {
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;
Expand Down Expand Up @@ -685,7 +690,7 @@ @implementation FSTLevelDBTargetDocumentKey
std::string result;
WriteTableName(&result, kTargetDocumentsTable);
WriteTargetID(&result, targetID);
WriteResourcePath(&result, documentKey.path);
WriteResourcePath(&result, [FSTResourcePath resourcePathWithCPPResourcePath:documentKey.path]);
WriteTerminator(&result);
return result;
}
Expand Down Expand Up @@ -719,7 +724,7 @@ @implementation FSTLevelDBDocumentTargetKey
+ (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;
Expand Down Expand Up @@ -754,7 +759,7 @@ @implementation FSTLevelDBRemoteDocumentKey
+ (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;
}
Expand Down
12 changes: 7 additions & 5 deletions Firestore/Source/Local/FSTLevelDBMutationQueue.mm
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,9 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(FSTBatchID)batchID
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<Iterator> indexIterator(_db->NewIterator(StandardReadOptions()));
indexIterator->Seek(indexPrefix);

Expand Down Expand Up @@ -467,7 +468,7 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(FSTBatchID)batchID
// 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;
}

Expand Down Expand Up @@ -614,8 +615,9 @@ - (FSTMutationBatch *)decodedMutationBatch:(Slice)slice {
#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<Iterator> indexIterator(_db->NewIterator(StandardReadOptions()));
indexIterator->Seek(indexPrefix);

Expand Down
4 changes: 3 additions & 1 deletion Firestore/Source/Local/FSTLevelDBQueryCache.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -339,7 +340,8 @@ - (FSTDocumentKeySet *)matchingKeysForTargetID:(FSTTargetID)targetID {
#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<Iterator> indexIterator(_db->NewIterator([FSTLevelDB standardReadOptions]));
indexIterator->Seek(indexPrefix);

Expand Down
2 changes: 1 addition & 1 deletion Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ - (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query {
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];
Expand Down
Loading

0 comments on commit 8311c64

Please sign in to comment.