Skip to content

Commit

Permalink
add converters and port paths to FSTQuery (firebase#869)
Browse files Browse the repository at this point in the history
* add converters and fix FSTQuery.{h,m} only

* address changes

* a change forget to address

* add a dummy function to make inline-only-library buildable
  • Loading branch information
zxu123 committed Mar 5, 2018
1 parent 9b5b4d8 commit 1c40e7a
Show file tree
Hide file tree
Showing 24 changed files with 485 additions and 195 deletions.
39 changes: 23 additions & 16 deletions Firestore/Example/Tests/Core/FSTQueryTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,16 @@
#import "Firestore/Example/Tests/Util/FSTHelpers.h"

#include "Firestore/core/src/firebase/firestore/model/database_id.h"
#include "Firestore/core/src/firebase/firestore/model/field_path.h"
#include "Firestore/core/src/firebase/firestore/model/resource_path.h"
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
#include "Firestore/core/test/firebase/firestore/testutil/testutil.h"

namespace testutil = firebase::firestore::testutil;
namespace util = firebase::firestore::util;
using firebase::firestore::model::DatabaseId;
using firebase::firestore::model::FieldPath;
using firebase::firestore::model::ResourcePath;

NS_ASSUME_NONNULL_BEGIN

Expand All @@ -41,8 +47,10 @@ - (FSTQuery *)queryByAddingSortBy:(NSString *)key ascending:(BOOL)ascending;
@implementation FSTQuery (Tests)

- (FSTQuery *)queryByAddingSortBy:(NSString *)key ascending:(BOOL)ascending {
return [self queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(key)
ascending:ascending]];
return [self
queryByAddingSortOrder:[FSTSortOrder
sortOrderWithFieldPath:testutil::Field(util::MakeStringView(key))
ascending:ascending]];
}

@end
Expand All @@ -55,11 +63,11 @@ @implementation FSTQueryTests
- (void)testConstructor {
FSTResourcePath *path =
[FSTResourcePath pathWithSegments:@[ @"rooms", @"Firestore", @"messages", @"0001" ]];
FSTQuery *query = [FSTQuery queryWithPath:path];
FSTQuery *query = [FSTQuery queryWithPath:[path toCPPResourcePath]];
XCTAssertNotNil(query);

XCTAssertEqual(query.sortOrders.count, 1);
XCTAssertEqualObjects(query.sortOrders[0].field.canonicalString, kDocumentKeyPath);
XCTAssertEqual(query.sortOrders[0].field.CanonicalString(), FieldPath::kDocumentKeyPath);
XCTAssertEqual(query.sortOrders[0].ascending, YES);

XCTAssertEqual(query.explicitSortOrders.count, 0);
Expand All @@ -68,17 +76,17 @@ - (void)testConstructor {
- (void)testOrderBy {
FSTQuery *query = FSTTestQuery(@"rooms/Firestore/messages");
query =
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"length")
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("length")
ascending:NO]];

XCTAssertEqual(query.sortOrders.count, 2);
XCTAssertEqualObjects(query.sortOrders[0].field.canonicalString, @"length");
XCTAssertEqual(query.sortOrders[0].field.CanonicalString(), "length");
XCTAssertEqual(query.sortOrders[0].ascending, NO);
XCTAssertEqualObjects(query.sortOrders[1].field.canonicalString, kDocumentKeyPath);
XCTAssertEqual(query.sortOrders[1].field.CanonicalString(), FieldPath::kDocumentKeyPath);
XCTAssertEqual(query.sortOrders[1].ascending, NO);

XCTAssertEqual(query.explicitSortOrders.count, 1);
XCTAssertEqualObjects(query.explicitSortOrders[0].field.canonicalString, @"length");
XCTAssertEqual(query.explicitSortOrders[0].field.CanonicalString(), "length");
XCTAssertEqual(query.explicitSortOrders[0].ascending, NO);
}

Expand Down Expand Up @@ -211,7 +219,7 @@ - (void)testDoesNotMatchComplexObjectsForFilters {

- (void)testDoesntRemoveComplexObjectsWithOrderBy {
FSTQuery *query1 = [FSTTestQuery(@"collection")
queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"sort")
queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("sort")
ascending:YES]];

FSTDocument *doc1 = FSTTestDoc(@"collection/1", 0, @{ @"sort" : @2 }, NO);
Expand Down Expand Up @@ -305,9 +313,8 @@ - (void)assertCorrectComparisonsWithArray:(NSArray *)array comparator:(NSCompara

- (void)testSortsDocumentsInTheCorrectOrder {
FSTQuery *query = FSTTestQuery(@"collection");
query =
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"sort")
ascending:YES]];
query = [query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("sort")
ascending:YES]];

