Implement the Composite Filter class (#9734)

* Implement the Composite Filter class

* Polish code

* add tests for composite filter

* fix broken test

* Improve the implementation of flattern filters

* change the flatten_filters implementation with some other things

* Change the implementation of Filter in Composite Filter

* Address feedback

* Match Android Tests

Author: cherylEnkidu

Firestore/Example/Firestore.xcodeproj/project.pbxproj

@@ -27,6 +27,7 @@
 #import "Firestore/Example/Tests/Util/FSTHelpers.h"
 #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"
 
+#import "Firestore/Source/API/FIRFilter+Internal.h"
 #include "Firestore/core/test/unit/testutil/app_testing.h"
 
 using firebase::firestore::testutil::AppForUnitTesting;
@@ -602,6 +603,92 @@ - (void)testQueriesUsingInAndDocumentIdMustHaveProperDocumentReferencesInArray {
       reason);
 }
 
+- (void)testInvalidQueryFilters {
+  FIRCollectionReference *collection = [self collectionRef];
+
+  // Multiple inequalities, one of which is inside a nested composite filter.
+  NSString *reason = @"Invalid Query. All where filters with an inequality (notEqual, lessThan, "
+                      "lessThanOrEqual, greaterThan, or greaterThanOrEqual) must be on the same "
+                      "field. But you have inequality filters on 'c' and 'r'";
+
+  NSArray<FIRFilter *> *array1 = @[
+    [FIRFilter andFilterWithFilters:@[
+      [FIRFilter filterWhereField:@"a" isEqualTo:@"b"], [FIRFilter filterWhereField:@"c"
+                                                                      isGreaterThan:@"d"]
+    ]],
+    [FIRFilter andFilterWithFilters:@[
+      [FIRFilter filterWhereField:@"e" isEqualTo:@"f"], [FIRFilter filterWhereField:@"g"
+                                                                          isEqualTo:@"h"]
+    ]]
+  ];
+
+  FSTAssertThrows(
+      [[collection queryWhereFilter:[FIRFilter orFilterWithFilters:array1]] queryWhereField:@"r"
+                                                                              isGreaterThan:@"s"],
+      reason);
+
+  // OrderBy and inequality on different fields. Inequality inside a nested composite filter.
+  reason = @"Invalid query. You have a where filter with an inequality (notEqual, lessThan, "
+            "lessThanOrEqual, greaterThan, or greaterThanOrEqual) on field 'c' and so you must "
+            "also use 'c' as your first queryOrderedBy field, but your first queryOrderedBy is "
+            "currently on field 'r' instead.";
+
+  FSTAssertThrows([[collection queryWhereFilter:[FIRFilter orFilterWithFilters:array1]]
+                      queryOrderedByField:@"r"],
+                  reason);
+
+  // Conflicting operations within a composite filter.
+  reason = @"Invalid Query. You cannot use 'notIn' filters with 'in' filters.";
+
+  NSArray<FIRFilter *> *array2 = @[
+    [FIRFilter andFilterWithFilters:@[
+      [FIRFilter filterWhereField:@"a" isEqualTo:@"b"], [FIRFilter filterWhereField:@"c"
+                                                                                 in:@[ @"d", @"e" ]]
+    ]],
+    [FIRFilter andFilterWithFilters:@[
+      [FIRFilter filterWhereField:@"e" isEqualTo:@"f"], [FIRFilter filterWhereField:@"c"
+                                                                              notIn:@[ @"f", @"g" ]]
+    ]]
+  ];
+
+  FSTAssertThrows([collection queryWhereFilter:[FIRFilter orFilterWithFilters:array2]], reason);
+
+  // Conflicting operations between a field filter and a composite filter.
+  NSArray<FIRFilter *> *array3 = @[
+    [FIRFilter andFilterWithFilters:@[
+      [FIRFilter filterWhereField:@"a" isEqualTo:@"b"], [FIRFilter filterWhereField:@"c"
+                                                                                 in:@[ @"d", @"e" ]]
+    ]],
+    [FIRFilter andFilterWithFilters:@[
+      [FIRFilter filterWhereField:@"e" isEqualTo:@"f"], [FIRFilter filterWhereField:@"g"
+                                                                          isEqualTo:@"h"]
+    ]]
+  ];
+
+  NSArray<NSString *> *array4 = @[ @"j", @"k" ];
+
+  FSTAssertThrows(
+      [[collection queryWhereFilter:[FIRFilter orFilterWithFilters:array3]] queryWhereField:@"i"
+                                                                                      notIn:array4],
+      reason);
+
+  // Conflicting operations between two composite filters.
+  NSArray<FIRFilter *> *array5 = @[
+    [FIRFilter andFilterWithFilters:@[
+      [FIRFilter filterWhereField:@"i" isEqualTo:@"j"], [FIRFilter filterWhereField:@"l"
+                                                                              notIn:@[ @"m", @"n" ]]
+    ]],
+    [FIRFilter andFilterWithFilters:@[
+      [FIRFilter filterWhereField:@"o" isEqualTo:@"p"], [FIRFilter filterWhereField:@"q"
+                                                                          isEqualTo:@"r"]
+    ]]
+  ];
+
+  FSTAssertThrows([[collection queryWhereFilter:[FIRFilter orFilterWithFilters:array3]]
+                      queryWhereFilter:[FIRFilter orFilterWithFilters:array5]],
+                  reason);
+}
+
 - (void)testQueryInequalityFieldMustMatchFirstOrderByField {
   FIRCollectionReference *coll = [self.db collectionWithPath:@"collection"];
   FIRQuery *base = [coll queryWhereField:@"x" isGreaterThanOrEqualTo:@32];

Firestore/Example/Tests/Integration/API/FIRValidationTests.mm

@@ -15,7 +15,9 @@
  */
 
 #import <Foundation/Foundation.h>
-#include "Firestore/core/src/core/field_filter.h"
+#include "Firestore/Protos/nanopb/google/firestore/v1/query.nanopb.h"
+#import "Firestore/core/src/core/composite_filter.h"
+#import "Firestore/core/src/core/field_filter.h"
 
 // TODO(orquery): This class will become public API. Change visibility and add documentation.
 
@@ -93,15 +95,26 @@ NS_SWIFT_NAME(Filter)
                               notIn:(nonnull NSArray<id> *)values
     NS_SWIFT_NAME(whereField(_:notIn:));
 
++ (FIRFilter *)orFilterWithFilters:(NSArray<FIRFilter *> *)filters NS_SWIFT_NAME(orFilter(_:));
+
++ (FIRFilter *)andFilterWithFilters:(NSArray<FIRFilter *> *)filters NS_SWIFT_NAME(andFilter(_:));
+
 @end
 
-/** Internal FIRFilter properties we don't want exposed in our public header files. */
-@interface FIRFilter (Internal)
+/** Exposed internally */
+@interface FSTUnaryFilter : FIRFilter
 
 @property(nonatomic, strong, readonly) FIRFieldPath *fieldPath;
-@property(nonatomic, readonly) firebase::firestore::core::FieldFilter::Operator op;
+@property(nonatomic, readonly) firebase::firestore::core::FieldFilter::Operator unaryOp;
 @property(nonatomic, strong, readonly) id value;
 
 @end
 
+@interface FSTCompositeFilter : FIRFilter
+
+@property(nonatomic, strong, readonly) NSArray<FIRFilter *> *filters;
+@property(nonatomic, readonly) firebase::firestore::core::CompositeFilter::Operator compOp;
+
+@end
+
 NS_ASSUME_NONNULL_END

Firestore/Source/API/FIRFilter+Internal.h

@@ -20,6 +20,7 @@
 using firebase::firestore::model::FieldPath;
 using firebase::firestore::util::MakeString;
 using firebase::firestore::core::FieldFilter;
+using firebase::firestore::core::CompositeFilter;
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -31,55 +32,81 @@
 
 }  // namespace
 
