Skip to content

Commit 6d9f334

Browse files
cherylEnkiducherylEnkidu
authored
Remove some query restrictions and add integration tests (#10449)
* Adding test * Remove more restrictions
1 parent 09aae23 commit 6d9f334

File tree

3 files changed

+190
-102
lines changed

3 files changed

+190
-102
lines changed

‎Firestore/Example/Tests/Integration/API/FIRQueryTests.mm‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,4 +1020,184 @@ - (void)testOrQueriesWithArrayMembership {
10201020
matchesResult:@[ @"doc1", @"doc4", @"doc6" ]];
10211021
}
10221022

1023+
- (void)testMultipleInOps {
1024+
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
1025+
@"doc1" : @{@"a" : @1, @"b" : @0},
1026+
@"doc2" : @{@"b" : @1},
1027+
@"doc3" : @{@"a" : @3, @"b" : @2},
1028+
@"doc4" : @{@"a" : @1, @"b" : @3},
1029+
@"doc5" : @{@"a" : @1},
1030+
@"doc6" : @{@"a" : @2}
1031+
}];
1032+
1033+
// Two IN operations on different fields with disjunction.
1034+
FIRFilter *filter1 = [FIRFilter orFilterWithFilters:@[
1035+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
1036+
in:@[ @0, @2 ]]
1037+
]];
1038+
[self checkOnlineAndOfflineQuery:[[collRef queryWhereFilter:filter1] queryOrderedByField:@"a"]
1039+
matchesResult:@[ @"doc1", @"doc6", @"doc3" ]];
1040+
1041+
// Two IN operations on different fields with conjunction.
1042+
FIRFilter *filter2 = [FIRFilter andFilterWithFilters:@[
1043+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
1044+
in:@[ @0, @2 ]]
1045+
]];
1046+
[self checkOnlineAndOfflineQuery:[[collRef queryWhereFilter:filter2] queryOrderedByField:@"a"]
1047+
matchesResult:@[ @"doc3" ]];
1048+
1049+
// Two IN operations on the same field.
1050+
// a IN [1,2,3] && a IN [0,1,4] should result in "a==1".
1051+
FIRFilter *filter3 = [FIRFilter andFilterWithFilters:@[
1052+
[FIRFilter filterWhereField:@"a" in:@[ @1, @2, @3 ]],
1053+
[FIRFilter filterWhereField:@"a" in:@[ @0, @1, @4 ]]
1054+
]];
1055+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter3]
1056+
matchesResult:@[ @"doc1", @"doc4", @"doc5" ]];
1057+
1058+
// a IN [2,3] && a IN [0,1,4] is never true and so the result should be an empty set.
1059+
FIRFilter *filter4 = [FIRFilter andFilterWithFilters:@[
1060+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"a"
1061+
in:@[ @0, @1, @4 ]]
1062+
]];
1063+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter4] matchesResult:@[]];
1064+
1065+
// a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]).
1066+
FIRFilter *filter5 = [FIRFilter orFilterWithFilters:@[
1067+
[FIRFilter filterWhereField:@"a" in:@[ @0, @3 ]], [FIRFilter filterWhereField:@"a"
1068+
in:@[ @0, @2 ]]
1069+
]];
1070+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter5]
1071+
matchesResult:@[ @"doc3", @"doc6" ]];
1072+
1073+
// Nested composite filter on the same field.
1074+
FIRFilter *filter6 = [FIRFilter andFilterWithFilters:@[
1075+
[FIRFilter filterWhereField:@"a" in:@[ @1, @3 ]], [FIRFilter orFilterWithFilters:@[
1076+
[FIRFilter filterWhereField:@"a" in:@[ @0, @2 ]], [FIRFilter andFilterWithFilters:@[
1077+
[FIRFilter filterWhereField:@"b" isGreaterThanOrEqualTo:@1],
1078+
[FIRFilter filterWhereField:@"a" in:@[ @1, @3 ]]
1079+
]]
1080+
]]
1081+
]];
1082+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter6]
1083+
matchesResult:@[ @"doc3", @"doc4" ]];
1084+
1085+
// Nested composite filter on different fields.
1086+
FIRFilter *filter7 = [FIRFilter andFilterWithFilters:@[
1087+
[FIRFilter filterWhereField:@"b" in:@[ @0, @3 ]], [FIRFilter orFilterWithFilters:@[
1088+
[FIRFilter filterWhereField:@"b" in:@[ @1 ]], [FIRFilter andFilterWithFilters:@[
1089+
[FIRFilter filterWhereField:@"b" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"a"
1090+
in:@[ @1, @3 ]]
1091+
]]
1092+
]]
1093+
]];
1094+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter7] matchesResult:@[ @"doc4" ]];
1095+
}
1096+
1097+
- (void)testUseInWithArrayContainsAny {
1098+
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
1099+
@"doc1" : @{@"a" : @1, @"b" : @[ @0 ]},
1100+
@"doc2" : @{@"b" : @[ @1 ]},
1101+
@"doc3" : @{@"a" : @3, @"b" : @[ @2, @7 ], @"c" : @10},
1102+
@"doc4" : @{@"a" : @1, @"b" : @[ @3, @7 ]},
1103+
@"doc5" : @{@"a" : @1},
1104+
@"doc6" : @{@"a" : @2, @"c" : @20}
1105+
}];
1106+
1107+
FIRFilter *filter1 = [FIRFilter orFilterWithFilters:@[
1108+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
1109+
arrayContainsAny:@[ @0, @7 ]]
1110+
]];
1111+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter1]
1112+
matchesResult:@[ @"doc1", @"doc3", @"doc4", @"doc6" ]];
1113+
1114+
FIRFilter *filter2 = [FIRFilter andFilterWithFilters:@[
1115+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
1116+
arrayContainsAny:@[ @0, @7 ]]
1117+
]];
1118+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter2] matchesResult:@[ @"doc3" ]];
1119+
1120+
FIRFilter *filter3 = [FIRFilter orFilterWithFilters:@[
1121+
[FIRFilter andFilterWithFilters:@[
1122+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"c"
1123+
isEqualTo:@10]
1124+
]],
1125+
[FIRFilter filterWhereField:@"b" arrayContainsAny:@[ @0, @7 ]]
1126+
]];
1127+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter3]
1128+
matchesResult:@[ @"doc1", @"doc3", @"doc4" ]];
1129+
1130+
FIRFilter *filter4 = [FIRFilter andFilterWithFilters:@[
1131+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter orFilterWithFilters:@[
1132+
[FIRFilter filterWhereField:@"b" arrayContainsAny:@[ @0, @7 ]],
1133+
[FIRFilter filterWhereField:@"c" isEqualTo:@20]
1134+
]]
1135+
]];
1136+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter4]
1137+
matchesResult:@[ @"doc3", @"doc6" ]];
1138+
}
1139+
1140+
- (void)testUseInWithArrayContains {
1141+
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
1142+
@"doc1" : @{@"a" : @1, @"b" : @[ @0 ]},
1143+
@"doc2" : @{@"b" : @[ @1 ]},
1144+
@"doc3" : @{@"a" : @3, @"b" : @[ @2, @7 ]},
1145+
@"doc4" : @{@"a" : @1, @"b" : @[ @3, @7 ]},
1146+
@"doc5" : @{@"a" : @1},
1147+
@"doc6" : @{@"a" : @2}
1148+
}];
1149+
1150+
FIRFilter *filter1 = [FIRFilter orFilterWithFilters:@[
1151+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
1152+
arrayContainsAny:@[ @3 ]]
1153+
]];
1154+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter1]
1155+
matchesResult:@[ @"doc3", @"doc4", @"doc6" ]];
1156+
1157+
FIRFilter *filter2 = [FIRFilter andFilterWithFilters:@[
1158+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter filterWhereField:@"b"
1159+
arrayContains:@7]
1160+
]];
1161+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter2] matchesResult:@[ @"doc3" ]];
1162+
1163+
FIRFilter *filter3 = [FIRFilter orFilterWithFilters:@[
1164+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter andFilterWithFilters:@[
1165+
[FIRFilter filterWhereField:@"b" arrayContains:@3], [FIRFilter filterWhereField:@"a"
1166+
isEqualTo:@1]
1167+
]]
1168+
]];
1169+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter3]
1170+
matchesResult:@[ @"doc3", @"doc4", @"doc6" ]];
1171+
1172+
FIRFilter *filter4 = [FIRFilter andFilterWithFilters:@[
1173+
[FIRFilter filterWhereField:@"a" in:@[ @2, @3 ]], [FIRFilter orFilterWithFilters:@[
1174+
[FIRFilter filterWhereField:@"b" arrayContains:@7], [FIRFilter filterWhereField:@"a"
1175+
isEqualTo:@1]
1176+
]]
1177+
]];
1178+
[self checkOnlineAndOfflineQuery:[collRef queryWhereFilter:filter4] matchesResult:@[ @"doc3" ]];
1179+
}
1180+
1181+
- (void)testOrderByEquality {
1182+
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
1183+
@"doc1" : @{@"a" : @1, @"b" : @[ @0 ]},
1184+
@"doc2" : @{@"b" : @[ @1 ]},
1185+
@"doc3" : @{@"a" : @3, @"b" : @[ @2, @7 ], @"c" : @10},
1186+
@"doc4" : @{@"a" : @1, @"b" : @[ @3, @7 ]},
1187+
@"doc5" : @{@"a" : @1},
1188+
@"doc6" : @{@"a" : @2, @"c" : @20}
1189+
}];
1190+
1191+
[self checkOnlineAndOfflineQuery:[[collRef queryWhereFilter:[FIRFilter filterWhereField:@"a"
1192+
isEqualTo:@1]]
1193+
queryOrderedByField:@"a"]
1194+
matchesResult:@[ @"doc1", @"doc4", @"doc5" ]];
1195+
1196+
[self checkOnlineAndOfflineQuery:[[collRef
1197+
queryWhereFilter:[FIRFilter filterWhereField:@"a"
1198+
in:@[ @2, @3 ]]]
1199+
queryOrderedByField:@"a"]
1200+
matchesResult:@[ @"doc6", @"doc3" ]];
1201+
}
1202+
10231203
@end

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