// clang-format off
NSArray<FSTDocument *> *docs = @[
Expand Down Expand Up @@ -335,10 +342,10 @@ - (void)testSortsDocumentsInTheCorrectOrder {
- (void)testSortsDocumentsUsingMultipleFields {
FSTQuery *query = FSTTestQuery(@"collection");
query =
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"sort1")
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("sort1")
ascending:YES]];
query =
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"sort2")
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("sort2")
ascending:YES]];

// clang-format off
Expand All @@ -362,10 +369,10 @@ - (void)testSortsDocumentsUsingMultipleFields {
- (void)testSortsDocumentsWithDescendingToo {
FSTQuery *query = FSTTestQuery(@"collection");
query =
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"sort1")
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("sort1")
ascending:NO]];
query =
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"sort2")
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("sort2")
ascending:NO]];

// clang-format off
Expand Down
24 changes: 14 additions & 10 deletions Firestore/Example/Tests/Core/FSTViewTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@

#import "Firestore/Example/Tests/Util/FSTHelpers.h"

#include "Firestore/core/src/firebase/firestore/model/resource_path.h"
#include "Firestore/core/test/firebase/firestore/testutil/testutil.h"

namespace testutil = firebase::firestore::testutil;
using firebase::firestore::model::ResourcePath;

NS_ASSUME_NONNULL_BEGIN

@interface FSTViewTests : XCTestCase
Expand All @@ -39,8 +45,7 @@ @implementation FSTViewTests

/** Returns a new empty query to use for testing. */
- (FSTQuery *)queryForMessages {
return [FSTQuery
queryWithPath:[FSTResourcePath pathWithSegments:@[ @"rooms", @"eros", @"messages" ]]];
return [FSTQuery queryWithPath:ResourcePath{"rooms", "eros", "messages"}];
}

