Skip to content

Commit c5a894c

Browse files
milaGGLmilaGGL
authored
Multiple inequality support (#5194)
1 parent dfa22c7 commit c5a894c

13 files changed

Lines changed: 907 additions & 274 deletions

File tree

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java‎

Lines changed: 594 additions & 76 deletions
Large diffs are not rendered by default.

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java‎

Lines changed: 51 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import static com.google.firebase.firestore.Filter.equalTo;
2020
import static com.google.firebase.firestore.Filter.greaterThan;
2121
import static com.google.firebase.firestore.Filter.inArray;
22+
import static com.google.firebase.firestore.Filter.lessThan;
23+
import static com.google.firebase.firestore.Filter.notEqualTo;
2224
import static com.google.firebase.firestore.Filter.notInArray;
2325
import static com.google.firebase.firestore.Filter.or;
2426
import static com.google.firebase.firestore.testutil.Assert.assertThrows;
@@ -532,41 +534,11 @@ public void queryOrderByKeyBoundsMustBeStringsWithoutSlashes() {
532534
+ "document path, but 'foo' is not because it contains an odd number of segments.");
533535
}
534536

535-
@Test
536-
public void queriesWithDifferentInequalityFieldsFail() {
537-
expectError(
538-
() -> testCollection().whereGreaterThan("x", 32).whereLessThan("y", "cat"),
539-
"All where filters with an inequality (notEqualTo, notIn, lessThan, "
540-
+ "lessThanOrEqualTo, greaterThan, or greaterThanOrEqualTo) must be on the "
541-
+ "same field. But you have filters on 'x' and 'y'");
542-
}
543-
544-
@Test
545-
public void queriesWithInequalityDifferentThanFirstOrderByFail() {
546-
CollectionReference collection = testCollection();
547-
String reason =
548-
"Invalid query. You have an inequality where filter (whereLessThan(), "
549-
+ "whereGreaterThan(), etc.) on field 'x' and so you must also have 'x' as "
550-
+ "your first orderBy() field, but your first orderBy() is currently on field 'y' "
551-
+ "instead.";
552-
expectError(() -> collection.whereGreaterThan("x", 32).orderBy("y"), reason);
553-
expectError(() -> collection.orderBy("y").whereGreaterThan("x", 32), reason);
554-
expectError(() -> collection.whereGreaterThan("x", 32).orderBy("y").orderBy("x"), reason);
555-
expectError(() -> collection.orderBy("y").orderBy("x").whereGreaterThan("x", 32), reason);
556-
expectError(() -> collection.orderBy("y").orderBy("x").whereNotEqualTo("x", 32), reason);
557-
}
558-
559537
@Test
560538
public void queriesWithMultipleNotEqualAndInequalitiesFail() {
561539
expectError(
562540
() -> testCollection().whereNotEqualTo("x", 32).whereNotEqualTo("x", 33),
563541
"Invalid Query. You cannot use more than one '!=' filter.");
564-
565-
expectError(
566-
() -> testCollection().whereNotEqualTo("x", 32).whereGreaterThan("y", 33),
567-
"All where filters with an inequality (notEqualTo, notIn, lessThan, "
568-
+ "lessThanOrEqualTo, greaterThan, or greaterThanOrEqualTo) must be on the "
569-
+ "same field. But you have filters on 'x' and 'y'");
570542
}
571543