-@interface FIRFilter ()
+@interface FSTUnaryFilter ()
 
 @property(nonatomic, strong, readwrite) FIRFieldPath *fieldPath;
-@property(nonatomic, readwrite) FieldFilter::Operator op;
+@property(nonatomic, readwrite) FieldFilter::Operator unaryOp;
 @property(nonatomic, strong, readwrite) id value;
 
 @end
 
-@implementation FIRFilter
-
-#pragma mark - Constructor Methods
+@implementation FSTUnaryFilter
 
 - (instancetype)initWithFIRFieldPath:(nonnull FIRFieldPath *)fieldPath
                                   op:(FieldFilter::Operator)op
                                value:(nonnull id)value {
   if (self = [super init]) {
     self.fieldPath = fieldPath;
-    self.op = op;
+    self.unaryOp = op;
     self.value = value;
   }
   return self;
 }
 
+@end
+
+@interface FSTCompositeFilter ()
+
+@property(nonatomic, strong, readwrite) NSArray<FIRFilter *> *filters;
+@property(nonatomic, readwrite) CompositeFilter::Operator compOp;
+
+@end
+
+@implementation FSTCompositeFilter
+
+- (instancetype)initWithFilters:(nonnull NSArray<FIRFilter *> *)filters
+                             op:(CompositeFilter::Operator)op {
+  if (self = [super init]) {
+    self.filters = filters;
+    self.compOp = op;
+  }
+  return self;
+}
+
+@end
+
+@implementation FIRFilter
+
+#pragma mark - Constructor Methods
+
 + (FIRFilter *)filterWhereField:(nonnull NSString *)field isEqualTo:(nonnull id)value {
   return [self filterWhereFieldPath:MakeFIRFieldPath(field) isEqualTo:value];
 }
 
 + (FIRFilter *)filterWhereFieldPath:(nonnull FIRFieldPath *)field isEqualTo:(nonnull id)value {
-  return [[FIRFilter alloc] initWithFIRFieldPath:field op:FieldFilter::Operator::Equal value:value];
+  return [[FSTUnaryFilter alloc] initWithFIRFieldPath:field
+                                                   op:FieldFilter::Operator::Equal
+                                                value:value];
 }
 
 + (FIRFilter *)filterWhereField:(nonnull NSString *)field isNotEqualTo:(nonnull id)value {
   return [self filterWhereFieldPath:MakeFIRFieldPath(field) isNotEqualTo:value];
 }
 
 + (FIRFilter *)filterWhereFieldPath:(nonnull FIRFieldPath *)field isNotEqualTo:(nonnull id)value {
-  return [[FIRFilter alloc] initWithFIRFieldPath:field
-                                              op:FieldFilter::Operator::NotEqual
-                                           value:value];
+  return [[FSTUnaryFilter alloc] initWithFIRFieldPath:field
+                                                   op:FieldFilter::Operator::NotEqual
+                                                value:value];
 }
 
 + (FIRFilter *)filterWhereField:(nonnull NSString *)field isGreaterThan:(nonnull id)value {
   return [self filterWhereFieldPath:MakeFIRFieldPath(field) isGreaterThan:value];
 }
 
 + (FIRFilter *)filterWhereFieldPath:(nonnull FIRFieldPath *)field isGreaterThan:(nonnull id)value {
-  return [[FIRFilter alloc] initWithFIRFieldPath:field
-                                              op:FieldFilter::Operator::GreaterThan
-                                           value:value];
+  return [[FSTUnaryFilter alloc] initWithFIRFieldPath:field
+                                                   op:FieldFilter::Operator::GreaterThan
+                                                value:value];
 }
 
 + (FIRFilter *)filterWhereField:(nonnull NSString *)field isGreaterThanOrEqualTo:(nonnull id)value {
@@ -88,19 +115,19 @@ + (FIRFilter *)filterWhereField:(nonnull NSString *)field isGreaterThanOrEqualTo
 
 + (FIRFilter *)filterWhereFieldPath:(nonnull FIRFieldPath *)field
              isGreaterThanOrEqualTo:(nonnull id)value {
-  return [[FIRFilter alloc] initWithFIRFieldPath:field
-                                              op:FieldFilter::Operator::GreaterThanOrEqual
-                                           value:value];
+  return [[FSTUnaryFilter alloc] initWithFIRFieldPath:field
+                                                   op:FieldFilter::Operator::GreaterThanOrEqual
+                                                value:value];
 }
 
 + (FIRFilter *)filterWhereField:(nonnull NSString *)field isLessThan:(nonnull id)value {
   return [self filterWhereFieldPath:MakeFIRFieldPath(field) isLessThan:value];
 }
 
 + (FIRFilter *)filterWhereFieldPath:(nonnull FIRFieldPath *)field isLessThan:(nonnull id)value {
-  return [[FIRFilter alloc] initWithFIRFieldPath:field
-                                              op:FieldFilter::Operator::LessThan
-                                           value:value];
+  return [[FSTUnaryFilter alloc] initWithFIRFieldPath:field
+                                                   op:FieldFilter::Operator::LessThan
+                                                value:value];
 }
 
 + (FIRFilter *)filterWhereField:(nonnull NSString *)field isLessThanOrEqualTo:(nonnull id)value {
@@ -109,19 +136,19 @@ + (FIRFilter *)filterWhereField:(nonnull NSString *)field isLessThanOrEqualTo:(n
 
 + (FIRFilter *)filterWhereFieldPath:(nonnull FIRFieldPath *)field
                 isLessThanOrEqualTo:(nonnull id)value {
-  return [[FIRFilter alloc] initWithFIRFieldPath:field
-                                              op:FieldFilter::Operator::LessThanOrEqual
-                                           value:value];
+  return [[FSTUnaryFilter alloc] initWithFIRFieldPath:field
+                                                   op:FieldFilter::Operator::LessThanOrEqual
+                                                value:value];
 }
 
 + (FIRFilter *)filterWhereField:(nonnull NSString *)field arrayContains:(nonnull id)value {
   return [self filterWhereFieldPath:MakeFIRFieldPath(field) arrayContains:value];
 }
 
 + (FIRFilter *)filterWhereFieldPath:(nonnull FIRFieldPath *)field arrayContains:(nonnull id)value {
-  return [[FIRFilter alloc] initWithFIRFieldPath:field
-                                              op:FieldFilter::Operator::ArrayContains
-                                           value:value];
+  return [[FSTUnaryFilter alloc] initWithFIRFieldPath:field
+                                                   op:FieldFilter::Operator::ArrayContains
+                                                value:value];
 }
 
 + (FIRFilter *)filterWhereField:(nonnull NSString *)field
@@ -131,17 +158,19 @@ + (FIRFilter *)filterWhereField:(nonnull NSString *)field
 
 + (FIRFilter *)filterWhereFieldPath:(nonnull FIRFieldPath *)field
                    arrayContainsAny:(nonnull NSArray<id> *)values {
-  return [[FIRFilter alloc] initWithFIRFieldPath:field
-                                              op:FieldFilter::Operator::ArrayContainsAny
-                                           value:values];
+  return [[FSTUnaryFilter alloc] initWithFIRFieldPath:field
+                                                   op:FieldFilter::Operator::ArrayContainsAny
+                                                value:values];
 }
 
 + (FIRFilter *)filterWhereField:(nonnull NSString *)field in:(nonnull NSArray<id> *)values {
   return [self filterWhereFieldPath:MakeFIRFieldPath(field) in:values];
 }
 
 + (FIRFilter *)filterWhereFieldPath:(nonnull FIRFieldPath *)field in:(nonnull NSArray<id> *)values {
-  return [[FIRFilter alloc] initWithFIRFieldPath:field op:FieldFilter::Operator::In value:values];
+  return [[FSTUnaryFilter alloc] initWithFIRFieldPath:field
+                                                   op:FieldFilter::Operator::In
+                                                value:values];
 }
 
 + (FIRFilter *)filterWhereField:(nonnull NSString *)field notIn:(nonnull NSArray<id> *)values {
@@ -150,11 +179,18 @@ + (FIRFilter *)filterWhereField:(nonnull NSString *)field notIn:(nonnull NSArray
 
 + (FIRFilter *)filterWhereFieldPath:(nonnull FIRFieldPath *)field
                               notIn:(nonnull NSArray<id> *)values {
-  return [[FIRFilter alloc] initWithFIRFieldPath:field
-                                              op:FieldFilter::Operator::NotIn
-                                           value:values];
+  return [[FSTUnaryFilter alloc] initWithFIRFieldPath:field
+                                                   op:FieldFilter::Operator::NotIn
+                                                value:values];
 }
 
++ (FIRFilter *)orFilterWithFilters:(NSArray<FIRFilter *> *)filters {
+  return [[FSTCompositeFilter alloc] initWithFilters:filters op:CompositeFilter::Operator::Or];
+}
+
++ (FIRFilter *)andFilterWithFilters:(NSArray<FIRFilter *> *)filters {
+  return [[FSTCompositeFilter alloc] initWithFilters:filters op:CompositeFilter::Operator::And];
+}
 @end
 
 NS_ASSUME_NONNULL_END

Firestore/Source/API/FIRFilter.mm

@@ -21,6 +21,8 @@
 #include "Firestore/core/src/api/api_fwd.h"
 #include "Firestore/core/src/core/core_fwd.h"
 
+@class FIRFilter;
+
 namespace api = firebase::firestore::api;
 namespace core = firebase::firestore::core;
 
@@ -42,6 +44,9 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (const api::Query &)apiQuery;
 
+// TODO(orquery): This method will become public API. Change visibility and add documentation.
+- (FIRQuery *)queryWhereFilter:(FIRFilter *)filter;
+
 @end
 
 NS_ASSUME_NONNULL_END