- (void)testAddsDocumentsBasedOnQuery {
Expand Down Expand Up @@ -128,7 +133,7 @@ - (void)testDoesNotReturnNilForFirstChanges {
- (void)testFiltersDocumentsBasedOnQueryWithFilter {
FSTQuery *query = [self queryForMessages];
FSTRelationFilter *filter =
[FSTRelationFilter filterWithField:FSTTestFieldPath(@"sort")
[FSTRelationFilter filterWithField:testutil::Field("sort")
filterOperator:FSTRelationFilterOperatorLessThanOrEqual
value:[FSTDoubleValue doubleValue:2]];
query = [query queryByAddingFilter:filter];
Expand Down Expand Up @@ -160,7 +165,7 @@ - (void)testFiltersDocumentsBasedOnQueryWithFilter {
- (void)testUpdatesDocumentsBasedOnQueryWithFilter {
FSTQuery *query = [self queryForMessages];
FSTRelationFilter *filter =
[FSTRelationFilter filterWithField:FSTTestFieldPath(@"sort")
[FSTRelationFilter filterWithField:testutil::Field("sort")
filterOperator:FSTRelationFilterOperatorLessThanOrEqual
value:[FSTDoubleValue doubleValue:2]];
query = [query queryByAddingFilter:filter];
Expand Down Expand Up @@ -232,9 +237,8 @@ - (void)testRemovesDocumentsForQueryWithLimit {

- (void)testDoesntReportChangesForDocumentBeyondLimitOfQuery {
FSTQuery *query = [self queryForMessages];
query =
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"num")
ascending:YES]];
query = [query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("num")
ascending:YES]];
query = [query queryBySettingLimit:2];
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:[FSTDocumentKeySet keySet]];

Expand Down Expand Up @@ -385,7 +389,7 @@ - (void)testReturnsNeedsRefillOnDeleteInLimitQuery {
- (void)testReturnsNeedsRefillOnReorderInLimitQuery {
FSTQuery *query = [self queryForMessages];
query =
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"order")
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("order")
ascending:YES]];
query = [query queryBySettingLimit:2];
FSTDocument *doc1 = FSTTestDoc(@"rooms/eros/messages/0", 0, @{ @"order" : @1 }, NO);
Expand Down Expand Up @@ -419,7 +423,7 @@ - (void)testReturnsNeedsRefillOnReorderInLimitQuery {
- (void)testDoesntNeedRefillOnReorderWithinLimit {
FSTQuery *query = [self queryForMessages];
query =
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"order")
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("order")
ascending:YES]];
query = [query queryBySettingLimit:3];
FSTDocument *doc1 = FSTTestDoc(@"rooms/eros/messages/0", 0, @{ @"order" : @1 }, NO);
Expand Down Expand Up @@ -449,7 +453,7 @@ - (void)testDoesntNeedRefillOnReorderWithinLimit {
- (void)testDoesntNeedRefillOnReorderAfterLimitQuery {
FSTQuery *query = [self queryForMessages];
query =
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"order")
[query queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("order")
ascending:YES]];
query = [query queryBySettingLimit:3];
FSTDocument *doc1 = FSTTestDoc(@"rooms/eros/messages/0", 0, @{ @"order" : @1 }, NO);
Expand Down
24 changes: 24 additions & 0 deletions Firestore/Example/Tests/Model/FSTPathTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,30 @@ - (void)testCanonicalStringOfSubstring {
XCTAssertEqualObjects([[pathHead pathByRemovingFirstSegment] canonicalString], @"bar");
}

- (void)testRoundTrip {
FSTFieldPath *path = [FSTFieldPath pathWithSegments:@[ @"rooms", @"Eros", @"messages" ]];
XCTAssertEqualObjects(path, [FSTFieldPath fieldPathWithCPPFieldPath:[path toCPPFieldPath]]);

const firebase::firestore::model::FieldPath cppPath{"rooms", "Eros", "messages"};
XCTAssertEqual(cppPath, [[FSTFieldPath fieldPathWithCPPFieldPath:cppPath] toCPPFieldPath]);
}

@end

@interface FSTResourcePathTests : XCTestCase
@end

@implementation FSTResourcePathTests

- (void)testRoundTrip {
FSTResourcePath *path = [FSTResourcePath pathWithSegments:@[ @"rooms", @"Eros", @"messages" ]];
XCTAssertEqualObjects(path,
[FSTResourcePath resourcePathWithCPPResourcePath:[path toCPPResourcePath]]);

const firebase::firestore::model::ResourcePath cppPath{"rooms", "Eros", "messages"};
XCTAssertEqual(cppPath,
[[FSTResourcePath resourcePathWithCPPResourcePath:cppPath] toCPPResourcePath]);
}

@end
NS_ASSUME_NONNULL_END
6 changes: 4 additions & 2 deletions Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@

#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 testutil = firebase::firestore::testutil;
namespace util = firebase::firestore::util;
using firebase::firestore::model::DatabaseId;

Expand Down Expand Up @@ -581,7 +583,7 @@ - (void)unaryFilterTestWithValue:(id)value

- (void)testEncodesSortOrders {
FSTQuery *q = [FSTTestQuery(@"docs")
queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"prop")
queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("prop")
ascending:YES]];
FSTQueryData *model = [self queryDataForQuery:q];

Expand All @@ -601,7 +603,7 @@ - (void)testEncodesSortOrders {

- (void)testEncodesSortOrdersDescending {
FSTQuery *q = [FSTTestQuery(@"rooms/1/messages/10/attachments")
queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(@"prop")
queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:testutil::Field("prop")
ascending:NO]];
FSTQueryData *model = [self queryDataForQuery:q];

Expand Down
15 changes: 8 additions & 7 deletions Firestore/Example/Tests/Util/FSTHelpers.mm
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@
}

FSTQuery *FSTTestQuery(NSString *path) {
return [FSTQuery queryWithPath:FSTTestPath(path)];
return [FSTQuery queryWithPath:[FSTTestPath(path) toCPPResourcePath]];
}

id<FSTFilter> FSTTestFilter(NSString *field, NSString *opString, id value) {
Expand All @@ -206,12 +206,12 @@
FSTFieldValue *data = FSTTestFieldValue(value);
if ([data isEqual:[FSTDoubleValue nanValue]]) {
FSTCAssert(op == FSTRelationFilterOperatorEqual, @"Must use == with NAN.");
return [[FSTNanFilter alloc] initWithField:path];
return [[FSTNanFilter alloc] initWithField:[path toCPPFieldPath]];
} else if ([data isEqual:[FSTNullValue nullValue]]) {
FSTCAssert(op == FSTRelationFilterOperatorEqual, @"Must use == with Null.");
return [[FSTNullFilter alloc] initWithField:path];
return [[FSTNullFilter alloc] initWithField:[path toCPPFieldPath]];
} else {
return [FSTRelationFilter filterWithField:path filterOperator:op value:data];
return [FSTRelationFilter filterWithField:[path toCPPFieldPath] filterOperator:op value:data];
}
}

Expand All @@ -225,13 +225,14 @@
} else {
FSTCFail(@"Unsupported direction: %@", direction);
}
return [FSTSortOrder sortOrderWithFieldPath:path ascending:ascending];
return [FSTSortOrder sortOrderWithFieldPath:[path toCPPFieldPath] ascending:ascending];
}

