Multiple inequality support (#5194)

Author: milaGGL

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

@@ -19,6 +19,8 @@
 import static com.google.firebase.firestore.Filter.equalTo;
 import static com.google.firebase.firestore.Filter.greaterThan;
 import static com.google.firebase.firestore.Filter.inArray;
+import static com.google.firebase.firestore.Filter.lessThan;
+import static com.google.firebase.firestore.Filter.notEqualTo;
 import static com.google.firebase.firestore.Filter.notInArray;
 import static com.google.firebase.firestore.Filter.or;
 import static com.google.firebase.firestore.testutil.Assert.assertThrows;
@@ -532,41 +534,11 @@ public void queryOrderByKeyBoundsMustBeStringsWithoutSlashes() {
             + "document path, but 'foo' is not because it contains an odd number of segments.");
   }
 
-  @Test
-  public void queriesWithDifferentInequalityFieldsFail() {
-    expectError(
-        () -> testCollection().whereGreaterThan("x", 32).whereLessThan("y", "cat"),
-        "All where filters with an inequality (notEqualTo, notIn, lessThan, "
-            + "lessThanOrEqualTo, greaterThan, or greaterThanOrEqualTo) must be on the "
-            + "same field. But you have filters on 'x' and 'y'");
-  }
-
-  @Test
-  public void queriesWithInequalityDifferentThanFirstOrderByFail() {
-    CollectionReference collection = testCollection();
-    String reason =
-        "Invalid query. You have an inequality where filter (whereLessThan(), "
-            + "whereGreaterThan(), etc.) on field 'x' and so you must also have 'x' as "
-            + "your first orderBy() field, but your first orderBy() is currently on field 'y' "
-            + "instead.";
-    expectError(() -> collection.whereGreaterThan("x", 32).orderBy("y"), reason);
-    expectError(() -> collection.orderBy("y").whereGreaterThan("x", 32), reason);
-    expectError(() -> collection.whereGreaterThan("x", 32).orderBy("y").orderBy("x"), reason);
-    expectError(() -> collection.orderBy("y").orderBy("x").whereGreaterThan("x", 32), reason);
-    expectError(() -> collection.orderBy("y").orderBy("x").whereNotEqualTo("x", 32), reason);
-  }
-
   @Test
   public void queriesWithMultipleNotEqualAndInequalitiesFail() {
     expectError(
         () -> testCollection().whereNotEqualTo("x", 32).whereNotEqualTo("x", 33),
         "Invalid Query. You cannot use more than one '!=' filter.");
-
-    expectError(
-        () -> testCollection().whereNotEqualTo("x", 32).whereGreaterThan("y", 33),
-        "All where filters with an inequality (notEqualTo, notIn, lessThan, "
-            + "lessThanOrEqualTo, greaterThan, or greaterThanOrEqualTo) must be on the "
-            + "same field. But you have filters on 'x' and 'y'");
   }
 
   @Test
