Merge e6a5c16 into 7e4521a

Author: MarkDuckworth

packages/firestore/lite/index.ts

@@ -33,7 +33,8 @@ export {
   getAggregate,
   count,
   sum,
-  average
+  average,
+  aggregateFieldEqual
 } from '../src/lite-api/aggregate';
 
 export {

packages/firestore/src/api.ts

@@ -21,7 +21,8 @@ export {
   getAggregateFromServer,
   count,
   sum,
-  average
+  average,
+  aggregateFieldEqual
 } from './api/aggregate';
 
 export {

packages/firestore/src/api/aggregate.ts

@@ -20,6 +20,7 @@ import { AggregateImpl } from '../core/aggregate';
 import { firestoreClientRunAggregateQuery } from '../core/firestore_client';
 import { count } from '../lite-api/aggregate';
 import { AggregateQuerySnapshot } from '../lite-api/aggregate_types';
+import { AggregateAlias } from '../model/aggregate_alias';
 import { ObjectValue } from '../model/object_value';
 import { cast } from '../util/input_validation';
 import { mapToArray } from '../util/obj';
@@ -31,7 +32,8 @@ export {
   aggregateQuerySnapshotEqual,
   count,
   sum,
-  average
+  average,
+  aggregateFieldEqual
 } from '../lite-api/aggregate';
 
 /**
@@ -107,11 +109,8 @@ export function getAggregateFromServer<T extends AggregateSpec>(
   const client = ensureFirestoreConfigured(firestore);
 
   const internalAggregates = mapToArray(aggregateSpec, (aggregate, alias) => {
-    // TODO (sum/avg) should alias validation be performed or should that be
-    // delegated to the backend?
-
     return new AggregateImpl(
-      alias,
+      new AggregateAlias(alias),
       aggregate._aggregateType,
       aggregate._internalFieldPath
     );

packages/firestore/src/core/aggregate.ts

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { AggregateAlias } from '../model/aggregate_alias';
 import { FieldPath } from '../model/path';
 
 /**
@@ -28,7 +29,7 @@ export type AggregateType = 'count' | 'avg' | 'sum';
  */
 export interface Aggregate {
   readonly fieldPath?: FieldPath;
-  readonly alias: string;
+  readonly alias: AggregateAlias;
   readonly aggregateType: AggregateType;
 }
 
@@ -37,7 +38,7 @@ export interface Aggregate {
  */
 export class AggregateImpl implements Aggregate {
   constructor(
-    readonly alias: string,
+    readonly alias: AggregateAlias,
     readonly aggregateType: AggregateType,
     readonly fieldPath?: FieldPath
   ) {}

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

@@ -18,6 +18,7 @@
 import { deepEqual } from '@firebase/util';
 
 import { AggregateImpl } from '../core/aggregate';
+import { AggregateAlias } from '../model/aggregate_alias';
 import { ObjectValue } from '../model/object_value';
 import { invokeRunAggregationQueryRpc } from '../remote/datastore';
 import { cast } from '../util/input_validation';
@@ -94,11 +95,8 @@ export function getAggregate<T extends AggregateSpec>(
   const datastore = getDatastore(firestore);
 
   const internalAggregates = mapToArray(aggregateSpec, (aggregate, alias) => {
-    // TODO (sum/avg) should alias validation be performed or should that be
-    // delegated to the backend?
-
     return new AggregateImpl(
-      alias,
+      new AggregateAlias(alias),
       aggregate._aggregateType,
       aggregate._internalFieldPath
     );
@@ -164,6 +162,7 @@ export function count(): AggregateField<number> {
  *
  * @param left Compare this AggregateField to the `right`.
  * @param right Compare this AggregateField to the `left`.
+ * @internal TODO (sum/avg) remove when public
  */
 export function aggregateFieldEqual(
   left: AggregateField<unknown>,

packages/firestore/src/model/aggregate_alias.ts

@@ -0,0 +1,48 @@
+/**
+ * @license
+ * 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.
+ */
+
+const aliasRegExp = /^[_a-zA-Z][_a-zA-Z0-9]*(?:\.[_a-zA-Z][_a-zA-Z0-9]*)*$/;
+
+/**
+ * An alias for aggregation results.
+ * @internal
+ */
+export class AggregateAlias {
+  /**
+   * @internal
+   * @param alias Un-escaped alias representation
+   */
+  constructor(private alias: string) {}
+
+  /**
+   * Returns true if the string could be used as an alias.
+   */
+  private static isValidAlias(value: string): boolean {
+    return aliasRegExp.test(value);
+  }
+
+  /**
+   * Return an escaped and quoted string representation of the alias.
+   */
+  canonicalString(): string {
+    let alias = this.alias.replace(/\\/g, '\\\\').replace(/`/g, '\\`');
+    if (!AggregateAlias.isValidAlias(alias)) {
+      alias = '`' + alias + '`';
+    }
+    return alias;
+  }
+}

packages/firestore/src/remote/serializer.ts

@@ -915,19 +915,19 @@ export function toRunAggregationQueryRequest(
   aggregates.forEach(aggregate => {
     if (aggregate.aggregateType === 'count') {
       aggregations.push({
-        alias: aggregate.alias,
+        alias: aggregate.alias.canonicalString(),
         count: {}
       });
     } else if (aggregate.aggregateType === 'avg') {
       aggregations.push({
-        alias: aggregate.alias,
+        alias: aggregate.alias.canonicalString(),
         avg: {
           field: toFieldPathReference(aggregate.fieldPath!)
         }
       });
     } else if (aggregate.aggregateType === 'sum') {
       aggregations.push({
-        alias: aggregate.alias,
+        alias: aggregate.alias.canonicalString(),
         sum: {
           field: toFieldPathReference(aggregate.fieldPath!)
         }