Lines changed: 0 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -774,23 +774,6 @@ - (void)testQueriesWithMultipleNotEqualAndInequalitiesFail {
774774
"the same field. But you have inequality filters on 'x' and 'y'");
775775
}
776776

777-
- (void)testQueriesWithMultipleArrayFiltersFail {
778-
FIRCollectionReference *coll = [self.db collectionWithPath:@"collection"];
779-
FSTAssertThrows([[coll queryWhereField:@"foo" arrayContains:@1] queryWhereField:@"foo"
780-
arrayContains:@2],
781-
@"Invalid Query. You cannot use more than one 'arrayContains' filter.");
782-
783-
FSTAssertThrows(
784-
[[coll queryWhereField:@"foo" arrayContains:@1] queryWhereField:@"foo"
785-
arrayContainsAny:@[ @2 ]],
786-
@"Invalid Query. You cannot use 'arrayContainsAny' filters with 'arrayContains' filters.");
787-
788-
FSTAssertThrows(
789-
[[coll queryWhereField:@"foo" arrayContainsAny:@[ @1 ]] queryWhereField:@"foo"
790-
arrayContains:@2],
791-
@"Invalid Query. You cannot use 'arrayContains' filters with 'arrayContainsAny' filters.");
792-
}
793-
794777
- (void)testQueriesWithNotEqualAndNotInFiltersFail {
795778
FIRCollectionReference *coll = [self.db collectionWithPath:@"collection"];
796779

@@ -805,25 +788,11 @@ - (void)testQueriesWithNotEqualAndNotInFiltersFail {
805788

806789
- (void)testQueriesWithMultipleDisjunctiveFiltersFail {
807790
FIRCollectionReference *coll = [self.db collectionWithPath:@"collection"];
808-
FSTAssertThrows([[coll queryWhereField:@"foo" in:@[ @1 ]] queryWhereField:@"foo" in:@[ @2 ]],
809-
@"Invalid Query. You cannot use more than one 'in' filter.");
810-
811-
FSTAssertThrows([[coll queryWhereField:@"foo" arrayContainsAny:@[ @1 ]] queryWhereField:@"foo"
812-
arrayContainsAny:@[ @2 ]],
813-
@"Invalid Query. You cannot use more than one 'arrayContainsAny' filter.");
814791

815792
FSTAssertThrows([[coll queryWhereField:@"foo" notIn:@[ @1 ]] queryWhereField:@"foo"
816793
notIn:@[ @2 ]],
817794
@"Invalid Query. You cannot use more than one 'notIn' filter.");
818795

819-
FSTAssertThrows([[coll queryWhereField:@"foo" arrayContainsAny:@[ @1 ]] queryWhereField:@"foo"
820-
in:@[ @2 ]],
821-
@"Invalid Query. You cannot use 'in' filters with 'arrayContainsAny' filters.");
822-
823-
FSTAssertThrows([[coll queryWhereField:@"foo" in:@[ @1 ]] queryWhereField:@"foo"
824-
arrayContainsAny:@[ @2 ]],
825-
@"Invalid Query. You cannot use 'arrayContainsAny' filters with 'in' filters.");
826-
827796
FSTAssertThrows(
828797
[[coll queryWhereField:@"foo" arrayContainsAny:@[ @1 ]] queryWhereField:@"foo" notIn:@[ @2 ]],
829798
@"Invalid Query. You cannot use 'notIn' filters with 'arrayContainsAny' filters.");
@@ -837,31 +806,6 @@ - (void)testQueriesWithMultipleDisjunctiveFiltersFail {
837806

838807
FSTAssertThrows([[coll queryWhereField:@"foo" notIn:@[ @1 ]] queryWhereField:@"foo" in:@[ @2 ]],
839808
@"Invalid Query. You cannot use 'in' filters with 'notIn' filters.");
840-
841-
// This is redundant with the above tests, but makes sure our validation doesn't get confused.
842-
FSTAssertThrows([[[coll queryWhereField:@"foo"
843-
in:@[ @1 ]] queryWhereField:@"foo"
844-
arrayContains:@2] queryWhereField:@"foo"
845-
arrayContainsAny:@[ @2 ]],
846-
@"Invalid Query. You cannot use 'arrayContainsAny' filters with 'in' filters.");
847-
848-
FSTAssertThrows(
849-
[[[coll queryWhereField:@"foo"
850-
arrayContains:@1] queryWhereField:@"foo" in:@[ @2 ]] queryWhereField:@"foo"
851-
arrayContainsAny:@[ @2 ]],
852-
@"Invalid Query. You cannot use 'arrayContainsAny' filters with 'arrayContains' filters.");
853-
854-
FSTAssertThrows([[[coll queryWhereField:@"foo"
855-
notIn:@[ @1 ]] queryWhereField:@"foo"
856-
arrayContains:@2] queryWhereField:@"foo"
857-
arrayContainsAny:@[ @2 ]],
858-
@"Invalid Query. You cannot use 'arrayContains' filters with 'notIn' filters.");
859-
860-
FSTAssertThrows([[[coll queryWhereField:@"foo"
861-
arrayContains:@1] queryWhereField:@"foo"
862-
in:@[ @2 ]] queryWhereField:@"foo"
863-
notIn:@[ @2 ]],
864-
@"Invalid Query. You cannot use 'notIn' filters with 'arrayContains' filters.");
865809
}
866810

867811
- (void)testQueriesCanUseInWithArrayContain {
@@ -873,18 +817,6 @@ - (void)testQueriesCanUseInWithArrayContain {
873817
XCTAssertNoThrow([[coll queryWhereField:@"foo" in:@[ @1 ]] queryWhereField:@"foo"
874818
arrayContains:@2],
875819
@"IN with arrayContains works.");
876-
877-
FSTAssertThrows([[[coll queryWhereField:@"foo"
878-
in:@[ @1 ]] queryWhereField:@"foo"
879-
arrayContains:@2] queryWhereField:@"foo"
880-
arrayContains:@3],
881-
@"Invalid Query. You cannot use more than one 'arrayContains' filter.");
882-
883-
FSTAssertThrows([[[coll queryWhereField:@"foo"
884-
arrayContains:@1] queryWhereField:@"foo"
885-
in:@[ @2 ]] queryWhereField:@"foo"
886-
in:@[ @3 ]],
887-
@"Invalid Query. You cannot use more than one 'in' filter.");
888820
}
889821

890822
- (void)testQueriesInAndArrayContainsAnyArrayRules {
@@ -898,18 +830,6 @@ - (void)testQueriesInAndArrayContainsAnyArrayRules {
898830

899831
FSTAssertThrows([coll queryWhereField:@"foo" arrayContainsAny:@[]],
900832
@"Invalid Query. A non-empty array is required for 'arrayContainsAny' filters.");
901-
902-
// The 10 element max includes duplicates.
903-
NSArray *values = @[ @1, @2, @3, @4, @5, @6, @7, @8, @9, @9, @9 ];
904-
FSTAssertThrows(
905-
[coll queryWhereField:@"foo" in:values],
906-
@"Invalid Query. 'in' filters support a maximum of 10 elements in the value array.");
907-
FSTAssertThrows([coll queryWhereField:@"foo" arrayContainsAny:values],
908-
@"Invalid Query. 'arrayContainsAny' filters support a maximum of 10 elements"
909-
" in the value array.");
910-
FSTAssertThrows(
911-
[coll queryWhereField:@"foo" notIn:values],
912-
@"Invalid Query. 'notIn' filters support a maximum of 10 elements in the value array.");
913833
}
914834

915835
#pragma mark - GeoPoint Validation

‎Firestore/core/src/api/query_core.cc‎

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -77,30 +77,24 @@ namespace {
7777
* Given an operator, returns the set of operators that cannot be used with
7878
* it.
7979
*
80-
* Operators in a query must adhere to the following set of rules:
81-
* 1. Only one array operator is allowed.
82-
* 2. Only one disjunctive operator is allowed.
83-
* 3. NOT_EQUAL cannot be used with another NOT_EQUAL operator.
84-
* 4. NOT_IN cannot be used with array, disjunctive, or NOT_EQUAL operators.
80+
* This is not a comprehensive check, and this function should be removed in the
81+
* long term.
82+
* Validations should occur in the Firestore backend.
8583
*
86-
* Array operators: ARRAY_CONTAINS, ARRAY_CONTAINS_ANY
87-
* Disjunctive operators: IN, ARRAY_CONTAINS_ANY, NOT_IN
84+
* Operators in a query must adhere to the following set of rules:
85+
* 1. Only one inequality per query.
86+
* 2. NOT_IN cannot be used with array, disjunctive, or NOT_EQUAL operators.
8887
*/
8988
static std::vector<Operator> ConflictingOps(Operator op) {
9089
switch (op) {
9190
case Operator::NotEqual:
9291
return {Operator::NotEqual, Operator::NotIn};
93-
case Operator::ArrayContains:
94-
return {Operator::ArrayContains, Operator::ArrayContainsAny,
95-
Operator::NotIn};
96-
case Operator::In:
97-
return {Operator::ArrayContainsAny, Operator::In, Operator::NotIn};
9892
case Operator::ArrayContainsAny:
99-
return {Operator::ArrayContains, Operator::ArrayContainsAny, Operator::In,
100-
Operator::NotIn};
93+
case Operator::In:
94+
return {Operator::NotIn};
10195
case Operator::NotIn:
102-
return {Operator::ArrayContains, Operator::ArrayContainsAny, Operator::In,
103-
Operator::NotIn, Operator::NotEqual};
96+
return {Operator::ArrayContainsAny, Operator::In, Operator::NotIn,
97+
Operator::NotEqual};
10498
default:
10599
return {};
106100
}
@@ -413,12 +407,6 @@ void Query::ValidateDisjunctiveFilterElements(
413407
" filters.",
414408
Describe(op));
415409
}
416-
if (value.array_value.values_count > 10) {
417-
ThrowInvalidArgument(
418-
"Invalid Query. '%s' filters support a maximum of 10"
419-
" elements in the value array.",
420-
Describe(op));
421-
}
422410
}
423411

424412
Message<google_firestore_v1_Value> Query::ParseExpectedReferenceValue(

0 commit comments

Comments
 (0)