From 8cde3157d989b56a5a1d051dfd977fd972a3e770 Mon Sep 17 00:00:00 2001 From: Ehsan Date: Mon, 24 Apr 2023 09:51:39 -0700 Subject: [PATCH 01/13] feat: SUM / AVG (#1218) * feat: SUM / AVG (real files) * Remove aggregate field duplicates (if any). * Clean up and fixes. * Clean up comments, and add Nonnull where possible. * Add more public docs. * More cleanup. * Update hashCode and equals for AggregateQuery. * Address code review comments. more to come. * fix test name. * Better comment. * Fix alias encoding. * Remove TODO. * Revert the way alias is constructed. * Backport test updates. fix format. fix import stmt. --- .../cloud/firestore/AggregateField.java | 127 +++ .../cloud/firestore/AggregateQuery.java | 93 +- .../firestore/AggregateQuerySnapshot.java | 110 ++- .../com/google/cloud/firestore/Query.java | 24 +- .../cloud/firestore/AggregateFieldTest.java | 93 ++ .../firestore/AggregateQuerySnapshotTest.java | 61 +- .../cloud/firestore/AggregateQueryTest.java | 55 +- .../firestore/it/ITQueryAggregationsTest.java | 815 ++++++++++++++++++ .../cloud/firestore/it/ITQueryCountTest.java | 68 +- .../google/cloud/firestore/it/TestHelper.java | 29 + 10 files changed, 1368 insertions(+), 107 deletions(-) create mode 100644 google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java create mode 100644 google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateFieldTest.java create mode 100644 google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java new file mode 100644 index 000000000..76fad5b8f --- /dev/null +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java @@ -0,0 +1,127 @@ +/* + * 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; + +import java.util.Objects; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +public abstract class AggregateField { + @Nonnull + public static CountAggregateField count() { + return new CountAggregateField(); + } + + @Nonnull + public static SumAggregateField sum(@Nonnull String field) { + return new SumAggregateField(FieldPath.fromDotSeparatedString(field)); + } + + @Nonnull + public static SumAggregateField sum(@Nonnull FieldPath fieldPath) { + return new SumAggregateField(fieldPath); + } + + @Nonnull + public static AverageAggregateField average(@Nonnull String field) { + return new AverageAggregateField(FieldPath.fromDotSeparatedString(field)); + } + + @Nonnull + public static AverageAggregateField average(@Nonnull FieldPath fieldPath) { + return new AverageAggregateField(fieldPath); + } + + @Nullable FieldPath fieldPath; + + /** Returns the alias used internally for this aggregate field. */ + @Nonnull + String getAlias() { + // Use $operator_$field format if it's an aggregation of a specific field. For example: sum_foo. + // Use $operator format if there's no field. For example: count. + return getOperator() + (fieldPath == null ? "" : "_" + fieldPath.getEncodedPath()); + } + + /** + * Returns the field on which the aggregation takes place. Returns an empty string if there's no + * field (e.g. for count). + */ + @Nonnull + String getFieldPath() { + return fieldPath == null ? "" : fieldPath.getEncodedPath(); + } + + /** Returns a string representation of this aggregation's operator. For example: "sum" */ + abstract @Nonnull String getOperator(); + + /** + * Returns true if the given object is equal to this object. Two `AggregateField` objects are + * considered equal if they have the same operator and operate on the same field. + */ + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof AggregateField)) { + return false; + } + AggregateField otherAggregateField = (AggregateField) other; + return getOperator().equals(otherAggregateField.getOperator()) + && getFieldPath().equals(otherAggregateField.getFieldPath()); + } + + /** Calculates and returns the hash code for this object. */ + @Override + public int hashCode() { + return Objects.hash(getOperator(), getFieldPath()); + } + + public static class SumAggregateField extends AggregateField { + private SumAggregateField(@Nonnull FieldPath field) { + fieldPath = field; + } + + @Override + @Nonnull + public String getOperator() { + return "sum"; + } + } + + public static class AverageAggregateField extends AggregateField { + private AverageAggregateField(@Nonnull FieldPath field) { + fieldPath = field; + } + + @Override + @Nonnull + public String getOperator() { + return "average"; + } + } + + public static class CountAggregateField extends AggregateField { + private CountAggregateField() {} + + @Override + @Nonnull + public String getOperator() { + return "count"; + } + } +} diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java index e99dce925..ca5af9ec0 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java @@ -25,13 +25,10 @@ import com.google.api.gax.rpc.StreamController; import com.google.cloud.Timestamp; import com.google.cloud.firestore.v1.FirestoreSettings; -import com.google.firestore.v1.RunAggregationQueryRequest; -import com.google.firestore.v1.RunAggregationQueryResponse; -import com.google.firestore.v1.RunQueryRequest; -import com.google.firestore.v1.StructuredAggregationQuery; -import com.google.firestore.v1.Value; +import com.google.firestore.v1.*; +import com.google.firestore.v1.StructuredAggregationQuery.Aggregation; import com.google.protobuf.ByteString; -import java.util.Set; +import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -49,8 +46,11 @@ public class AggregateQuery { @Nonnull private final Query query; - AggregateQuery(@Nonnull Query query) { + @Nonnull private List aggregateFieldList; + + AggregateQuery(@Nonnull Query query, @Nonnull List aggregateFields) { this.query = query; + this.aggregateFieldList = aggregateFields; } /** Returns the query whose aggregations will be calculated by this object. */ @@ -112,9 +112,9 @@ long getStartTimeNanos() { return startTimeNanos; } - void deliverResult(long count, Timestamp readTime) { + void deliverResult(@Nonnull Map data, Timestamp readTime) { if (isFutureCompleted.compareAndSet(false, true)) { - future.set(new AggregateQuerySnapshot(AggregateQuery.this, readTime, count)); + future.set(new AggregateQuerySnapshot(AggregateQuery.this, readTime, data)); } } @@ -145,26 +145,13 @@ public void onResponse(RunAggregationQueryResponse response) { // Close the stream to avoid it dangling, since we're not expecting any more responses. streamController.cancel(); - // Extract the count and read time from the RunAggregationQueryResponse. + // Extract the aggregations and read time from the RunAggregationQueryResponse. Timestamp readTime = Timestamp.fromProto(response.getReadTime()); - Value value = response.getResult().getAggregateFieldsMap().get(ALIAS_COUNT); - if (value == null) { - throw new IllegalArgumentException( - "RunAggregationQueryResponse is missing required alias: " + ALIAS_COUNT); - } else if (value.getValueTypeCase() != Value.ValueTypeCase.INTEGER_VALUE) { - throw new IllegalArgumentException( - "RunAggregationQueryResponse alias " - + ALIAS_COUNT - + " has incorrect type: " - + value.getValueTypeCase()); - } - long count = value.getIntegerValue(); // Deliver the result; even though the `RunAggregationQuery` RPC is a "streaming" RPC, meaning - // that `onResponse()` can be called multiple times, it _should_ only be called once for count - // queries. But even if it is called more than once, `responseDeliverer` will drop superfluous - // results. - responseDeliverer.deliverResult(count, readTime); + // that `onResponse()` can be called multiple times, it _should_ only be called once. But even + // if it is called more than once, `responseDeliverer` will drop superfluous results. + responseDeliverer.deliverResult(response.getResult().getAggregateFieldsMap(), readTime); } @Override @@ -215,12 +202,32 @@ RunAggregationQueryRequest toProto(@Nullable final ByteString transactionId) { request.getStructuredAggregationQueryBuilder(); structuredAggregationQuery.setStructuredQuery(runQueryRequest.getStructuredQuery()); - StructuredAggregationQuery.Aggregation.Builder aggregation = - StructuredAggregationQuery.Aggregation.newBuilder(); - aggregation.setCount(StructuredAggregationQuery.Aggregation.Count.getDefaultInstance()); - aggregation.setAlias(ALIAS_COUNT); - structuredAggregationQuery.addAggregations(aggregation); - + // We use a Set here to automatically remove duplicates. + Set aggregations = new HashSet<>(); + for (AggregateField aggregateField : aggregateFieldList) { + // If there's a field for this aggregation, build its proto. + StructuredQuery.FieldReference field = null; + if (!aggregateField.getFieldPath().isEmpty()) { + field = + StructuredQuery.FieldReference.newBuilder() + .setFieldPath(aggregateField.getFieldPath()) + .build(); + } + // Build the aggregation proto. + Aggregation.Builder aggregation = Aggregation.newBuilder(); + if (aggregateField instanceof AggregateField.CountAggregateField) { + aggregation.setCount(Aggregation.Count.getDefaultInstance()); + } else if (aggregateField instanceof AggregateField.SumAggregateField) { + aggregation.setSum(Aggregation.Sum.newBuilder().setField(field).build()); + } else if (aggregateField instanceof AggregateField.AverageAggregateField) { + aggregation.setAvg(Aggregation.Avg.newBuilder().setField(field).build()); + } else { + throw new RuntimeException("Unsupported aggregation"); + } + aggregation.setAlias(aggregateField.getAlias()); + aggregations.add(aggregation.build()); + } + structuredAggregationQuery.addAllAggregations(aggregations); return request.build(); } @@ -243,7 +250,23 @@ public static AggregateQuery fromProto(Firestore firestore, RunAggregationQueryR .setStructuredQuery(proto.getStructuredAggregationQuery().getStructuredQuery()) .build(); Query query = Query.fromProto(firestore, runQueryRequest); - return new AggregateQuery(query); + + List aggregateFields = new ArrayList<>(); + List aggregations = proto.getStructuredAggregationQuery().getAggregationsList(); + aggregations.forEach( + aggregation -> { + if (aggregation.hasCount()) { + aggregateFields.add(AggregateField.count()); + } else if (aggregation.hasAvg()) { + aggregateFields.add( + AggregateField.average(aggregation.getAvg().getField().getFieldPath())); + } else if (aggregation.hasSum()) { + aggregateFields.add(AggregateField.sum(aggregation.getSum().getField().getFieldPath())); + } else { + throw new RuntimeException("Unsupported aggregation."); + } + }); + return new AggregateQuery(query, aggregateFields); } /** @@ -253,7 +276,7 @@ public static AggregateQuery fromProto(Firestore firestore, RunAggregationQueryR */ @Override public int hashCode() { - return query.hashCode(); + return Objects.hash(query, aggregateFieldList); } /** @@ -280,6 +303,6 @@ public boolean equals(Object object) { return false; } AggregateQuery other = (AggregateQuery) object; - return query.equals(other.query); + return query.equals(other.query) && aggregateFieldList.equals(other.aggregateFieldList); } } diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuerySnapshot.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuerySnapshot.java index 5b1a17119..ff7b99906 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuerySnapshot.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuerySnapshot.java @@ -18,8 +18,11 @@ import com.google.api.core.InternalExtensionOnly; import com.google.cloud.Timestamp; +import com.google.firestore.v1.Value; +import java.util.Map; import java.util.Objects; import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** The results of executing an {@link AggregateQuery}. */ @InternalExtensionOnly @@ -27,12 +30,15 @@ public class AggregateQuerySnapshot { @Nonnull private final AggregateQuery query; @Nonnull private final Timestamp readTime; - private final long count; + @Nonnull private final Map data; - AggregateQuerySnapshot(@Nonnull AggregateQuery query, @Nonnull Timestamp readTime, long count) { + AggregateQuerySnapshot( + @Nonnull AggregateQuery query, + @Nonnull Timestamp readTime, + @Nonnull Map data) { this.query = query; this.readTime = readTime; - this.count = count; + this.data = data; } /** Returns the query that was executed to produce this result. */ @@ -49,7 +55,99 @@ public Timestamp getReadTime() { /** Returns the number of documents in the result set of the underlying query. */ public long getCount() { - return count; + AggregateField countField = AggregateField.count(); + Object value = get(countField); + if (value == null) { + throw new IllegalArgumentException( + "RunAggregationQueryResponse alias " + countField.getAlias() + " is null"); + } else if (!(value instanceof Long)) { + throw new IllegalArgumentException( + "RunAggregationQueryResponse alias " + + countField.getAlias() + + " has incorrect type: " + + value.getClass().getName()); + } + return (Long) value; + } + + /** Returns the number of documents in the result set of the underlying query. */ + @SuppressWarnings({"unused"}) + public long get(@Nonnull AggregateField.CountAggregateField unused) { + return getCount(); + } + + /** + * Returns the result of the given aggregation from the server without coercion of data types. + * Throws java.lang.RuntimeException if the `aggregateField` was not requested when calling + * `query.aggregate(...)`. + * + * @param aggregateField The aggregation for which the value is requested. + * @return The result of the given aggregation. + */ + @Nullable + public Object get(@Nonnull AggregateField aggregateField) { + if (!data.containsKey(aggregateField.getAlias())) { + throw new IllegalArgumentException( + "'" + + aggregateField.getOperator() + + "(" + + aggregateField.getFieldPath() + + ")" + + "' was not requested in the aggregation query."); + } + Value value = data.get(aggregateField.getAlias()); + if (value.hasNullValue()) { + return null; + } else if (value.hasDoubleValue()) { + return value.getDoubleValue(); + } else if (value.hasIntegerValue()) { + return value.getIntegerValue(); + } else { + throw new IllegalStateException("Found aggregation result that is not an integer nor double"); + } + } + + /** + * Returns the result of the given average aggregation. Since the result of an average aggregation + * performed by the server is always a double, this convenience overload can be used in lieu of + * the above `get` method. Throws java.lang.RuntimeException if the `aggregateField` was not + * requested when calling `query.aggregate(...)`. + * + * @param averageAggregateField The average aggregation for which the value is requested. + * @return The result of the given average aggregation. + */ + @Nullable + public Double get(@Nonnull AggregateField.AverageAggregateField averageAggregateField) { + return (Double) get((AggregateField) averageAggregateField); + } + + /** + * Returns the result of the given aggregation as a double. Coerces all numeric values and throws + * a RuntimeException if the result of the aggregate is non-numeric. In the case of coercion of + * long to double, uses java.lang.Long.doubleValue to perform the conversion, and may result in a + * loss of precision. + * + * @param aggregateField The aggregation for which the value is requested. + * @return The result of the given average aggregation as a double. + */ + @Nullable + public Double getDouble(@Nonnull AggregateField aggregateField) { + Number result = (Number) get(aggregateField); + return result == null ? null : result.doubleValue(); + } + + /** + * Returns the result of the given aggregation as a long. Coerces all numeric values and throws a + * RuntimeException if the result of the aggregate is non-numeric. In case of coercion of double + * to long, uses java.lang.Double.longValue to perform the conversion. + * + * @param aggregateField The aggregation for which the value is requested. + * @return The result of the given average aggregation as a long. + */ + @Nullable + public Long getLong(@Nonnull AggregateField aggregateField) { + Number result = (Number) get(aggregateField); + return result == null ? null : result.longValue(); } /** @@ -79,7 +177,7 @@ public boolean equals(Object object) { AggregateQuerySnapshot other = (AggregateQuerySnapshot) object; // Don't check `readTime`, because `DocumentSnapshot.equals()` doesn't either. - return query.equals(other.query) && count == other.count; + return query.equals(other.query) && data.equals(other.data); } /** @@ -89,6 +187,6 @@ public boolean equals(Object object) { */ @Override public int hashCode() { - return Objects.hash(query, count); + return Objects.hash(query, data); } } 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 6dd52d8fd..ab8d08a12 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 @@ -61,6 +61,7 @@ import io.opencensus.trace.AttributeValue; import io.opencensus.trace.Tracing; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.Iterator; @@ -1874,7 +1875,28 @@ boolean shouldRetryQuery( */ @Nonnull public AggregateQuery count() { - return new AggregateQuery(this); + return new AggregateQuery(this, Collections.singletonList(AggregateField.count())); + } + + /** + * Calculates the specified aggregations over the documents in the result set of the given query, + * without actually downloading the documents. + * + *

Using this function to perform aggregations is efficient because only the final aggregation + * values, not the documents' data, is downloaded. This function can even perform aggregations of + * the documents if the result set would be prohibitively large to download entirely (e.g. + * thousands of documents). + * + * @return an {@link AggregateQuery} that performs aggregations on the documents in the result set + * of this query. + */ + @Nonnull + public AggregateQuery aggregate( + @Nonnull AggregateField aggregateField1, @Nonnull AggregateField... aggregateFields) { + List aggregateFieldList = new ArrayList<>(); + aggregateFieldList.add(aggregateField1); + aggregateFieldList.addAll(Arrays.asList(aggregateFields)); + return new AggregateQuery(this, aggregateFieldList); } /** diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateFieldTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateFieldTest.java new file mode 100644 index 000000000..3b86b1f96 --- /dev/null +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateFieldTest.java @@ -0,0 +1,93 @@ +/* + * Copyright 2022 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; + +import static com.google.common.truth.Truth.assertThat; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class AggregateFieldTest { + + @Test + public void staticCountCreatesCountAggregateField() { + Object count = AggregateField.count(); + assertThat(count instanceof AggregateField.CountAggregateField).isTrue(); + } + + @Test + public void staticSumCreatesSumAggregateField() { + Object sum = AggregateField.sum("foo"); + assertThat(sum instanceof AggregateField.SumAggregateField).isTrue(); + } + + @Test + public void staticAverageCreatesAverageAggregateField() { + Object average = AggregateField.average("foo"); + assertThat(average instanceof AggregateField.AverageAggregateField).isTrue(); + } + + @Test + public void setsProperFieldPathForSum() { + AggregateField.SumAggregateField sum = AggregateField.sum("foo"); + assertThat(sum.getFieldPath().equals("foo")).isTrue(); + } + + @Test + public void setsProperFieldPathForAverage() { + AggregateField.AverageAggregateField average = AggregateField.average("foo"); + assertThat(average.getFieldPath().equals("foo")).isTrue(); + } + + @Test + public void setsProperOperatorForCount() { + AggregateField.CountAggregateField count = AggregateField.count(); + assertThat(count.getOperator().equals("count")).isTrue(); + } + + @Test + public void setsProperOperatorForSum() { + AggregateField.SumAggregateField sum = AggregateField.sum("foo"); + assertThat(sum.getOperator().equals("sum")).isTrue(); + } + + @Test + public void setsProperOperatorForAverage() { + AggregateField.AverageAggregateField average = AggregateField.average("foo"); + assertThat(average.getOperator().equals("average")).isTrue(); + } + + @Test + public void setsProperFieldPathWithEscapeChars() { + AggregateField.SumAggregateField sum = AggregateField.sum("has`invalid"); + assertThat(sum.getFieldPath().equals("`has\\`invalid`")).isTrue(); + } + + @Test + public void sumSetsProperAliasWithEscapeChars() { + AggregateField.SumAggregateField sum = AggregateField.sum("has`invalid"); + assertThat(sum.getAlias().equals("sum_`has\\`invalid`")).isTrue(); + } + + @Test + public void averageSetsProperAliasWithEscapeChars() { + AggregateField.AverageAggregateField average = AggregateField.average("has`invalid"); + assertThat(average.getAlias().equals("average_`has\\`invalid`")).isTrue(); + } +} diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQuerySnapshotTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQuerySnapshotTest.java index a7f586428..5fd1e822b 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQuerySnapshotTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQuerySnapshotTest.java @@ -19,6 +19,10 @@ import static com.google.common.truth.Truth.assertThat; import com.google.cloud.Timestamp; +import com.google.firestore.v1.Value; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -36,116 +40,125 @@ public class AggregateQuerySnapshotTest { private Timestamp sampleTimestamp; private Timestamp sampleTimestamp2; + private Map data42; + private Map data24; + @Before public void initializeSampleObjects() { - sampleAggregateQuery = new AggregateQuery(mockQuery); - sampleAggregateQuery2 = new AggregateQuery(mockQuery2); + sampleAggregateQuery = + new AggregateQuery(mockQuery, Collections.singletonList(AggregateField.count())); + sampleAggregateQuery2 = + new AggregateQuery(mockQuery2, Collections.singletonList(AggregateField.count())); sampleTimestamp = Timestamp.ofTimeSecondsAndNanos(42, 42); sampleTimestamp2 = Timestamp.ofTimeSecondsAndNanos(24, 24); + data42 = new HashMap<>(); + data42.put("count", Value.newBuilder().setIntegerValue(42).build()); + data24 = new HashMap<>(); + data24.put("count", Value.newBuilder().setIntegerValue(24).build()); } @Test public void getQueryShouldReturnTheAggregateQuerySpecifiedToTheConstructor() { AggregateQuerySnapshot snapshot = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); assertThat(snapshot.getQuery()).isSameInstanceAs(sampleAggregateQuery); } @Test public void getReadTimeShouldReturnTheTimestampSpecifiedToTheConstructor() { AggregateQuerySnapshot snapshot = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); assertThat(snapshot.getReadTime()).isSameInstanceAs(sampleTimestamp); } @Test public void getCountShouldReturnTheCountSpecifiedToTheConstructor() { AggregateQuerySnapshot snapshot = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); assertThat(snapshot.getCount()).isEqualTo(42); } @Test public void hashCodeShouldReturnSameHashCodeWhenConstructedWithSameObjects() { AggregateQuerySnapshot snapshot1 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); AggregateQuerySnapshot snapshot2 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); assertThat(snapshot1.hashCode()).isEqualTo(snapshot2.hashCode()); } @Test public void hashCodeShouldReturnDifferentHashCodeWhenConstructedDifferentAggregateQuery() { AggregateQuerySnapshot snapshot1 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); AggregateQuerySnapshot snapshot2 = - new AggregateQuerySnapshot(sampleAggregateQuery2, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery2, sampleTimestamp, data42); assertThat(snapshot1.hashCode()).isNotEqualTo(snapshot2.hashCode()); } @Test public void hashCodeShouldReturnSameHashCodeWhenConstructedDifferentTimestamp() { AggregateQuerySnapshot snapshot1 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); AggregateQuerySnapshot snapshot2 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp2, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp2, data42); assertThat(snapshot1.hashCode()).isEqualTo(snapshot2.hashCode()); } @Test public void hashCodeShouldReturnDifferentHashCodeWhenConstructedDifferentCount() { AggregateQuerySnapshot snapshot1 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); AggregateQuerySnapshot snapshot2 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 24); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data24); assertThat(snapshot1.hashCode()).isNotEqualTo(snapshot2.hashCode()); } @Test public void equalsShouldReturnFalseWhenGivenNull() { AggregateQuerySnapshot snapshot = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); assertThat(snapshot.equals(null)).isFalse(); } @Test public void equalsShouldReturnFalseWhenGivenADifferentObject() { AggregateQuerySnapshot snapshot = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); assertThat(snapshot.equals("Not An AggregateQuerySnapshot")).isFalse(); } @Test public void equalsShouldReturnFalseWhenGivenAnAggregateQuerySnapshotWithADifferentQuery() { AggregateQuerySnapshot snapshot1 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); AggregateQuerySnapshot snapshot2 = - new AggregateQuerySnapshot(sampleAggregateQuery2, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery2, sampleTimestamp, data42); assertThat(snapshot1.equals(snapshot2)).isFalse(); } @Test public void equalsShouldReturnTrueWhenGivenAnAggregateQuerySnapshotWithADifferentReadTime() { AggregateQuerySnapshot snapshot1 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); AggregateQuerySnapshot snapshot2 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp2, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp2, data42); assertThat(snapshot1.equals(snapshot2)).isTrue(); } @Test public void equalsShouldReturnFalseWhenGivenAnAggregateQuerySnapshotWithADifferentCount() { AggregateQuerySnapshot snapshot1 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); AggregateQuerySnapshot snapshot2 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 24); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data24); assertThat(snapshot1.equals(snapshot2)).isFalse(); } @Test public void equalsShouldReturnTrueWhenGivenTheSameAggregateQuerySnapshotInstance() { AggregateQuerySnapshot snapshot = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); assertThat(snapshot.equals(snapshot)).isTrue(); } @@ -153,9 +166,9 @@ public void equalsShouldReturnTrueWhenGivenTheSameAggregateQuerySnapshotInstance public void equalsShouldReturnTrueWhenGivenAnAggregateQuerySnapshotConstructedWithTheSameArguments() { AggregateQuerySnapshot snapshot1 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); AggregateQuerySnapshot snapshot2 = - new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, 42); + new AggregateQuerySnapshot(sampleAggregateQuery, sampleTimestamp, data42); assertThat(snapshot1.equals(snapshot2)).isTrue(); } } diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQueryTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQueryTest.java index d7e513474..80c758bd6 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQueryTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQueryTest.java @@ -16,10 +16,15 @@ package com.google.cloud.firestore; +import static com.google.cloud.firestore.AggregateField.*; import static com.google.common.truth.Truth.assertThat; +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; import static org.mockito.Mockito.mock; import com.google.cloud.firestore.spi.v1.FirestoreRpc; +import java.util.List; +import java.util.Objects; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -33,45 +38,73 @@ public class AggregateQueryTest { @Test public void getQueryShouldReturnTheQuerySpecifiedToTheConstructor() { - AggregateQuery aggregateQuery = new AggregateQuery(mockQuery); + AggregateQuery aggregateQuery = new AggregateQuery(mockQuery, singletonList(count())); assertThat(aggregateQuery.getQuery()).isSameInstanceAs(mockQuery); } @Test - public void hashCodeShouldReturnHashCodeOfUnderlyingQuery() { - AggregateQuery aggregateQuery = new AggregateQuery(mockQuery); - assertThat(aggregateQuery.hashCode()).isEqualTo(mockQuery.hashCode()); + public void hashCodeShouldReturnHashCodeOfUnderlyingQueryAndAggregateFieldList() { + List list = asList(count(), sum("foo"), average("bar")); + AggregateQuery aggregateQuery = new AggregateQuery(mockQuery, list); + assertThat(aggregateQuery.hashCode()).isEqualTo(Objects.hash(mockQuery, list)); } @Test public void equalsShouldReturnFalseWhenGivenNull() { - AggregateQuery aggregateQuery = new AggregateQuery(mockQuery); + AggregateQuery aggregateQuery = + new AggregateQuery(mockQuery, asList(count(), sum("foo"), average("bar"))); assertThat(aggregateQuery.equals(null)).isFalse(); } @Test public void equalsShouldReturnFalseWhenGivenADifferentObject() { - AggregateQuery aggregateQuery = new AggregateQuery(mockQuery); + AggregateQuery aggregateQuery = + new AggregateQuery(mockQuery, asList(count(), sum("foo"), average("bar"))); assertThat(aggregateQuery.equals("Not An AggregateQuery")).isFalse(); } @Test public void equalsShouldReturnFalseWhenGivenAnAggregateQueryWithADifferentQuery() { - AggregateQuery aggregateQuery1 = new AggregateQuery(mockQuery); - AggregateQuery aggregateQuery2 = new AggregateQuery(mockQuery2); + AggregateQuery aggregateQuery1 = new AggregateQuery(mockQuery, singletonList(count())); + AggregateQuery aggregateQuery2 = new AggregateQuery(mockQuery2, singletonList(count())); + assertThat(aggregateQuery1.equals(aggregateQuery2)).isFalse(); + } + + @Test + public void equalsShouldReturnFalseWhenGivenAnAggregateQueryWithDifferentAggregations() { + AggregateQuery aggregateQuery1 = new AggregateQuery(mockQuery, singletonList(count())); + AggregateQuery aggregateQuery2 = new AggregateQuery(mockQuery, singletonList(sum("foo"))); + assertThat(aggregateQuery1.equals(aggregateQuery2)).isFalse(); + } + + @Test + public void equalsShouldReturnFalseWhenGivenAnAggregateQueryWithDifferentAggregationOrder() { + AggregateQuery aggregateQuery1 = + new AggregateQuery(mockQuery, asList(sum("foo"), average("bar"))); + AggregateQuery aggregateQuery2 = + new AggregateQuery(mockQuery, asList(average("bar"), sum("foo"))); assertThat(aggregateQuery1.equals(aggregateQuery2)).isFalse(); } @Test public void equalsShouldReturnTrueWhenGivenTheSameAggregateQueryInstance() { - AggregateQuery aggregateQuery = new AggregateQuery(mockQuery); + AggregateQuery aggregateQuery = new AggregateQuery(mockQuery, singletonList(count())); assertThat(aggregateQuery.equals(aggregateQuery)).isTrue(); } @Test public void equalsShouldReturnTrueWhenGivenAnAggregateQueryWithTheSameQuery() { - AggregateQuery aggregateQuery1 = new AggregateQuery(mockQuery); - AggregateQuery aggregateQuery2 = new AggregateQuery(mockQuery); + AggregateQuery aggregateQuery1 = new AggregateQuery(mockQuery, singletonList(count())); + AggregateQuery aggregateQuery2 = new AggregateQuery(mockQuery, singletonList(count())); + assertThat(aggregateQuery1.equals(aggregateQuery2)).isTrue(); + } + + @Test + public void equalsShouldReturnTrueWhenGivenMultipleAggregationsWithTheSameQuery() { + AggregateQuery aggregateQuery1 = + new AggregateQuery(mockQuery, asList(count(), sum("foo"), average("bar"))); + AggregateQuery aggregateQuery2 = + new AggregateQuery(mockQuery, asList(count(), sum("foo"), average("bar"))); assertThat(aggregateQuery1.equals(aggregateQuery2)).isTrue(); } diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java new file mode 100644 index 000000000..9d9b5d3aa --- /dev/null +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java @@ -0,0 +1,815 @@ +package com.google.cloud.firestore.it; + +import static com.google.cloud.firestore.AggregateField.average; +import static com.google.cloud.firestore.AggregateField.sum; +import static com.google.cloud.firestore.LocalFirestoreHelper.autoId; +import static com.google.cloud.firestore.LocalFirestoreHelper.map; +import static com.google.cloud.firestore.it.TestHelper.await; +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.assertThrows; +import static org.junit.Assume.assumeFalse; + +import com.google.cloud.firestore.*; +import com.google.common.base.Preconditions; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class ITQueryAggregationsTest { + private Firestore firestore; + + @Before + public void setUpFirestore() { + // TODO: stop using emulator for these tests once prod is ready. + firestore = FirestoreOptions.newBuilder().setHost("localhost:8080").build().getService(); + Preconditions.checkNotNull( + firestore, + "Error instantiating Firestore. Check that the service account credentials were properly set."); + } + + @After + public void tearDownFirestore() throws Exception { + if (firestore != null) { + firestore.close(); + firestore = null; + } + } + + private CollectionReference testCollection() { + String collectionPath = "java-" + autoId(); + return firestore.collection(collectionPath); + } + + private CollectionReference testCollection(String name) { + return firestore.collection("java-" + name + "-" + autoId()); + } + + private CollectionReference testCollectionWithDocs(Map> docs) + throws InterruptedException { + CollectionReference collection = testCollection(); + CollectionReference writer = firestore.collection(collection.getId()); + writeAllDocs(writer, docs); + return collection; + } + + public static void writeAllDocs( + CollectionReference collection, Map> docs) + throws InterruptedException { + for (Map.Entry> doc : docs.entrySet()) { + await(collection.document(doc.getKey()).set(doc.getValue())); + } + } + + private static Map> testDocs1 = + map( + "a", + map("author", "authorA", "title", "titleA", "pages", 100, "foo", 1, "bar", 2, "baz", 3), + "b", + map("author", "authorB", "title", "titleB", "pages", 50, "foo", 1, "bar", 2, "baz", 3)); + + @Test + public void canRunCountUsingAggregationMethod() throws Exception { + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateQuerySnapshot snapshot = collection.aggregate(AggregateField.count()).get().get(); + assertThat(snapshot.getCount()).isEqualTo(2); + } + + @Test + public void canGetDuplicateAggregations() throws Exception { + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateQuerySnapshot snapshot = + collection + .aggregate(AggregateField.count(), AggregateField.count(), sum("pages"), sum("pages")) + .get() + .get(); + assertThat(snapshot.getCount()).isEqualTo(2); + assertThat(snapshot.get(sum("pages"))).isEqualTo(150); + } + + @Test + public void aggregateErrorMessageIfIndexIsMissing() throws Exception { + assumeFalse( + "Skip this test when running against the emulator because it does not require composite index creation.", + isRunningAgainstFirestoreEmulator(firestore)); + + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateQuery aggregateQuery = + collection + .whereEqualTo("key1", 42) + .whereLessThan("key2", 42) + .aggregate(AggregateField.count()); + ExecutionException executionException = + assertThrows(ExecutionException.class, () -> aggregateQuery.get().get()); + assertThat(executionException) + .hasCauseThat() + .hasMessageThat() + .containsMatch("/index.*https:\\/\\/console\\.firebase\\.google\\.com/"); + } + + @Test + public void canRunSumQuery() throws Exception { + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("pages")).get().get(); + assertThat(snapshot.get(sum("pages"))).isEqualTo(150); + } + + @Test + public void canRunAverageQuery() throws Exception { + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateQuerySnapshot snapshot = collection.aggregate(average("pages")).get().get(); + assertThat(snapshot.get(average("pages"))).isEqualTo(75.0); + } + + @Test + public void canGetMultipleAggregationsInTheSameQuery() throws Exception { + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateQuerySnapshot snapshot = + collection.aggregate(sum("pages"), average("pages"), AggregateField.count()).get().get(); + assertThat(snapshot.get(sum("pages"))).isEqualTo(150); + assertThat(snapshot.get(average("pages"))).isEqualTo(75.0); + assertThat(snapshot.get(AggregateField.count())).isEqualTo(2); + } + + @Test + public void getCorrectTypeForSumLong() throws Exception { + Map> testDocs = map("a", map("foo", 100), "b", map("foo", 100)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("foo")).get().get(); + Object sum = snapshot.get(sum("foo")); + assertThat(sum instanceof Long).isTrue(); + } + + @Test + public void getCorrectTypeForSumDouble() throws Exception { + Map> testDocs = map("a", map("foo", 100.5), "b", map("foo", 100)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("foo")).get().get(); + Object sum = snapshot.get(sum("foo")); + assertThat(sum instanceof Double).isTrue(); + } + + @Test + public void getCorrectTypeForSumNaN() throws Exception { + Map> testDocs = + map("a", map("foo", 100.5), "b", map("foo", Double.NaN)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("foo")).get().get(); + Object sum = snapshot.get(sum("foo")); + assertThat(sum instanceof Double).isTrue(); + assertThat(sum.equals(Double.NaN)); + } + + @Test + public void getCorrectTypeForAverageDouble() throws Exception { + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateQuerySnapshot snapshot = collection.aggregate(average("pages")).get().get(); + Object average = snapshot.get((AggregateField) average("pages")); + assertThat(average instanceof Double).isTrue(); + } + + @Test + public void getCorrectTypeForAverageNaN() throws Exception { + Map> testDocs = + map("a", map("foo", 100.5), "b", map("foo", Double.NaN)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("foo")).get().get(); + Object sum = snapshot.get(average("foo")); + assertThat(sum instanceof Double).isTrue(); + assertThat(sum.equals(Double.NaN)); + } + + @Test + public void getCorrectTypeForAverageNull() throws Exception { + CollectionReference collection = testCollection(); + AggregateQuerySnapshot snapshot = collection.aggregate(average("bar")).get().get(); + Object sum = snapshot.get(average("bar")); + assertThat(sum == null).isTrue(); + } + + @Test + public void canPerformMaxAggregations() throws Exception { + // TODO: Update this test once aggregate de-duplication is implemented and more aggregation + // types are available. + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateField f1 = sum("pages"); + AggregateField f2 = average("pages"); + AggregateField f3 = AggregateField.count(); + AggregateField f4 = sum("foo"); + AggregateField f5 = sum("bar"); + AggregateQuerySnapshot snapshot = collection.aggregate(f1, f2, f3, f4, f5).get().get(); + assertThat(snapshot.get(f1)).isEqualTo(150); + assertThat(snapshot.get(f2)).isEqualTo(75.0); + assertThat(snapshot.get(f3)).isEqualTo(2); + assertThat(snapshot.get(f4)).isEqualTo(2); + assertThat(snapshot.get(f5)).isEqualTo(4); + } + + @Test + public void cannotPerformMoreThanMaxAggregations() throws Exception { + // TODO: Update this test once aggregate de-duplication is implemented and more aggregation + // types are available. + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateField f1 = sum("pages"); + AggregateField f2 = average("pages"); + AggregateField f3 = AggregateField.count(); + AggregateField f4 = sum("foo"); + AggregateField f5 = sum("bar"); + AggregateField f6 = sum("baz"); + Exception exception = null; + try { + collection.aggregate(f1, f2, f3, f4, f5, f6).get().get(); + } catch (Exception e) { + exception = e; + } + assertThat(exception).isNotNull(); + assertThat(exception.getMessage()).contains("maximum number of aggregations"); + } + + @Test + public void aggregateQueriesSupportCollectionGroups() throws Exception { + String collectionGroupId = "myColGroupId" + autoId(); + Map data = map("x", 2); + // Setting documents at the following paths: + // `${collectionGroupId}/cg-doc1`, + // `abc/123/${collectionGroupId}/cg-doc2`, + // `zzz${collectionGroupId}/cg-doc3`, + // `abc/123/zzz${collectionGroupId}/cg-doc4`, + // `abc/123/zzz/${collectionGroupId}` + await(firestore.collection(collectionGroupId).document("cg-doc1").set(data)); + await( + firestore + .collection("abc") + .document("123") + .collection(collectionGroupId) + .document("cg-doc2") + .set(data)); + await(firestore.collection("zzz" + collectionGroupId).document("cg-doc3").set(data)); + await( + firestore + .collection("abc") + .document("123") + .collection("zzz" + collectionGroupId) + .document("cg-doc4") + .set(data)); + await( + firestore + .collection("abc") + .document("123") + .collection("zzz") + .document(collectionGroupId) + .set(data)); + CollectionGroup collectionGroup = firestore.collectionGroup(collectionGroupId); + AggregateQuerySnapshot snapshot = + collectionGroup.aggregate(AggregateField.count(), sum("x"), average("x")).get().get(); + assertThat(snapshot.get(AggregateField.count())).isEqualTo(2); + assertThat(snapshot.get(sum("x"))).isEqualTo(4); + assertThat(snapshot.get(average("x"))).isEqualTo(2); + } + + @Test + public void performsAggregationsOnDocumentsWithAllaggregatedFields() throws Exception { + Map> testDocs = + map( + "a", + map("author", "authorA", "title", "titleA", "pages", 100, "year", 1980), + "b", + map("author", "authorB", "title", "titleB", "pages", 50, "year", 2020), + "c", + map("author", "authorC", "title", "titleC", "pages", 150, "year", 2021), + "d", + map("author", "authorD", "title", "titleD", "pages", 50)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = + collection + .aggregate(sum("pages"), average("pages"), average("year"), AggregateField.count()) + .get() + .get(); + assertThat(snapshot.get(sum("pages"))).isEqualTo(300); + assertThat(snapshot.get(average("pages"))).isEqualTo(100); + assertThat(snapshot.get(average("year"))).isEqualTo(2007); + assertThat(snapshot.get(AggregateField.count())).isEqualTo(3); + } + + @Test + public void performsAggregationsWhenNaNExistsForSomeFieldValues() throws Exception { + Map> testDocs = + map( + "a", + map( + "author", "authorA", "title", "titleA", "pages", 100, "year", 1980, "rating", + 5), + "b", + map("author", "authorB", "title", "titleB", "pages", 50, "year", 2020, "rating", 4), + "c", + map( + "author", + "authorC", + "title", + "titleC", + "pages", + 100, + "year", + 1980, + "rating", + Double.NaN), + "d", + map( + "author", "authorD", "title", "titleD", "pages", 50, "year", 2020, "rating", + 0)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = + collection.aggregate(sum("rating"), sum("pages"), average("year")).get().get(); + assertThat(snapshot.get(sum("rating"))).isEqualTo(Double.NaN); + assertThat(snapshot.get(sum("pages"))).isEqualTo(300); + assertThat(snapshot.get(average("year"))).isEqualTo(2000); + } + + @Test + public void throwsAnErrorWhenGettingTheResultOfAnUnrequestedAggregation() throws Exception { + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("pages")).get().get(); + Exception exception = null; + try { + snapshot.get(average("pages")); + } catch (Exception e) { + exception = e; + } + assertThat(exception).isNotNull(); + assertThat(exception.getMessage()) + .isEqualTo("'average(pages)' was not requested in the aggregation query."); + exception = null; + try { + snapshot.get(sum("foo")); + } catch (RuntimeException e) { + exception = e; + } + assertThat(exception).isNotNull(); + assertThat(exception.getMessage()) + .isEqualTo("'sum(foo)' was not requested in the aggregation query."); + } + + @Test + public void performsAggregationWhenUsingInOperator() throws Exception { + Map> testDocs = + map( + "a", + map( + "author", "authorA", "title", "titleA", "pages", 100, "year", 1980, "rating", + 5), + "b", + map("author", "authorB", "title", "titleB", "pages", 50, "year", 2020, "rating", 4), + "c", + map( + "author", "authorC", "title", "titleC", "pages", 100, "year", 1980, "rating", + 3), + "d", + map( + "author", "authorD", "title", "titleD", "pages", 50, "year", 2020, "rating", + 0)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = + collection + .whereIn("rating", asList(5, 3)) + .aggregate( + sum("rating"), + average("rating"), + sum("pages"), + average("pages"), + AggregateField.count()) + .get() + .get(); + assertThat(snapshot.get(sum("rating"))).isEqualTo(8); + assertThat(snapshot.get(average("rating"))).isEqualTo(4); + assertThat(snapshot.get(sum("pages"))).isEqualTo(200); + assertThat(snapshot.get(average("pages"))).isEqualTo(100); + assertThat(snapshot.get(AggregateField.count())).isEqualTo(2); + } + + @Test + public void performsAggregationWhenUsingArrayContainsAnyOperator() throws Exception { + Map> testDocs = + map( + "a", + map( + "author", + "authorA", + "title", + "titleA", + "pages", + 100, + "year", + 1980, + "rating", + asList(5, 1000)), + "b", + map( + "author", "authorB", "title", "titleB", "pages", 50, "year", 2020, "rating", + asList(4)), + "c", + map( + "author", + "authorC", + "title", + "titleC", + "pages", + 100, + "year", + 1980, + "rating", + asList(2222, 3)), + "d", + map( + "author", "authorD", "title", "titleD", "pages", 50, "year", 2020, "rating", + asList(0))); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = + collection + .whereArrayContainsAny("rating", asList(5, 3)) + .aggregate( + sum("rating"), + average("rating"), + sum("pages"), + average("pages"), + AggregateField.count()) + .get() + .get(); + assertThat(snapshot.get(sum("rating"))).isEqualTo(0); + assertThat(snapshot.get(average("rating"))).isEqualTo(null); + assertThat(snapshot.get(sum("pages"))).isEqualTo(200); + assertThat(snapshot.get(average("pages"))).isEqualTo(100); + assertThat(snapshot.get(AggregateField.count())).isEqualTo(2); + } + + @Test + public void performsAggregationsOnNestedMapValues() throws Exception { + Map> testDocs = + map( + "a", + map( + "author", + "authorA", + "title", + "titleA", + "metadata", + map("pages", 100, "rating", map("critic", 2, "user", 5))), + "b", + map( + "author", + "authorB", + "title", + "titleB", + "metadata", + map("pages", 50, "rating", map("critic", 4, "user", 4)))); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = + collection + .aggregate( + sum("metadata.pages"), + average("metadata.pages"), + average("metadata.rating.critic"), + sum("metadata.rating.user"), + AggregateField.count()) + .get() + .get(); + assertThat(snapshot.get(sum("metadata.pages"))).isEqualTo(150); + assertThat(snapshot.get(average("metadata.pages"))).isEqualTo(75); + assertThat(snapshot.get(average("metadata.rating.critic"))).isEqualTo(3); + assertThat(snapshot.get(sum("metadata.rating.user"))).isEqualTo(9); + assertThat(snapshot.get(AggregateField.count())).isEqualTo(2); + } + + @Test + public void performsSumThatResultsInFloat() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 5), + "b", map("author", "authorB", "title", "titleB", "rating", 4.5), + "c", map("author", "authorC", "title", "titleC", "rating", 3)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); + Object sum = snapshot.get(sum("rating")); + assertThat(sum instanceof Double).isTrue(); + assertThat(sum).isEqualTo(12.5); + } + + @Test + public void performsSumOfIntsAndFloatsThatResultsInInt() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 5), + "b", map("author", "authorB", "title", "titleB", "rating", 4.5), + "c", map("author", "authorC", "title", "titleC", "rating", 3.5)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); + Object sum = snapshot.get(sum("rating")); + assertThat(sum instanceof Long).isTrue(); + assertThat(sum).isEqualTo(13); + } + + @Test + public void performsSumThatOverflowsMaxLong() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", Long.MAX_VALUE), + "b", map("author", "authorB", "title", "titleB", "rating", Long.MAX_VALUE)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); + Object sum = snapshot.get(sum("rating")); + assertThat(sum instanceof Double).isTrue(); + assertThat(sum).isEqualTo((double) Long.MAX_VALUE + (double) Long.MAX_VALUE); + } + + @Test + public void performsSumThatCanOverflowIntegerValuesDuringAccumulation() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", Long.MAX_VALUE), + "b", map("author", "authorB", "title", "titleB", "rating", 1), + "c", map("author", "authorC", "title", "titleC", "rating", -101)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); + Object sum = snapshot.get(sum("rating")); + assertThat(sum instanceof Long).isTrue(); + assertThat(sum).isEqualTo(Long.MAX_VALUE - 100); + } + + @Test + public void performsSumThatIsNegative() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", Long.MAX_VALUE), + "b", map("author", "authorB", "title", "titleB", "rating", -Long.MAX_VALUE), + "c", map("author", "authorC", "title", "titleC", "rating", -101), + "d", map("author", "authorD", "title", "titleD", "rating", -10000)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); + assertThat(snapshot.get(sum("rating"))).isEqualTo(-10101); + } + + @Test + public void performsSumThatIsPositiveInfinity() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", Double.MAX_VALUE), + "b", map("author", "authorB", "title", "titleB", "rating", Double.MAX_VALUE)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); + Object sum = snapshot.get(sum("rating")); + assertThat(sum instanceof Double).isTrue(); + assertThat(sum).isEqualTo(Double.POSITIVE_INFINITY); + assertThat(snapshot.getDouble(sum("rating"))).isEqualTo(Double.POSITIVE_INFINITY); + assertThat(snapshot.getLong(sum("rating"))).isEqualTo(Long.MAX_VALUE); + } + + @Test + public void performsSumThatIsNegativeInfinity() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", -Double.MAX_VALUE), + "b", map("author", "authorB", "title", "titleB", "rating", -Double.MAX_VALUE)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); + Object sum = snapshot.get(sum("rating")); + assertThat(sum instanceof Double).isTrue(); + assertThat(sum).isEqualTo(Double.NEGATIVE_INFINITY); + assertThat(snapshot.getDouble(sum("rating"))).isEqualTo(Double.NEGATIVE_INFINITY); + assertThat(snapshot.getLong(sum("rating"))).isEqualTo(Long.MIN_VALUE); + } + + @Test + public void performsSumThatIsValidButCouldOverflowDuringAggregation() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", Double.MAX_VALUE), + "b", map("author", "authorB", "title", "titleB", "rating", Double.MAX_VALUE), + "c", map("author", "authorC", "title", "titleC", "rating", -Double.MAX_VALUE), + "d", map("author", "authorD", "title", "titleD", "rating", -Double.MAX_VALUE), + "e", map("author", "authorE", "title", "titleE", "rating", Double.MAX_VALUE), + "f", map("author", "authorF", "title", "titleF", "rating", -Double.MAX_VALUE), + "g", map("author", "authorG", "title", "titleG", "rating", -Double.MAX_VALUE), + "h", map("author", "authorH", "title", "titleH", "rating", Double.MAX_VALUE)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); + Object sum = snapshot.get(sum("rating")); + assertThat(sum instanceof Long).isTrue(); + assertThat(sum).isEqualTo(0); + } + + @Test + public void performsSumThatIncludesNaN() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 5), + "b", map("author", "authorB", "title", "titleB", "rating", 4), + "c", map("author", "authorC", "title", "titleC", "rating", Double.NaN), + "d", map("author", "authorD", "title", "titleD", "rating", 0)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); + assertThat(snapshot.get(sum("rating"))).isEqualTo(Double.NaN); + } + + @Test + public void performsSumOverResultSetOfZeroDocuments() throws Exception { + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateQuerySnapshot snapshot = + collection.whereGreaterThan("pages", 200).aggregate(sum("pages")).get().get(); + assertThat(snapshot.get(sum("pages"))).isEqualTo(0); + } + + @Test + public void performsSumOnlyOnNumericFields() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 5), + "b", map("author", "authorB", "title", "titleB", "rating", 4), + "c", map("author", "authorC", "title", "titleC", "rating", "3"), + "d", map("author", "authorD", "title", "titleD", "rating", 1)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = + collection.aggregate(sum("rating"), AggregateField.count()).get().get(); + assertThat(snapshot.get(sum("rating"))).isEqualTo(10); + assertThat(snapshot.get(AggregateField.count())).isEqualTo(4); + } + + @Test + public void performsSumOfMinIEEE754() throws Exception { + Map> testDocs = + map("a", map("author", "authorA", "title", "titleA", "rating", Double.MIN_VALUE)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); + assertThat(snapshot.get(sum("rating"))).isEqualTo(Double.MIN_VALUE); + } + + @Test + public void performsAverageOfIntsThatResultsInAnInt() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 10), + "b", map("author", "authorB", "title", "titleB", "rating", 5), + "c", map("author", "authorC", "title", "titleC", "rating", 0)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("rating")).get().get(); + assertThat(snapshot.get(average("rating"))).isEqualTo(5); + assertThat(snapshot.getLong(average("rating"))).isEqualTo(5L); + assertThat(snapshot.getDouble(average("rating"))).isEqualTo(5.0); + } + + @Test + public void performsAverageOfFloatsThatResultsInAnInt() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 10.5), + "b", map("author", "authorB", "title", "titleB", "rating", 9.5)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("rating")).get().get(); + assertThat(snapshot.get(average("rating")) instanceof Double).isTrue(); + assertThat(snapshot.get(average("rating"))).isEqualTo(10); + assertThat(snapshot.getLong(average("rating"))).isEqualTo(10L); + assertThat(snapshot.getDouble(average("rating"))).isEqualTo(10.0); + } + + @Test + public void performsAverageOfFloatsAndIntsThatResultsInAnInt() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 10), + "b", map("author", "authorB", "title", "titleB", "rating", 9.5), + "c", map("author", "authorC", "title", "titleC", "rating", 10.5)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("rating")).get().get(); + assertThat(snapshot.get(average("rating"))).isEqualTo(10); + assertThat(snapshot.getLong(average("rating"))).isEqualTo(10L); + assertThat(snapshot.getDouble(average("rating"))).isEqualTo(10.0); + } + + @Test + public void performsAverageOfFloatsThatResultsInAFloat() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 5.5), + "b", map("author", "authorB", "title", "titleB", "rating", 4.5), + "c", map("author", "authorC", "title", "titleC", "rating", 3.5)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("rating")).get().get(); + assertThat(snapshot.get(average("rating"))).isEqualTo(4.5); + assertThat(snapshot.getDouble(average("rating"))).isEqualTo(4.5); + assertThat(snapshot.getLong(average("rating"))).isEqualTo(4L); + } + + @Test + public void performsAverageOfFloatsAndIntsThatResultsInAFloat() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 8.6), + "b", map("author", "authorB", "title", "titleB", "rating", 9), + "c", map("author", "authorC", "title", "titleC", "rating", 10)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("rating")).get().get(); + assertThat(snapshot.get(average("rating"))).isEqualTo(27.6 / 3); + assertThat(snapshot.getDouble(average("rating"))).isEqualTo(27.6 / 3); + assertThat(snapshot.getLong(average("rating"))).isEqualTo(9L); + } + + @Test + public void performsAverageOfIntsThatResultsInAFloat() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 10), + "b", map("author", "authorB", "title", "titleB", "rating", 9)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("rating")).get().get(); + assertThat(snapshot.get(average("rating"))).isEqualTo(9.5); + assertThat(snapshot.getDouble(average("rating"))).isEqualTo(9.5d); + assertThat(snapshot.getLong(average("rating"))).isEqualTo(9L); + } + + @Test + public void performsAverageCausingUnderflow() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", Double.MIN_VALUE), + "b", map("author", "authorB", "title", "titleB", "rating", 0)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("rating")).get().get(); + assertThat(snapshot.get(average("rating"))).isEqualTo(0); + assertThat(snapshot.getDouble(average("rating"))).isEqualTo(0.0d); + assertThat(snapshot.getLong(average("rating"))).isEqualTo(0L); + } + + @Test + public void performsAverageOfMinIEEE754() throws Exception { + Map> testDocs = + map("a", map("author", "authorA", "title", "titleA", "rating", Double.MIN_VALUE)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("rating")).get().get(); + assertThat(snapshot.get(average("rating"))).isEqualTo(Double.MIN_VALUE); + assertThat(snapshot.getDouble(average("rating"))).isEqualTo(Double.MIN_VALUE); + assertThat(snapshot.getLong(average("rating"))).isEqualTo(0); + } + + @Test + public void performsAverageThatCouldOverflowIEEE754DuringAccumulation() throws Exception { + Map> testDocs = + map( + "a", + map("author", "authorA", "title", "titleA", "rating", Double.MAX_VALUE), + "b", + map("author", "authorB", "title", "titleB", "rating", Double.MAX_VALUE)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("rating")).get().get(); + assertThat(snapshot.get(average("rating"))).isEqualTo(Double.POSITIVE_INFINITY); + assertThat(snapshot.getDouble(average("rating"))).isEqualTo(Double.POSITIVE_INFINITY); + assertThat(snapshot.getLong(average("rating"))).isEqualTo(Long.MAX_VALUE); + } + + @Test + public void performsAverageThatIncludesNaN() throws Exception { + Map> testDocs = + map( + "a", + map("author", "authorA", "title", "titleA", "rating", 5), + "b", + map("author", "authorB", "title", "titleB", "rating", 4), + "c", + map("author", "authorC", "title", "titleC", "rating", Double.NaN), + "d", + map("author", "authorD", "title", "titleD", "rating", 0)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = collection.aggregate(average("rating")).get().get(); + assertThat(snapshot.get(average("rating"))).isEqualTo(Double.NaN); + assertThat(snapshot.getDouble(average("rating"))).isEqualTo(Double.NaN); + assertThat(snapshot.getLong(average("rating"))).isEqualTo(0L); + } + + @Test + public void performsAverageOverResultSetOfZeroDocuments() throws Exception { + CollectionReference collection = testCollectionWithDocs(testDocs1); + AggregateQuerySnapshot snapshot = + collection.whereGreaterThan("pages", 200).aggregate(average("pages")).get().get(); + assertThat(snapshot.get(average("pages"))).isEqualTo(null); + assertThat(snapshot.getDouble(average("pages"))).isEqualTo(null); + assertThat(snapshot.getLong(average("pages"))).isEqualTo(null); + } + + @Test + public void performsAverageOnlyOnNumericFields() throws Exception { + Map> testDocs = + map( + "a", map("author", "authorA", "title", "titleA", "rating", 5), + "b", map("author", "authorB", "title", "titleB", "rating", 4), + "c", map("author", "authorC", "title", "titleC", "rating", "3"), + "d", map("author", "authorD", "title", "titleD", "rating", 6)); + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = + collection.aggregate(average("rating"), AggregateField.count()).get().get(); + assertThat(snapshot.get(average("rating"))).isEqualTo(5); + assertThat(snapshot.get(AggregateField.count())).isEqualTo(4); + } +} 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 index b825ac86e..63f3a0d82 100644 --- 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.await; import static com.google.cloud.firestore.it.TestHelper.isRunningAgainstFirestoreEmulator; import static com.google.common.truth.Truth.assertThat; import static java.util.Collections.singletonMap; @@ -26,6 +27,7 @@ import com.google.api.core.ApiFuture; import com.google.auto.value.AutoValue; import com.google.cloud.Timestamp; +import com.google.cloud.firestore.AggregateField; import com.google.cloud.firestore.AggregateQuery; import com.google.cloud.firestore.AggregateQuerySnapshot; import com.google.cloud.firestore.CollectionGroup; @@ -40,9 +42,6 @@ import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.atomic.AtomicBoolean; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TestName; @@ -133,6 +132,13 @@ public void countShouldReturnNumberOfDocumentsForCollectionGroups() throws Excep assertThat(snapshot.getCount()).isEqualTo(13); } + @Test + public void aggregateQuerySupportsCollectionGroups() throws Exception { + CollectionGroup collectionGroup = createCollectionGroupWithDocuments(13); + AggregateQuerySnapshot snapshot = collectionGroup.aggregate(AggregateField.count()).get().get(); + assertThat(snapshot.getCount()).isEqualTo(13); + } + @Test public void countShouldReturnNumberOfDocumentsForPartitionQuery() throws Exception { CollectionReference collection = createCollectionWithDocuments(3).collection(); @@ -150,6 +156,15 @@ public void inFlightCountQueriesShouldCompleteSuccessfullyWhenFirestoreIsClosed( assertThat(task.get().getCount()).isEqualTo(20); } + @Test + public void inFlightAggregateQueriesShouldCompleteSuccessfullyWhenFirestoreIsClosed() + throws Exception { + CollectionReference collection = createCollectionWithDocuments(20).collection(); + ApiFuture task = collection.aggregate(AggregateField.count()).get(); + collection.getFirestore().close(); + assertThat(task.get().getCount()).isEqualTo(20); + } + @Test public void inFlightCountQueriesShouldCompleteSuccessfullyWhenFirestoreIsShutDownGracefully() throws Exception { @@ -160,10 +175,20 @@ public void inFlightCountQueriesShouldCompleteSuccessfullyWhenFirestoreIsShutDow } @Test - public void inFlightCountQueriesShouldRunToCompletionWhenFirestoreIsShutDownForcefully() + public void + inFlightAggregationQueriesShouldCompleteSuccessfullyWhenFirestoreIsShutDownGracefully() + throws Exception { + CollectionReference collection = createCollectionWithDocuments(20).collection(); + ApiFuture task = collection.aggregate(AggregateField.count()).get(); + collection.getFirestore().shutdown(); + assertThat(task.get().getCount()).isEqualTo(20); + } + + @Test + public void inFlightAggregateQueriesShouldRunToCompletionWhenFirestoreIsShutDownForcefully() throws Exception { CollectionReference collection = createCollectionWithDocuments(20).collection(); - ApiFuture task = collection.count().get(); + ApiFuture task = collection.aggregate(AggregateField.count()).get(); collection.getFirestore().shutdownNow(); await(task); } @@ -176,6 +201,14 @@ public void countQueriesShouldFailIfStartedOnAClosedFirestoreInstance() throws E assertThrows(IllegalStateException.class, aggregateQuery::get); } + @Test + public void aggregateQueriesShouldFailIfStartedOnAClosedFirestoreInstance() throws Exception { + CollectionReference collection = createEmptyCollection(); + AggregateQuery aggregateQuery = collection.aggregate(AggregateField.count()); + collection.getFirestore().close(); + assertThrows(IllegalStateException.class, aggregateQuery::get); + } + @Test public void countQueriesShouldFailIfStartedOnAShutDownFirestoreInstance() throws Exception { CollectionReference collection = createEmptyCollection(); @@ -371,31 +404,6 @@ private static long msFromTimestamp(Timestamp timestamp) { return (timestamp.getSeconds() * 1_000) + (timestamp.getNanos() / 1_000_000); } - /** - * Blocks the calling thread until the given future completes. Note that this method does not - * check the success or failure of the future; it returns regardless of its success or failure. - */ - private static void await(ApiFuture future) throws InterruptedException { - AtomicBoolean done = new AtomicBoolean(false); - ExecutorService executor = Executors.newSingleThreadExecutor(); - future.addListener( - () -> { - synchronized (done) { - done.set(true); - done.notifyAll(); - } - }, - executor); - - synchronized (done) { - while (!done.get()) { - done.wait(); - } - } - - executor.shutdown(); - } - @AutoValue abstract static class CreatedCollectionInfo { 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 index e8439bde1..54cfd86f0 100644 --- 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 @@ -16,7 +16,11 @@ package com.google.cloud.firestore.it; +import com.google.api.core.ApiFuture; import com.google.cloud.firestore.Firestore; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicBoolean; public final class TestHelper { /** Make constructor private to prevent creating instances. */ @@ -26,4 +30,29 @@ private TestHelper() {} static boolean isRunningAgainstFirestoreEmulator(Firestore firestore) { return firestore.getOptions().getHost().startsWith("localhost:"); } + + /** + * Blocks the calling thread until the given future completes. Note that this method does not + * check the success or failure of the future; it returns regardless of its success or failure. + */ + public static void await(ApiFuture future) throws InterruptedException { + AtomicBoolean done = new AtomicBoolean(false); + ExecutorService executor = Executors.newSingleThreadExecutor(); + future.addListener( + () -> { + synchronized (done) { + done.set(true); + done.notifyAll(); + } + }, + executor); + + synchronized (done) { + while (!done.get()) { + done.wait(); + } + } + + executor.shutdown(); + } } From 8cdcf5435a9207eeda8e81342e852f713c16b2b2 Mon Sep 17 00:00:00 2001 From: Ehsan Date: Wed, 2 Aug 2023 15:01:25 -0700 Subject: [PATCH 02/13] feat: Add long alias support for aggregations. (#1267) * feat: Add long alias support for aggregations. * address comments. * Better method name and replace hardcoded "count" with "aggregate_0". * Remove duplicate aggregations. * add static import. * Fix tests. All tests pass. --- .../cloud/firestore/AggregateQuery.java | 34 +++++++---- .../cloud/firestore/LocalFirestoreHelper.java | 35 ++++++------ .../cloud/firestore/QueryCountTest.java | 46 +++++++-------- .../cloud/firestore/TransactionTest.java | 25 +------- .../firestore/it/ITQueryAggregationsTest.java | 57 ++++++++++++++++--- 5 files changed, 114 insertions(+), 83 deletions(-) diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java index ca5af9ec0..067ec0185 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java @@ -36,21 +36,16 @@ /** A query that calculates aggregations over an underlying query. */ @InternalExtensionOnly public class AggregateQuery { - - /** - * The "alias" to specify in the {@link RunAggregationQueryRequest} proto when running a count - * query. The actual value is not meaningful, but will be used to get the count out of the {@link - * RunAggregationQueryResponse}. - */ - private static final String ALIAS_COUNT = "count"; - @Nonnull private final Query query; @Nonnull private List aggregateFieldList; + @Nonnull private Map aliasMap; + AggregateQuery(@Nonnull Query query, @Nonnull List aggregateFields) { this.query = query; this.aggregateFieldList = aggregateFields; + this.aliasMap = new HashMap<>(); } /** Returns the query whose aggregations will be calculated by this object. */ @@ -114,7 +109,9 @@ long getStartTimeNanos() { void deliverResult(@Nonnull Map data, Timestamp readTime) { if (isFutureCompleted.compareAndSet(false, true)) { - future.set(new AggregateQuerySnapshot(AggregateQuery.this, readTime, data)); + Map mappedData = new HashMap<>(); + data.forEach((serverAlias, value) -> mappedData.put(aliasMap.get(serverAlias), value)); + future.set(new AggregateQuerySnapshot(AggregateQuery.this, readTime, mappedData)); } } @@ -202,9 +199,18 @@ RunAggregationQueryRequest toProto(@Nullable final ByteString transactionId) { request.getStructuredAggregationQueryBuilder(); structuredAggregationQuery.setStructuredQuery(runQueryRequest.getStructuredQuery()); - // We use a Set here to automatically remove duplicates. - Set aggregations = new HashSet<>(); + // We use this set to remove duplicate aggregates. e.g. `aggregate(sum("foo"), sum("foo"))` + HashSet uniqueAggregates = new HashSet<>(); + List aggregations = new ArrayList<>(); + int aggregationNum = 0; for (AggregateField aggregateField : aggregateFieldList) { + // `getAlias()` provides a unique representation of an AggregateField. + boolean isNewAggregateField = uniqueAggregates.add(aggregateField.getAlias()); + if (!isNewAggregateField) { + // This is a duplicate AggregateField. We don't need to include it in the request. + continue; + } + // If there's a field for this aggregation, build its proto. StructuredQuery.FieldReference field = null; if (!aggregateField.getFieldPath().isEmpty()) { @@ -224,7 +230,11 @@ RunAggregationQueryRequest toProto(@Nullable final ByteString transactionId) { } else { throw new RuntimeException("Unsupported aggregation"); } - aggregation.setAlias(aggregateField.getAlias()); + // Map all client-side aliases to a unique short-form alias. + // This avoids issues with client-side aliases that exceed the 1500-byte string size limit. + String serverAlias = "aggregate_" + aggregationNum++; + aliasMap.put(serverAlias, aggregateField.getAlias()); + aggregation.setAlias(serverAlias); aggregations.add(aggregation.build()); } structuredAggregationQuery.addAllAggregations(aggregations); diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/LocalFirestoreHelper.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/LocalFirestoreHelper.java index 896a51e3a..b86cc0bc9 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/LocalFirestoreHelper.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/LocalFirestoreHelper.java @@ -341,24 +341,24 @@ public static Answer queryResponseWithDone( } } - public static Answer aggregationQueryResponse() { - return aggregationQueryResponse(42); + public static Answer countQueryResponse() { + return countQueryResponse(42); } - public static Answer aggregationQueryResponse(int count) { - return aggregationQueryResponse(count, null); + public static Answer countQueryResponse(int count) { + return countQueryResponse(count, null); } - public static Answer aggregationQueryResponse( + public static Answer countQueryResponse( int count, @Nullable Timestamp readTime) { return streamingResponse( new RunAggregationQueryResponse[] { - createRunAggregationQueryResponse(count, readTime), + createCountQueryResponse(count, readTime), }, /*throwable=*/ null); } - public static Answer aggregationQueryResponse(Throwable throwable) { + public static Answer countQueryResponse(Throwable throwable) { return streamingResponse(new RunAggregationQueryResponse[] {}, throwable); } @@ -366,8 +366,7 @@ public static Answer aggregationQueryResponses( int count1, int count2) { return streamingResponse( new RunAggregationQueryResponse[] { - createRunAggregationQueryResponse(count1, null), - createRunAggregationQueryResponse(count2, null), + createCountQueryResponse(count1, null), createCountQueryResponse(count2, null), }, /*throwable=*/ null); } @@ -376,17 +375,17 @@ public static Answer aggregationQueryResponses( int count1, Throwable throwable) { return streamingResponse( new RunAggregationQueryResponse[] { - createRunAggregationQueryResponse(count1, null), + createCountQueryResponse(count1, null), }, throwable); } - private static RunAggregationQueryResponse createRunAggregationQueryResponse( + private static RunAggregationQueryResponse createCountQueryResponse( int count, @Nullable Timestamp timestamp) { RunAggregationQueryResponse.Builder builder = RunAggregationQueryResponse.newBuilder(); builder.setResult( AggregationResult.newBuilder() - .putAggregateFields("count", Value.newBuilder().setIntegerValue(count).build()) + .putAggregateFields("aggregate_0", Value.newBuilder().setIntegerValue(count).build()) .build()); if (timestamp != null) { builder.setReadTime(timestamp.toProto()); @@ -750,11 +749,11 @@ public static RunQueryRequest query( return request.build(); } - public static RunAggregationQueryRequest aggregationQuery() { - return aggregationQuery((String) null); + public static RunAggregationQueryRequest countQuery() { + return countQuery((String) null); } - public static RunAggregationQueryRequest aggregationQuery(@Nullable String transactionId) { + public static RunAggregationQueryRequest countQuery(@Nullable String transactionId) { RunQueryRequest runQueryRequest = query(TRANSACTION_ID, false); RunAggregationQueryRequest.Builder request = @@ -765,7 +764,7 @@ public static RunAggregationQueryRequest aggregationQuery(@Nullable String trans .setStructuredQuery(runQueryRequest.getStructuredQuery()) .addAggregations( Aggregation.newBuilder() - .setAlias("count") + .setAlias("aggregate_0") .setCount(Aggregation.Count.getDefaultInstance()))); if (transactionId != null) { @@ -775,7 +774,7 @@ public static RunAggregationQueryRequest aggregationQuery(@Nullable String trans return request.build(); } - public static RunAggregationQueryRequest aggregationQuery(RunQueryRequest runQueryRequest) { + public static RunAggregationQueryRequest countQuery(RunQueryRequest runQueryRequest) { return RunAggregationQueryRequest.newBuilder() .setParent(runQueryRequest.getParent()) .setStructuredAggregationQuery( @@ -783,7 +782,7 @@ public static RunAggregationQueryRequest aggregationQuery(RunQueryRequest runQue .setStructuredQuery(runQueryRequest.getStructuredQuery()) .addAggregations( Aggregation.newBuilder() - .setAlias("count") + .setAlias("aggregate_0") .setCount(Aggregation.Count.getDefaultInstance()))) .build(); } diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryCountTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryCountTest.java index 3cf05648e..14e6faa3d 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryCountTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryCountTest.java @@ -17,9 +17,8 @@ package com.google.cloud.firestore; import static com.google.cloud.firestore.LocalFirestoreHelper.COLLECTION_ID; -import static com.google.cloud.firestore.LocalFirestoreHelper.aggregationQuery; -import static com.google.cloud.firestore.LocalFirestoreHelper.aggregationQueryResponse; import static com.google.cloud.firestore.LocalFirestoreHelper.aggregationQueryResponses; +import static com.google.cloud.firestore.LocalFirestoreHelper.countQueryResponse; import static com.google.cloud.firestore.LocalFirestoreHelper.endAt; import static com.google.cloud.firestore.LocalFirestoreHelper.limit; import static com.google.cloud.firestore.LocalFirestoreHelper.order; @@ -77,7 +76,7 @@ public void before() { @Test public void countShouldBeZeroForEmptyCollection() throws Exception { - doAnswer(aggregationQueryResponse(0)) + doAnswer(countQueryResponse(0)) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -88,7 +87,7 @@ public void countShouldBeZeroForEmptyCollection() throws Exception { @Test public void countShouldBe99ForCollectionWith99Documents() throws Exception { - doAnswer(aggregationQueryResponse(99)) + doAnswer(countQueryResponse(99)) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -99,19 +98,19 @@ public void countShouldBe99ForCollectionWith99Documents() throws Exception { @Test public void countShouldMakeCorrectRequestForACollection() throws Exception { - doAnswer(aggregationQueryResponse(0)) + doAnswer(countQueryResponse(0)) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); CollectionReference collection = firestoreMock.collection(COLLECTION_ID); collection.count().get(); - assertThat(runAggregationQuery.getValue()).isEqualTo(aggregationQuery()); + assertThat(runAggregationQuery.getValue()).isEqualTo(LocalFirestoreHelper.countQuery()); } @Test public void countShouldMakeCorrectRequestForAComplexQuery() throws Exception { - doAnswer(aggregationQueryResponse(0)) + doAnswer(countQueryResponse(0)) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -119,7 +118,7 @@ public void countShouldMakeCorrectRequestForAComplexQuery() throws Exception { assertThat(runAggregationQuery.getValue()) .isEqualTo( - aggregationQuery( + LocalFirestoreHelper.countQuery( query( limit(42), order("foo", StructuredQuery.Direction.DESCENDING), @@ -129,7 +128,7 @@ public void countShouldMakeCorrectRequestForAComplexQuery() throws Exception { @Test public void shouldReturnReadTimeFromResponse() throws Exception { - doAnswer(aggregationQueryResponse(99, Timestamp.ofTimeSecondsAndNanos(123, 456))) + doAnswer(countQueryResponse(99, Timestamp.ofTimeSecondsAndNanos(123, 456))) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -163,7 +162,7 @@ public void shouldIgnoreExtraErrors() throws Exception { @Test public void shouldPropagateErrors() throws Exception { Exception exception = new Exception(); - doAnswer(aggregationQueryResponse(exception)) + doAnswer(countQueryResponse(exception)) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -180,7 +179,7 @@ public void aggregateQueryGetQueryShouldReturnCorrectValue() throws Exception { @Test public void aggregateQuerySnapshotGetQueryShouldReturnCorrectValue() throws Exception { - doAnswer(aggregationQueryResponse()) + doAnswer(countQueryResponse()) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -192,8 +191,8 @@ public void aggregateQuerySnapshotGetQueryShouldReturnCorrectValue() throws Exce @Test public void shouldNotRetryIfExceptionIsNotFirestoreException() { - doAnswer(aggregationQueryResponse(new NotFirestoreException())) - .doAnswer(aggregationQueryResponse()) + doAnswer(countQueryResponse(new NotFirestoreException())) + .doAnswer(countQueryResponse()) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -204,8 +203,8 @@ public void shouldNotRetryIfExceptionIsNotFirestoreException() { @Test public void shouldRetryIfExceptionIsFirestoreExceptionWithRetryableStatus() throws Exception { - doAnswer(aggregationQueryResponse(new FirestoreException("reason", Status.INTERNAL))) - .doAnswer(aggregationQueryResponse(42)) + doAnswer(countQueryResponse(new FirestoreException("reason", Status.INTERNAL))) + .doAnswer(countQueryResponse(42)) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -217,8 +216,9 @@ public void shouldRetryIfExceptionIsFirestoreExceptionWithRetryableStatus() thro @Test public void shouldNotRetryIfExceptionIsFirestoreExceptionWithNonRetryableStatus() { - doAnswer(aggregationQueryResponse(new FirestoreException("reason", Status.INVALID_ARGUMENT))) - .doAnswer(aggregationQueryResponse()) + doReturn(Duration.ZERO).when(firestoreMock).getTotalRequestTimeout(); + doAnswer(countQueryResponse(new FirestoreException("reason", Status.INVALID_ARGUMENT))) + .doAnswer(countQueryResponse()) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -232,8 +232,8 @@ public void shouldNotRetryIfExceptionIsFirestoreExceptionWithNonRetryableStatus( shouldRetryIfExceptionIsFirestoreExceptionWithRetryableStatusWithInfiniteTimeoutWindow() throws Exception { doReturn(Duration.ZERO).when(firestoreMock).getTotalRequestTimeout(); - doAnswer(aggregationQueryResponse(new FirestoreException("reason", Status.INTERNAL))) - .doAnswer(aggregationQueryResponse(42)) + doAnswer(countQueryResponse(new FirestoreException("reason", Status.INTERNAL))) + .doAnswer(countQueryResponse(42)) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -247,8 +247,8 @@ public void shouldNotRetryIfExceptionIsFirestoreExceptionWithNonRetryableStatus( public void shouldRetryIfExceptionIsFirestoreExceptionWithRetryableStatusWithinTimeoutWindow() throws Exception { doReturn(Duration.ofDays(999)).when(firestoreMock).getTotalRequestTimeout(); - doAnswer(aggregationQueryResponse(new FirestoreException("reason", Status.INTERNAL))) - .doAnswer(aggregationQueryResponse(42)) + doAnswer(countQueryResponse(new FirestoreException("reason", Status.INTERNAL))) + .doAnswer(countQueryResponse(42)) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); @@ -269,8 +269,8 @@ public void shouldRetryIfExceptionIsFirestoreExceptionWithRetryableStatusWithinT .when(clockMock) .nanoTime(); doReturn(Duration.ofSeconds(5)).when(firestoreMock).getTotalRequestTimeout(); - doAnswer(aggregationQueryResponse(new FirestoreException("reason", Status.INTERNAL))) - .doAnswer(aggregationQueryResponse(42)) + doAnswer(countQueryResponse(new FirestoreException("reason", Status.INTERNAL))) + .doAnswer(countQueryResponse(42)) .when(firestoreMock) .streamRequest(runAggregationQuery.capture(), streamObserverCapture.capture(), any()); diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java index 4c559486b..653ffe19e 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java @@ -16,26 +16,7 @@ package com.google.cloud.firestore; -import static com.google.cloud.firestore.LocalFirestoreHelper.IMMEDIATE_RETRY_SETTINGS; -import static com.google.cloud.firestore.LocalFirestoreHelper.SINGLE_FIELD_PROTO; -import static com.google.cloud.firestore.LocalFirestoreHelper.TRANSACTION_ID; -import static com.google.cloud.firestore.LocalFirestoreHelper.aggregationQuery; -import static com.google.cloud.firestore.LocalFirestoreHelper.aggregationQueryResponse; -import static com.google.cloud.firestore.LocalFirestoreHelper.begin; -import static com.google.cloud.firestore.LocalFirestoreHelper.beginResponse; -import static com.google.cloud.firestore.LocalFirestoreHelper.commit; -import static com.google.cloud.firestore.LocalFirestoreHelper.commitResponse; -import static com.google.cloud.firestore.LocalFirestoreHelper.create; -import static com.google.cloud.firestore.LocalFirestoreHelper.delete; -import static com.google.cloud.firestore.LocalFirestoreHelper.get; -import static com.google.cloud.firestore.LocalFirestoreHelper.getAll; -import static com.google.cloud.firestore.LocalFirestoreHelper.getAllResponse; -import static com.google.cloud.firestore.LocalFirestoreHelper.query; -import static com.google.cloud.firestore.LocalFirestoreHelper.queryResponse; -import static com.google.cloud.firestore.LocalFirestoreHelper.rollback; -import static com.google.cloud.firestore.LocalFirestoreHelper.rollbackResponse; -import static com.google.cloud.firestore.LocalFirestoreHelper.set; -import static com.google.cloud.firestore.LocalFirestoreHelper.update; +import static com.google.cloud.firestore.LocalFirestoreHelper.*; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -697,7 +678,7 @@ public void getAggregateQuery() throws Exception { .sendRequest( requestCapture.capture(), ArgumentMatchers.>any()); - doAnswer(aggregationQueryResponse(42)) + doAnswer(countQueryResponse(42)) .when(firestoreMock) .streamRequest( requestCapture.capture(), streamObserverCapture.capture(), ArgumentMatchers.any()); @@ -711,7 +692,7 @@ public void getAggregateQuery() throws Exception { assertEquals(3, requests.size()); assertEquals(begin(), requests.get(0)); - assertEquals(aggregationQuery(TRANSACTION_ID), requests.get(1)); + assertEquals(countQuery(TRANSACTION_ID), requests.get(1)); assertEquals(commit(TRANSACTION_ID), requests.get(2)); } diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java index 9d9b5d3aa..7c24f7418 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java @@ -10,6 +10,7 @@ import static java.util.Arrays.asList; import static org.junit.Assert.assertThrows; import static org.junit.Assume.assumeFalse; +import static org.junit.Assume.assumeTrue; import com.google.cloud.firestore.*; import com.google.common.base.Preconditions; @@ -81,6 +82,27 @@ public void canRunCountUsingAggregationMethod() throws Exception { assertThat(snapshot.getCount()).isEqualTo(2); } + @Test + public void allowsAliasesForLongestFieldNames() throws Exception { + // The longest field name allowed is 1499 characters long. + // Ensure that sum(longestField) and average(longestField) work. + StringBuilder builder = new StringBuilder(1500); + for (int i = 0; i < 1499; i++) { + builder.append("k"); + } + String longestField = builder.toString(); + Map> testDocs = + map("a", map(longestField, 2), "b", map(longestField, 4)); + + CollectionReference collection = testCollectionWithDocs(testDocs); + AggregateQuerySnapshot snapshot = + collection.aggregate(AggregateField.sum(longestField)).get().get(); + assertThat(snapshot.get(AggregateField.sum(longestField))).isEqualTo(6); + AggregateQuerySnapshot snapshot2 = + collection.aggregate(AggregateField.average(longestField)).get().get(); + assertThat(snapshot2.get(AggregateField.average(longestField))).isEqualTo(3.0); + } + @Test public void canGetDuplicateAggregations() throws Exception { CollectionReference collection = testCollectionWithDocs(testDocs1); @@ -110,7 +132,7 @@ public void aggregateErrorMessageIfIndexIsMissing() throws Exception { assertThat(executionException) .hasCauseThat() .hasMessageThat() - .containsMatch("/index.*https:\\/\\/console\\.firebase\\.google\\.com/"); + .containsMatch("index.*https:\\/\\/console\\.firebase\\.google\\.com"); } @Test @@ -195,8 +217,9 @@ public void getCorrectTypeForAverageNull() throws Exception { @Test public void canPerformMaxAggregations() throws Exception { - // TODO: Update this test once aggregate de-duplication is implemented and more aggregation - // types are available. + assumeTrue( + "Skip this test when running against prod because it requires composite index creation.", + isRunningAgainstFirestoreEmulator(firestore)); CollectionReference collection = testCollectionWithDocs(testDocs1); AggregateField f1 = sum("pages"); AggregateField f2 = average("pages"); @@ -234,6 +257,9 @@ public void cannotPerformMoreThanMaxAggregations() throws Exception { @Test public void aggregateQueriesSupportCollectionGroups() throws Exception { + assumeTrue( + "Skip this test when running against prod because it requires composite index creation.", + isRunningAgainstFirestoreEmulator(firestore)); String collectionGroupId = "myColGroupId" + autoId(); Map data = map("x", 2); // Setting documents at the following paths: @@ -274,7 +300,10 @@ public void aggregateQueriesSupportCollectionGroups() throws Exception { } @Test - public void performsAggregationsOnDocumentsWithAllaggregatedFields() throws Exception { + public void performsAggregationsOnDocumentsWithAllAggregatedFields() throws Exception { + assumeTrue( + "Skip this test when running against prod because it requires composite index creation.", + isRunningAgainstFirestoreEmulator(firestore)); Map> testDocs = map( "a", @@ -299,6 +328,9 @@ public void performsAggregationsOnDocumentsWithAllaggregatedFields() throws Exce @Test public void performsAggregationsWhenNaNExistsForSomeFieldValues() throws Exception { + assumeTrue( + "Skip this test when running against prod because it requires composite index creation.", + isRunningAgainstFirestoreEmulator(firestore)); Map> testDocs = map( "a", @@ -357,6 +389,9 @@ public void throwsAnErrorWhenGettingTheResultOfAnUnrequestedAggregation() throws @Test public void performsAggregationWhenUsingInOperator() throws Exception { + assumeTrue( + "Skip this test when running against prod because it requires composite index creation.", + isRunningAgainstFirestoreEmulator(firestore)); Map> testDocs = map( "a", @@ -394,6 +429,9 @@ public void performsAggregationWhenUsingInOperator() throws Exception { @Test public void performsAggregationWhenUsingArrayContainsAnyOperator() throws Exception { + assumeTrue( + "Skip this test when running against prod because it requires composite index creation.", + isRunningAgainstFirestoreEmulator(firestore)); Map> testDocs = map( "a", @@ -449,6 +487,9 @@ public void performsAggregationWhenUsingArrayContainsAnyOperator() throws Except @Test public void performsAggregationsOnNestedMapValues() throws Exception { + assumeTrue( + "Skip this test when running against prod because it requires composite index creation.", + isRunningAgainstFirestoreEmulator(firestore)); Map> testDocs = map( "a", @@ -509,8 +550,8 @@ public void performsSumOfIntsAndFloatsThatResultsInInt() throws Exception { CollectionReference collection = testCollectionWithDocs(testDocs); AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); Object sum = snapshot.get(sum("rating")); - assertThat(sum instanceof Long).isTrue(); - assertThat(sum).isEqualTo(13); + assertThat(sum instanceof Double).isTrue(); + assertThat(sum).isEqualTo(13.0); } @Test @@ -598,8 +639,8 @@ public void performsSumThatIsValidButCouldOverflowDuringAggregation() throws Exc CollectionReference collection = testCollectionWithDocs(testDocs); AggregateQuerySnapshot snapshot = collection.aggregate(sum("rating")).get().get(); Object sum = snapshot.get(sum("rating")); - assertThat(sum instanceof Long).isTrue(); - assertThat(sum).isEqualTo(0); + assertThat(sum instanceof Double).isTrue(); + assertThat(sum).isAnyOf(0, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY); } @Test From 2c0c473e155a2a1ef623db894c60da68cc809b71 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Tue, 8 Aug 2023 16:36:45 -0700 Subject: [PATCH 03/13] Address comments. --- .../cloud/firestore/AggregateField.java | 33 +++++++++++++++++ .../firestore/it/ITQueryAggregationsTest.java | 36 ++----------------- 2 files changed, 35 insertions(+), 34 deletions(-) diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java index 76fad5b8f..a4cdb9080 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java @@ -20,32 +20,62 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +/** Represents an aggregation that can be performed by Firestore. */ public abstract class AggregateField { + /** + * Create a {@link CountAggregateField} object that can be used to compute the count of documents + * in the result set of a query. + */ @Nonnull public static CountAggregateField count() { return new CountAggregateField(); } + /** + * Create a {@link SumAggregateField} object that can be used to compute the sum of a specified + * field over a range of documents in the result set of a query. + * + * @param field Specifies the field to sum across the result set. + */ @Nonnull public static SumAggregateField sum(@Nonnull String field) { return new SumAggregateField(FieldPath.fromDotSeparatedString(field)); } + /** + * Create a {@link SumAggregateField} object that can be used to compute the sum of a specified + * field over a range of documents in the result set of a query. + * + * @param fieldPath Specifies the field to sum across the result set. + */ @Nonnull public static SumAggregateField sum(@Nonnull FieldPath fieldPath) { return new SumAggregateField(fieldPath); } + /** + * Create an {@link AverageAggregateField} object that can be used to compute the average of a + * specified field over a range of documents in the result set of a query. + * + * @param field Specifies the field to average across the result set. + */ @Nonnull public static AverageAggregateField average(@Nonnull String field) { return new AverageAggregateField(FieldPath.fromDotSeparatedString(field)); } + /** + * Create an {@link AverageAggregateField} object that can be used to compute the average of a + * specified field over a range of documents in the result set of a query. + * + * @param fieldPath Specifies the field to average across the result set. + */ @Nonnull public static AverageAggregateField average(@Nonnull FieldPath fieldPath) { return new AverageAggregateField(fieldPath); } + /** The field over which the aggregation is performed. */ @Nullable FieldPath fieldPath; /** Returns the alias used internally for this aggregate field. */ @@ -91,6 +121,7 @@ public int hashCode() { return Objects.hash(getOperator(), getFieldPath()); } + /** Represents a "sum" aggregation that can be performed by Firestore. */ public static class SumAggregateField extends AggregateField { private SumAggregateField(@Nonnull FieldPath field) { fieldPath = field; @@ -103,6 +134,7 @@ public String getOperator() { } } + /** Represents an "average" aggregation that can be performed by Firestore. */ public static class AverageAggregateField extends AggregateField { private AverageAggregateField(@Nonnull FieldPath field) { fieldPath = field; @@ -115,6 +147,7 @@ public String getOperator() { } } + /** Represents a "count" aggregation that can be performed by Firestore. */ public static class CountAggregateField extends AggregateField { private CountAggregateField() {} diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java index 7c24f7418..c9bb8efd7 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java @@ -13,36 +13,14 @@ import static org.junit.Assume.assumeTrue; import com.google.cloud.firestore.*; -import com.google.common.base.Preconditions; import java.util.Map; import java.util.concurrent.ExecutionException; -import org.junit.After; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class ITQueryAggregationsTest { - private Firestore firestore; - - @Before - public void setUpFirestore() { - // TODO: stop using emulator for these tests once prod is ready. - firestore = FirestoreOptions.newBuilder().setHost("localhost:8080").build().getService(); - Preconditions.checkNotNull( - firestore, - "Error instantiating Firestore. Check that the service account credentials were properly set."); - } - - @After - public void tearDownFirestore() throws Exception { - if (firestore != null) { - firestore.close(); - firestore = null; - } - } - +public class ITQueryAggregationsTest extends ITBaseTest { private CollectionReference testCollection() { String collectionPath = "java-" + autoId(); return firestore.collection(collectionPath); @@ -487,9 +465,6 @@ public void performsAggregationWhenUsingArrayContainsAnyOperator() throws Except @Test public void performsAggregationsOnNestedMapValues() throws Exception { - assumeTrue( - "Skip this test when running against prod because it requires composite index creation.", - isRunningAgainstFirestoreEmulator(firestore)); Map> testDocs = map( "a", @@ -511,18 +486,11 @@ public void performsAggregationsOnNestedMapValues() throws Exception { CollectionReference collection = testCollectionWithDocs(testDocs); AggregateQuerySnapshot snapshot = collection - .aggregate( - sum("metadata.pages"), - average("metadata.pages"), - average("metadata.rating.critic"), - sum("metadata.rating.user"), - AggregateField.count()) + .aggregate(sum("metadata.pages"), average("metadata.pages"), AggregateField.count()) .get() .get(); assertThat(snapshot.get(sum("metadata.pages"))).isEqualTo(150); assertThat(snapshot.get(average("metadata.pages"))).isEqualTo(75); - assertThat(snapshot.get(average("metadata.rating.critic"))).isEqualTo(3); - assertThat(snapshot.get(sum("metadata.rating.user"))).isEqualTo(9); assertThat(snapshot.get(AggregateField.count())).isEqualTo(2); } From 909083a4d05514aa0072124d969b2ba5e86079bd Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Wed, 9 Aug 2023 16:55:20 -0700 Subject: [PATCH 04/13] Improve the documentation to match strongly typed languages. --- .../cloud/firestore/AggregateField.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java index a4cdb9080..da94dc2bb 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java @@ -25,6 +25,11 @@ public abstract class AggregateField { /** * Create a {@link CountAggregateField} object that can be used to compute the count of documents * in the result set of a query. + * + *

