diff --git a/README.md b/README.md index 47d5b0f7b..ab879c902 100644 --- a/README.md +++ b/README.md @@ -57,13 +57,13 @@ implementation 'com.google.cloud:google-cloud-firestore' If you are using Gradle without BOM, add this to your dependencies: ```Groovy -implementation 'com.google.cloud:google-cloud-firestore:3.14.2' +implementation 'com.google.cloud:google-cloud-firestore:3.14.3' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-firestore" % "3.14.2" +libraryDependencies += "com.google.cloud" % "google-cloud-firestore" % "3.14.3" ``` @@ -222,7 +222,7 @@ Java is a registered trademark of Oracle and/or its affiliates. [kokoro-badge-link-5]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-firestore/java11.html [stability-image]: https://img.shields.io/badge/stability-stable-green [maven-version-image]: https://img.shields.io/maven-central/v/com.google.cloud/google-cloud-firestore.svg -[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-firestore/3.14.2 +[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-firestore/3.14.3 [authentication]: https://github.com/googleapis/google-cloud-java#authentication [auth-scopes]: https://developers.google.com/identity/protocols/oauth2/scopes [predefined-iam-roles]: https://cloud.google.com/iam/docs/understanding-roles#predefined_roles diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FieldPath.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FieldPath.java index 752b7ea09..90425ba75 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FieldPath.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FieldPath.java @@ -20,6 +20,8 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.firestore.v1.StructuredQuery; +import java.util.ArrayList; +import java.util.List; import java.util.regex.Pattern; import javax.annotation.Nonnull; @@ -83,6 +85,62 @@ static FieldPath fromDotSeparatedString(String field) { return empty().append(field); } + /** + * Creates a {@code FieldPath} from a server-encoded field path. + * + *

Copied from Firebase Android SDK: + * https://github.com/firebase/firebase-android-sdk/blob/2d3b2be7d2d00d693eb74986f20a6265c918848f/firebase-firestore/src/main/java/com/google/firebase/firestore/model/FieldPath.java#L47 + */ + public static FieldPath fromServerFormat(String path) { + List res = new ArrayList<>(); + StringBuilder builder = new StringBuilder(); + + int i = 0; + + // If we're inside '`' backticks, then we should ignore '.' dots. + boolean inBackticks = false; + + while (i < path.length()) { + char c = path.charAt(i); + if (c == '\\') { + if (i + 1 == path.length()) { + throw new IllegalArgumentException("Trailing escape character is not allowed"); + } + i++; + builder.append(path.charAt(i)); + } else if (c == '.') { + if (!inBackticks) { + String elem = builder.toString(); + if (elem.isEmpty()) { + throw new IllegalArgumentException( + "Invalid field path (" + + path + + "). Paths must not be empty, begin with '.', end with '.', or contain '..'"); + } + builder = new StringBuilder(); + res.add(elem); + } else { + // escaped, append to current segment + builder.append(c); + } + } else if (c == '`') { + inBackticks = !inBackticks; + } else { + builder.append(c); + } + i++; + } + String lastElem = builder.toString(); + if (lastElem.isEmpty()) { + throw new IllegalArgumentException( + "Invalid field path (" + + path + + "). Paths must not be empty, begin with '.', end with '.', or contain '..'"); + } + res.add(lastElem); + return FieldPath.of(res.toArray(new String[0])); + } + /** Returns an empty field path. */ static FieldPath empty() { // NOTE: This is not static since it would create a circular class dependency during 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 index f53b44b9b..011ad5a50 100644 --- 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 @@ -63,10 +63,13 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Logger; @@ -285,7 +288,9 @@ boolean isInequalityFilter() { return operator.equals(GREATER_THAN) || operator.equals(GREATER_THAN_OR_EQUAL) || operator.equals(LESS_THAN) - || operator.equals(LESS_THAN_OR_EQUAL); + || operator.equals(LESS_THAN_OR_EQUAL) + || operator.equals(NOT_EQUAL) + || operator.equals(NOT_IN); } @Nullable @@ -327,6 +332,11 @@ static final class FieldOrder { this.direction = direction; } + FieldOrder(String field, Direction direction) { + this.fieldReference = FieldPath.fromServerFormat(field).toProto(); + this.direction = direction; + } + Order toProto() { Order.Builder result = Order.newBuilder(); result.setField(fieldReference); @@ -462,39 +472,57 @@ private static boolean isUnaryComparison(@Nullable Object value) { return value == null || value.equals(Double.NaN) || value.equals(Float.NaN); } - /** Computes the backend ordering semantics for DocumentSnapshot cursors. */ - private ImmutableList createImplicitOrderBy() { - List implicitOrders = new ArrayList<>(options.getFieldOrders()); - - // If no explicit ordering is specified, use the first inequality to define an implicit order. - if (implicitOrders.isEmpty()) { - for (FilterInternal filter : options.getFilters()) { - FieldReference fieldReference = filter.getFirstInequalityField(); - if (fieldReference != null) { - implicitOrders.add(new FieldOrder(fieldReference, Direction.ASCENDING)); - break; + /** Returns the sorted set of inequality filter fields used in this query. */ + private SortedSet getInequalityFilterFields() { + SortedSet result = new TreeSet<>(); + + for (FilterInternal filter : options.getFilters()) { + for (FieldFilterInternal subFilter : filter.getFlattenedFilters()) { + if (subFilter.isInequalityFilter()) { + result.add(FieldPath.fromServerFormat(subFilter.fieldReference.getFieldPath())); } } } - boolean hasDocumentId = false; - for (FieldOrder fieldOrder : implicitOrders) { - if (FieldPath.isDocumentId(fieldOrder.fieldReference.getFieldPath())) { - hasDocumentId = true; + return result; + } + + /** Computes the backend ordering semantics for DocumentSnapshot cursors. */ + ImmutableList createImplicitOrderBy() { + // Any explicit order by fields should be added as is. + List result = new ArrayList<>(options.getFieldOrders()); + + HashSet fieldsNormalized = new HashSet<>(); + for (FieldOrder order : result) { + fieldsNormalized.add(order.fieldReference.getFieldPath()); + } + + /** The order of the implicit ordering always matches the last explicit order by. */ + Direction lastDirection = + result.isEmpty() ? Direction.ASCENDING : result.get(result.size() - 1).direction; + + /** + * Any inequality fields not explicitly ordered should be implicitly ordered in a + * lexicographical order. When there are multiple inequality filters on the same field, the + * field should be added only once. + * + *

