Skip to content

Commit 51e4059

Browse files
Add long-alias support for aggregations. (#7244)
* Add long-alias support for aggregations.
1 parent bddc177 commit 51e4059

File tree

7 files changed

+69
-66
lines changed

7 files changed

+69
-66
lines changed

packages/firestore/src/api/aggregate.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import { AggregateImpl } from '../core/aggregate';
2020
import { firestoreClientRunAggregateQuery } from '../core/firestore_client';
2121
import { count } from '../lite-api/aggregate';
2222
import { AggregateQuerySnapshot } from '../lite-api/aggregate_types';
23-
import { AggregateAlias } from '../model/aggregate_alias';
2423
import { ApiClientObjectMap, Value } from '../protos/firestore_proto_api';
2524
import { cast } from '../util/input_validation';
2625
import { mapToArray } from '../util/obj';
@@ -110,7 +109,7 @@ export function getAggregateFromServer<T extends AggregateSpec>(
110109

111110
const internalAggregates = mapToArray(aggregateSpec, (aggregate, alias) => {
112111
return new AggregateImpl(
113-
new AggregateAlias(alias),
112+
alias,
114113
aggregate._aggregateType,
115114
aggregate._internalFieldPath
116115
);

packages/firestore/src/core/aggregate.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { AggregateAlias } from '../model/aggregate_alias';
1918
import { FieldPath } from '../model/path';
2019

2120
/**
@@ -29,7 +28,7 @@ export type AggregateType = 'count' | 'avg' | 'sum';
2928
*/
3029
export interface Aggregate {
3130
readonly fieldPath?: FieldPath;
32-
readonly alias: AggregateAlias;
31+
readonly alias: string;
3332
readonly aggregateType: AggregateType;
3433
}
3534

@@ -38,7 +37,7 @@ export interface Aggregate {
3837
*/
3938
export class AggregateImpl implements Aggregate {
4039
constructor(
41-
readonly alias: AggregateAlias,
40+
readonly alias: string,
4241
readonly aggregateType: AggregateType,
4342
readonly fieldPath?: FieldPath
4443
) {}

packages/firestore/src/lite-api/aggregate.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import { deepEqual } from '@firebase/util';
1919

2020
import { AggregateImpl } from '../core/aggregate';
21-
import { AggregateAlias } from '../model/aggregate_alias';
2221
import { ApiClientObjectMap, Value } from '../protos/firestore_proto_api';
2322
import { invokeRunAggregationQueryRpc } from '../remote/datastore';
2423
import { cast } from '../util/input_validation';
@@ -96,7 +95,7 @@ export function getAggregate<T extends AggregateSpec>(
9695

9796
const internalAggregates = mapToArray(aggregateSpec, (aggregate, alias) => {
9897
return new AggregateImpl(
99-
new AggregateAlias(alias),
98+
alias,
10099
aggregate._aggregateType,
101100
aggregate._internalFieldPath
102101
);

packages/firestore/src/model/aggregate_alias.ts

Lines changed: 0 additions & 48 deletions
This file was deleted.

packages/firestore/src/remote/datastore.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ export async function invokeRunAggregationQueryRpc(
246246
aggregates: Aggregate[]
247247
): Promise<ApiClientObjectMap<Value>> {
248248
const datastoreImpl = debugCast(datastore, DatastoreImpl);
249-
const request = toRunAggregationQueryRequest(
249+
const { request, aliasMap } = toRunAggregationQueryRequest(
250250
datastoreImpl.serializer,
251251
queryToTarget(query),
252252
aggregates
@@ -277,7 +277,22 @@ export async function invokeRunAggregationQueryRpc(
277277
'aggregationQueryResponse.result.aggregateFields'
278278
);
279279

280-
return filteredResult[0].result.aggregateFields;
280+
// Remap the short-form aliases that were sent to the server
281+
// to the client-side aliases. Users will access the results
282+
// using the client-side alias.
283+
const unmappedAggregateFields = filteredResult[0].result?.aggregateFields;
284+
const remappedFields = Object.keys(unmappedAggregateFields).reduce<
285+
ApiClientObjectMap<Value>
286+
>((accumulator, key) => {
287+
debugAssert(
288+
!isNullOrUndefined(aliasMap[key]),
289+
`'${key}' not present in aliasMap result`
290+
);
291+
accumulator[aliasMap[key]] = unmappedAggregateFields[key]!;
292+
return accumulator;
293+
}, {});
294+
295+
return remappedFields;
281296
}
282297

283298
export function newPersistentWriteStream(

packages/firestore/src/remote/serializer.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -891,26 +891,38 @@ export function toRunAggregationQueryRequest(
891891
serializer: JsonProtoSerializer,
892892
target: Target,
893893
aggregates: Aggregate[]
894-
): ProtoRunAggregationQueryRequest {
894+
): {
895+
request: ProtoRunAggregationQueryRequest;
896+
aliasMap: Record<string, string>;
897+
} {
895898
const queryTarget = toQueryTarget(serializer, target);
899+
const aliasMap: Record<string, string> = {};
896900

897901
const aggregations: ProtoAggregation[] = [];
902+
let aggregationNum = 0;
903+
898904
aggregates.forEach(aggregate => {
905+
// Map all client-side aliases to a unique short-form
906+
// alias. This avoids issues with client-side aliases that
907+
// exceed the 1500-byte string size limit.
908+
const serverAlias = `aggregate_${aggregationNum++}`;
909+
aliasMap[serverAlias] = aggregate.alias;
910+
899911
if (aggregate.aggregateType === 'count') {
900912
aggregations.push({
901-
alias: aggregate.alias.canonicalString(),
913+
alias: serverAlias,
902914
count: {}
903915
});
904916
} else if (aggregate.aggregateType === 'avg') {
905917
aggregations.push({
906-
alias: aggregate.alias.canonicalString(),
918+
alias: serverAlias,
907919
avg: {
908920
field: toFieldPathReference(aggregate.fieldPath!)
909921
}
910922
});
911923
} else if (aggregate.aggregateType === 'sum') {
912924
aggregations.push({
913-
alias: aggregate.alias.canonicalString(),
925+
alias: serverAlias,
914926
sum: {
915927
field: toFieldPathReference(aggregate.fieldPath!)
916928
}
@@ -919,11 +931,14 @@ export function toRunAggregationQueryRequest(
919931
});
920932

921933
return {
922-
structuredAggregationQuery: {
923-
aggregations,
924-
structuredQuery: queryTarget.structuredQuery
934+
request: {
935+
structuredAggregationQuery: {
936+
aggregations,
937+
structuredQuery: queryTarget.structuredQuery
938+
},
939+
parent: queryTarget.parent
925940
},
926-
parent: queryTarget.parent
941+
aliasMap
927942
};
928943
}
929944

packages/firestore/test/integration/api/aggregation.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,30 @@ apiDescribe('Aggregation queries', (persistence: boolean) => {
218218
});
219219
});
220220

221+
it('allows aliases with length greater than 1500 bytes', () => {
222+
// Alias string length is bytes of UTF-8 encoded alias + 1;
223+
let longAlias = '';
224+
for (let i = 0; i < 150; i++) {
225+
longAlias += '0123456789';
226+
}
227+
228+
const longerAlias = longAlias + longAlias;
229+
230+
const testDocs = {
231+
a: { num: 3 },
232+
b: { num: 5 }
233+
};
234+
return withTestCollection(persistence, testDocs, async coll => {
235+
const snapshot = await getAggregateFromServer(coll, {
236+
[longAlias]: count(),
237+
[longerAlias]: count()
238+
});
239+
240+
expect(snapshot.data()[longAlias]).to.equal(2);
241+
expect(snapshot.data()[longerAlias]).to.equal(2);
242+
});
243+
});
244+
221245
it('can get duplicate aggregations using getAggregationFromServer', () => {
222246
const testDocs = {
223247
a: { author: 'authorA', title: 'titleA' },
@@ -432,7 +456,7 @@ apiDescribe.skip(
432456
});
433457

434458
await expect(promise).to.eventually.be.rejectedWith(
435-
/INVALID_ARGUMENT.*maximum number of aggregations/
459+
/maximum number of aggregations/
436460
);
437461
});
438462
});

0 commit comments

Comments
 (0)