From 5c6789c48b588b55a133a0b27530c3af06631c7d Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 21 Oct 2024 13:49:25 +0200 Subject: [PATCH 1/5] Align behavior completely --- packages/shell-api/src/collection.ts | 16 +++++----- packages/shell-api/src/database.spec.ts | 17 +++++++++-- packages/shell-api/src/database.ts | 40 +++++++++++++++++-------- 3 files changed, 52 insertions(+), 21 deletions(-) diff --git a/packages/shell-api/src/collection.ts b/packages/shell-api/src/collection.ts index a8c699ff5c..699b8a17d5 100644 --- 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 } + options: AggregateOptions & { explain?: never } ): Promise; async aggregate( pipeline: Document[], - options: Document & { explain: ExplainVerbosityLike } + options: AggregateOptions & { explain: ExplainVerbosityLike } ): Promise; async aggregate(...stages: Document[]): Promise; @returnsPromise @returnType('AggregationCursor') @apiVersions([1]) - async aggregate(...args: any[]): Promise { - let options; - let pipeline; + async aggregate(...args: unknown[]): Promise { + 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 index a78e971012..c9d3dc1289 100644 --- 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: {} }], + {} ); }); diff --git a/packages/shell-api/src/database.ts b/packages/shell-api/src/database.ts index 7c6ce4a3b6..e00b578bee 100644 --- 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; + async aggregate( + pipeline: Document[], + options?: AggregateOptions + ): Promise; + async aggregate(...stages: Document[]): Promise; @returnsPromise @returnType('AggregationCursor') @apiVersions([1]) - async aggregate( - pipelineOrSingleStage: Document | Document[], - options?: Document - ): Promise { - if ('background' in (options ?? {})) { + async aggregate(...args: unknown[]): Promise { + 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 { + async sql( + sqlString: string, + options?: AggregateOptions + ): Promise { 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.' From 51f3704be842e89b3b3e269e3cc1db0ad0b4bf07 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 21 Oct 2024 13:50:22 +0200 Subject: [PATCH 2/5] Fix typecheck test --- packages/shell-api/src/database.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/shell-api/src/database.spec.ts b/packages/shell-api/src/database.spec.ts index c9d3dc1289..4cc7cb7e22 100644 --- a/packages/shell-api/src/database.spec.ts +++ b/packages/shell-api/src/database.spec.ts @@ -2904,7 +2904,9 @@ describe('Database', function () { it('runs a $sql aggregation', async function () { const serviceProviderCursor = stubInterface(); 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, [ @@ -2917,7 +2919,7 @@ describe('Database', function () { }, }, ], - { options: true } + { serializeFunctions: true } ); }); From f765124d9ef76a40894df91ad8bbae5b21a7b36c Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 21 Oct 2024 13:55:31 +0200 Subject: [PATCH 3/5] Fix some typescript --- packages/shell-api/src/collection.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/shell-api/src/collection.ts b/packages/shell-api/src/collection.ts index 699b8a17d5..054e9ef3f1 100644 --- a/packages/shell-api/src/collection.ts +++ b/packages/shell-api/src/collection.ts @@ -158,14 +158,14 @@ export default class Collection extends ShellApiWithMongoClass { * * @returns {Promise} The promise of aggregation results. */ - async aggregate( - pipeline: Document[], - options: AggregateOptions & { explain?: never } - ): Promise; async aggregate( pipeline: Document[], options: AggregateOptions & { explain: ExplainVerbosityLike } ): Promise; + async aggregate( + pipeline: Document[], + options: AggregateOptions + ): Promise; async aggregate(...stages: Document[]): Promise; @returnsPromise @returnType('AggregationCursor') From 0df728a9b94af7ee5f1bcbdc4a495bb58efe4b10 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 21 Oct 2024 14:49:58 +0200 Subject: [PATCH 4/5] Let options also be undefined for collections --- packages/shell-api/src/collection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shell-api/src/collection.ts b/packages/shell-api/src/collection.ts index 054e9ef3f1..b2818792ab 100644 --- a/packages/shell-api/src/collection.ts +++ b/packages/shell-api/src/collection.ts @@ -164,7 +164,7 @@ export default class Collection extends ShellApiWithMongoClass { ): Promise; async aggregate( pipeline: Document[], - options: AggregateOptions + options?: AggregateOptions ): Promise; async aggregate(...stages: Document[]): Promise; @returnsPromise From 02c74f1d82f20a7eb7bac4c2cae240ec7ab72565 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 22 Oct 2024 14:00:01 +0200 Subject: [PATCH 5/5] align return types --- packages/shell-api/src/collection.ts | 2 +- packages/shell-api/src/database.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/shell-api/src/collection.ts b/packages/shell-api/src/collection.ts index b2818792ab..911dcc45df 100644 --- a/packages/shell-api/src/collection.ts +++ b/packages/shell-api/src/collection.ts @@ -170,7 +170,7 @@ export default class Collection extends ShellApiWithMongoClass { @returnsPromise @returnType('AggregationCursor') @apiVersions([1]) - async aggregate(...args: unknown[]): Promise { + async aggregate(...args: unknown[]): Promise { let options: AggregateOptions; let pipeline: Document[]; if (args.length === 0 || Array.isArray(args[0])) { diff --git a/packages/shell-api/src/database.ts b/packages/shell-api/src/database.ts index e00b578bee..c148cc14e1 100644 --- a/packages/shell-api/src/database.ts +++ b/packages/shell-api/src/database.ts @@ -431,7 +431,7 @@ export default class Database extends ShellApiWithMongoClass { @returnsPromise @returnType('AggregationCursor') @apiVersions([1]) - async aggregate(...args: unknown[]): Promise { + async aggregate(...args: unknown[]): Promise { let options: AggregateOptions; let pipeline: Document[]; if (args.length === 0 || Array.isArray(args[0])) {