Backporting - return aggregation results as a map from the remote layer instead of as an ObjectValue.

Author: MarkDuckworth

packages/firestore/src/api/aggregate.ts

@@ -21,13 +21,14 @@ 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';
 
 import { ensureFirestoreConfigured, Firestore } from './database';
 import { ExpUserDataWriter } from './reference_impl';
 
+
 export {
   aggregateQuerySnapshotEqual,
   count,
@@ -136,7 +137,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/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 {
   canUseNetwork,
@@ -527,8 +527,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

@@ -19,7 +19,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';
@@ -115,7 +115,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/remote/datastore.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';
 
 /**
@@ -242,7 +243,7 @@ export async function invokeRunAggregationQueryRpc(
   datastore: Datastore,
   query: Query,
   aggregates: Aggregate[]
-): Promise<ObjectValue> {
+): Promise<ApiClientObjectMap<Value>> {
   const datastoreImpl = debugCast(datastore, DatastoreImpl);
   const request = toRunAggregationQueryRequest(
     datastoreImpl.serializer,
@@ -266,8 +267,16 @@ 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'
+  );
 
-  return fromAggregationResult(filteredResult[0]);
+  return filteredResult[0].result.aggregateFields;
 }
 
 export function newPersistentWriteStream(

packages/firestore/src/remote/serializer.ts

@@ -81,7 +81,6 @@ import {
   Precondition as ProtoPrecondition,
   QueryTarget as ProtoQueryTarget,
   RunAggregationQueryRequest as ProtoRunAggregationQueryRequest,
-  RunAggregationQueryResponse as ProtoRunAggregationQueryResponse,
   Aggregation as ProtoAggregation,
   Status as ProtoStatus,
   Target as ProtoTarget,
@@ -438,22 +437,6 @@ export function fromDocument(
   return hasCommittedMutations ? result.setHasCommittedMutations() : result;
 }
 
-export function fromAggregationResult(
-  aggregationQueryResponse: ProtoRunAggregationQueryResponse
-): ObjectValue {
-  assertPresent(
-    aggregationQueryResponse.result,
-    'aggregationQueryResponse.result'
-  );
-  assertPresent(
-    aggregationQueryResponse.result.aggregateFields,
-    'aggregationQueryResponse.result.aggregateFields'
-  );
-  return new ObjectValue({
-    mapValue: { fields: aggregationQueryResponse.result?.aggregateFields }
-  });
-}
-
 function fromFound(
   serializer: JsonProtoSerializer,
   doc: ProtoBatchGetDocumentsResponse