From 30355c50eb7bab88d8f10465232ce7d014f1e755 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 14 Oct 2024 14:13:17 +0200 Subject: [PATCH 1/5] Add error on throw --- packages/shell-api/src/collection.ts | 7 +++++-- packages/shell-api/src/database.spec.ts | 11 +++++++++++ packages/shell-api/src/database.ts | 9 +++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/packages/shell-api/src/collection.ts b/packages/shell-api/src/collection.ts index 1723425bb1..04b116b573 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 { async aggregate( pipeline: Document[], options: Document & { explain: ExplainVerbosityLike } - ): Promise; + ): Promise; async aggregate(...stages: Document[]): Promise; @returnsPromise @returnType('AggregationCursor') @@ -191,7 +191,10 @@ export default class Collection extends ShellApiWithMongoClass { this._database._name, this._name, pipeline, - { ...(await this._database._baseOptions()), ...aggOptions }, + { + ...(await this._database._baseOptions()), + ...aggOptions, + }, dbOptions ); const cursor = new AggregationCursor(this._mongo, providerCursor); diff --git a/packages/shell-api/src/database.spec.ts b/packages/shell-api/src/database.spec.ts index d4fd662e2f..c9684bd0eb 100644 --- a/packages/shell-api/src/database.spec.ts +++ b/packages/shell-api/src/database.spec.ts @@ -390,6 +390,17 @@ describe('Database', function () { serviceProviderCursor = stubInterface(); }); + it('throws if the given argument is not an array', async function () { + let caughtError: MongoshInvalidInputError | undefined; + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + await database.aggregate({} as any).catch((err) => { + caughtError = err; + }); + expect(caughtError?.message).contains( + 'Aggregate pipeline argument must be an array' + ); + }); + it('calls serviceProvider.aggregateDb with pipleline and options', async function () { await database.aggregate([{ $piplelineStage: {} }], { options: true }); diff --git a/packages/shell-api/src/database.ts b/packages/shell-api/src/database.ts index 1765c92998..e038999d7f 100644 --- a/packages/shell-api/src/database.ts +++ b/packages/shell-api/src/database.ts @@ -432,6 +432,14 @@ export default class Database extends ShellApiWithMongoClass { ); } assertArgsDefinedType([pipeline], [true], 'Database.aggregate'); + + if (!Array.isArray(pipeline)) { + throw new MongoshInvalidInputError( + 'Aggregate pipeline argument must be an array', + CommonErrors.InvalidArgument + ); + } + this._emitDatabaseApiCall('aggregate', { options, pipeline }); const { aggOptions, dbOptions, explain } = adaptAggregateOptions(options); @@ -1429,6 +1437,7 @@ export default class Database extends ShellApiWithMongoClass { CommonErrors.CommandFailed ); } + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion for (const cmdDescription of Object.values(result.commands) as Document[]) { if ('slaveOk' in cmdDescription) { cmdDescription.secondaryOk = cmdDescription.slaveOk; From bf52c7f14930e42de59429cfde29b8acf8d86f18 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 15 Oct 2024 11:33:42 +0200 Subject: [PATCH 2/5] add single stage support instead --- packages/shell-api/src/database.spec.ts | 19 +++++++++---------- packages/shell-api/src/database.ts | 14 +++++++++++++- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/packages/shell-api/src/database.spec.ts b/packages/shell-api/src/database.spec.ts index c9684bd0eb..a78e971012 100644 --- a/packages/shell-api/src/database.spec.ts +++ b/packages/shell-api/src/database.spec.ts @@ -390,19 +390,18 @@ describe('Database', function () { serviceProviderCursor = stubInterface(); }); - it('throws if the given argument is not an array', async function () { - let caughtError: MongoshInvalidInputError | undefined; - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument - await database.aggregate({} as any).catch((err) => { - caughtError = err; - }); - expect(caughtError?.message).contains( - 'Aggregate pipeline argument must be an array' + it('calls serviceProvider.aggregateDb with pipleline and options', async function () { + await database.aggregate([{ $piplelineStage: {} }], { options: true }); + + expect(serviceProvider.aggregateDb).to.have.been.calledWith( + database._name, + [{ $piplelineStage: {} }], + { options: true } ); }); - it('calls serviceProvider.aggregateDb with pipleline and options', async function () { - await database.aggregate([{ $piplelineStage: {} }], { options: true }); + it('supports a single aggregation stage', async function () { + await database.aggregate({ $piplelineStage: {} }, { options: true }); expect(serviceProvider.aggregateDb).to.have.been.calledWith( database._name, diff --git a/packages/shell-api/src/database.ts b/packages/shell-api/src/database.ts index e038999d7f..f129bac109 100644 --- a/packages/shell-api/src/database.ts +++ b/packages/shell-api/src/database.ts @@ -412,6 +412,14 @@ export default class Database extends ShellApiWithMongoClass { return await this._runAdminCommand(cmd, {}); } + async aggregate( + singleStage: Document, + options?: Document + ): Promise; + async aggregate( + pipeline: Document[], + options?: Document + ): Promise; /** * Run an aggregation against the db. * @@ -423,7 +431,7 @@ export default class Database extends ShellApiWithMongoClass { @returnType('AggregationCursor') @apiVersions([1]) async aggregate( - pipeline: Document[], + pipelineOrSingleStage: Document | Document[], options?: Document ): Promise { if ('background' in (options ?? {})) { @@ -431,6 +439,10 @@ export default class Database extends ShellApiWithMongoClass { aggregateBackgroundOptionNotSupportedHelp ); } + const pipeline: Document[] = Array.isArray(pipelineOrSingleStage) + ? pipelineOrSingleStage + : [pipelineOrSingleStage]; + assertArgsDefinedType([pipeline], [true], 'Database.aggregate'); if (!Array.isArray(pipeline)) { From f896fbd1479edd5214b3114f7a86b36cdc8419e1 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 15 Oct 2024 11:39:54 +0200 Subject: [PATCH 3/5] remove array check --- packages/shell-api/src/database.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/shell-api/src/database.ts b/packages/shell-api/src/database.ts index f129bac109..3ba7fc7f4d 100644 --- a/packages/shell-api/src/database.ts +++ b/packages/shell-api/src/database.ts @@ -445,13 +445,6 @@ export default class Database extends ShellApiWithMongoClass { assertArgsDefinedType([pipeline], [true], 'Database.aggregate'); - if (!Array.isArray(pipeline)) { - throw new MongoshInvalidInputError( - 'Aggregate pipeline argument must be an array', - CommonErrors.InvalidArgument - ); - } - this._emitDatabaseApiCall('aggregate', { options, pipeline }); const { aggOptions, dbOptions, explain } = adaptAggregateOptions(options); From d24305c5a5064e40b9191e6acd7977ca405e279d Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 17 Oct 2024 09:57:43 +0200 Subject: [PATCH 4/5] revert collection.ts changes --- packages/shell-api/src/collection.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/shell-api/src/collection.ts b/packages/shell-api/src/collection.ts index 04b116b573..1723425bb1 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 { async aggregate( pipeline: Document[], options: Document & { explain: ExplainVerbosityLike } - ): Promise; + ): Promise; async aggregate(...stages: Document[]): Promise; @returnsPromise @returnType('AggregationCursor') @@ -191,10 +191,7 @@ export default class Collection extends ShellApiWithMongoClass { this._database._name, this._name, pipeline, - { - ...(await this._database._baseOptions()), - ...aggOptions, - }, + { ...(await this._database._baseOptions()), ...aggOptions }, dbOptions ); const cursor = new AggregationCursor(this._mongo, providerCursor); From 5bde533c0b999229bbcaf4d76eff215d5de69a03 Mon Sep 17 00:00:00 2001 From: Gagik Amaryan Date: Thu, 17 Oct 2024 18:30:13 +0200 Subject: [PATCH 5/5] Update packages/shell-api/src/database.ts Co-authored-by: Anna Henningsen --- packages/shell-api/src/database.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/shell-api/src/database.ts b/packages/shell-api/src/database.ts index 3ba7fc7f4d..7c6ce4a3b6 100644 --- a/packages/shell-api/src/database.ts +++ b/packages/shell-api/src/database.ts @@ -412,14 +412,6 @@ export default class Database extends ShellApiWithMongoClass { return await this._runAdminCommand(cmd, {}); } - async aggregate( - singleStage: Document, - options?: Document - ): Promise; - async aggregate( - pipeline: Document[], - options?: Document - ): Promise; /** * Run an aggregation against the db. *