@@ -584,9 +556,7 @@ public void queriesWithNotEqualAndNotInFiltersFail() {
   public void queriesWithMultipleDisjunctiveFiltersFail() {
     expectError(
         () -> testCollection().whereNotIn("foo", asList(1, 2)).whereNotIn("bar", asList(1, 2)),
-        "All where filters with an inequality (notEqualTo, notIn, lessThan, "
-            + "lessThanOrEqualTo, greaterThan, or greaterThanOrEqualTo) must be on the "
-            + "same field. But you have filters on 'foo' and 'bar'");
+        "Invalid Query. You cannot use more than one 'not_in' filter.");
 
     expectError(
         () ->
@@ -707,37 +677,71 @@ public void queriesUsingInAndDocumentIdMustHaveProperDocumentReferencesInArray()
   }
 
   @Test
-  public void testInvalidQueryFilters() {
+  public void testConflictingOperatorsInCompositeFilters() {
     CollectionReference collection = testCollection();
+    // Composite queries can validate conflicting operators in multiple inequality.
+    String reason = "Invalid Query. You cannot use more than one '!=' filter.";
+    expectError(
+        () ->
+            collection
+                .where(
+                    or(
+                        and(lessThan("a", "b"), inArray("c", Arrays.asList("d", "e"))),
+                        and(equalTo("e", "f"), notEqualTo("g", "h"))))
+                .where(
+                    or(
+                        and(greaterThan("i", "j"), greaterThan("k", "l")),
+                        and(notEqualTo("o", "p"), lessThan("q", "r")))),
+        reason);
 
-    // Multiple inequalities, one of which is inside a nested composite filter.
-    String reason =
-        "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'";
+    reason = "Invalid Query. You cannot use more than one 'not_in' filter.";
     expectError(
         () ->
             collection
                 .where(
                     or(
-                        and(equalTo("a", "b"), greaterThan("c", "d")),
-                        and(equalTo("e", "f"), equalTo("g", "h"))))
-                .where(greaterThan("r", "s")),
+                        and(lessThan("a", "b"), notInArray("c", Arrays.asList("d", "e"))),
+                        and(equalTo("e", "f"), lessThan("g", "j"))))
+                .where(
+                    or(
+                        and(greaterThan("i", "j"), greaterThan("k", "l")),
+                        and(notInArray("o", Arrays.asList("p", "q")), lessThan("q", "r")))),
         reason);
 
-    // OrderBy and inequality on different fields. Inequality inside a nested composite filter.
-    reason =
-        "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.";
+    reason = "Invalid Query. You cannot use 'not_in' filters with '!=' filters.";
     expectError(
         () ->
             collection
                 .where(
                     or(
-                        and(equalTo("a", "b"), greaterThan("c", "d")),
-                        and(equalTo("e", "f"), equalTo("g", "h"))))
-                .orderBy("r"),
+                        and(lessThan("a", "b"), greaterThan("c", "d")),
+                        and(equalTo("e", "f"), notEqualTo("g", "h"))))
+                .where(
+                    or(
+                        and(greaterThan("i", "j"), greaterThan("k", "l")),
+                        and(notInArray("o", Arrays.asList("p", "q")), lessThan("q", "r")))),
         reason);
 
-    // Conflicting operations within a composite filter.
     reason = "Invalid Query. You cannot use 'not_in' filters with 'in' filters.";