NSComparator FSTTestDocComparator(NSString *fieldPath) {
FSTQuery *query = [FSTTestQuery(@"docs")
queryByAddingSortOrder:[FSTSortOrder sortOrderWithFieldPath:FSTTestFieldPath(fieldPath)
ascending:YES]];
queryByAddingSortOrder:[FSTSortOrder
sortOrderWithFieldPath:[FSTTestFieldPath(fieldPath) toCPPFieldPath]
ascending:YES]];
return [query comparator];
}

Expand Down
31 changes: 20 additions & 11 deletions Firestore/Source/API/FIRCollectionReference.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
#import "Firestore/Source/Util/FSTAssert.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;
using firebase::firestore::util::CreateAutoId;

NS_ASSUME_NONNULL_BEGIN
Expand Down Expand Up @@ -57,7 +62,8 @@ - (instancetype)initWithPath:(FSTResourcePath *)path firestore:(FIRFirestore *)f
"number of segments, but %@ has %d",
path.canonicalString, path.length);
}
self = [super initWithQuery:[FSTQuery queryWithPath:path] firestore:firestore];
self =
[super initWithQuery:[FSTQuery queryWithPath:[path toCPPResourcePath]] firestore:firestore];
return self;
}

Expand Down Expand Up @@ -87,30 +93,33 @@ - (NSUInteger)hash {
}

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

- (FIRDocumentReference *_Nullable)parent {
FSTResourcePath *parentPath = [self.query.path pathByRemovingLastSegment];
if (parentPath.isEmpty) {
const ResourcePath parentPath = self.query.path.PopLast();
if (parentPath.empty()) {
return nil;
} else {
FSTDocumentKey *key = [FSTDocumentKey keyWithPath:parentPath];
FSTDocumentKey *key =
[FSTDocumentKey keyWithPath:[FSTResourcePath resourcePathWithCPPResourcePath:parentPath]];
return [FIRDocumentReference referenceWithKey:key firestore:self.firestore];
}
}

- (NSString *)path {
return [self.query.path canonicalString];
return util::WrapNSString(self.query.path.CanonicalString());
}

- (FIRDocumentReference *)documentWithPath:(NSString *)documentPath {
if (!documentPath) {
FSTThrowInvalidArgument(@"Document path cannot be nil.");
}
FSTResourcePath *subPath = [FSTResourcePath pathWithString:documentPath];
FSTResourcePath *path = [self.query.path pathByAppendingPath:subPath];
return [FIRDocumentReference referenceWithPath:path firestore:self.firestore];
const ResourcePath subPath = ResourcePath::FromString(util::MakeStringView(documentPath));
const ResourcePath path = self.query.path.Append(subPath);
return
[FIRDocumentReference referenceWithPath:[FSTResourcePath resourcePathWithCPPResourcePath:path]
firestore:self.firestore];
}

- (FIRDocumentReference *)addDocumentWithData:(NSDictionary<NSString *, id> *)data {
Expand All @@ -126,9 +135,9 @@ - (FIRDocumentReference *)addDocumentWithData:(NSDictionary<NSString *, id> *)da
}

- (FIRDocumentReference *)documentWithAutoID {
NSString *autoID = [NSString stringWithUTF8String:CreateAutoId().c_str()];
const ResourcePath path = self.query.path.Append(CreateAutoId());
FSTDocumentKey *key =
[FSTDocumentKey keyWithPath:[self.query.path pathByAppendingSegment:autoID]];
[FSTDocumentKey keyWithPath:[FSTResourcePath resourcePathWithCPPResourcePath:path]];
return [FIRDocumentReference referenceWithKey:key firestore:self.firestore];
}

Expand Down
2 changes: 1 addition & 1 deletion Firestore/Source/API/FIRDocumentReference.mm
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ - (void)getDocumentWithCompletion:(void (^)(FIRDocumentSnapshot *_Nullable docum
addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions
listener:(FIRDocumentSnapshotBlock)listener {
FIRFirestore *firestore = self.firestore;
FSTQuery *query = [FSTQuery queryWithPath:self.key.path];
FSTQuery *query = [FSTQuery queryWithPath:[self.key.path toCPPResourcePath]];
FSTDocumentKey *key = self.key;

FSTViewSnapshotHandler snapshotHandler = ^(FSTViewSnapshot *snapshot, NSError *error) {
Expand Down
Loading

0 comments on commit 1c40e7a

Please sign in to comment.