From 82ef0886bf89339bfe7a1855e697e61959eeb486 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Wed, 11 Jul 2018 21:59:49 -0400 Subject: Move creation of FSTFilter objects to static create method on FSTFilter. (#1512) Rather than previously inlining it in the calling code. This is to unify filter creation across platforms. (This change involves altering FSTFilter from a protocol to an abstract class.) --- Firestore/Example/Tests/Core/FSTQueryTests.mm | 17 +++--- .../Example/Tests/Remote/FSTSerializerBetaTests.mm | 5 +- Firestore/Example/Tests/Util/FSTHelpers.h | 4 +- Firestore/Example/Tests/Util/FSTHelpers.mm | 13 ++--- Firestore/Source/API/FIRQuery.mm | 25 +++------ Firestore/Source/Core/FSTQuery.h | 38 +++++++++----- Firestore/Source/Core/FSTQuery.mm | 61 ++++++++++++++++------ Firestore/Source/Remote/FSTSerializerBeta.mm | 16 +++--- 8 files changed, 101 insertions(+), 78 deletions(-) (limited to 'Firestore') diff --git a/Firestore/Example/Tests/Core/FSTQueryTests.mm b/Firestore/Example/Tests/Core/FSTQueryTests.mm index 6118d0f..362a94c 100644 --- a/Firestore/Example/Tests/Core/FSTQueryTests.mm +++ b/Firestore/Example/Tests/Core/FSTQueryTests.mm @@ -287,20 +287,19 @@ NS_ASSUME_NONNULL_BEGIN FSTQuery *baseQuery = FSTTestQuery("collection"); FSTDocument *doc1 = FSTTestDoc("collection/doc", 0, @{ @"tags" : @[ @"foo", @1, @YES ] }, NO); - NSArray> *matchingFilters = - @[ FSTTestFilter("tags", @"==", @[ @"foo", @1, @YES ]) ]; + NSArray *matchingFilters = @[ FSTTestFilter("tags", @"==", @[ @"foo", @1, @YES ]) ]; - NSArray> *nonMatchingFilters = @[ + NSArray *nonMatchingFilters = @[ FSTTestFilter("tags", @"==", @"foo"), FSTTestFilter("tags", @"==", @[ @"foo", @1 ]), FSTTestFilter("tags", @"==", @[ @"foo", @YES, @1 ]), ]; - for (id filter in matchingFilters) { + for (FSTFilter *filter in matchingFilters) { XCTAssertTrue([[baseQuery queryByAddingFilter:filter] matchesDocument:doc1]); } - for (id filter in nonMatchingFilters) { + for (FSTFilter *filter in nonMatchingFilters) { XCTAssertFalse([[baseQuery queryByAddingFilter:filter] matchesDocument:doc1]); } } @@ -311,7 +310,7 @@ NS_ASSUME_NONNULL_BEGIN FSTTestDoc("collection/doc", 0, @{ @"tags" : @{@"foo" : @"foo", @"a" : @0, @"b" : @YES, @"c" : @(NAN)} }, NO); - NSArray> *matchingFilters = @[ + NSArray *matchingFilters = @[ FSTTestFilter("tags", @"==", @{ @"foo" : @"foo", @"a" : @0, @@ -325,7 +324,7 @@ NS_ASSUME_NONNULL_BEGIN FSTTestFilter("tags.foo", @"==", @"foo") ]; - NSArray> *nonMatchingFilters = @[ + NSArray *nonMatchingFilters = @[ FSTTestFilter("tags", @"==", @"foo"), FSTTestFilter("tags", @"==", @{ @"foo" : @"foo", @"a" : @0, @@ -333,11 +332,11 @@ NS_ASSUME_NONNULL_BEGIN }) ]; - for (id filter in matchingFilters) { + for (FSTFilter *filter in matchingFilters) { XCTAssertTrue([[baseQuery queryByAddingFilter:filter] matchesDocument:doc1]); } - for (id filter in nonMatchingFilters) { + for (FSTFilter *filter in nonMatchingFilters) { XCTAssertFalse([[baseQuery queryByAddingFilter:filter] matchesDocument:doc1]); } } diff --git a/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm b/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm index da47aaa..51e99a8 100644 --- a/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm +++ b/Firestore/Example/Tests/Remote/FSTSerializerBetaTests.mm @@ -476,7 +476,7 @@ NS_ASSUME_NONNULL_BEGIN } - (void)testEncodesRelationFilter { - FSTRelationFilter *input = FSTTestFilter("item.part.top", @"==", @"food"); + FSTRelationFilter *input = (FSTRelationFilter *)FSTTestFilter("item.part.top", @"==", @"food"); GCFSStructuredQuery_Filter *actual = [self.serializer encodedRelationFilter:input]; GCFSStructuredQuery_Filter *expected = [GCFSStructuredQuery_Filter message]; @@ -488,7 +488,8 @@ NS_ASSUME_NONNULL_BEGIN } - (void)testEncodesArrayContainsFilter { - FSTRelationFilter *input = FSTTestFilter("item.tags", @"array_contains", @"food"); + FSTRelationFilter *input = + (FSTRelationFilter *)FSTTestFilter("item.tags", @"array_contains", @"food"); GCFSStructuredQuery_Filter *actual = [self.serializer encodedRelationFilter:input]; GCFSStructuredQuery_Filter *expected = [GCFSStructuredQuery_Filter message]; diff --git a/Firestore/Example/Tests/Util/FSTHelpers.h b/Firestore/Example/Tests/Util/FSTHelpers.h index 7946c06..8e5a86f 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.h +++ b/Firestore/Example/Tests/Util/FSTHelpers.h @@ -34,6 +34,7 @@ @class FSTDocumentKeyReference; @class FSTDocumentSet; @class FSTFieldValue; +@class FSTFilter; @class FSTLocalViewChanges; @class FSTPatchMutation; @class FSTQuery; @@ -46,7 +47,6 @@ @class FSTView; @class FSTViewSnapshot; @class FSTObjectValue; -@protocol FSTFilter; NS_ASSUME_NONNULL_BEGIN @@ -242,7 +242,7 @@ FSTQuery *FSTTestQuery(const absl::string_view path); * A convenience method to create a FSTFilter using a string representation for both field * and operator (<, <=, ==, >=, >, array_contains). */ -id FSTTestFilter(const absl::string_view field, NSString *op, id value); +FSTFilter *FSTTestFilter(const absl::string_view field, NSString *op, id value); /** A convenience method for creating sort orders. */ FSTSortOrder *FSTTestOrderBy(const absl::string_view field, NSString *direction); diff --git a/Firestore/Example/Tests/Util/FSTHelpers.mm b/Firestore/Example/Tests/Util/FSTHelpers.mm index 8ece82f..58513b0 100644 --- a/Firestore/Example/Tests/Util/FSTHelpers.mm +++ b/Firestore/Example/Tests/Util/FSTHelpers.mm @@ -179,7 +179,7 @@ FSTQuery *FSTTestQuery(const absl::string_view path) { return [FSTQuery queryWithPath:testutil::Resource(path)]; } -id FSTTestFilter(const absl::string_view field, NSString *opString, id value) { +FSTFilter *FSTTestFilter(const absl::string_view field, NSString *opString, id value) { const FieldPath path = testutil::Field(field); FSTRelationFilterOperator op; if ([opString isEqualToString:@"<"]) { @@ -199,15 +199,8 @@ id FSTTestFilter(const absl::string_view field, NSString *opString, i } FSTFieldValue *data = FSTTestFieldValue(value); - if ([data isEqual:[FSTDoubleValue nanValue]]) { - HARD_ASSERT(op == FSTRelationFilterOperatorEqual, "Must use == with NAN."); - return [[FSTNanFilter alloc] initWithField:path]; - } else if ([data isEqual:[FSTNullValue nullValue]]) { - HARD_ASSERT(op == FSTRelationFilterOperatorEqual, "Must use == with Null."); - return [[FSTNullFilter alloc] initWithField:path]; - } else { - return [FSTRelationFilter filterWithField:path filterOperator:op value:data]; - } + + return [FSTFilter filterWithField:path filterOperator:op value:data]; } FSTSortOrder *FSTTestOrderBy(const absl::string_view field, NSString *direction) { diff --git a/Firestore/Source/API/FIRQuery.mm b/Firestore/Source/API/FIRQuery.mm index 08c912d..cdf8bc9 100644 --- a/Firestore/Source/API/FIRQuery.mm +++ b/Firestore/Source/API/FIRQuery.mm @@ -487,26 +487,13 @@ addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions fieldValue = [self.firestore.dataConverter parsedQueryValue:value]; } - id filter; - if ([fieldValue isEqual:[FSTNullValue nullValue]]) { - if (filterOperator != FSTRelationFilterOperatorEqual) { - FSTThrowInvalidUsage(@"InvalidQueryException", - @"Invalid Query. You can only perform equality comparisons on nil / " - "NSNull."); - } - filter = [[FSTNullFilter alloc] initWithField:fieldPath]; - } else if ([fieldValue isEqual:[FSTDoubleValue nanValue]]) { - if (filterOperator != FSTRelationFilterOperatorEqual) { - FSTThrowInvalidUsage(@"InvalidQueryException", - @"Invalid Query. You can only perform equality comparisons on NaN."); - } - filter = [[FSTNanFilter alloc] initWithField:fieldPath]; - } else { - filter = [FSTRelationFilter filterWithField:fieldPath - filterOperator:filterOperator - value:fieldValue]; - [self validateNewRelationFilter:filter]; + FSTFilter *filter = + [FSTFilter filterWithField:fieldPath filterOperator:filterOperator value:fieldValue]; + + if ([filter isKindOfClass:[FSTRelationFilter class]]) { + [self validateNewRelationFilter:(FSTRelationFilter *)filter]; } + return [FIRQuery referenceWithQuery:[self.query queryByAddingFilter:filter] firestore:self.firestore]; } diff --git a/Firestore/Source/Core/FSTQuery.h b/Firestore/Source/Core/FSTQuery.h index e38d3dd..884ac20 100644 --- a/Firestore/Source/Core/FSTQuery.h +++ b/Firestore/Source/Core/FSTQuery.h @@ -38,15 +38,27 @@ typedef NS_ENUM(NSInteger, FSTRelationFilterOperator) { }; /** Interface used for all query filters. */ -@protocol FSTFilter +@interface FSTFilter : NSObject -/** Returns the field the Filter operates over. */ +/** + * Creates a filter for the provided path, operator, and value. + * + * Note that if the relational operator is FSTRelationFilterOperatorEqual and + * the value is [FSTNullValue nullValue] or [FSTDoubleValue nanValue], this + * will return the appropriate FSTNullFilter or FSTNanFilter class instead of a + * FSTRelationFilter. + */ ++ (instancetype)filterWithField:(const firebase::firestore::model::FieldPath &)field + filterOperator:(FSTRelationFilterOperator)op + value:(FSTFieldValue *)value; + +/** Returns the field the Filter operates over. Abstract method. */ - (const firebase::firestore::model::FieldPath &)field; -/** Returns true if a document matches the filter. */ +/** Returns true if a document matches the filter. Abstract method. */ - (BOOL)matchesDocument:(FSTDocument *)document; -/** A unique ID identifying the filter; used when serializing queries. */ +/** A unique ID identifying the filter; used when serializing queries. Abstract method. */ - (NSString *)canonicalID; @end @@ -55,7 +67,7 @@ typedef NS_ENUM(NSInteger, FSTRelationFilterOperator) { * FSTRelationFilter is a document filter constraint on a query with a single relation operator. * It is similar to NSComparisonPredicate, except customized for Firestore semantics. */ -@interface FSTRelationFilter : NSObject +@interface FSTRelationFilter : FSTFilter /** * Creates a new constraint for filtering documents. @@ -65,9 +77,9 @@ typedef NS_ENUM(NSInteger, FSTRelationFilterOperator) { * @param value A constant value to compare @a field to. The RHS of the expression. * @return A new instance of FSTRelationFilter. */ -+ (instancetype)filterWithField:(firebase::firestore::model::FieldPath)field - filterOperator:(FSTRelationFilterOperator)filterOperator - value:(FSTFieldValue *)value; +- (instancetype)initWithField:(firebase::firestore::model::FieldPath)field + filterOperator:(FSTRelationFilterOperator)filterOperator + value:(FSTFieldValue *)value; - (instancetype)init NS_UNAVAILABLE; @@ -86,14 +98,14 @@ typedef NS_ENUM(NSInteger, FSTRelationFilterOperator) { @end /** Filter that matches NULL values. */ -@interface FSTNullFilter : NSObject +@interface FSTNullFilter : FSTFilter - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithField:(firebase::firestore::model::FieldPath)field NS_DESIGNATED_INITIALIZER; @end /** Filter that matches NAN values. */ -@interface FSTNanFilter : NSObject +@interface FSTNanFilter : FSTFilter - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithField:(firebase::firestore::model::FieldPath)field NS_DESIGNATED_INITIALIZER; @@ -162,7 +174,7 @@ typedef NS_ENUM(NSInteger, FSTRelationFilterOperator) { * Initializes a query with all of its components directly. */ - (instancetype)initWithPath:(firebase::firestore::model::ResourcePath)path - filterBy:(NSArray> *)filters + filterBy:(NSArray *)filters orderBy:(NSArray *)sortOrders limit:(NSInteger)limit startAt:(nullable FSTBound *)startAtBound @@ -198,7 +210,7 @@ typedef NS_ENUM(NSInteger, FSTRelationFilterOperator) { * @param filter The predicate to filter by. * @return the new FSTQuery. */ -- (instancetype)queryByAddingFilter:(id)filter; +- (instancetype)queryByAddingFilter:(FSTFilter *)filter; /** * Creates a new FSTQuery with an additional ordering constraint. @@ -255,7 +267,7 @@ typedef NS_ENUM(NSInteger, FSTRelationFilterOperator) { - (const firebase::firestore::model::ResourcePath &)path; /** The filters on the documents returned by the query. */ -@property(nonatomic, strong, readonly) NSArray> *filters; +@property(nonatomic, strong, readonly) NSArray *filters; /** The maximum number of results to return, or NSNotFound if no limit. */ @property(nonatomic, assign, readonly) NSInteger limit; diff --git a/Firestore/Source/Core/FSTQuery.mm b/Firestore/Source/Core/FSTQuery.mm index abec474..fd6a999 100644 --- a/Firestore/Source/Core/FSTQuery.mm +++ b/Firestore/Source/Core/FSTQuery.mm @@ -23,6 +23,8 @@ #import "Firestore/Source/API/FIRFirestore+Internal.h" #import "Firestore/Source/Model/FSTDocument.h" #import "Firestore/Source/Model/FSTFieldValue.h" +#import "Firestore/Source/Util/FSTClasses.h" +#import "Firestore/Source/Util/FSTUsageValidation.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" @@ -66,6 +68,43 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe } } +@implementation FSTFilter + ++ (instancetype)filterWithField:(const FieldPath &)field + filterOperator:(FSTRelationFilterOperator)op + value:(FSTFieldValue *)value { + if ([value isEqual:[FSTNullValue nullValue]]) { + if (op != FSTRelationFilterOperatorEqual) { + FSTThrowInvalidUsage(@"InvalidQueryException", + @"Invalid Query. You can only perform equality comparisons on nil / " + "NSNull."); + } + return [[FSTNullFilter alloc] initWithField:field]; + } else if ([value isEqual:[FSTDoubleValue nanValue]]) { + if (op != FSTRelationFilterOperatorEqual) { + FSTThrowInvalidUsage(@"InvalidQueryException", + @"Invalid Query. You can only perform equality comparisons on NaN."); + } + return [[FSTNanFilter alloc] initWithField:field]; + } else { + return [[FSTRelationFilter alloc] initWithField:field filterOperator:op value:value]; + } +} + +- (const FieldPath &)field { + @throw FSTAbstractMethodException(); // NOLINT +} + +- (BOOL)matchesDocument:(FSTDocument *)document { + @throw FSTAbstractMethodException(); // NOLINT +} + +- (NSString *)canonicalID { + @throw FSTAbstractMethodException(); // NOLINT +} + +@end + #pragma mark - FSTRelationFilter @interface FSTRelationFilter () { @@ -99,14 +138,6 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe #pragma mark - Constructor methods -+ (instancetype)filterWithField:(FieldPath)field - filterOperator:(FSTRelationFilterOperator)filterOperator - value:(FSTFieldValue *)value { - return [[FSTRelationFilter alloc] initWithField:std::move(field) - filterOperator:filterOperator - value:value]; -} - - (instancetype)initWithField:(FieldPath)field filterOperator:(FSTRelationFilterOperator)filterOperator value:(FSTFieldValue *)value { @@ -513,7 +544,7 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe * @param limit If not NSNotFound, only this many results will be returned. */ - (instancetype)initWithPath:(ResourcePath)path - filterBy:(NSArray> *)filters + filterBy:(NSArray *)filters orderBy:(NSArray *)sortOrders limit:(NSInteger)limit startAt:(nullable FSTBound *)startAtBound @@ -541,7 +572,7 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe } - (instancetype)initWithPath:(ResourcePath)path - filterBy:(NSArray> *)filters + filterBy:(NSArray *)filters orderBy:(NSArray *)sortOrders limit:(NSInteger)limit startAt:(nullable FSTBound *)startAtBound @@ -630,7 +661,7 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe return self.memoizedSortOrders; } -- (instancetype)queryByAddingFilter:(id)filter { +- (instancetype)queryByAddingFilter:(FSTFilter *)filter { HARD_ASSERT(!DocumentKey::IsDocumentKey(_path), "No filtering allowed for document query"); const FieldPath *newInequalityField = nullptr; @@ -715,7 +746,7 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe } - (const FieldPath *)inequalityFilterField { - for (id filter in self.filters) { + for (FSTFilter *filter in self.filters) { if ([filter isKindOfClass:[FSTRelationFilter class]] && ((FSTRelationFilter *)filter).isInequality) { return &filter.field; @@ -725,7 +756,7 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe } - (BOOL)hasArrayContainsFilter { - for (id filter in self.filters) { + for (FSTFilter *filter in self.filters) { if ([filter isKindOfClass:[FSTRelationFilter class]] && ((FSTRelationFilter *)filter).filterOperator == FSTRelationFilterOperatorArrayContains) { return YES; @@ -758,7 +789,7 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe // Add filters. [canonicalID appendString:@"|f:"]; - for (id predicate in self.filters) { + for (FSTFilter *predicate in self.filters) { [canonicalID appendFormat:@"%@", [predicate canonicalID]]; } @@ -822,7 +853,7 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe /** Returns YES if the document matches all of the filters in the receiver. */ - (BOOL)filtersMatchDocument:(FSTDocument *)document { - for (id filter in self.filters) { + for (FSTFilter *filter in self.filters) { if (![filter matchesDocument:document]) { return NO; } diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 263fe6d..81c6958 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -802,7 +802,7 @@ NS_ASSUME_NONNULL_BEGIN path = path.Append(util::MakeString(from.collectionId)); } - NSArray> *filterBy; + NSArray *filterBy; if (query.hasWhere) { filterBy = [self decodedFilters:query.where]; } else { @@ -841,14 +841,14 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark Filters -- (GCFSStructuredQuery_Filter *_Nullable)encodedFilters:(NSArray> *)filters { +- (GCFSStructuredQuery_Filter *_Nullable)encodedFilters:(NSArray *)filters { if (filters.count == 0) { return nil; } NSMutableArray *protos = [NSMutableArray array]; - for (id filter in filters) { + for (FSTFilter *filter in filters) { if ([filter isKindOfClass:[FSTRelationFilter class]]) { - [protos addObject:[self encodedRelationFilter:filter]]; + [protos addObject:[self encodedRelationFilter:(FSTRelationFilter *)filter]]; } else { [protos addObject:[self encodedUnaryFilter:filter]]; } @@ -864,8 +864,8 @@ NS_ASSUME_NONNULL_BEGIN return composite; } -- (NSArray> *)decodedFilters:(GCFSStructuredQuery_Filter *)proto { - NSMutableArray> *result = [NSMutableArray array]; +- (NSArray *)decodedFilters:(GCFSStructuredQuery_Filter *)proto { + NSMutableArray *result = [NSMutableArray array]; NSArray *filters; if (proto.filterTypeOneOfCase == @@ -913,7 +913,7 @@ NS_ASSUME_NONNULL_BEGIN return [FSTRelationFilter filterWithField:fieldPath filterOperator:filterOperator value:value]; } -- (GCFSStructuredQuery_Filter *)encodedUnaryFilter:(id)filter { +- (GCFSStructuredQuery_Filter *)encodedUnaryFilter:(FSTFilter *)filter { GCFSStructuredQuery_Filter *proto = [GCFSStructuredQuery_Filter message]; proto.unaryFilter.field = [self encodedFieldPath:filter.field]; if ([filter isKindOfClass:[FSTNanFilter class]]) { @@ -926,7 +926,7 @@ NS_ASSUME_NONNULL_BEGIN return proto; } -- (id)decodedUnaryFilter:(GCFSStructuredQuery_UnaryFilter *)proto { +- (FSTFilter *)decodedUnaryFilter:(GCFSStructuredQuery_UnaryFilter *)proto { FieldPath field = FieldPath::FromServerFormat(util::MakeString(proto.field.fieldPath)); switch (proto.op) { case GCFSStructuredQuery_UnaryFilter_Operator_IsNan: -- cgit v1.2.3