572544
@Test
@@ -584,9 +556,7 @@ public void queriesWithNotEqualAndNotInFiltersFail() {
584556
public void queriesWithMultipleDisjunctiveFiltersFail() {
585557
expectError(
586558
() -> testCollection().whereNotIn("foo", asList(1, 2)).whereNotIn("bar", asList(1, 2)),
587-
"All where filters with an inequality (notEqualTo, notIn, lessThan, "
588-
+ "lessThanOrEqualTo, greaterThan, or greaterThanOrEqualTo) must be on the "
589-
+ "same field. But you have filters on 'foo' and 'bar'");
559+
"Invalid Query. You cannot use more than one 'not_in' filter.");
590560

591561
expectError(
592562
() ->
@@ -707,37 +677,71 @@ public void queriesUsingInAndDocumentIdMustHaveProperDocumentReferencesInArray()
707677
}
708678

709679
@Test
710-
public void testInvalidQueryFilters() {
680+
public void testConflictingOperatorsInCompositeFilters() {
711681
CollectionReference collection = testCollection();
682+
// Composite queries can validate conflicting operators in multiple inequality.
683+
String reason = "Invalid Query. You cannot use more than one '!=' filter.";
684+
expectError(
685+
() ->
686+
collection
687+
.where(
688+
or(
689+
and(lessThan("a", "b"), inArray("c", Arrays.asList("d", "e"))),
690+
and(equalTo("e", "f"), notEqualTo("g", "h"))))
691+
.where(
692+
or(
693+
and(greaterThan("i", "j"), greaterThan("k", "l")),
694+
and(notEqualTo("o", "p"), lessThan("q", "r")))),
695+
reason);
712696

713-
// Multiple inequalities, one of which is inside a nested composite filter.
714-
String reason =
715-
"All where filters with an inequality (notEqualTo, notIn, lessThan, lessThanOrEqualTo, greaterThan, or greaterThanOrEqualTo) must be on the same field. But you have filters on 'c' and 'r'";
697+
reason = "Invalid Query. You cannot use more than one 'not_in' filter.";
716698
expectError(
717699
() ->
718700
collection
719701
.where(
720702
or(
721-
and(equalTo("a", "b"), greaterThan("c", "d")),
722-
and(equalTo("e", "f"), equalTo("g", "h"))))
723-
.where(greaterThan("r", "s")),
703+
and(lessThan("a", "b"), notInArray("c", Arrays.asList("d", "e"))),
704+
and(equalTo("e", "f"), lessThan("g", "j"))))
705+
.where(
706+
or(
707+
and(greaterThan("i", "j"), greaterThan("k", "l")),
708+
and(notInArray("o", Arrays.asList("p", "q")), lessThan("q", "r")))),
724709
reason);
725710

726-
// OrderBy and inequality on different fields. Inequality inside a nested composite filter.
727-
reason =
728-
"Invalid query. You have an inequality where filter (whereLessThan(), whereGreaterThan(), etc.) on field 'c' and so you must also have 'c' as your first orderBy() field, but your first orderBy() is currently on field 'r' instead.";
711+
reason = "Invalid Query. You cannot use 'not_in' filters with '!=' filters.";
729712
expectError(
730713
() ->
731714
collection
732715
.where(
733716
or(
734-
and(equalTo("a", "b"), greaterThan("c", "d")),
735-
and(equalTo("e", "f"), equalTo("g", "h"))))
736-
.orderBy("r"),
717+
and(lessThan("a", "b"), greaterThan("c", "d")),
718+
and(equalTo("e", "f"), notEqualTo("g", "h"))))
719+
.where(
720+
or(
721+
and(greaterThan("i", "j"), greaterThan("k", "l")),
722+
and(notInArray("o", Arrays.asList("p", "q")), lessThan("q", "r")))),
737723
reason);
738724

