Skip to content

Commit

Permalink
fix(java): fix error message returned for invalid snapshot listener i…
Browse files Browse the repository at this point in the history
…nequality filter (#1093)
  • Loading branch information
milaGGL committed Jan 24, 2023
1 parent 32ba8ac commit bf7f4a3
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]");
}

/**
*
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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:");
}
}

0 comments on commit bf7f4a3

Please sign in to comment.