Skip to content

Commit abfa774

Browse files
authored
Add long alias support on SDK side for SUM/AVG (firebase#4961)
* Add long alias * Address comments
1 parent 34af203 commit abfa774

File tree

3 files changed

+62
-6
lines changed

3 files changed

+62
-6
lines changed

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/AggregationTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,4 +1228,30 @@ public void testAggregateFailWithGoodMessageIfMissingIndex() {
12281228
Truth.assertThat(cause).hasMessageThat().ignoringCase().contains("index");
12291229
Truth.assertThat(cause).hasMessageThat().contains("https://console.firebase.google.com");
12301230
}
1231+
1232+
@Test
1233+
public void allowsAliasesLongerThan1500Bytes() {
1234+
assumeTrue(
1235+
"Skip this test if running against production because sum/avg is only support "
1236+
+ "in emulator currently.",
1237+
isRunningAgainstEmulator());
1238+
// The longest field name allowed is 1500. The alias chosen by the client is <op>_<fieldName>.
1239+
// If the field name is
1240+
// 1500 bytes, the alias will be longer than 1500, which is the limit for aliases. This is to
1241+
// make sure the client
1242+
// can handle this corner case correctly.
1243+
1244+
StringBuilder builder = new StringBuilder(1500);
1245+
for (int i = 0; i < 1500; i++) {
1246+
builder.append("a");
1247+
}
1248+
String longField = builder.toString();
1249+
Map<String, Map<String, Object>> testDocs = map("a", map(longField, 1), "b", map(longField, 2));
1250+
CollectionReference collection = testCollectionWithDocs(testDocs);
1251+
1252+
AggregateQuerySnapshot snapshot =
1253+
waitFor(collection.aggregate(sum(longField)).get(AggregateSource.SERVER));
1254+
1255+
assertEquals(snapshot.get(sum(longField)), 3L);
1256+
}
12311257
}

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/Datastore.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.firebase.firestore.remote;
1616

17+
import static com.google.firebase.firestore.util.Assert.hardAssert;
1718
import static com.google.firebase.firestore.util.Util.exceptionFromStatus;
1819

1920
import android.content.Context;
@@ -225,8 +226,9 @@ public Task<Map<String, Value>> runAggregateQuery(
225226
Query query, List<AggregateField> aggregateFields) {
226227
com.google.firestore.v1.Target.QueryTarget encodedQueryTarget =
227228
serializer.encodeQueryTarget(query.toTarget());
229+
HashMap<String, String> aliasMap = new HashMap<>();
228230
StructuredAggregationQuery structuredAggregationQuery =
229-
serializer.encodeStructuredAggregationQuery(encodedQueryTarget, aggregateFields);
231+
serializer.encodeStructuredAggregationQuery(encodedQueryTarget, aggregateFields, aliasMap);
230232

231233
RunAggregationQueryRequest.Builder request = RunAggregationQueryRequest.newBuilder();
232234
request.setParent(encodedQueryTarget.getParent());
@@ -246,8 +248,20 @@ public Task<Map<String, Value>> runAggregateQuery(
246248
throw task.getException();
247249
}
248250

251+
Map<String, Value> result = new HashMap<>();
249252
RunAggregationQueryResponse response = task.getResult();
250-
return response.getResult().getAggregateFieldsMap();
253+
254+
// Remap the short-form aliases that were sent to the server to the client-side
255+
// aliases. Users will access the results using the client-side alias.
256+
for (Map.Entry<String, Value> entry :
257+
response.getResult().getAggregateFieldsMap().entrySet()) {
258+
hardAssert(
259+
aliasMap.containsKey(entry.getKey()),
260+
"%s not present in aliasMap",
261+
entry.getKey());
262+
result.put(aliasMap.get(entry.getKey()), entry.getValue());
263+
}
264+
return result;
251265
});
252266
}
253267

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -633,14 +633,30 @@ public com.google.firebase.firestore.core.Target decodeQueryTarget(QueryTarget t
633633
}
634634

635635
StructuredAggregationQuery encodeStructuredAggregationQuery(
636-
QueryTarget encodedQueryTarget, List<AggregateField> aggregateFields) {
636+
QueryTarget encodedQueryTarget,
637+
List<AggregateField> aggregateFields,
638+
HashMap<String, String> aliasMap) {
637639
StructuredAggregationQuery.Builder structuredAggregationQuery =
638640
StructuredAggregationQuery.newBuilder();
639641
structuredAggregationQuery.setStructuredQuery(encodedQueryTarget.getStructuredQuery());
640642

641-
// We use a Set here to automatically remove duplicates.
642-
Set<StructuredAggregationQuery.Aggregation> aggregations = new HashSet<>();
643+
List<StructuredAggregationQuery.Aggregation> aggregations = new ArrayList<>();
644+
645+
HashSet<String> uniqueFields = new HashSet<>();
646+
int aliasID = 1;
643647
for (AggregateField aggregateField : aggregateFields) {
648+
// The code block below is used to deduplicate the same aggregate fields.
649+
// If two aggregateFields are identical, their aliases would be the same.
650+
// Therefore, when adding duplicated alias into uniqueFields, the size of uniqueFields
651+
// won't increase, and we can skip this aggregateField processing.
652+
final int count = uniqueFields.size();
653+
uniqueFields.add(aggregateField.getAlias());
654+
if (count == uniqueFields.size()) {
655+
continue;
656+
}
657+
String serverAlias = "aggregate_" + aliasID++;
658+
aliasMap.put(serverAlias, aggregateField.getAlias());
659+
644660
StructuredAggregationQuery.Aggregation.Builder aggregation =
645661
StructuredAggregationQuery.Aggregation.newBuilder();
646662
StructuredQuery.FieldReference fieldPath =
@@ -660,7 +676,7 @@ StructuredAggregationQuery encodeStructuredAggregationQuery(
660676
throw new RuntimeException("Unsupported aggregation");
661677
}
662678

663-
aggregation.setAlias(aggregateField.getAlias());
679+
aggregation.setAlias(serverAlias);
664680
aggregations.add(aggregation.build());
665681
}
666682
structuredAggregationQuery.addAllAggregations(aggregations);

0 commit comments

Comments
 (0)