The result of a count operation will always be a 64-bit integer value. + * + * @return The `CountAggregateField` object that can be used to compute the count of documents in + * the result set of a query. */ @Nonnull public static CountAggregateField count() { @@ -35,7 +40,20 @@ public static CountAggregateField count() { * Create a {@link SumAggregateField} object that can be used to compute the sum of a specified * field over a range of documents in the result set of a query. * + *

The result of a sum operation will always be a 64-bit integer value, a double, or NaN. + * + *

    + *
  • Summing over zero documents or fields will result in 0L. + *
  • Summing over NaN will result in a double value representing NaN. + *
  • A sum that overflows the maximum representable 64-bit integer value will result in a + * double return value. This may result in lost precision of the result. + *
  • A sum that overflows the maximum representable double value will result in a double + * return value representing infinity. + *
+ * * @param field Specifies the field to sum across the result set. + * @return The `SumAggregateField` object that can be used to compute the sum of a specified field + * over a range of documents in the result set of a query. */ @Nonnull public static SumAggregateField sum(@Nonnull String field) { @@ -46,7 +64,20 @@ public static SumAggregateField sum(@Nonnull String field) { * Create a {@link SumAggregateField} object that can be used to compute the sum of a specified * field over a range of documents in the result set of a query. * + *

The result of a sum operation will always be a 64-bit integer value, a double, or NaN. + * + *

    + *
  • Summing over zero documents or fields will result in 0L. + *
  • Summing over NaN will result in a double value representing NaN. + *
  • A sum that overflows the maximum representable 64-bit integer value will result in a + * double return value. This may result in lost precision of the result. + *
  • A sum that overflows the maximum representable double value will result in a double + * return value representing infinity. + *
+ * * @param fieldPath Specifies the field to sum across the result set. + * @return The `SumAggregateField` object that can be used to compute the sum of a specified field + * over a range of documents in the result set of a query. */ @Nonnull public static SumAggregateField sum(@Nonnull FieldPath fieldPath) { @@ -57,7 +88,16 @@ public static SumAggregateField sum(@Nonnull FieldPath fieldPath) { * Create an {@link AverageAggregateField} object that can be used to compute the average of a * specified field over a range of documents in the result set of a query. * + *

The result of an average operation will always be a 64-bit integer value, a double, or NaN. + * + *

    + *
  • Averaging over zero documents or fields will result in a double value representing NaN. + *
  • Averaging over NaN will result in a double value representing NaN. + *
+ * * @param field Specifies the field to average across the result set. + * @return The `AverageAggregateField` object that can be used to compute the average of a + * specified field over a range of documents in the result set of a query. */ @Nonnull public static AverageAggregateField average(@Nonnull String field) { @@ -68,7 +108,16 @@ public static AverageAggregateField average(@Nonnull String field) { * Create an {@link AverageAggregateField} object that can be used to compute the average of a * specified field over a range of documents in the result set of a query. * + *

The result of an average operation will always be a 64-bit integer value, a double, or NaN. + * + *

    + *
  • Averaging over zero documents or fields will result in a double value representing NaN. + *
  • Averaging over NaN will result in a double value representing NaN. + *
+ * * @param fieldPath Specifies the field to average across the result set. + * @return The `AverageAggregateField` object that can be used to compute the average of a + * specified field over a range of documents in the result set of a query. */ @Nonnull public static AverageAggregateField average(@Nonnull FieldPath fieldPath) { From 4a39b8a6dca9421a6496140cbdad26d8386c8bba Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Wed, 9 Aug 2023 16:58:49 -0700 Subject: [PATCH 05/13] Do not use wildcard import. --- .../java/com/google/cloud/firestore/AggregateQuery.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java index 067ec0185..fb1a876ab 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java @@ -25,8 +25,13 @@ import com.google.api.gax.rpc.StreamController; import com.google.cloud.Timestamp; import com.google.cloud.firestore.v1.FirestoreSettings; -import com.google.firestore.v1.*; +import com.google.firestore.v1.RunAggregationQueryRequest; +import com.google.firestore.v1.RunAggregationQueryResponse; +import com.google.firestore.v1.RunQueryRequest; +import com.google.firestore.v1.StructuredAggregationQuery; import com.google.firestore.v1.StructuredAggregationQuery.Aggregation; +import com.google.firestore.v1.StructuredQuery; +import com.google.firestore.v1.Value; import com.google.protobuf.ByteString; import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; From fd773daa06b6d9175564eb95a6ffea505c4398b2 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Wed, 9 Aug 2023 17:02:53 -0700 Subject: [PATCH 06/13] Do not use wildcard import (2). --- .../java/com/google/cloud/firestore/AggregateQuery.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java index fb1a876ab..d513c39d1 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateQuery.java @@ -33,7 +33,13 @@ import com.google.firestore.v1.StructuredQuery; import com.google.firestore.v1.Value; import com.google.protobuf.ByteString; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nonnull; import javax.annotation.Nullable; From 25a55975dc5b802cacd040dbff31036a8cc43844 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Wed, 9 Aug 2023 17:07:01 -0700 Subject: [PATCH 07/13] Do not use wildcard import (3). --- .../cloud/firestore/AggregateQueryTest.java | 4 +++- .../cloud/firestore/TransactionTest.java | 21 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQueryTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQueryTest.java index 80c758bd6..9423ad1bb 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQueryTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/AggregateQueryTest.java @@ -16,7 +16,9 @@ package com.google.cloud.firestore; -import static com.google.cloud.firestore.AggregateField.*; +import static com.google.cloud.firestore.AggregateField.average; +import static com.google.cloud.firestore.AggregateField.count; +import static com.google.cloud.firestore.AggregateField.sum; import static com.google.common.truth.Truth.assertThat; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java index 653ffe19e..eeef59e0c 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java @@ -16,7 +16,26 @@ package com.google.cloud.firestore; -import static com.google.cloud.firestore.LocalFirestoreHelper.*; +import static com.google.cloud.firestore.LocalFirestoreHelper.IMMEDIATE_RETRY_SETTINGS; +import static com.google.cloud.firestore.LocalFirestoreHelper.SINGLE_FIELD_PROTO; +import static com.google.cloud.firestore.LocalFirestoreHelper.TRANSACTION_ID; +import static com.google.cloud.firestore.LocalFirestoreHelper.begin; +import static com.google.cloud.firestore.LocalFirestoreHelper.beginResponse; +import static com.google.cloud.firestore.LocalFirestoreHelper.commit; +import static com.google.cloud.firestore.LocalFirestoreHelper.commitResponse; +import static com.google.cloud.firestore.LocalFirestoreHelper.countQuery; +import static com.google.cloud.firestore.LocalFirestoreHelper.countQueryResponse; +import static com.google.cloud.firestore.LocalFirestoreHelper.create; +import static com.google.cloud.firestore.LocalFirestoreHelper.delete; +import static com.google.cloud.firestore.LocalFirestoreHelper.get; +import static com.google.cloud.firestore.LocalFirestoreHelper.getAll; +import static com.google.cloud.firestore.LocalFirestoreHelper.getAllResponse; +import static com.google.cloud.firestore.LocalFirestoreHelper.query; +import static com.google.cloud.firestore.LocalFirestoreHelper.queryResponse; +import static com.google.cloud.firestore.LocalFirestoreHelper.rollback; +import static com.google.cloud.firestore.LocalFirestoreHelper.rollbackResponse; +import static com.google.cloud.firestore.LocalFirestoreHelper.set; +import static com.google.cloud.firestore.LocalFirestoreHelper.update; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; From 3a0cc6946bcd23937ac5b6ccbc7ea0ddbe9b98d8 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Wed, 9 Aug 2023 17:10:48 -0700 Subject: [PATCH 08/13] Do not use wildcard import (4). --- .../google/cloud/firestore/it/ITQueryAggregationsTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java index c9bb8efd7..e6739eb7f 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java @@ -12,7 +12,11 @@ import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; -import com.google.cloud.firestore.*; +import com.google.cloud.firestore.AggregateField; +import com.google.cloud.firestore.AggregateQuery; +import com.google.cloud.firestore.AggregateQuerySnapshot; +import com.google.cloud.firestore.CollectionGroup; +import com.google.cloud.firestore.CollectionReference; import java.util.Map; import java.util.concurrent.ExecutionException; import org.junit.Test; From 036e6879b082517cd518b149ac6420fdaed72c4c Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Fri, 11 Aug 2023 12:00:35 -0700 Subject: [PATCH 09/13] Fix the javadoc. --- .../main/java/com/google/cloud/firestore/AggregateField.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java index da94dc2bb..ee3af88ca 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/AggregateField.java @@ -88,7 +88,7 @@ public static SumAggregateField sum(@Nonnull FieldPath fieldPath) { * Create an {@link AverageAggregateField} object that can be used to compute the average of a * specified field over a range of documents in the result set of a query. * - *

The result of an average operation will always be a 64-bit integer value, a double, or NaN. + *

The result of an average operation will always be a double or NaN. * *

    *
  • Averaging over zero documents or fields will result in a double value representing NaN. @@ -108,7 +108,7 @@ public static AverageAggregateField average(@Nonnull String field) { * Create an {@link AverageAggregateField} object that can be used to compute the average of a * specified field over a range of documents in the result set of a query. * - *

    The result of an average operation will always be a 64-bit integer value, a double, or NaN. + *

    The result of an average operation will always be a double or NaN. * *

      *
    • Averaging over zero documents or fields will result in a double value representing NaN. From 43b55c2521d8e35fb77f99a1ba47a2867243a3c7 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 2 Oct 2023 15:31:55 -0700 Subject: [PATCH 10/13] Add license header, and remove unused test code. --- .../google/cloud/firestore/QueryCountTest.java | 1 - .../firestore/it/ITQueryAggregationsTest.java | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryCountTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryCountTest.java index 14e6faa3d..78222ecfa 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryCountTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryCountTest.java @@ -216,7 +216,6 @@ public void shouldRetryIfExceptionIsFirestoreExceptionWithRetryableStatus() thro @Test public void shouldNotRetryIfExceptionIsFirestoreExceptionWithNonRetryableStatus() { - doReturn(Duration.ZERO).when(firestoreMock).getTotalRequestTimeout(); doAnswer(countQueryResponse(new FirestoreException("reason", Status.INVALID_ARGUMENT))) .doAnswer(countQueryResponse()) .when(firestoreMock) diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java index e6739eb7f..031323cd5 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java @@ -1,3 +1,19 @@ +/* + * 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 static com.google.cloud.firestore.AggregateField.average; From c3b0fe2619453f24fc67096d78989972afac5e2f Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Mon, 2 Oct 2023 22:35:01 +0000 Subject: [PATCH 11/13] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 9d63d5003..e5431b291 100644 --- a/README.md +++ b/README.md @@ -50,20 +50,20 @@ If you are using Maven without the BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.21.0') +implementation platform('com.google.cloud:libraries-bom:26.24.0') 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.13.8' +implementation 'com.google.cloud:google-cloud-firestore:3.14.4' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-firestore" % "3.13.8" +libraryDependencies += "com.google.cloud" % "google-cloud-firestore" % "3.14.4" ``` @@ -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.13.8 +[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-firestore/3.14.4 [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 From 25be35e917576e8108d9d06e400d363579e9be29 Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 2 Oct 2023 15:56:46 -0700 Subject: [PATCH 12/13] better regex. --- .../com/google/cloud/firestore/it/ITQueryAggregationsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java index 031323cd5..40372e199 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java @@ -130,7 +130,7 @@ public void aggregateErrorMessageIfIndexIsMissing() throws Exception { assertThat(executionException) .hasCauseThat() .hasMessageThat() - .containsMatch("index.*https:\\/\\/console\\.firebase\\.google\\.com"); + .containsMatch("FAILED_PRECONDITION:.*index.*"); } @Test From 9a72f3db09e4a85c9b1e69fbe191da15ad0fdc5b Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Thu, 5 Oct 2023 16:31:30 -0700 Subject: [PATCH 13/13] Add more tests for cursors. --- .../firestore/it/ITQueryAggregationsTest.java | 250 +++++++++++++++++- .../cloud/firestore/it/ITQueryCountTest.java | 49 +++- 2 files changed, 293 insertions(+), 6 deletions(-) diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java index 40372e199..265151446 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITQueryAggregationsTest.java @@ -28,13 +28,10 @@ import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; -import com.google.cloud.firestore.AggregateField; -import com.google.cloud.firestore.AggregateQuery; -import com.google.cloud.firestore.AggregateQuerySnapshot; -import com.google.cloud.firestore.CollectionGroup; -import com.google.cloud.firestore.CollectionReference; +import com.google.cloud.firestore.*; import java.util.Map; import java.util.concurrent.ExecutionException; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -841,4 +838,247 @@ public void performsAverageOnlyOnNumericFields() throws Exception { assertThat(snapshot.get(average("rating"))).isEqualTo(5); assertThat(snapshot.get(AggregateField.count())).isEqualTo(4); } + + // Currently not allowed because it requires __name__, num index. + @Ignore + @Test + public void aggregatesWithDocumentReferenceCursors() throws Exception { + Map> testDocs = + map( + "a", map("num", 1), + "b", map("num", 2), + "c", map("num", 3), + "d", map("num", 4), + "e", map("num", 5)); + CollectionReference collection = testCollectionWithDocs(testDocs); + + AggregateQuerySnapshot snapshot = + collection + .orderBy(FieldPath.documentId()) + .startAfter(collection.document("c")) + .aggregate(sum("num")) + .get() + .get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(9); + + snapshot = + collection + .orderBy(FieldPath.documentId()) + .startAt(collection.document("c")) + .aggregate(sum("num")) + .get() + .get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(12); + + snapshot = + collection + .orderBy(FieldPath.documentId()) + .endBefore(collection.document("c")) + .aggregate(sum("num")) + .get() + .get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(3); + + snapshot = + collection + .orderBy(FieldPath.documentId()) + .endAt(collection.document("c")) + .aggregate(sum("num")) + .get() + .get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(6); + } + + CollectionReference addTwoDocsForCursorTesting() throws InterruptedException { + Map> testDocs = + map( + "a", map("num", 5, "foo", 1), + "b", map("num", 7, "foo", 2)); + return testCollectionWithDocs(testDocs); + } + + @Test + public void aggregateWithNoFilterNoOrderByNoCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = collection.aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(12); + } + + @Test + public void aggregateWithEqualityFilterNoOrderByNoCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = collection.whereEqualTo("num", 5).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(5); + } + + @Test + public void aggregateWithInequalityFilterNoOrderByNoCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = collection.whereGreaterThan("num", 5).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + @Test + public void aggregateWithNoFilterExplicitOrderByNoCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = collection.orderBy("num").aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(12); + } + + @Test + public void aggregateWithEqualityFilterExplicitOrderByNoCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = collection.whereEqualTo("num", 5).orderBy("num").aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(5); + } + + @Test + public void aggregateWithInequalityFilterExplicitOrderByNoCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = + collection.whereGreaterThan("num", 5).orderBy("num").aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + @Test + public void aggregateNoFilterExplicitOrderByFieldValueCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = collection.orderBy("num").startAfter(5).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + // This is expected to fail because it requires the `__name__, num` index. + @Ignore + @Test + public void aggregateNoFilterExplicitOrderByDocumentReferenceCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = + collection + .orderBy(FieldPath.documentId()) + .startAfter(collection.document("a")) + .aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + // This is expected to fail because it requires the `__name__, num` index. + @Ignore + @Test + public void aggregateNoFilterNoOrderByDocumentReferenceCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = collection.startAfter(collection.document("a")).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + // This is expected to fail because it requires the `foo, __name__, num` index. + @Ignore + @Test + public void aggregateNoFilterExplicitOrderByDocumentSnapshotCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + DocumentSnapshot docSnapshot = collection.document("a").get().get(); + AggregateQuery query = collection.orderBy("foo").startAfter(docSnapshot).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + // This just happens to work because the orderBy field matches the aggregation field. + @Test + public void aggregateNoFilterExplicitOrderByDocumentSnapshotCursor2() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + DocumentSnapshot docSnapshot = collection.document("a").get().get(); + AggregateQuery query = collection.orderBy("num").startAfter(docSnapshot).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + @Test + public void aggregateEqualityFilterExplicitOrderByFieldValueCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = + collection.whereEqualTo("num", 5).orderBy("num").startAt(5).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(5); + } + + @Test + public void aggregateInequalityFilterExplicitOrderByFieldValueCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = + collection.whereGreaterThan("num", 5).orderBy("num").startAt(6).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + // This is expected to fail because it requires the `__name__, num` index. + @Ignore + @Test + public void aggregateEqualityFilterExplicitOrderByDocumentReferenceCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = + collection + .whereEqualTo("num", 7) + .orderBy(FieldPath.documentId()) + .startAfter(collection.document("a")) + .aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + // Full orderBy is provided. + @Test + public void aggregateInequalityFilterExplicitOrderByDocumentReferenceCursor() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + AggregateQuery query = + collection + .whereGreaterThan("num", 0) + .orderBy("num") + .orderBy(FieldPath.documentId()) + .startAfter(5, collection.document("a")) + .aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + // This is expected to fail because it requires the `__name__, num` index. + @Ignore + @Test + public void aggregateEqualityFilterNoOrderByDocumentSnapshotReference() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + DocumentSnapshot docSnapshot = collection.document("a").get().get(); + AggregateQuery query = + collection.whereEqualTo("num", 7).startAfter(docSnapshot).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + // This just happens to work because the orderBy field matches the aggregation field. + @Test + public void aggregateInequalityFilterNoOrderByDocumentSnapshotReference() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + DocumentSnapshot docSnapshot = collection.document("a").get().get(); + AggregateQuery query = + collection.whereGreaterThan("num", 0).startAfter(docSnapshot).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } + + // This is expected to fail because it requires the `foo, __name__, num` index. + @Ignore + @Test + public void aggregateInequalityFilterNoOrderByDocumentSnapshotReference2() throws Exception { + CollectionReference collection = addTwoDocsForCursorTesting(); + DocumentSnapshot docSnapshot = collection.document("a").get().get(); + AggregateQuery query = + collection.whereGreaterThan("foo", 0).startAfter(docSnapshot).aggregate(sum("num")); + AggregateQuerySnapshot snapshot = query.get().get(); + assertThat(snapshot.get(sum("num"))).isEqualTo(7); + } } 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 index 63f3a0d82..22e371ca2 100644 --- 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 @@ -33,6 +33,7 @@ import com.google.cloud.firestore.CollectionGroup; import com.google.cloud.firestore.CollectionReference; import com.google.cloud.firestore.DocumentReference; +import com.google.cloud.firestore.FieldPath; import com.google.cloud.firestore.Query; import com.google.cloud.firestore.QueryDocumentSnapshot; import com.google.cloud.firestore.TransactionOptions; @@ -103,7 +104,7 @@ public void countShouldRespectOrderBy() throws Exception { } @Test - public void countShouldRespectStartAtAndEndAt() throws Exception { + public void countShouldRespectStartAtAndEndAtWithDocumentSnapshotCursor() throws Exception { CollectionReference collection = createCollectionWithDocuments(10).collection(); List documentSnapshots = collection.get().get().getDocuments(); AggregateQuerySnapshot snapshot = @@ -116,6 +117,52 @@ public void countShouldRespectStartAtAndEndAt() throws Exception { assertThat(snapshot.getCount()).isEqualTo(6); } + @Test + public void countShouldRespectStartAtAndEndAtWithDocumentReferenceCursor() throws Exception { + CollectionReference collection = createCollectionWithDocuments(10).collection(); + List documentSnapshots = collection.get().get().getDocuments(); + AggregateQuerySnapshot snapshot = + collection + .orderBy(FieldPath.documentId()) + .startAt(documentSnapshots.get(2).getReference()) + .endAt(documentSnapshots.get(7).getReference()) + .count() + .get() + .get(); + assertThat(snapshot.getCount()).isEqualTo(6); + } + + @Test + public void countShouldRespectStartAfterAndEndBeforeWithDocumentSnapshotCursor() + throws Exception { + CollectionReference collection = createCollectionWithDocuments(10).collection(); + List documentSnapshots = collection.get().get().getDocuments(); + AggregateQuerySnapshot snapshot = + collection + .startAfter(documentSnapshots.get(2)) + .endBefore(documentSnapshots.get(7)) + .count() + .get() + .get(); + assertThat(snapshot.getCount()).isEqualTo(4); + } + + @Test + public void countShouldRespectStartAfterAndEndBeforeWithDocumentReferenceCursor() + throws Exception { + CollectionReference collection = createCollectionWithDocuments(10).collection(); + List documentSnapshots = collection.get().get().getDocuments(); + AggregateQuerySnapshot snapshot = + collection + .orderBy(FieldPath.documentId()) + .startAfter(documentSnapshots.get(2).getReference()) + .endBefore(documentSnapshots.get(7).getReference()) + .count() + .get() + .get(); + assertThat(snapshot.getCount()).isEqualTo(4); + } + @Test public void countQueriesShouldFailIfCollectionNameIsInvalid() { CollectionReference collection = createEmptyCollection().document().collection("__invalid__");