739-
// Conflicting operations within a composite filter.
740725
reason = "Invalid Query. You cannot use 'not_in' filters with 'in' filters.";
726+
expectError(
727+
() ->
728+
collection
729+
.where(
730+
or(
731+
and(lessThan("a", "b"), inArray("c", Arrays.asList("d", "e"))),
732+
and(equalTo("e", "f"), lessThan("g", "j"))))
733+
.where(
734+
or(
735+
and(greaterThan("i", "j"), greaterThan("k", "l")),
736+
and(notInArray("o", Arrays.asList("p", "q")), lessThan("q", "r")))),
737+
reason);
738+
}
739+
740+
@Test
741+
public void testInvalidQueryFilters() {
742+
CollectionReference collection = testCollection();
743+
// Conflicting operations within a composite filter.
744+
String reason = "Invalid Query. You cannot use 'not_in' filters with 'in' filters.";
741745
expectError(
742746
() ->
743747
collection.where(

‎firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -487,14 +487,6 @@ private com.google.firebase.firestore.core.Filter parseFilter(Filter filter) {
487487
return parseCompositeFilter((Filter.CompositeFilter) filter);
488488
}
489489

490-
private void validateOrderByField(com.google.firebase.firestore.model.FieldPath field) {
491-
com.google.firebase.firestore.model.FieldPath inequalityField = query.inequalityField();
492-
if (query.getFirstOrderByField() == null && inequalityField != null) {
493-
494-
validateOrderByFieldMatchesInequality(field, inequalityField);
495-
}
496-
}
497-
498490
/**
499491
* Parses the given documentIdValue into a ReferenceValue, throwing appropriate errors if the
500492
* value is anything other than a DocumentReference or String, or if the string is malformed.
@@ -544,21 +536,6 @@ private void validateDisjunctiveFilterElements(Object value, Operator op) {
544536
}
545537
}
546538

547-
private void validateOrderByFieldMatchesInequality(
548-
com.google.firebase.firestore.model.FieldPath orderBy,
549-
com.google.firebase.firestore.model.FieldPath inequality) {
550-
if (!orderBy.equals(inequality)) {
551-
String inequalityString = inequality.canonicalString();
552-
throw new IllegalArgumentException(
553-
String.format(
554-
"Invalid query. You have an inequality where filter (whereLessThan(), "
555-
+ "whereGreaterThan(), etc.) on field '%s' and so you must also have '%s' as "
556-
+ "your first orderBy() field, but your first orderBy() is currently on field "
557-
+ "'%s' instead.",
558-
inequalityString, inequalityString, orderBy.canonicalString()));
559-
}
560-
}
561-
562539
/**
563540
* Given an operator, returns the set of operators that cannot be used with it.
564541
*
@@ -592,24 +569,7 @@ private void validateNewFieldFilter(
592569
com.google.firebase.firestore.core.Query query,
593570
com.google.firebase.firestore.core.FieldFilter fieldFilter) {
594571
Operator filterOp = fieldFilter.getOperator();
595-
if (fieldFilter.isInequality()) {
596-
com.google.firebase.firestore.model.FieldPath existingInequality = query.inequalityField();
597-
com.google.firebase.firestore.model.FieldPath newInequality = fieldFilter.getField();
598572

599-
if (existingInequality != null && !existingInequality.equals(newInequality)) {
600-
throw new IllegalArgumentException(
601-
String.format(
602-
"All where filters with an inequality (notEqualTo, notIn, lessThan, "
603-
+ "lessThanOrEqualTo, greaterThan, or greaterThanOrEqualTo) must be on the "
604-
+ "same field. But you have filters on '%s' and '%s'",
605-
existingInequality.canonicalString(), newInequality.canonicalString()));
606-
}
607-
com.google.firebase.firestore.model.FieldPath firstOrderByField =
608-
query.getFirstOrderByField();
609-
if (firstOrderByField != null) {
610-
validateOrderByFieldMatchesInequality(firstOrderByField, newInequality);
611-
}
612-
}
613573
Operator conflictingOp = findOpInsideFilters(query.getFilters(), conflictingOps(filterOp));
614574
if (conflictingOp != null) {
615575
// We special case when it's a duplicate op to give a slightly clearer error message.
@@ -717,7 +677,6 @@ private Query orderBy(
717677
"Invalid query. You must not call Query.endAt() or Query.endBefore() before "
718678
+ "calling Query.orderBy().");
719679
}
720-
validateOrderByField(fieldPath);
721680
OrderBy.Direction dir =
722681
direction == Direction.ASCENDING
723682
? OrderBy.Direction.ASCENDING

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/CompositeFilter.java‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import android.text.TextUtils;
1818
import androidx.annotation.Nullable;
1919
import com.google.firebase.firestore.model.Document;
20-
import com.google.firebase.firestore.model.FieldPath;
2120
import com.google.firebase.firestore.util.Function;
2221
import java.util.ArrayList;
2322
import java.util.Collections;
@@ -74,19 +73,6 @@ public List<FieldFilter> getFlattenedFilters() {
7473
return Collections.unmodifiableList(memoizedFlattenedFilters);
7574
}
7675

77-
/**
78-
* Returns the first inequality filter contained within this composite filter. Returns {@code
79-
* null} if it does not contain any inequalities.
80-
*/
81-
@Override
82-
public FieldPath getFirstInequalityField() {
83-
FieldFilter found = findFirstMatchingFilter(f -> f.isInequality());
84-
if (found != null) {
85-
return found.getField();
86-
}
87-
return null;
88-
}
89-
9076
public boolean isConjunction() {
9177
return operator == Operator.AND;
9278
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/FieldFilter.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -172,14 +172,6 @@ public List<Filter> getFilters() {
172172
return Collections.singletonList(this);
173173
}
174174

175-
@Override
176-
public FieldPath getFirstInequalityField() {
177-
if (isInequality()) {
178-
return getField();
179-
}
180-
return null;
181-
}
182-
183175
@Override
184176
public String toString() {
185177
return getCanonicalId();

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/Filter.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@
1414

1515
package com.google.firebase.firestore.core;
1616

17-
import androidx.annotation.Nullable;
1817
import com.google.firebase.firestore.model.Document;
19-
import com.google.firebase.firestore.model.FieldPath;
2018
import java.util.List;
2119

2220
public abstract class Filter {
@@ -31,8 +29,4 @@ public abstract class Filter {
3129

3230
/** Returns a list of all filters that are contained within this filter */
3331
public abstract List<Filter> getFilters();
34-
35-
/** Returns the field of the first filter that's an inequality, or null if none. */
36-
@Nullable
37-
public abstract FieldPath getFirstInequalityField();
3832
}

0 commit comments

Comments
 (0)