fix(shell-api): Align database and collection aggregate functions MON…

…GOSH-1868 (#2229)
mongodb-js · Oct 22, 2024 · c65a5b5 · c65a5b5
1 parent 42c83d6
commit c65a5b5
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 25 deletions.
diff --git a/packages/shell-api/src/collection.ts b/packages/shell-api/src/collection.ts
@@ -66,6 +66,7 @@ import type {
   UpdateOptions,
   DropCollectionOptions,
   CheckMetadataConsistencyOptions,
+  AggregateOptions,
 } from '@mongosh/service-provider-core';
 import type { RunCommandCursor, Database } from './index';
 import {
@@ -159,26 +160,27 @@ export default class Collection extends ShellApiWithMongoClass {
    */
   async aggregate(
     pipeline: Document[],
-    options: Document & { explain?: never }
-  ): Promise<AggregationCursor>;
+    options: AggregateOptions & { explain: ExplainVerbosityLike }
+  ): Promise<Document>;
   async aggregate(
     pipeline: Document[],
-    options: Document & { explain: ExplainVerbosityLike }
-  ): Promise<Document>;
+    options?: AggregateOptions
+  ): Promise<AggregationCursor>;
   async aggregate(...stages: Document[]): Promise<AggregationCursor>;
   @returnsPromise
   @returnType('AggregationCursor')
   @apiVersions([1])
-  async aggregate(...args: any[]): Promise<any> {
-    let options;
-    let pipeline;
+  async aggregate(...args: unknown[]): Promise<AggregationCursor | Document> {
+    let options: AggregateOptions;
+    let pipeline: Document[];
     if (args.length === 0 || Array.isArray(args[0])) {
       options = args[1] || {};
-      pipeline = args[0] || [];
+      pipeline = (args[0] as Document[]) || [];
     } else {
       options = {};
-      pipeline = args || [];
+      pipeline = (args as Document[]) || [];
     }
+
     if ('background' in options) {
       await this._instanceState.printWarning(
         aggregateBackgroundOptionNotSupportedHelp

diff --git a/packages/shell-api/src/database.spec.ts b/packages/shell-api/src/database.spec.ts
@@ -401,12 +401,25 @@ describe('Database', function () {
       });
 
       it('supports a single aggregation stage', async function () {
-        await database.aggregate({ $piplelineStage: {} }, { options: true });
+        await database.aggregate({ $piplelineStage: {} });
 
         expect(serviceProvider.aggregateDb).to.have.been.calledWith(
           database._name,
           [{ $piplelineStage: {} }],
-          { options: true }
+          {}
+        );
+      });
+
+      it('supports passing args as aggregation stages', async function () {
+        await database.aggregate(
+          { $piplelineStage: {} },
+          { $piplelineStage2: {} }
+        );
+
+        expect(serviceProvider.aggregateDb).to.have.been.calledWith(
+          database._name,
+          [{ $piplelineStage: {} }, { $piplelineStage2: {} }],
+          {}
         );
       });
 
@@ -2891,7 +2904,9 @@ describe('Database', function () {
       it('runs a $sql aggregation', async function () {
         const serviceProviderCursor = stubInterface<ServiceProviderAggCursor>();
         serviceProvider.aggregateDb.returns(serviceProviderCursor as any);
-        await database.sql('SELECT * FROM somecollection;', { options: true });
+        await database.sql('SELECT * FROM somecollection;', {
+          serializeFunctions: true,
+        });
         expect(serviceProvider.aggregateDb).to.have.been.calledWith(
           database._name,
           [
@@ -2904,7 +2919,7 @@ describe('Database', function () {
               },
             },
           ],
-          { options: true }
+          { serializeFunctions: true }
         );
       });
 

diff --git a/packages/shell-api/src/database.ts b/packages/shell-api/src/database.ts
@@ -52,6 +52,8 @@ import type {
   CreateEncryptedCollectionOptions,
   CheckMetadataConsistencyOptions,
   RunCommandOptions,
+  ExplainVerbosityLike,
+  AggregateOptions,
 } from '@mongosh/service-provider-core';
 
 export type CollectionNamesWithTypes = {
@@ -413,27 +415,38 @@ export default class Database extends ShellApiWithMongoClass {
   }
 
   /**
-   * Run an aggregation against the db.
+   * Run an aggregation against the database. Accepts array pipeline and options object OR stages as individual arguments.
    *
-   * @param pipeline
-   * @param options
    * @returns {Promise} The promise of aggregation results.
    */
+  async aggregate(
+    pipeline: Document[],
+    options: AggregateOptions & { explain: ExplainVerbosityLike }
+  ): Promise<Document>;
+  async aggregate(
+    pipeline: Document[],
+    options?: AggregateOptions
+  ): Promise<AggregationCursor>;
+  async aggregate(...stages: Document[]): Promise<AggregationCursor>;
   @returnsPromise
   @returnType('AggregationCursor')
   @apiVersions([1])
-  async aggregate(
-    pipelineOrSingleStage: Document | Document[],
-    options?: Document
-  ): Promise<AggregationCursor> {
-    if ('background' in (options ?? {})) {
+  async aggregate(...args: unknown[]): Promise<AggregationCursor | Document> {
+    let options: AggregateOptions;
+    let pipeline: Document[];
+    if (args.length === 0 || Array.isArray(args[0])) {
+      options = args[1] || {};
+      pipeline = (args[0] as Document[]) || [];
+    } else {
+      options = {};
+      pipeline = (args as Document[]) || [];
+    }
+
+    if ('background' in options) {
       await this._instanceState.printWarning(
         aggregateBackgroundOptionNotSupportedHelp
       );
     }
-    const pipeline: Document[] = Array.isArray(pipelineOrSingleStage)
-      ? pipelineOrSingleStage
-      : [pipelineOrSingleStage];
 
     assertArgsDefinedType([pipeline], [true], 'Database.aggregate');
 
@@ -1731,7 +1744,10 @@ export default class Database extends ShellApiWithMongoClass {
   @serverVersions(['4.4.0', ServerVersions.latest])
   @returnsPromise
   @returnType('AggregationCursor')
-  async sql(sqlString: string, options?: Document): Promise<AggregationCursor> {
+  async sql(
+    sqlString: string,
+    options?: AggregateOptions
+  ): Promise<AggregationCursor> {
     this._emitDatabaseApiCall('sql', { sqlString: sqlString, options });
     await this._instanceState.shellApi.print(
       'Note: this is an experimental feature that may be subject to change in future releases.'