Note: `SortedSet` sorts the key field before other fields. However, we want the + * key field to be sorted last. + */ + SortedSet inequalityFields = getInequalityFilterFields(); + for (FieldPath field : inequalityFields) { + if (!fieldsNormalized.contains(field.toString()) + && !FieldPath.isDocumentId(field.toString())) { + result.add(new FieldOrder(field.toProto(), lastDirection)); } } - if (!hasDocumentId) { - // Add implicit sorting by name, using the last specified direction. - Direction lastDirection = - implicitOrders.isEmpty() - ? Direction.ASCENDING - : implicitOrders.get(implicitOrders.size() - 1).direction; - - implicitOrders.add(new FieldOrder(FieldPath.documentId().toProto(), lastDirection)); + // Add the document key field to the last if it is not explicitly ordered. + if (!fieldsNormalized.contains(FieldPath.documentId().toString())) { + result.add(new FieldOrder(FieldPath.documentId().toProto(), lastDirection)); } - return ImmutableList.builder().addAll(implicitOrders).build(); + return ImmutableList.builder().addAll(result).build(); } private Cursor createCursor( @@ -506,11 +534,12 @@ private Cursor createCursor( if (FieldPath.isDocumentId(path)) { fieldValues.add(documentSnapshot.getReference()); } else { - FieldPath fieldPath = FieldPath.fromDotSeparatedString(path); + FieldPath fieldPath = FieldPath.fromServerFormat(path); Preconditions.checkArgument( documentSnapshot.contains(fieldPath), "Field '%s' is missing in the provided DocumentSnapshot. Please provide a document " - + "that contains values for all specified orderBy() and where() constraints."); + + "that contains values for all specified orderBy() and where() constraints.", + fieldPath); fieldValues.add(documentSnapshot.get(fieldPath)); } } 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 index 4d27fd815..280c6e02a 100644 --- 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 @@ -51,6 +51,7 @@ import com.google.api.gax.rpc.ResponseObserver; import com.google.cloud.Timestamp; import com.google.cloud.firestore.Query.ComparisonFilterInternal; +import com.google.cloud.firestore.Query.FieldOrder; import com.google.cloud.firestore.Query.FilterInternal; import com.google.cloud.firestore.spi.v1.FirestoreRpc; import com.google.common.io.BaseEncoding; @@ -66,6 +67,7 @@ import io.grpc.Status; import java.lang.reflect.Method; import java.lang.reflect.Proxy; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Iterator; @@ -1353,4 +1355,170 @@ public void ensureFromProtoWorksWithAProxy() throws InvalidProtocolBufferExcepti assertFalse(comparisonFilter.isInequalityFilter()); assertEquals(Value.newBuilder().setBooleanValue(true).build(), comparisonFilter.value); } + + @Test + public void inequalityFiltersImplicitlyOrderedLexicographicallyOnCharacters() { + Query query_ = + query + .whereLessThan("a", "value") + .whereGreaterThanOrEqualTo("a", "value") + .whereGreaterThan("aa", "value") + .whereGreaterThan("b", "value") + .whereGreaterThan("A", "value"); + + List orderFields = new ArrayList<>(); + orderFields.add(new FieldOrder("A", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("aa", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("b", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder(FieldPath.documentId().toProto(), Query.Direction.ASCENDING)); + + assertEquals(orderFields, query_.createImplicitOrderBy()); + } + + @Test + public void inequalityFiltersImplicitlyOrderedLexicographicallyOnCharactersAndNumbers() { + Query query_ = + query + .whereLessThan("a", "value") + .whereGreaterThan("1", "value") + .whereGreaterThan("19", "value") + .whereGreaterThan("2", "value"); + + List orderFields = new ArrayList<>(); + orderFields.add(new FieldOrder("1", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("19", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("2", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder(FieldPath.documentId().toProto(), Query.Direction.ASCENDING)); + + assertEquals(orderFields, query_.createImplicitOrderBy()); + } + + @Test + public void inequalityFiltersImplicitlyOrderedLexicographicallyOnNestedFields() { + Query query_ = + query + .whereLessThan("a", "value") + .whereGreaterThan("aa", "value") + .whereGreaterThan("a.a", "value"); + + List orderFields = new ArrayList<>(); + orderFields.add(new FieldOrder("a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("a.a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("aa", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder(FieldPath.documentId().toProto(), Query.Direction.ASCENDING)); + + assertEquals(orderFields, query_.createImplicitOrderBy()); + } + + @Test + public void inequalityFiltersImplicitlyOrderedLexicographicallyOnSpecialCharacters() { + Query query_ = + query + .whereLessThan("a", "value") + .whereGreaterThan("_a", "value") + .whereGreaterThan("a.a", "value"); + + List orderFields = new ArrayList<>(); + orderFields.add(new FieldOrder("_a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("a.a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder(FieldPath.documentId().toProto(), Query.Direction.ASCENDING)); + + assertEquals(orderFields, query_.createImplicitOrderBy()); + } + + @Test + public void inequalityFiltersImplicitlyOrderedLexicographicallyOnFieldNameWithDot() { + Query query_ = + query + .whereLessThan("a", "value") + .whereGreaterThan(FieldPath.of("a.a"), "value") + .whereGreaterThan("a.z", "value"); + + List orderFields = new ArrayList<>(); + orderFields.add(new FieldOrder("a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("a.z", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("`a.a`", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder(FieldPath.documentId().toProto(), Query.Direction.ASCENDING)); + + assertEquals(orderFields, query_.createImplicitOrderBy()); + } + + @Test + public void inequalityFiltersImplicitlyOrderedLexicographicallyInCompositeFilter() { + Query query_ = + query.where( + and( + lessThan("a", "value"), + and( + or(greaterThanOrEqualTo("b", "value"), lessThanOrEqualTo("c", "value")), + or(greaterThan("d", "value"), equalTo("e", "value"))))); + + List orderFields = new ArrayList<>(); + orderFields.add(new FieldOrder("a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("b", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("c", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("d", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder(FieldPath.documentId().toProto(), Query.Direction.ASCENDING)); + + assertEquals(orderFields, query_.createImplicitOrderBy()); + } + + @Test + public void inequalityFiltersImplicitlyOrderedLexicographicallyWithExplicitOrderBys() { + Query query_ = + query + .whereLessThan("b", "value") + .whereGreaterThan("a", "value") + .whereGreaterThan("z", "value") + .orderBy("z", Query.Direction.ASCENDING); + + List orderFields = new ArrayList<>(); + orderFields.add(new FieldOrder("z", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("b", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder(FieldPath.documentId().toProto(), Query.Direction.ASCENDING)); + + assertEquals(orderFields, query_.createImplicitOrderBy()); + } + + @Test + public void + inequalityFiltersImplicitlyOrderedLexicographicallyFollowingExplicitOrderByDirection() { + Query query_ = + query + .whereLessThan("b", "value") + .whereGreaterThan("a", "value") + .orderBy("z", Query.Direction.DESCENDING); + + List orderFields = new ArrayList<>(); + orderFields.add(new FieldOrder("z", Query.Direction.DESCENDING)); + orderFields.add(new FieldOrder("a", Query.Direction.DESCENDING)); + orderFields.add(new FieldOrder("b", Query.Direction.DESCENDING)); + orderFields.add(new FieldOrder(FieldPath.documentId().toProto(), Query.Direction.DESCENDING)); + + assertEquals(orderFields, query_.createImplicitOrderBy()); + } + + @Test + public void + inequalityFiltersImplicitlyOrderedLexicographicallyFollowingLastExplicitOrderByDirection() { + Query query_ = + query + .whereLessThan("b", "value") + .whereGreaterThan("a", "value") + .orderBy("z", Query.Direction.DESCENDING) + .orderBy("c", Query.Direction.ASCENDING); + + List orderFields = new ArrayList<>(); + orderFields.add(new FieldOrder("z", Query.Direction.DESCENDING)); + orderFields.add(new FieldOrder("c", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("a", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder("b", Query.Direction.ASCENDING)); + orderFields.add(new FieldOrder(FieldPath.documentId().toProto(), Query.Direction.ASCENDING)); + + assertEquals(orderFields, query_.createImplicitOrderBy()); + } } diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryTest.java index cc3613d35..444d31dd5 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryTest.java @@ -21,13 +21,10 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assume.assumeTrue; -import com.google.cloud.firestore.CollectionReference; -import com.google.cloud.firestore.Filter; -import com.google.cloud.firestore.LocalFirestoreHelper; -import com.google.cloud.firestore.Query; +import com.google.cloud.firestore.*; import com.google.cloud.firestore.Query.Direction; -import com.google.cloud.firestore.QuerySnapshot; import java.util.Arrays; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -392,4 +389,480 @@ public void testOrderByEquality() Query query2 = collection.where(Filter.inArray("a", asList(2, 3))).orderBy("a"); checkQuerySnapshotContainsDocuments(query2, "doc6", "doc3"); } + + /** Multiple Inequality */ + @Test + public void multipleInequalityOnDifferentFields() throws Exception { + // TODO(MIEQ): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because multiple inequality is " + + "not supported yet.", + isRunningAgainstFirestoreEmulator(firestore)); + + CollectionReference collection = + testCollectionWithDocs( + map( + "doc1", map("key", "a", "sort", 0, "v", 0), + "doc2", map("key", "b", "sort", 3, "v", 1), + "doc3", map("key", "c", "sort", 1, "v", 3), + "doc4", map("key", "d", "sort", 2, "v", 2))); + + Query query1 = + collection + .whereNotEqualTo("key", "a") + .whereLessThanOrEqualTo("sort", 2) + .whereGreaterThan("v", 2); + checkQuerySnapshotContainsDocuments(query1, "doc3"); + + // Duplicate inequality fields + Query query2 = + collection + .whereNotEqualTo("key", "a") + .whereLessThanOrEqualTo("sort", 2) + .whereGreaterThan("sort", 1); + checkQuerySnapshotContainsDocuments(query2, "doc4"); + + // With multiple IN + Query query3 = + collection + .whereGreaterThanOrEqualTo("key", "a") + .whereLessThanOrEqualTo("sort", 2) + .whereIn("v", asList(2, 3, 4)) + .whereIn("sort", asList(2, 3)); + checkQuerySnapshotContainsDocuments(query3, "doc4"); + + // With NOT-IN + Query query4 = + collection + .whereGreaterThanOrEqualTo("key", "a") + .whereLessThanOrEqualTo("sort", 2) + .whereNotIn("v", asList(2, 4, 5)); + checkQuerySnapshotContainsDocuments(query4, "doc1", "doc3"); + + // With orderby + Query query5 = + collection + .whereGreaterThanOrEqualTo("key", "a") + .whereLessThanOrEqualTo("sort", 2) + .orderBy("v", Direction.DESCENDING); + checkQuerySnapshotContainsDocuments(query5, "doc3", "doc4", "doc1"); + + // With limit + Query query6 = + collection + .whereGreaterThanOrEqualTo("key", "a") + .whereLessThanOrEqualTo("sort", 2) + .orderBy("v", Direction.DESCENDING) + .limit(2); + checkQuerySnapshotContainsDocuments(query6, "doc3", "doc4"); + + // With limitToLast + Query query7 = + collection + .whereGreaterThanOrEqualTo("key", "a") + .whereLessThanOrEqualTo("sort", 2) + .orderBy("v", Direction.DESCENDING) + .limitToLast(2); + checkQuerySnapshotContainsDocuments(query7, "doc4", "doc1"); + } + + @Test + public void multipleInequalityOnSpecialValues() throws Exception { + // TODO(MIEQ): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because multiple inequality is " + + "not supported yet.", + isRunningAgainstFirestoreEmulator(firestore)); + + CollectionReference collection = + testCollectionWithDocs( + map( + "doc1", map("key", "a", "sort", 0, "v", 0), + "doc2", map("key", "b", "sort", Double.NaN, "v", 1), + "doc3", map("key", "c", "sort", null, "v", 3), + "doc4", map("key", "d", "v", 2), + "doc5", map("key", "e", "sort", 0), + "doc6", map("key", "f", "sort", 1, "v", 1))); + + Query query1 = collection.whereNotEqualTo("key", "a").whereLessThanOrEqualTo("sort", 2); + checkQuerySnapshotContainsDocuments(query1, "doc5", "doc6"); + + Query query2 = + collection + .whereNotEqualTo("key", "a") + .whereLessThanOrEqualTo("sort", 2) + .whereLessThanOrEqualTo("v", 1); + checkQuerySnapshotContainsDocuments(query2, "doc6"); + } + + @Test + public void multipleInequalityWithArrayMembership() throws Exception { + // TODO(MIEQ): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because multiple inequality is " + + "not supported yet.", + isRunningAgainstFirestoreEmulator(firestore)); + + CollectionReference collection = + testCollectionWithDocs( + map( + "doc1", + map("key", "a", "sort", 0, "v", asList(0)), + "doc2", + map("key", "b", "sort", 1, "v", asList(0, 1, 3)), + "doc3", + map("key", "c", "sort", 1, "v", asList()), + "doc4", + map("key", "d", "sort", 2, "v", asList(1)), + "doc5", + map("key", "e", "sort", 3, "v", asList(2, 4)), + "doc6", + map("key", "f", "sort", 4, "v", Arrays.asList(Double.NaN)), + "doc7", + map("key", "g", "sort", 4, "v", Collections.singletonList(null)))); + + Query query1 = + collection + .whereNotEqualTo("key", "a") + .whereGreaterThanOrEqualTo("sort", 1) + .whereArrayContains("v", 0); + checkQuerySnapshotContainsDocuments(query1, "doc2"); + + Query query2 = + collection + .whereNotEqualTo("key", "a") + .whereGreaterThanOrEqualTo("sort", 1) + .whereArrayContainsAny("v", asList(0, 1)); + checkQuerySnapshotContainsDocuments(query2, "doc2", "doc4"); + } + + private static Map nestedObject(int number) { + return map( + "name", + String.format("room %d", number), + "metadata", + map("createdAt", number), + "field", + String.format("field %d", number), + "field.dot", + number, + "field\\slash", + number); + } + + // Use cursor in following test cases to add implicit order by fields in the sdk and compare the + // result with the query fields normalized in the server. + @Test + public void multipleInequalityWithNestedField() throws Exception { + // TODO(MIEQ): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because multiple inequality is " + + "not supported yet.", + isRunningAgainstFirestoreEmulator(firestore)); + + CollectionReference collection = + testCollectionWithDocs( + map( + "doc1", nestedObject(400), + "doc2", nestedObject(200), + "doc3", nestedObject(100), + "doc4", nestedObject(300))); + + // ordered by: name asc, metadata.createdAt asc, __name__ asc + Query query1 = + collection + .whereLessThanOrEqualTo("metadata.createdAt", 500) + .whereGreaterThan("metadata.createdAt", 100) + .whereNotEqualTo("name", "room 200") + .orderBy("name"); + DocumentSnapshot docSnap = collection.document("doc4").get().get(); + Query query1WithCursor = query1.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query1, "doc4", "doc1"); + checkQuerySnapshotContainsDocuments(query1WithCursor, "doc4", "doc1"); + + // ordered by: name desc, field desc, field.dot desc, field\\slash desc, __name__ desc + Query query2 = + collection + .whereGreaterThanOrEqualTo("field", "field 100") + .whereNotEqualTo(FieldPath.of("field.dot"), 300) + .whereLessThan("field\\slash", 400) + .orderBy("name", Direction.DESCENDING); + docSnap = collection.document("doc2").get().get(); + Query query2WithCursor = query2.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query2, "doc2", "doc3"); + checkQuerySnapshotContainsDocuments(query2WithCursor, "doc2", "doc3"); + } + + @Test + public void multipleInequalityWithCompositeFilters() throws Exception { + // TODO(MIEQ): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because multiple inequality is " + + "not supported yet.", + isRunningAgainstFirestoreEmulator(firestore)); + + CollectionReference collection = + testCollectionWithDocs( + map( + "doc1", + map("key", "a", "sort", 0, "v", 5), + "doc2", + map("key", "aa", "sort", 4, "v", 4), + "doc3", + map("key", "c", "sort", 3, "v", 3), + "doc4", + map("key", "b", "sort", 2, "v", 2), + "doc5", + map("key", "b", "sort", 2, "v", 1), + "doc6", + map("key", "b", "sort", 0, "v", 0))); + + // Implicitly ordered by: 'key' asc, 'sort' asc, 'v' asc, __name__ asc + Query query1 = + collection.where( + Filter.or( + Filter.and(Filter.equalTo("key", "b"), Filter.lessThanOrEqualTo("sort", 2)), + Filter.and(Filter.notEqualTo("key", "b"), Filter.greaterThan("v", 4)))); + DocumentSnapshot docSnap = collection.document("doc1").get().get(); + Query query1WithCursor = query1.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query1, "doc1", "doc6", "doc5", "doc4"); + checkQuerySnapshotContainsDocuments(query1WithCursor, "doc1", "doc6", "doc5", "doc4"); + + // Ordered by: 'sort' desc, 'key' asc, 'v' asc, __name__ asc + Query query2 = + collection + .where( + Filter.or( + Filter.and(Filter.equalTo("key", "b"), Filter.lessThanOrEqualTo("sort", 2)), + Filter.and(Filter.notEqualTo("key", "b"), Filter.greaterThan("v", 4)))) + .orderBy("sort", Direction.DESCENDING) + .orderBy("key"); + docSnap = collection.document("doc5").get().get(); + Query query2WithCursor = query2.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query2, "doc5", "doc4", "doc1", "doc6"); + checkQuerySnapshotContainsDocuments(query2WithCursor, "doc5", "doc4", "doc1", "doc6"); + + // Implicitly ordered by: 'key' asc, 'sort' asc, 'v' asc, __name__ asc + Query query3 = + collection.where( + Filter.and( + Filter.or( + Filter.and(Filter.equalTo("key", "b"), Filter.lessThanOrEqualTo("sort", 4)), + Filter.and(Filter.notEqualTo("key", "b"), Filter.greaterThanOrEqualTo("v", 4))), + Filter.or( + Filter.and( + Filter.greaterThan("key", "b"), Filter.greaterThanOrEqualTo("sort", 1)), + Filter.and(Filter.lessThan("key", "b"), Filter.greaterThan("v", 0))))); + docSnap = collection.document("doc1").get().get(); + Query query3WithCursor = query3.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query3, "doc1", "doc2"); + checkQuerySnapshotContainsDocuments(query3WithCursor, "doc1", "doc2"); + } + + @Test + public void multipleInequalityFieldsWillBeImplicitlyOrderedLexicographicallyByServer() + throws Exception { + // TODO(MIEQ): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because multiple inequality is " + + "not supported yet.", + isRunningAgainstFirestoreEmulator(firestore)); + + CollectionReference collection = + testCollectionWithDocs( + map( + "doc1", + map("key", "a", "sort", 0, "v", 5), + "doc2", + map("key", "aa", "sort", 4, "v", 4), + "doc3", + map("key", "b", "sort", 3, "v", 3), + "doc4", + map("key", "b", "sort", 2, "v", 2), + "doc5", + map("key", "b", "sort", 2, "v", 1), + "doc6", + map("key", "b", "sort", 0, "v", 0))); + + DocumentSnapshot docSnap = collection.document("doc2").get().get(); + + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + Query query1 = + collection + .whereNotEqualTo("key", "a") + .whereGreaterThan("sort", 1) + .whereIn("v", asList(1, 2, 3, 4)); + Query query1WithCursor = query1.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query1, "doc2", "doc4", "doc5", "doc3"); + checkQuerySnapshotContainsDocuments(query1WithCursor, "doc2", "doc4", "doc5", "doc3"); + + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + Query query2 = + collection + .whereGreaterThan("sort", 1) + .whereNotEqualTo("key", "a") + .whereIn("v", asList(1, 2, 3, 4)); + Query query2WithCursor = query2.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query2, "doc2", "doc4", "doc5", "doc3"); + checkQuerySnapshotContainsDocuments(query2WithCursor, "doc2", "doc4", "doc5", "doc3"); + } + + @Test + public void multipleInequalityWithMultipleExplicitOrderBy() throws Exception { + // TODO(MIEQ): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because multiple inequality is " + + "not supported yet.", + isRunningAgainstFirestoreEmulator(firestore)); + + CollectionReference collection = + testCollectionWithDocs( + map( + "doc1", + map("key", "a", "sort", 5, "v", 0), + "doc2", + map("key", "aa", "sort", 4, "v", 0), + "doc3", + map("key", "b", "sort", 3, "v", 1), + "doc4", + map("key", "b", "sort", 2, "v", 1), + "doc5", + map("key", "bb", "sort", 1, "v", 1), + "doc6", + map("key", "c", "sort", 0, "v", 2))); + + DocumentSnapshot docSnap = collection.document("doc2").get().get(); + + // Ordered by: 'v' asc, 'key' asc, 'sort' asc, __name__ asc + Query query1 = + collection.whereGreaterThan("key", "a").whereGreaterThanOrEqualTo("sort", 1).orderBy("v"); + Query query1WithCursor = query1.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query1, "doc2", "doc4", "doc3", "doc5"); + checkQuerySnapshotContainsDocuments(query1WithCursor, "doc2", "doc4", "doc3", "doc5"); + + // Ordered by: 'v asc, 'sort' asc, 'key' asc, __name__ asc + Query query2 = + collection + .whereGreaterThan("key", "a") + .whereGreaterThanOrEqualTo("sort", 1) + .orderBy("v") + .orderBy("sort"); + Query query2WithCursor = query2.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query2, "doc2", "doc5", "doc4", "doc3"); + checkQuerySnapshotContainsDocuments(query2WithCursor, "doc2", "doc5", "doc4", "doc3"); + + docSnap = collection.document("doc5").get().get(); + + // Implicit order by matches the direction of last explicit order by. + // Ordered by: 'v' desc, 'key' desc, 'sort' desc, __name__ desc + Query query3 = + collection + .whereGreaterThan("key", "a") + .whereGreaterThanOrEqualTo("sort", 1) + .orderBy("v", Direction.DESCENDING); + Query query3WithCursor = query3.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query3, "doc5", "doc3", "doc4", "doc2"); + checkQuerySnapshotContainsDocuments(query3WithCursor, "doc5", "doc3", "doc4", "doc2"); + + // Ordered by: 'v desc, 'sort' asc, 'key' asc, __name__ asc + Query query4 = + collection + .whereGreaterThan("key", "a") + .whereGreaterThanOrEqualTo("sort", 1) + .orderBy("v", Direction.DESCENDING) + .orderBy("sort"); + Query query4WithCursor = query4.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query4, "doc5", "doc4", "doc3", "doc2"); + checkQuerySnapshotContainsDocuments(query4WithCursor, "doc5", "doc4", "doc3", "doc2"); + } + + @Test + public void multipleInequalityFieldsInAggregateQuery() throws Exception { + // TODO(MIEQ): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because multiple inequality is " + + "not supported yet.", + isRunningAgainstFirestoreEmulator(firestore)); + + CollectionReference collection = + testCollectionWithDocs( + map( + "doc1", + map("key", "a", "sort", 5, "v", 0), + "doc2", + map("key", "aa", "sort", 4, "v", 0), + "doc3", + map("key", "b", "sort", 3, "v", 1), + "doc4", + map("key", "b", "sort", 2, "v", 1), + "doc5", + map("key", "bb", "sort", 1, "v", 1))); + + AggregateQuery query = + collection + .whereGreaterThan("key", "a") + .whereGreaterThanOrEqualTo("sort", 1) + .orderBy("v") + .count(); + assertThat(query.get().get().getCount()).isEqualTo(4); + // TODO(MIEQ): Add sum and average when they are public. + } + + @Test + public void multipleInequalityFieldsWithDocumentKey() throws Exception { + // TODO(MIEQ): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because multiple inequality is " + + "not supported yet.", + isRunningAgainstFirestoreEmulator(firestore)); + + CollectionReference collection = + testCollectionWithDocs( + map( + "doc1", + map("key", "a", "sort", 5), + "doc2", + map("key", "aa", "sort", 4), + "doc3", + map("key", "b", "sort", 3), + "doc4", + map("key", "b", "sort", 2), + "doc5", + map("key", "bb", "sort", 1))); + + DocumentSnapshot docSnap = collection.document("doc2").get().get(); + + // Document Key in inequality field will implicitly ordered to the last. + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + Query query1 = + collection + .whereGreaterThan("sort", 1) + .whereNotEqualTo("key", "a") + .whereLessThan(FieldPath.documentId(), "doc5"); + Query query1WithCursor = query1.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query1, "doc2", "doc4", "doc3"); + checkQuerySnapshotContainsDocuments(query1WithCursor, "doc2", "doc4", "doc3"); + + // Changing filters order will not affect implicit order. + // Implicitly ordered by: 'key' asc, 'sort' asc, __name__ asc + Query query2 = + collection + .whereLessThan(FieldPath.documentId(), "doc5") + .whereGreaterThan("sort", 1) + .whereNotEqualTo("key", "a"); + Query query2WithCursor = query2.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query2, "doc2", "doc4", "doc3"); + checkQuerySnapshotContainsDocuments(query2WithCursor, "doc2", "doc4", "doc3"); + + // Ordered by: 'sort' desc, 'key' desc, __name__ desc + Query query3 = + collection + .whereLessThan(FieldPath.documentId(), "doc5") + .whereGreaterThan("sort", 1) + .whereNotEqualTo("key", "a") + .orderBy("sort", Direction.DESCENDING); + Query query3WithCursor = query3.startAt(docSnap); + checkQuerySnapshotContainsDocuments(query3, "doc2", "doc3", "doc4"); + checkQuerySnapshotContainsDocuments(query3WithCursor, "doc2", "doc3", "doc4"); + } } 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 index fe3e621b7..6f80f916c 100644 --- 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,14 +17,12 @@ 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; @@ -139,64 +137,6 @@ 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() - .isIn( - asList( - "Backend ended Listen stream: Cannot have inequality filters on multiple properties: [foo, bar]", - "Backend ended Listen stream: Cannot have inequality filters on multiple properties: [bar, foo]")); - } - /** * * 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 index 950cbe485..a7d84d441 100644 --- 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 @@ -37,7 +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 static org.junit.Assume.assumeTrue; import com.google.api.core.ApiFuture; import com.google.api.core.ApiFutures; @@ -489,25 +489,20 @@ public void multipleInequalityQueryOnSamePropertiesShouldBeSupported() throws Ex 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", + public void multipleInequalityQueryOnDifferentPropertiesShouldBeSupported() throws Exception { + // TODO(MIEQ): Enable this test against production when possible. + assumeTrue( + "Skip this test if running against production because multiple inequality is " + + "not supported yet.", 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]"); + QuerySnapshot querySnapshot = + randomColl.whereGreaterThan("foo", 0).whereLessThanOrEqualTo("bar", 2).get().get(); + assertEquals(1, querySnapshot.size()); + assertEquals(1L, querySnapshot.getDocuments().get(0).get("foo")); } @Test