Mila/count implement count query

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

@@ -15,26 +15,23 @@
  * limitations under the License.
  */
 
+import { CompositeFilterOpEnum } from '../protos/firestore_proto_api';
 import { invokeRunAggregationQueryRpc } from '../remote/datastore';
+import { hardAssert } from '../util/assert';
 import { cast } from '../util/input_validation';
 import { getDatastore } from './components';
 import { Firestore } from './database';
-import { DocumentData, Query, queryEqual } from './reference';
+import { Query, queryEqual } from './reference';
 import { LiteUserDataWriter } from './reference_impl';
 
 /**
- * A {@code AggregateQuery} computes some aggregation statistics from the result set of a base
- * {@link Query}.
- *
- * <p><b>Subclassing Note</b>: Cloud Firestore classes are not meant to be subclassed except for use
- * in test mocks. Subclassing is not supported in production code and new SDK releases may break
- * code that does so.
+ * A `AggregateQuery` computes some aggregation statistics from the result set of
+ * a base `Query`.
  */
 export class AggregateQuery {
   readonly type = 'AggregateQuery';
   /**
-   * The query on which you called {@link countQuery} in order to get this
-   * `AggregateQuery`.
+   * The query on which you called `countQuery` in order to get this `AggregateQuery`.
    * Query type is set to unknown to avoid error caused by query type converter.
    * might change it back to T after testing if the error do exist or not
    */
@@ -44,15 +41,10 @@ export class AggregateQuery {
   constructor(query: Query<unknown>) {
     this.query = query;
   }
-
 }
 
 /**
- * A {@code AggregateQuerySnapshot} contains results of a {@link AggregateQuery}.
- *
- * <p><b>Subclassing Note</b>: Cloud Firestore classes are not meant to be subclassed except for use
- * in test mocks. Subclassing is not supported in production code and new SDK releases may break
- * code that does so.
+ * A `AggregateQuerySnapshot` contains results of a `AggregateQuery`.
  */
 export class AggregateQuerySnapshot {
   readonly type = 'AggregateQuerySnapshot';
@@ -73,15 +65,12 @@ export class AggregateQuerySnapshot {
 }
 
 /**
- * Creates an {@link AggregateQuery} counting the number of documents matching this query.
+ * Creates an `AggregateQuery` counting the number of documents matching this query.
  *
- * @return An {@link AggregateQuery} object that can be used to count the number of documents in
+ * @return An `AggregateQuery` object that can be used to count the number of documents in
  * the result set of this query.
  */
 export function countQuery(query: Query<unknown>): AggregateQuery {
-  /**
-   * TODO(mila): add the "count" aggregateField to the params after the AggregateQuery is updated.
-   */
   return new AggregateQuery(query);
 }
 
@@ -94,25 +83,24 @@ export function getAggregateFromServerDirect(
 
   return invokeRunAggregationQueryRpc(datastore, aggregateQuery).then(
     result => {
-      const aggregationFields = new Map<string, any>();
-      /**
-       * while getting aggregation fields from server direct, it should have only
-       * one RunAggregationQueryResponse returned.
-       * But we used streaming rpc here, so we will have an array of
-       * (one, or possibly more) RunAggregationQueryResponse. For this specific
-       * function, we get the first RunAggregationQueryResponse only.
-       */
-      for (const [key, value] of Object.entries(result[0])) {
-        aggregationFields.set(key, userDataWriter.convertValue(value));
-      }
+      hardAssert(
+        result[0] !== undefined,
+        'Aggregation fields are missing from result.'
+      );
+
+      const countField = (result[0] as any).count_alias;
+      hardAssert(
+        countField !== undefined,
+        'Count field is missing from result.'
+      );
+
+      const countAggregateResult = userDataWriter.convertValue(countField);
+      hardAssert(
+        typeof countAggregateResult === 'number',
+        'Count aggeragte field is not a number: ' + countAggregateResult
+      );
       return Promise.resolve(
-        new AggregateQuerySnapshot(
-          aggregateQuery,
-          //return the count value , or null if count is undfined
-          aggregationFields.has('count_alias')
-            ? aggregationFields.get('count_alias')
-            : null
-        )
+        new AggregateQuerySnapshot(aggregateQuery, countAggregateResult)
       );
     }
   );

packages/firestore/src/remote/datastore.ts

@@ -254,8 +254,8 @@ export async function invokeRunAggregationQueryRpc(
   });
   return (
     response
-      // Omit RunQueryResponses that only contain readTimes.
-      .filter(proto => !!proto.result && !!proto.result.aggregateFields)
+      // Omit RunAggregationQueryResponse that only contain readTimes.
+      .filter(proto => !!proto.result)
       .map(proto => proto.result!.aggregateFields!)
   );
 }

packages/firestore/src/remote/serializer.ts

@@ -77,13 +77,13 @@ import {
   OrderDirection as ProtoOrderDirection,
   Precondition as ProtoPrecondition,
   QueryTarget as ProtoQueryTarget,
+  RunAggregationQueryRequest as ProtoRunAggregationQueryRequest,
   Status as ProtoStatus,
   Target as ProtoTarget,
   TargetChangeTargetChangeType as ProtoTargetChangeTargetChangeType,
   Timestamp as ProtoTimestamp,
   Write as ProtoWrite,
-  WriteResult as ProtoWriteResult,
-  RunAggregationQueryRequest as ProtoRunAggregationQueryRequest
+  WriteResult as ProtoWriteResult
 } from '../protos/firestore_proto_api';
 import { debugAssert, fail, hardAssert } from '../util/assert';
 import { ByteString } from '../util/byte_string';
@@ -857,62 +857,20 @@ export function toRunAggregationQueryRequest(
   serializer: JsonProtoSerializer,
   target: Target
 ): ProtoRunAggregationQueryRequest {
-  // Dissect the path into parent, collectionId, and optional key filter.
-  const result: ProtoRunAggregationQueryRequest = {
+  const queryTarget = toQueryTarget(serializer, target);
+
+  return {
     structuredAggregationQuery: {
       aggregations: [
-        //TODO: dynamically add the aggregate fields in future development
         {
-          count: {
-          },
-          alias:"count_alias"
-        },
+          count: {},
+          alias: 'count_alias'
+        }
       ],
-      structuredQuery: {}
-    }
+      structuredQuery: queryTarget.structuredQuery
+    },
+    parent: queryTarget.parent
   };
-  const path = target.path;
-  if (target.collectionGroup !== null) {
-    debugAssert(
-      path.length % 2 === 0,
-      'Collection Group queries should be within a document path or root.'
-    );
-    result.parent = toQueryPath(serializer, path);
-    result.structuredAggregationQuery!.structuredQuery!.from = [
-      {
-        collectionId: target.collectionGroup,
-        allDescendants: true
-      }
-    ];
-  } else {
-    debugAssert(
-      path.length % 2 !== 0,
-      'Document queries with filters are not supported.'
-    );
-    result.parent = toQueryPath(serializer, path.popLast());
-    result.structuredAggregationQuery!.structuredQuery!.from = [
-      { collectionId: path.lastSegment() }
-    ];
-  }
-
-  const where = toFilter(target.filters);
-  if (where) {
-    result.structuredAggregationQuery!.structuredQuery!.where = where;
-  }
-  /** QUESTION: in count query, do we need to add OrderBy? Number of
-   *  documents shouldn't be impacted by how the documents are ordered.
-   */
-  const orderBy = toOrder(target.orderBy);
-  if (orderBy) {
-    result.structuredAggregationQuery!.structuredQuery!.orderBy = orderBy;
-  }
-
-  const limit = toInt32Proto(serializer, target.limit);
-  if (limit !== null) {
-    result.structuredAggregationQuery!.structuredQuery!.limit = limit;
-  }
-
-  return result;
 }
 
 export function convertQueryTargetToQuery(target: ProtoQueryTarget): Query {