Skip to content

add converters and port paths to FSTQuery #869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 5, 2018
Merged

add converters and port paths to FSTQuery #869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa34bd1
add converters and fix FSTQuery.{h,m} only
zxu123 Mar 1, 2018
b9b3c83
address changes
zxu123 Mar 2, 2018
5010f30
a change forget to address
zxu123 Mar 2, 2018
0e54a99
add a dummy function to make inline-only-library buildable
zxu123 Mar 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 23 additions & 16 deletions Firestore/Example/Tests/Core/FSTQueryTests.mm
View file
Open in desktop
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]];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference (not in this PR since these are already obsolete) this can be handled more readably by adding an inline C++ constructor guarded by #if __OBJC__.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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