Merge remote-tracking branch 'origin/master' into TargetPurposeStringEnumValues_PR7257

Author: dconeybe

.changeset/olive-goats-greet.md

@@ -0,0 +1,5 @@
+---
+"@firebase/firestore": patch
+---
+
+Simplified the internal handling of aggregation results.

README.md

@@ -29,7 +29,7 @@ Please see [Environment Support](https://firebase.google.com/support/guides/envi
 #### Node.js
 
 Before you can start working on the Firebase JS SDK, you need to have Node.js
-installed on your machine. The currently supported versions are `10.15.0` or greater.
+installed on your machine. The currently supported versions are `10.15.0` through `16.6.0`.
 
 To download Node.js visit https://nodejs.org/en/download/.
 
@@ -43,9 +43,14 @@ In addition to Node.js we use `yarn` to facilitate multi package development.
 To install `yarn` follow the instructions listed on their website:
 https://yarnpkg.com/en/docs/install
 
+This repo currently supports building with yarn `1.x`. For instance, after installating yarn, run
+```bash
+$ yarn set version 1.22.11`
+```
+
 #### Java
 
-The closure compiler requires a modern Java installation. Java 8+ should be installed: http://www.oracle.com/technetwork/java/javase/downloads/jdk8-downloads-2133151.html
+The closure compiler requires a modern Java installation. Java 11+ should be installed: http://www.oracle.com/technetwork/java/javase/downloads/jdk8-downloads-2133151.html
 
 #### Verify Prerequisites
 
@@ -57,8 +62,8 @@ $ yarn -v
 $ java -version
 ```
 
-Your Node.js version should be `10.15.0` or greater, your `yarn` version should
-be `1.0.0` or greater, and your `java` version should be `1.8.0` or greater.
+Your `node` version should be between `10.15.0` and `16.6.0`, your `yarn` version should
+be between `1.0.0` and `1.22.11`, and your `java` version should be `11.0` or greater.
 
 _NOTE: We will update the documentation as new versions are required, however
 for continuing development on the SDK, staying up to date on the stable versions
@@ -109,7 +114,7 @@ will be overwritten below.
 
 Visit the "Realtime Database" section of the console and create a realtime
 database. When prompted to select the set of initial security rules, select
-any option (e.g. "Start in Production Mode") since these permission settings
+any option (e.g. "Start in Locked Mode") since these permission settings
 will be overwritten below.
 
 #### Storage Setup
@@ -127,8 +132,10 @@ order to run the tests, you will need to update your bucket's CORS rules.
     }
 ]
 ```
-2. Install `gsutil` from https://cloud.google.com/storage/docs/gsutil_install
-3. Run `gsutil cors set cors.json gs://<your-cloud-storage-bucket>`
+1. Install `gsutil` from https://cloud.google.com/storage/docs/gsutil_install
+1. You will need to login if this is your first time using `gsutil`. Run `gcloud auth login`
+and follow the instructions to login.
+1. Run `gsutil cors set cors.json gs://<your-cloud-storage-bucket>`
 
 For more information, visit https://firebase.google.com/docs/storage/web/download-files#cors_configuration
 

packages/firestore/src/api/aggregate.ts

@@ -20,8 +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 { ApiClientObjectMap, Value } from '../protos/firestore_proto_api';
 import { cast } from '../util/input_validation';
 import { mapToArray } from '../util/obj';
 
