fix: Fields used in whereIn should be equality filters (#216)

* fix: Fields used in whereIn should be equality filters * fix: proper naming.
googleapis · May 14, 2020 · 4a62633 · 4a62633
1 parent 81c20c5
commit 4a62633
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 6 deletions.
diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java
@@ -104,7 +104,7 @@ Value encodeValue() {
       return encodedValue;
     }
 
-    abstract boolean isEqualsFilter();
+    abstract boolean isInequalityFilter();
 
     abstract Filter toProto();
   }
@@ -117,8 +117,8 @@ private static class UnaryFilter extends FieldFilter {
     }
 
     @Override
-    boolean isEqualsFilter() {
-      return true;
+    boolean isInequalityFilter() {
+      return false;
     }
 
     Filter toProto() {
@@ -148,8 +148,11 @@ private static class ComparisonFilter extends FieldFilter {
     }
 
     @Override
-    boolean isEqualsFilter() {
-      return operator.equals(EQUAL);
+    boolean isInequalityFilter() {
+      return operator.equals(GREATER_THAN)
+          || operator.equals(GREATER_THAN_OR_EQUAL)
+          || operator.equals(LESS_THAN)
+          || operator.equals(LESS_THAN_OR_EQUAL);
     }
 
     Filter toProto() {
@@ -306,7 +309,7 @@ private ImmutableList<FieldOrder> createImplicitOrderBy() {
     if (implicitOrders.isEmpty()) {
       // If no explicit ordering is specified, use the first inequality to define an implicit order.
       for (FieldFilter fieldFilter : options.getFieldFilters()) {
-        if (!fieldFilter.isEqualsFilter()) {
+        if (fieldFilter.isInequalityFilter()) {
           implicitOrders.add(new FieldOrder(fieldFilter.fieldPath, Direction.ASCENDING));
           break;
         }

diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryTest.java
@@ -303,6 +303,39 @@ public void inQueriesWithReferenceArray() throws Exception {
     assertEquals(expectedRequest, runQuery.getValue());
   }
 
+  @Test
+  public void inQueriesFieldsNotUsedInOrderBy() throws Exception {
+    doAnswer(queryResponse())
+        .when(firestoreMock)
+        .streamRequest(
+            runQuery.capture(),
+            streamObserverCapture.capture(),
+            Matchers.<ServerStreamingCallable>any());
+
+    // Field "foo" used in `whereIn` should not appear in implicit orderBys in the resulting query.
+    query
+        .whereIn(FieldPath.of("foo"), Arrays.<Object>asList("value1", "value2"))
+        .startAt(SINGLE_FIELD_SNAPSHOT)
+        .get()
+        .get();
+
+    Value value =
+        Value.newBuilder()
+            .setArrayValue(
+                ArrayValue.newBuilder()
+                    .addValues(Value.newBuilder().setStringValue("value1").build())
+                    .addValues(Value.newBuilder().setStringValue("value2").build())
+                    .build())
+            .build();
+    RunQueryRequest expectedRequest =
+        query(
+            filter(Operator.IN, "foo", value),
+            order("__name__", Direction.ASCENDING),
+            startAt(reference(DOCUMENT_NAME), true));
+
+    assertEquals(expectedRequest, runQuery.getValue());
+  }
+
   @Test
   public void validatesInQueries() {
     try {