+    expectError(
+        () ->
+            collection
+                .where(
+                    or(
+                        and(lessThan("a", "b"), inArray("c", Arrays.asList("d", "e"))),
+                        and(equalTo("e", "f"), lessThan("g", "j"))))
+                .where(
+                    or(
+                        and(greaterThan("i", "j"), greaterThan("k", "l")),
+                        and(notInArray("o", Arrays.asList("p", "q")), lessThan("q", "r")))),
+        reason);
+  }
+
+  @Test
+  public void testInvalidQueryFilters() {
+    CollectionReference collection = testCollection();
+    // Conflicting operations within a composite filter.
+    String reason = "Invalid Query. You cannot use 'not_in' filters with 'in' filters.";
     expectError(
         () ->
             collection.where(

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

@@ -487,14 +487,6 @@ private com.google.firebase.firestore.core.Filter parseFilter(Filter filter) {
     return parseCompositeFilter((Filter.CompositeFilter) filter);
   }
 
-  private void validateOrderByField(com.google.firebase.firestore.model.FieldPath field) {
-    com.google.firebase.firestore.model.FieldPath inequalityField = query.inequalityField();
-    if (query.getFirstOrderByField() == null && inequalityField != null) {
-
-      validateOrderByFieldMatchesInequality(field, inequalityField);
-    }
-  }
-
   /**
    * Parses the given documentIdValue into a ReferenceValue, throwing appropriate errors if the
    * 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) {
     }
   }
 
-  private void validateOrderByFieldMatchesInequality(
-      com.google.firebase.firestore.model.FieldPath orderBy,
-      com.google.firebase.firestore.model.FieldPath inequality) {
-    if (!orderBy.equals(inequality)) {
-      String inequalityString = inequality.canonicalString();
-      throw new IllegalArgumentException(
-          String.format(
-              "Invalid query. You have an inequality where filter (whereLessThan(), "
-                  + "whereGreaterThan(), etc.) on field '%s' and so you must also have '%s' as "
-                  + "your first orderBy() field, but your first orderBy() is currently on field "
-                  + "'%s' instead.",
-              inequalityString, inequalityString, orderBy.canonicalString()));
-    }
-  }
-
   /**
    * Given an operator, returns the set of operators that cannot be used with it.
    *
@@ -592,24 +569,7 @@ private void validateNewFieldFilter(
       com.google.firebase.firestore.core.Query query,
       com.google.firebase.firestore.core.FieldFilter fieldFilter) {
     Operator filterOp = fieldFilter.getOperator();
-    if (fieldFilter.isInequality()) {
-      com.google.firebase.firestore.model.FieldPath existingInequality = query.inequalityField();
-      com.google.firebase.firestore.model.FieldPath newInequality = fieldFilter.getField();
 
-      if (existingInequality != null && !existingInequality.equals(newInequality)) {
-        throw new IllegalArgumentException(
-            String.format(
-                "All where filters with an inequality (notEqualTo, notIn, lessThan, "
-                    + "lessThanOrEqualTo, greaterThan, or greaterThanOrEqualTo) must be on the "
-                    + "same field. But you have filters on '%s' and '%s'",
-                existingInequality.canonicalString(), newInequality.canonicalString()));
-      }
-      com.google.firebase.firestore.model.FieldPath firstOrderByField =
-          query.getFirstOrderByField();
-      if (firstOrderByField != null) {
-        validateOrderByFieldMatchesInequality(firstOrderByField, newInequality);
-      }
-    }
     Operator conflictingOp = findOpInsideFilters(query.getFilters(), conflictingOps(filterOp));
     if (conflictingOp != null) {
       // We special case when it's a duplicate op to give a slightly clearer error message.
@@ -717,7 +677,6 @@ private Query orderBy(
           "Invalid query. You must not call Query.endAt() or Query.endBefore() before "
               + "calling Query.orderBy().");
     }
-    validateOrderByField(fieldPath);
     OrderBy.Direction dir =
         direction == Direction.ASCENDING
             ? OrderBy.Direction.ASCENDING

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

@@ -17,7 +17,6 @@
 import android.text.TextUtils;
 import androidx.annotation.Nullable;
 import com.google.firebase.firestore.model.Document;
-import com.google.firebase.firestore.model.FieldPath;
 import com.google.firebase.firestore.util.Function;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -74,19 +73,6 @@ public List<FieldFilter> getFlattenedFilters() {
     return Collections.unmodifiableList(memoizedFlattenedFilters);
   }
 
-  /**
-   * Returns the first inequality filter contained within this composite filter. Returns {@code
-   * null} if it does not contain any inequalities.
-   */
-  @Override
-  public FieldPath getFirstInequalityField() {
-    FieldFilter found = findFirstMatchingFilter(f -> f.isInequality());
-    if (found != null) {
-      return found.getField();
-    }
-    return null;
-  }
-
   public boolean isConjunction() {
     return operator == Operator.AND;
   }

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

@@ -172,14 +172,6 @@ public List<Filter> getFilters() {
     return Collections.singletonList(this);
   }
 
-  @Override
-  public FieldPath getFirstInequalityField() {
-    if (isInequality()) {
-      return getField();
-    }
-    return null;
-  }
-
   @Override
   public String toString() {
     return getCanonicalId();

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

@@ -14,9 +14,7 @@
 
 package com.google.firebase.firestore.core;
 
-import androidx.annotation.Nullable;
 import com.google.firebase.firestore.model.Document;
-import com.google.firebase.firestore.model.FieldPath;
 import java.util.List;
 
 public abstract class Filter {
@@ -31,8 +29,4 @@ public abstract class Filter {
 
   /** Returns a list of all filters that are contained within this filter */
   public abstract List<Filter> getFilters();
-
-  /** Returns the field of the first filter that's an inequality, or null if none. */
-  @Nullable
-  public abstract FieldPath getFirstInequalityField();
 }