diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index ca5fcc6ee74..0a387ba8907 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,5 +1,10 @@ # Unreleased -- [Fixed] Remove explicit MobileCoreServices library linkage from podspec. (#6850) +- [fixed] Remove explicit MobileCoreServices library linkage from podspec + (#6850). +- [fixed] Removed excess validation of null and NaN values in query filters. + This more closely aligns the SDK with the Firestore backend, which has always + accepted null and NaN for all operators, even though this isn't necessarily + useful. # v7.0.0 - [changed] **Breaking change:** Removed the `areTimestampsInSnapshotsEnabled` diff --git a/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm b/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm index 53041d9067a..4ca23933819 100644 --- a/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm @@ -452,7 +452,9 @@ - (void)testQueriesCanUseArrayContainsFilters { @"a" : @{@"array" : @[ @42 ]}, @"b" : @{@"array" : @[ @"a", @42, @"c" ]}, @"c" : @{@"array" : @[ @41.999, @"42", @{@"a" : @[ @42 ]} ]}, - @"d" : @{@"array" : @[ @42 ], @"array2" : @[ @"bingo" ]} + @"d" : @{@"array" : @[ @42 ], @"array2" : @[ @"bingo" ]}, + @"e" : @{@"array" : @[ [NSNull null] ]}, + @"f" : @{@"array" : @[ @(NAN) ]}, }; FIRCollectionReference *collection = [self collectionRefWithDocuments:testDocs]; @@ -462,8 +464,15 @@ - (void)testQueriesCanUseArrayContainsFilters { XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), (@[ testDocs[@"a"], testDocs[@"b"], testDocs[@"d"] ])); - // NOTE: The backend doesn't currently support null, NaN, objects, or arrays, so there isn't much - // of anything else interesting to test. + // With null. + snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"array" + arrayContains:[NSNull null]]]; + XCTAssertTrue(snapshot.isEmpty); + + // With NAN. + snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"array" + arrayContains:@(NAN)]]; + XCTAssertTrue(snapshot.isEmpty); } - (void)testQueriesCanUseInFilters { @@ -474,7 +483,9 @@ - (void)testQueriesCanUseInFilters { @"d" : @{@"zip" : @[ @98101 ]}, @"e" : @{@"zip" : @[ @"98101", @{@"zip" : @98101} ]}, @"f" : @{@"zip" : @{@"code" : @500}}, - @"g" : @{@"zip" : @[ @98101, @98102 ]} + @"g" : @{@"zip" : @[ @98101, @98102 ]}, + @"h" : @{@"zip" : [NSNull null]}, + @"i" : @{@"zip" : @(NAN)} }; FIRCollectionReference *collection = [self collectionRefWithDocuments:testDocs]; @@ -489,6 +500,24 @@ - (void)testQueriesCanUseInFilters { snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"zip" in:@[ @{@"code" : @500} ]]]; XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), (@[ testDocs[@"f"] ])); + + // With null. + snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"zip" in:@[ [NSNull null] ]]]; + XCTAssertTrue(snapshot.isEmpty); + + // With null and a value. + snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"zip" + in:@[ [NSNull null], @98101 ]]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), (@[ testDocs[@"a"] ])); + + // With NAN. + snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"zip" in:@[ @(NAN) ]]]; + XCTAssertTrue(snapshot.isEmpty); + + // With NAN and a value. + snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"zip" + in:@[ @(NAN), @98101 ]]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), (@[ testDocs[@"a"] ])); } - (void)testQueriesCanUseInFiltersWithDocIds { @@ -584,6 +613,9 @@ - (void)testQueriesCanUseArrayContainsAnyFilters { @"e" : @{@"array" : @[ @43 ]}, @"f" : @{@"array" : @[ @{@"a" : @42} ]}, @"g" : @{@"array" : @42}, + @"h" : @{@"array" : @[ [NSNull null] ]}, + @"g" : @{@"array" : @[ @(NAN) ]}, + }; FIRCollectionReference *collection = [self collectionRefWithDocuments:testDocs]; @@ -599,6 +631,26 @@ - (void)testQueriesCanUseArrayContainsAnyFilters { XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), (@[ testDocs[@"f"], ])); + + // With null. + snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"array" + arrayContainsAny:@[ [NSNull null] ]]]; + XCTAssertTrue(snapshot.isEmpty); + + // With null and a value. + snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"array" + arrayContainsAny:@[ [NSNull null], @43 ]]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), (@[ testDocs[@"e"] ])); + + // With NAN. + snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"array" + arrayContainsAny:@[ @(NAN) ]]]; + XCTAssertTrue(snapshot.isEmpty); + + // With NAN and a value. + snapshot = [self readDocumentSetForRef:[collection queryWhereField:@"array" + arrayContainsAny:@[ @(NAN), @43 ]]]; + XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), (@[ testDocs[@"e"] ])); } - (void)testCollectionGroupQueries { diff --git a/Firestore/Example/Tests/Integration/API/FIRValidationTests.mm b/Firestore/Example/Tests/Integration/API/FIRValidationTests.mm index ee6ed3db4d3..cdfe9040b8f 100644 --- a/Firestore/Example/Tests/Integration/API/FIRValidationTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRValidationTests.mm @@ -415,20 +415,6 @@ - (void)testQueryWithNonPositiveLimitFails { @"Invalid Query. Query limit (-1) is invalid. Limit must be positive."); } -- (void)testNonEqualityQueriesOnNullOrNaNFail { - NSString *expected = @"Invalid Query. Null supports only 'equalTo' and 'notEqualTo' comparisons."; - FSTAssertThrows([[self collectionRef] queryWhereField:@"a" isGreaterThan:nil], expected); - FSTAssertThrows([[self collectionRef] queryWhereField:@"a" isGreaterThan:[NSNull null]], - expected); - FSTAssertThrows([[self collectionRef] queryWhereField:@"a" arrayContains:nil], expected); - FSTAssertThrows([[self collectionRef] queryWhereField:@"a" arrayContains:[NSNull null]], - expected); - - expected = @"Invalid Query. NaN supports only 'equalTo' and 'notEqualTo' comparisons."; - FSTAssertThrows([[self collectionRef] queryWhereField:@"a" isGreaterThan:@(NAN)], expected); - FSTAssertThrows([[self collectionRef] queryWhereField:@"a" arrayContains:@(NAN)], expected); -} - - (void)testQueryCannotBeCreatedFromDocumentsMissingSortValues { FIRCollectionReference *testCollection = [self collectionRefWithDocuments:@{@"f" : @{@"v" : @"f", @"nosort" : @1.0}}]; @@ -787,20 +773,6 @@ - (void)testQueriesInAndArrayContainsAnyArrayRules { FSTAssertThrows( [coll queryWhereField:@"foo" notIn:values], @"Invalid Query. 'notIn' filters support a maximum of 10 elements in the value array."); - - NSArray *withNullValues = @[ @1, [NSNull null] ]; - FSTAssertThrows([coll queryWhereField:@"foo" in:withNullValues], - @"Invalid Query. 'in' filters cannot contain 'null' in the value array."); - FSTAssertThrows( - [coll queryWhereField:@"foo" arrayContainsAny:withNullValues], - @"Invalid Query. 'arrayContainsAny' filters cannot contain 'null' in the value array."); - - NSArray *withNaNValues = @[ @2, @(NAN) ]; - FSTAssertThrows([coll queryWhereField:@"foo" in:withNaNValues], - @"Invalid Query. 'in' filters cannot contain 'NaN' in the value array."); - FSTAssertThrows( - [coll queryWhereField:@"foo" arrayContainsAny:withNaNValues], - @"Invalid Query. 'arrayContainsAny' filters cannot contain 'NaN' in the value array."); } #pragma mark - GeoPoint Validation diff --git a/Firestore/core/src/api/query_core.cc b/Firestore/core/src/api/query_core.cc index 3d6f2df5e47..45440ef44f3 100644 --- a/Firestore/core/src/api/query_core.cc +++ b/Firestore/core/src/api/query_core.cc @@ -394,24 +394,6 @@ void Query::ValidateDisjunctiveFilterElements( " elements in the value array.", Describe(op)); } - - std::vector array = field_value.array_value(); - for (const auto& val : array) { - if (op == Operator::In || op == Operator::ArrayContainsAny) { - if (val.is_null()) { - ThrowInvalidArgument( - "Invalid Query. '%s' filters cannot contain 'null' in" - " the value array.", - Describe(op)); - } - if (val.is_nan()) { - ThrowInvalidArgument( - "Invalid Query. '%s' filters cannot contain 'NaN' in" - " the value array.", - Describe(op)); - } - } - } } FieldValue Query::ParseExpectedReferenceValue( diff --git a/Firestore/core/src/core/field_filter.cc b/Firestore/core/src/core/field_filter.cc index 89b5a499d8e..f0b066f5c77 100644 --- a/Firestore/core/src/core/field_filter.cc +++ b/Firestore/core/src/core/field_filter.cc @@ -94,25 +94,6 @@ FieldFilter FieldFilter::Create(FieldPath path, CanonicalName(op)); return KeyFieldFilter(std::move(path), op, std::move(value_rhs)); } - - } else if (value_rhs.type() == FieldValue::Type::Null) { - if (op != Filter::Operator::Equal && op != Filter::Operator::NotEqual) { - ThrowInvalidArgument( - "Invalid Query. Null supports only 'equalTo' and 'notEqualTo' " - "comparisons."); - } - Rep filter(std::move(path), op, std::move(value_rhs)); - return FieldFilter(std::make_shared(std::move(filter))); - - } else if (value_rhs.is_nan()) { - if (op != Filter::Operator::Equal && op != Filter::Operator::NotEqual) { - ThrowInvalidArgument( - "Invalid Query. NaN supports only 'equalTo' and 'notEqualTo' " - "comparisons."); - } - Rep filter(std::move(path), op, std::move(value_rhs)); - return FieldFilter(std::make_shared(std::move(filter))); - } else if (op == Operator::ArrayContains) { return ArrayContainsFilter(std::move(path), std::move(value_rhs));