fix(java): fix error message returned for invalid snapshot listener inequality filter (#1093)

milaGGL · web-flow · commit bf7f4a3b8731 · 2023-01-24T09:03:56.000-08:00
diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryCountTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryCountTest.java
@@ -17,6 +17,7 @@
 package com.google.cloud.firestore.it;
 
 import static com.google.cloud.firestore.LocalFirestoreHelper.autoId;
+import static com.google.cloud.firestore.it.TestHelper.isRunningAgainstFirestoreEmulator;
 import static com.google.common.truth.Truth.assertThat;
 import static java.util.Collections.singletonMap;
 import static org.junit.Assert.assertThrows;
@@ -247,7 +248,7 @@ public void aggregateQueryInATransactionShouldLockTheCountedDocuments() throws E
     assumeTrue(
         "Skip this test when running against production because "
             + "it appears that production is failing to lock the counted documents b/248152832",
-        isRunningAgainstFirestoreEmulator());
+        isRunningAgainstFirestoreEmulator(firestore));
 
     CollectionReference collection = createEmptyCollection();
     DocumentReference document = createDocumentInCollection(collection);
@@ -418,11 +419,6 @@ private static void await(ApiFuture<?> future) throws InterruptedException {
     executor.shutdown();
   }
 
-  /** Returns whether the tests are running against the Firestore emulator. */
-  private boolean isRunningAgainstFirestoreEmulator() {
-    return firestore.getOptions().getHost().startsWith("localhost:");
-  }
-
   @AutoValue
   abstract static class CreatedCollectionInfo {
 
diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryWatchTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryWatchTest.java
@@ -17,12 +17,14 @@
 package com.google.cloud.firestore.it;
 
 import static com.google.cloud.firestore.LocalFirestoreHelper.map;
+import static com.google.cloud.firestore.it.TestHelper.isRunningAgainstFirestoreEmulator;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static java.util.Arrays.asList;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
+import static org.junit.Assume.assumeFalse;
 
 import com.google.cloud.firestore.CollectionReference;
 import com.google.cloud.firestore.DocumentChange;
@@ -152,6 +154,63 @@ public void nonEmptyResults() throws Exception {
     listenerAssertions.removedIdsIsAnyOf(emptyList());
   }
 
+  /**
+   * Testing multiple inequality filters on same and different properties, and validate the error
+   * message returned for invalid filter.
+   */
+  @Test
+  public void inequalityFilterOnSamePropertiesShouldBeSupported() throws Exception {
+    setDocument("doc", map("foo", 1, "bar", 2));
+
+    final Query query = randomColl.whereGreaterThan("foo", 0).whereLessThanOrEqualTo("foo", 2);
+    QuerySnapshotEventListener listener =
+        QuerySnapshotEventListener.builder().setInitialEventCount(1).build();
+    ListenerRegistration registration = query.addSnapshotListener(listener);
+
+    try {
+      listener.eventsCountDownLatch.awaitInitialEvents();
+    } finally {
+      registration.remove();
+    }
+    ListenerAssertions listenerAssertions = listener.assertions();
+    listenerAssertions.noError();
+    listenerAssertions.eventCountIsAnyOf(Range.closed(1, 1));
+    listenerAssertions.addedIdsIsAnyOf(singletonList("doc"));
+    listenerAssertions.modifiedIdsIsAnyOf(emptyList());
+    listenerAssertions.removedIdsIsAnyOf(emptyList());
+  }
+
+  /** Based on https://github.com/googleapis/java-firestore/issues/1085 */
+  @Test
+  public void inequalityFilterOnDifferentPropertiesShouldThrow() throws Exception {
+    assumeFalse(
+        "Skip this test when running against emulator because the fix is only applied in the "
+            + "production",
+        isRunningAgainstFirestoreEmulator(firestore));
+
+    setDocument("doc1", map("foo", "1", "bar", 1));
+
+    final Query query = randomColl.whereGreaterThan("foo", "0").whereLessThan("bar", 2);
+    QuerySnapshotEventListener listener =
+        QuerySnapshotEventListener.builder().setExpectError().build();
+    ListenerRegistration registration = query.addSnapshotListener(listener);
+
+    try {
+      listener.eventsCountDownLatch.awaitError();
+    } finally {
+      registration.remove();
+    }
+
+    ListenerAssertions listenerAssertions = listener.assertions();
+    listenerAssertions.hasError();
+    FirestoreException error = listener.receivedEvents.get(0).error;
+    assertThat(error)
+        .hasMessageThat()
+        .ignoringCase()
+        .contains(
+            "Backend ended Listen stream: Cannot have inequality filters on multiple properties: [foo, bar]");
+  }
+
   /**
    *
    *
diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java
@@ -25,6 +25,7 @@
 import static com.google.cloud.firestore.LocalFirestoreHelper.UPDATE_SINGLE_FIELD_OBJECT;
 import static com.google.cloud.firestore.LocalFirestoreHelper.fullPath;
 import static com.google.cloud.firestore.LocalFirestoreHelper.map;
+import static com.google.cloud.firestore.it.TestHelper.isRunningAgainstFirestoreEmulator;
 import static com.google.common.truth.Truth.assertThat;
 import static java.util.Arrays.asList;
 import static org.junit.Assert.assertArrayEquals;
@@ -36,6 +37,7 @@
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeFalse;
 
 import com.google.api.core.ApiFuture;
 import com.google.api.core.ApiFutures;
@@ -488,6 +490,37 @@ public void greaterThanQuery() throws Exception {
     assertEquals(2L, querySnapshot.getDocuments().get(0).get("foo"));
   }
 
+  @Test
+  public void multipleInequalityQueryOnSamePropertiesShouldBeSupported() throws Exception {
+    addDocument("foo", 1);
+
+    QuerySnapshot querySnapshot =
+        randomColl.whereGreaterThan("foo", 0).whereLessThanOrEqualTo("foo", 2).get().get();
+    assertEquals(1, querySnapshot.size());
+    assertEquals(1L, querySnapshot.getDocuments().get(0).get("foo"));
+  }
+
+  /** Based on https://github.com/googleapis/java-firestore/issues/1085 */
+  @Test
+  public void multipleInequalityQueryOnDifferentPropertiesShouldThrow() throws Exception {
+    assumeFalse(
+        "Skip this test when running against emulator because the fix is only applied in the "
+            + "production",
+        isRunningAgainstFirestoreEmulator(firestore));
+
+    addDocument("foo", 1, "bar", 2);
+
+    ExecutionException executionException =
+        assertThrows(
+            ExecutionException.class,
+            () -> randomColl.whereGreaterThan("foo", 1).whereNotEqualTo("bar", 3).get().get());
+    assertThat(executionException)
+        .hasCauseThat()
+        .hasMessageThat()
+        .contains(
+            "INVALID_ARGUMENT: Cannot have inequality filters on multiple properties: [bar, foo]");
+  }
+
   @Test
   public void greaterThanOrEqualQuery() throws Exception {
     addDocument("foo", 1);
diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/TestHelper.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/TestHelper.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.cloud.firestore.it;
+
+import com.google.cloud.firestore.Firestore;
+
+public final class TestHelper {
+  /** Make constructor private to prevent creating instances. */
+  private TestHelper() {}
+
+  /** Returns whether the tests are running against the Firestore emulator. */
+  static boolean isRunningAgainstFirestoreEmulator(Firestore firestore) {
+    return firestore.getOptions().getHost().startsWith("localhost:");
+  }
+}