@@ -110,7 +109,7 @@ export function getAggregateFromServer<T extends AggregateSpec>(
 
   const internalAggregates = mapToArray(aggregateSpec, (aggregate, alias) => {
     return new AggregateImpl(
-      new AggregateAlias(alias),
+      alias,
       aggregate._aggregateType,
       aggregate._internalFieldPath
     );
@@ -136,7 +135,7 @@ export function getAggregateFromServer<T extends AggregateSpec>(
 function convertToAggregateQuerySnapshot<T extends AggregateSpec>(
   firestore: Firestore,
   query: Query<unknown>,
-  aggregateResult: ObjectValue
+  aggregateResult: ApiClientObjectMap<Value>
 ): AggregateQuerySnapshot<T> {
   const userDataWriter = new ExpUserDataWriter(firestore);
   const querySnapshot = new AggregateQuerySnapshot<T>(

packages/firestore/src/core/aggregate.ts

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

packages/firestore/src/core/firestore_client.ts

@@ -36,10 +36,10 @@ import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { FieldIndex } from '../model/field_index';
 import { Mutation } from '../model/mutation';
-import { ObjectValue } from '../model/object_value';
 import { toByteStreamReader } from '../platform/byte_stream_reader';
 import { newSerializer } from '../platform/serializer';
 import { newTextEncoder } from '../platform/text_serializer';
+import { ApiClientObjectMap, Value } from '../protos/firestore_proto_api';
 import { Datastore, invokeRunAggregationQueryRpc } from '../remote/datastore';
 import {
   RemoteStore,
@@ -526,8 +526,8 @@ export function firestoreClientRunAggregateQuery(
   client: FirestoreClient,
   query: Query,
   aggregates: Aggregate[]
-): Promise<ObjectValue> {
-  const deferred = new Deferred<ObjectValue>();
+): Promise<ApiClientObjectMap<Value>> {
+  const deferred = new Deferred<ApiClientObjectMap<Value>>();
 
   client.asyncQueue.enqueueAndForget(async () => {
     // TODO (sum/avg) should we update this to use the event manager?

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

@@ -18,8 +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 { ApiClientObjectMap, Value } from '../protos/firestore_proto_api';
 import { invokeRunAggregationQueryRpc } from '../remote/datastore';
 import { cast } from '../util/input_validation';
 import { mapToArray } from '../util/obj';
@@ -96,7 +95,7 @@ export function getAggregate<T extends AggregateSpec>(
 
   const internalAggregates = mapToArray(aggregateSpec, (aggregate, alias) => {
     return new AggregateImpl(
-      new AggregateAlias(alias),
+      alias,
       aggregate._aggregateType,
       aggregate._internalFieldPath
     );
@@ -115,7 +114,7 @@ export function getAggregate<T extends AggregateSpec>(
 function convertToAggregateQuerySnapshot<T extends AggregateSpec>(
   firestore: Firestore,
   query: Query<unknown>,
-  aggregateResult: ObjectValue
+  aggregateResult: ApiClientObjectMap<Value>
 ): AggregateQuerySnapshot<T> {
   const userDataWriter = new LiteUserDataWriter(firestore);
   const querySnapshot = new AggregateQuerySnapshot<T>(

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

@@ -16,8 +16,8 @@
  */
 
 import { AggregateType } from '../core/aggregate';
-import { ObjectValue } from '../model/object_value';
 import { FieldPath as InternalFieldPath } from '../model/path';
+import { ApiClientObjectMap, Value } from '../protos/firestore_proto_api';
 
 import { Query } from './reference';
 import { AbstractUserDataWriter } from './user_data_writer';
@@ -85,7 +85,7 @@ export class AggregateQuerySnapshot<T extends AggregateSpec> {
   constructor(
     query: Query<unknown>,
     private readonly _userDataWriter: AbstractUserDataWriter,
-    private readonly _data: ObjectValue
+    private readonly _data: ApiClientObjectMap<Value>
   ) {
     this.query = query;
   }
@@ -102,8 +102,8 @@ export class AggregateQuerySnapshot<T extends AggregateSpec> {
    * query.
    */
   data(): AggregateSpecData<T> {
-    return this._userDataWriter.convertValue(
-      this._data.value
+    return this._userDataWriter.convertObjectMap(
+      this._data
     ) as AggregateSpecData<T>;
   }
 }

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

@@ -32,10 +32,12 @@ import {
 import { TypeOrder } from '../model/type_order';
 import { typeOrder } from '../model/values';
 import {
+  ApiClientObjectMap,
   ArrayValue as ProtoArrayValue,
   LatLng as ProtoLatLng,
   MapValue as ProtoMapValue,
   Timestamp as ProtoTimestamp,
+  Value,
   Value as ProtoValue
 } from '../protos/firestore_proto_api';
 import { isValidResourceName } from '../remote/serializer';
@@ -91,9 +93,19 @@ export abstract class AbstractUserDataWriter {
   private convertObject(
     mapValue: ProtoMapValue,
     serverTimestampBehavior: ServerTimestampBehavior
+  ): DocumentData {
+    return this.convertObjectMap(mapValue.fields, serverTimestampBehavior);
+  }
+
+  /**
+   * @internal
+   */
+  convertObjectMap(
+    fields: ApiClientObjectMap<Value> | undefined,
+    serverTimestampBehavior: ServerTimestampBehavior = 'none'
   ): DocumentData {
     const result: DocumentData = {};
-    forEach(mapValue.fields, (key, value) => {
+    forEach(fields, (key, value) => {
       result[key] = this.convertValue(value, serverTimestampBehavior);
     });
     return result;

packages/firestore/src/model/aggregate_alias.ts

@@ -22,18 +22,20 @@ import { Query, queryToTarget } from '../core/query';
 import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { Mutation } from '../model/mutation';
-import { ObjectValue } from '../model/object_value';
 import {
+  ApiClientObjectMap,
   BatchGetDocumentsRequest as ProtoBatchGetDocumentsRequest,
   BatchGetDocumentsResponse as ProtoBatchGetDocumentsResponse,
   RunAggregationQueryRequest as ProtoRunAggregationQueryRequest,
   RunAggregationQueryResponse as ProtoRunAggregationQueryResponse,
   RunQueryRequest as ProtoRunQueryRequest,
-  RunQueryResponse as ProtoRunQueryResponse
+  RunQueryResponse as ProtoRunQueryResponse,
+  Value
 } from '../protos/firestore_proto_api';
 import { debugAssert, debugCast, hardAssert } from '../util/assert';
 import { AsyncQueue } from '../util/async_queue';
 import { Code, FirestoreError } from '../util/error';
+import { isNullOrUndefined } from '../util/types';
 
 import { Connection } from './connection';
 import {
@@ -50,8 +52,7 @@ import {
   toMutation,
   toName,
   toQueryTarget,
-  toRunAggregationQueryRequest,
-  fromAggregationResult
+  toRunAggregationQueryRequest
 } from './serializer';
 
 /**
@@ -243,9 +244,9 @@ export async function invokeRunAggregationQueryRpc(
   datastore: Datastore,
   query: Query,
   aggregates: Aggregate[]
-): Promise<ObjectValue> {
+): Promise<ApiClientObjectMap<Value>> {
   const datastoreImpl = debugCast(datastore, DatastoreImpl);
-  const request = toRunAggregationQueryRequest(
+  const { request, aliasMap } = toRunAggregationQueryRequest(
     datastoreImpl.serializer,
     queryToTarget(query),
     aggregates
@@ -267,8 +268,31 @@ export async function invokeRunAggregationQueryRpc(
     filteredResult.length === 1,
     'Aggregation fields are missing from result.'
   );
+  debugAssert(
+    !isNullOrUndefined(filteredResult[0].result),
+    'aggregationQueryResponse.result'
+  );
+  debugAssert(
+    !isNullOrUndefined(filteredResult[0].result.aggregateFields),
+    'aggregationQueryResponse.result.aggregateFields'
+  );
+
+  // Remap the short-form aliases that were sent to the server
+  // to the client-side aliases. Users will access the results
+  // using the client-side alias.
+  const unmappedAggregateFields = filteredResult[0].result?.aggregateFields;
+  const remappedFields = Object.keys(unmappedAggregateFields).reduce<
+    ApiClientObjectMap<Value>
+  >((accumulator, key) => {
+    debugAssert(
+      !isNullOrUndefined(aliasMap[key]),
+      `'${key}' not present in aliasMap result`
+    );
+    accumulator[aliasMap[key]] = unmappedAggregateFields[key]!;
+    return accumulator;
+  }, {});
 
-  return fromAggregationResult(filteredResult[0]);
+  return remappedFields;
 }
 
 export function newPersistentWriteStream(