Remove some query restrictions and add integration tests (#4231)

ehsannas · web-flow · commit 5ea5bd095b09 · 2023-02-15T15:30:42.000-08:00
* Remove multipl IN restriction and add an integration test.

* Update the ValidationTest to reflect the relaxed requirements.

* Remove obsolete test.

* new or query validation.

* Remove the word 'yet'.

* Fix wording.

* *Revert* the last commit (some new validation checks).

* Relax query restrictions.

* Skip Or query tests until supported in production.
diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java
@@ -1178,4 +1178,171 @@ public void testOrQueriesWithArrayMembership() {
         "doc4",
         "doc6");
   }
+
+  @Ignore
+  @Test
+  public void testMultipleInOps() {
+    Map<String, Map<String, Object>> testDocs =
+        map(
+            "doc1", map("a", 1, "b", 0),
+            "doc2", map("b", 1),
+            "doc3", map("a", 3, "b", 2),
+            "doc4", map("a", 1, "b", 3),
+            "doc5", map("a", 1),
+            "doc6", map("a", 2));
+    CollectionReference collection = testCollectionWithDocs(testDocs);
+
+    // Two IN operations on different fields with disjunction.
+    Query query1 =
+        collection
+            .where(Filter.or(Filter.inArray("a", asList(2, 3)), Filter.inArray("b", asList(0, 2))))
+            .orderBy("a");
+    checkOnlineAndOfflineResultsMatch(query1, "doc1", "doc6", "doc3");
+
+    // Two IN operations on different fields with conjunction.
+    Query query2 =
+        collection
+            .where(Filter.and(Filter.inArray("a", asList(2, 3)), Filter.inArray("b", asList(0, 2))))
+            .orderBy("a");
+    checkOnlineAndOfflineResultsMatch(query2, "doc3");
+
+    // Two IN operations on the same field.
+    // a IN [1,2,3] && a IN [0,1,4] should result in "a==1".
+    Query query3 =
+        collection.where(
+            Filter.and(Filter.inArray("a", asList(1, 2, 3)), Filter.inArray("a", asList(0, 1, 4))));
+    checkOnlineAndOfflineResultsMatch(query3, "doc1", "doc4", "doc5");
+
+    // a IN [2,3] && a IN [0,1,4] is never true and so the result should be an empty set.
+    Query query4 =
+        collection.where(
+            Filter.and(Filter.inArray("a", asList(2, 3)), Filter.inArray("a", asList(0, 1, 4))));
+    checkOnlineAndOfflineResultsMatch(query4);
+
+    // a IN [0,3] || a IN [0,2] should union them (similar to: a IN [0,2,3]).
+    Query query5 =
+        collection.where(
+            Filter.or(Filter.inArray("a", asList(0, 3)), Filter.inArray("a", asList(0, 2))));
+    checkOnlineAndOfflineResultsMatch(query5, "doc3", "doc6");
+
+    // Nested composite filter on the same field.
+    Query query6 =
+        collection.where(
+            Filter.and(
+                Filter.inArray("a", asList(1, 3)),
+                Filter.or(
+                    Filter.inArray("a", asList(0, 2)),
+                    Filter.and(
+                        Filter.greaterThanOrEqualTo("b", 1), Filter.inArray("a", asList(1, 3))))));
+    checkOnlineAndOfflineResultsMatch(query6, "doc3", "doc4");
+
+    // Nested composite filter on different fields.
+    Query query7 =
+        collection.where(
+            Filter.and(
+                Filter.inArray("b", asList(0, 3)),
+                Filter.or(
+                    Filter.inArray("b", asList(1)),
+                    Filter.and(
+                        Filter.inArray("b", asList(2, 3)), Filter.inArray("a", asList(1, 3))))));
+    checkOnlineAndOfflineResultsMatch(query7, "doc4");
+  }
+
+  @Ignore
+  @Test
+  public void testUsingInWithArrayContainsAny() {
+    Map<String, Map<String, Object>> testDocs =
+        map(
+            "doc1", map("a", 1, "b", asList(0)),
+            "doc2", map("b", asList(1)),
+            "doc3", map("a", 3, "b", asList(2, 7), "c", 10),
+            "doc4", map("a", 1, "b", asList(3, 7)),
+            "doc5", map("a", 1),
+            "doc6", map("a", 2, "c", 20));
+    CollectionReference collection = testCollectionWithDocs(testDocs);
+
+    Query query1 =
+        collection.where(
+            Filter.or(
+                Filter.inArray("a", asList(2, 3)), Filter.arrayContainsAny("b", asList(0, 7))));
+    checkOnlineAndOfflineResultsMatch(query1, "doc1", "doc3", "doc4", "doc6");
+
+    Query query2 =
+        collection.where(
+            Filter.and(
+                Filter.inArray("a", asList(2, 3)), Filter.arrayContainsAny("b", asList(0, 7))));
+    checkOnlineAndOfflineResultsMatch(query2, "doc3");
+
+    Query query3 =
+        collection.where(
+            Filter.or(
+                Filter.and(Filter.inArray("a", asList(2, 3)), Filter.equalTo("c", 10)),
+                Filter.arrayContainsAny("b", asList(0, 7))));
+    checkOnlineAndOfflineResultsMatch(query3, "doc1", "doc3", "doc4");
+
+    Query query4 =
+        collection.where(
+            Filter.and(
+                Filter.inArray("a", asList(2, 3)),
+                Filter.or(Filter.arrayContainsAny("b", asList(0, 7)), Filter.equalTo("c", 20))));
+    checkOnlineAndOfflineResultsMatch(query4, "doc3", "doc6");
+  }
+
+  @Ignore
+  @Test
+  public void testUsingInWithArrayContains() {
+    Map<String, Map<String, Object>> testDocs =
+        map(
+            "doc1", map("a", 1, "b", asList(0)),
+            "doc2", map("b", asList(1)),
+            "doc3", map("a", 3, "b", asList(2, 7)),
+            "doc4", map("a", 1, "b", asList(3, 7)),
+            "doc5", map("a", 1),
+            "doc6", map("a", 2));
+    CollectionReference collection = testCollectionWithDocs(testDocs);
+
+    Query query1 =
+        collection.where(
+            Filter.or(Filter.inArray("a", asList(2, 3)), Filter.arrayContains("b", 3)));
+    checkOnlineAndOfflineResultsMatch(query1, "doc3", "doc4", "doc6");
+
+    Query query2 =
+        collection.where(
+            Filter.and(Filter.inArray("a", asList(2, 3)), Filter.arrayContains("b", 7)));
+    checkOnlineAndOfflineResultsMatch(query2, "doc3");
+
+    Query query3 =
+        collection.where(
+            Filter.or(
+                Filter.inArray("a", asList(2, 3)),
+                Filter.and(Filter.arrayContains("b", 3), Filter.equalTo("a", 1))));
+    checkOnlineAndOfflineResultsMatch(query3, "doc3", "doc4", "doc6");
+
+    Query query4 =
+        collection.where(
+            Filter.and(
+                Filter.inArray("a", asList(2, 3)),
+                Filter.or(Filter.arrayContains("b", 7), Filter.equalTo("a", 1))));
+    checkOnlineAndOfflineResultsMatch(query4, "doc3");
+  }
+
+  @Ignore
+  @Test
+  public void testOrderByEquality() {
+    Map<String, Map<String, Object>> testDocs =
+        map(
+            "doc1", map("a", 1, "b", asList(0)),
+            "doc2", map("b", asList(1)),
+            "doc3", map("a", 3, "b", asList(2, 7), "c", 10),
+            "doc4", map("a", 1, "b", asList(3, 7)),
+            "doc5", map("a", 1),
+            "doc6", map("a", 2, "c", 20));
+    CollectionReference collection = testCollectionWithDocs(testDocs);
+
+    Query query1 = collection.where(Filter.equalTo("a", 1)).orderBy("a");
+    checkOnlineAndOfflineResultsMatch(query1, "doc1", "doc4", "doc5");
+
+    Query query2 = collection.where(Filter.inArray("a", asList(2, 3))).orderBy("a");
+    checkOnlineAndOfflineResultsMatch(query2, "doc6", "doc3");
+  }
 }
diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java
@@ -557,31 +557,6 @@ public void queriesWithMultipleNotEqualAndInequalitiesFail() {
             + "same field. But you have filters on 'x' and 'y'");
   }
 
-  @Test
-  public void queriesWithMultipleArrayFiltersFail() {
-    expectError(
-        () -> testCollection().whereArrayContains("foo", 1).whereArrayContains("foo", 2),
-        "Invalid Query. You cannot use more than one 'array_contains' filter.");
-
-    expectError(
-        () ->
-            testCollection()
-                .whereArrayContains("foo", 1)
-                .whereArrayContainsAny("foo", asList(1, 2)),
-        "Invalid Query. You cannot use 'array_contains_any' filters with 'array_contains' filters.");
-
-    expectError(
-        () ->
-            testCollection()
-                .whereArrayContainsAny("foo", asList(1, 2))
-                .whereArrayContains("foo", 1),
-        "Invalid Query. You cannot use 'array_contains' filters with 'array_contains_any' filters.");
-
-    expectError(
-        () -> testCollection().whereNotIn("foo", asList(1, 2)).whereArrayContains("foo", 1),
-        "Invalid Query. You cannot use 'array_contains' filters with 'not_in' filters.");
-  }
-
   @Test
   public void queriesWithNotEqualAndNotInFiltersFail() {
     expectError(
@@ -595,44 +570,12 @@ public void queriesWithNotEqualAndNotInFiltersFail() {
 
   @Test
   public void queriesWithMultipleDisjunctiveFiltersFail() {
-    expectError(
-        () -> testCollection().whereIn("foo", asList(1, 2)).whereIn("bar", asList(1, 2)),
-        "Invalid Query. You cannot use more than one 'in' filter.");
-
     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'");
 
-    expectError(
-        () ->
-            testCollection()
-                .whereArrayContainsAny("foo", asList(1, 2))
-                .whereArrayContainsAny("bar", asList(1, 2)),
-        "Invalid Query. You cannot use more than one 'array_contains_any' filter.");
-
-    expectError(
-        () ->
-            testCollection()
-                .whereArrayContainsAny("foo", asList(1, 2))
-                .whereIn("bar", asList(1, 2)),
-        "Invalid Query. You cannot use 'in' filters with 'array_contains_any' filters.");
-
-    expectError(
-        () ->
-            testCollection()
-                .whereIn("bar", asList(1, 2))
-                .whereArrayContainsAny("foo", asList(1, 2)),
-        "Invalid Query. You cannot use 'array_contains_any' filters with 'in' filters.");
-
-    expectError(
-        () ->
-            testCollection()
-                .whereArrayContainsAny("foo", asList(1, 2))
-                .whereNotIn("bar", asList(1, 2)),
-        "Invalid Query. You cannot use 'not_in' filters with 'array_contains_any' filters.");
-
     expectError(
         () ->
             testCollection()
@@ -647,61 +590,6 @@ public void queriesWithMultipleDisjunctiveFiltersFail() {
     expectError(
         () -> testCollection().whereIn("bar", asList(1, 2)).whereNotIn("foo", asList(1, 2)),
         "Invalid Query. You cannot use 'not_in' filters with 'in' filters.");
-
-    // This is redundant with the above tests, but makes sure our validation doesn't get confused.
-    expectError(
-        () ->
-            testCollection()
-                .whereIn("bar", asList(1, 2))
-                .whereArrayContains("foo", 1)
-                .whereArrayContainsAny("foo", asList(1, 2)),
-        "Invalid Query. You cannot use 'array_contains_any' filters with 'in' filters.");
-
-    expectError(
-        () ->
-            testCollection()
-                .whereArrayContains("foo", 1)
-                .whereIn("bar", asList(1, 2))
-                .whereArrayContainsAny("foo", asList(1, 2)),
-        "Invalid Query. You cannot use 'array_contains_any' filters with 'array_contains' filters.");
-
-    expectError(
-        () ->
-            testCollection()
-                .whereNotIn("bar", asList(1, 2))
-                .whereArrayContains("foo", 1)
-                .whereArrayContainsAny("foo", asList(1, 2)),
-        "Invalid Query. You cannot use 'array_contains' filters with 'not_in' filters.");
-
-    expectError(
-        () ->
-            testCollection()
-                .whereArrayContains("foo", 1)
-                .whereIn("foo", asList(1, 2))
-                .whereNotIn("bar", asList(1, 2)),
-        "Invalid Query. You cannot use 'not_in' filters with 'array_contains' filters.");
-  }
-
-  @Test
-  public void queriesCanUseInWithArrayContains() {
-    testCollection().whereArrayContains("foo", 1).whereIn("bar", asList(1, 2));
-    testCollection().whereIn("bar", asList(1, 2)).whereArrayContains("foo", 1);
-
-    expectError(
-        () ->
-            testCollection()
-                .whereIn("bar", asList(1, 2))
-                .whereArrayContains("foo", 1)
-                .whereArrayContains("foo", 1),
-        "Invalid Query. You cannot use more than one 'array_contains' filter.");
-
-    expectError(
-        () ->
-            testCollection()
-                .whereArrayContains("foo", 1)
-                .whereIn("bar", asList(1, 2))
-                .whereIn("bar", asList(1, 2)),
-        "Invalid Query. You cannot use more than one 'in' filter.");
   }
 
   @Test
@@ -717,22 +605,6 @@ public void queriesInAndArrayContainsAnyArrayRules() {
     expectError(
         () -> testCollection().whereArrayContainsAny("bar", asList()),
         "Invalid Query. A non-empty array is required for 'array_contains_any' filters.");
-
-    expectError(
-        // The 10 element max includes duplicates.
-        () -> testCollection().whereIn("bar", asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 9, 9)),
-        "Invalid Query. 'in' filters support a maximum of 10 elements in the value array.");
-
-    expectError(
-        // The 10 element max includes duplicates.
-        () -> testCollection().whereNotIn("bar", asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 9, 9)),
-        "Invalid Query. 'not_in' filters support a maximum of 10 elements in the value array.");
-
-    expectError(
-        // The 10 element max includes duplicates.
-        () ->
-            testCollection().whereArrayContainsAny("bar", asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 9, 9)),
-        "Invalid Query. 'array_contains_any' filters support a maximum of 10 elements in the value array.");
   }
 
   @Test
diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java
@@ -540,12 +540,6 @@ private void validateDisjunctiveFilterElements(Object value, Operator op) {
       throw new IllegalArgumentException(
           "Invalid Query. A non-empty array is required for '" + op.toString() + "' filters.");
     }
-    if (((List) value).size() > 10) {
-      throw new IllegalArgumentException(
-          "Invalid Query. '"
-              + op.toString()
-              + "' filters support a maximum of 10 elements in the value array.");
-    }
   }
 
   private void validateOrderByFieldMatchesInequality(
@@ -566,36 +560,26 @@ private void validateOrderByFieldMatchesInequality(
   /**
    * Given an operator, returns the set of operators that cannot be used with it.
    *
+   * <p>This is not a comprehensive check, and this function should be removed in the long term.
+   * Validations should occur in the Firestore backend.
+   *
    * <p>Operators in a query must adhere to the following set of rules:
    *
    * <ol>
-   *   <li>Only one array operator is allowed.
-   *   <li>Only one disjunctive operator is allowed.
-   *   <li>NOT_EQUAL cannot be used with another NOT_EQUAL operator.
+   *   <li>Only one inequality per query.
    *   <li>NOT_IN cannot be used with array, disjunctive, or NOT_EQUAL operators.
    * </ol>
-   *
-   * <p>Array operators: ARRAY_CONTAINS, ARRAY_CONTAINS_ANY Disjunctive operators: IN,
-   * ARRAY_CONTAINS_ANY, NOT_IN
    */
   private List<Operator> conflictingOps(Operator op) {
     switch (op) {
       case NOT_EQUAL:
         return Arrays.asList(Operator.NOT_EQUAL, Operator.NOT_IN);
-      case ARRAY_CONTAINS:
-        return Arrays.asList(Operator.ARRAY_CONTAINS, Operator.ARRAY_CONTAINS_ANY, Operator.NOT_IN);
-      case IN:
-        return Arrays.asList(Operator.ARRAY_CONTAINS_ANY, Operator.IN, Operator.NOT_IN);
       case ARRAY_CONTAINS_ANY:
-        return Arrays.asList(
-            Operator.ARRAY_CONTAINS, Operator.ARRAY_CONTAINS_ANY, Operator.IN, Operator.NOT_IN);
+      case IN:
+        return Arrays.asList(Operator.NOT_IN);
       case NOT_IN:
         return Arrays.asList(
-            Operator.ARRAY_CONTAINS,
-            Operator.ARRAY_CONTAINS_ANY,
-            Operator.IN,
-            Operator.NOT_IN,
-            Operator.NOT_EQUAL);
+            Operator.ARRAY_CONTAINS_ANY, Operator.IN, Operator.NOT_IN, Operator.NOT_EQUAL);
       default:
         return new ArrayList<>();
     }
diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/core/Query.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/core/Query.java
