From 8af288484bd516c2ef0bde3222d24bdf6c6b70e5 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 9 Oct 2024 11:37:00 -0600 Subject: [PATCH] implementation --- src/cursor/aggregation_cursor.ts | 53 ++++- src/cursor/find_cursor.ts | 56 +++++- src/explain.ts | 37 ++++ src/operations/command.ts | 15 +- src/operations/find.ts | 4 +- src/utils.ts | 27 --- test/integration/crud/explain.test.ts | 266 ++++++++++++++++++++++++++ test/tools/utils.ts | 26 +++ 8 files changed, 438 insertions(+), 46 deletions(-) diff --git a/src/cursor/aggregation_cursor.ts b/src/cursor/aggregation_cursor.ts index 056f28454ce..03ec11c0979 100644 --- a/src/cursor/aggregation_cursor.ts +++ b/src/cursor/aggregation_cursor.ts @@ -1,6 +1,11 @@ import type { Document } from '../bson'; import { MongoAPIError } from '../error'; -import type { ExplainCommandOptions, ExplainVerbosityLike } from '../explain'; +import { + Explain, + type ExplainCommandOptions, + type ExplainVerbosityLike, + validateExplainTimeoutOptions +} from '../explain'; import type { MongoClient } from '../mongo_client'; import { AggregateOperation, type AggregateOptions } from '../operations/aggregate'; import { executeOperation } from '../operations/execute_operation'; @@ -65,11 +70,20 @@ export class AggregationCursor extends AbstractCursor { /** @internal */ async _initialize(session: ClientSession): Promise { - const aggregateOperation = new AggregateOperation(this.namespace, this.pipeline, { + const options = { ...this.aggregateOptions, ...this.cursorOptions, session - }); + }; + try { + validateExplainTimeoutOptions(options, Explain.fromOptions(options)); + } catch { + throw new MongoAPIError( + 'timeoutMS cannot be used with explain when explain is specified in aggregateOptions' + ); + } + + const aggregateOperation = new AggregateOperation(this.namespace, this.pipeline, options); const response = await executeOperation(this.client, aggregateOperation, this.timeoutContext); @@ -77,14 +91,43 @@ export class AggregationCursor extends AbstractCursor { } /** Execute the explain for the cursor */ - async explain(verbosity?: ExplainVerbosityLike | ExplainCommandOptions): Promise { + async explain(): Promise; + async explain(verbosity: ExplainVerbosityLike | ExplainCommandOptions): Promise; + async explain(options: { timeoutMS?: number }): Promise; + async explain( + verbosity: ExplainVerbosityLike | ExplainCommandOptions, + options: { timeoutMS?: number } + ): Promise; + async explain( + verbosity?: ExplainVerbosityLike | ExplainCommandOptions | { timeoutMS?: number }, + options?: { timeoutMS?: number } + ): Promise { + let explain: ExplainVerbosityLike | ExplainCommandOptions | undefined; + let timeout: { timeoutMS?: number } | undefined; + if (verbosity == null && options == null) { + explain = true; + timeout = undefined; + } else if (verbosity != null && options == null) { + explain = + typeof verbosity !== 'object' + ? verbosity + : 'timeoutMS' in verbosity + ? undefined + : (verbosity as ExplainCommandOptions); + timeout = typeof verbosity === 'object' && 'timeoutMS' in verbosity ? verbosity : undefined; + } else { + explain = verbosity as ExplainCommandOptions; + timeout = options; + } + return ( await executeOperation( this.client, new AggregateOperation(this.namespace, this.pipeline, { ...this.aggregateOptions, // NOTE: order matters here, we may need to refine this ...this.cursorOptions, - explain: verbosity ?? true + ...timeout, + explain: explain ?? true }) ) ).shift(this.deserializationOptions); diff --git a/src/cursor/find_cursor.ts b/src/cursor/find_cursor.ts index 96b764dc7ff..3bb859df291 100644 --- a/src/cursor/find_cursor.ts +++ b/src/cursor/find_cursor.ts @@ -1,7 +1,12 @@ import { type Document } from '../bson'; import { CursorResponse } from '../cmap/wire_protocol/responses'; -import { MongoInvalidArgumentError, MongoTailableCursorError } from '../error'; -import { type ExplainCommandOptions, type ExplainVerbosityLike } from '../explain'; +import { MongoAPIError, MongoInvalidArgumentError, MongoTailableCursorError } from '../error'; +import { + Explain, + type ExplainCommandOptions, + type ExplainVerbosityLike, + validateExplainTimeoutOptions +} from '../explain'; import type { MongoClient } from '../mongo_client'; import type { CollationOptions } from '../operations/command'; import { CountOperation, type CountOptions } from '../operations/count'; @@ -63,11 +68,21 @@ export class FindCursor extends AbstractCursor { /** @internal */ async _initialize(session: ClientSession): Promise { - const findOperation = new FindOperation(this.namespace, this.cursorFilter, { + const options = { ...this.findOptions, // NOTE: order matters here, we may need to refine this ...this.cursorOptions, session - }); + }; + + try { + validateExplainTimeoutOptions(options, Explain.fromOptions(options)); + } catch { + throw new MongoAPIError( + 'timeoutMS cannot be used with explain when explain is specified in findOptions' + ); + } + + const findOperation = new FindOperation(this.namespace, this.cursorFilter, options); const response = await executeOperation(this.client, findOperation, this.timeoutContext); @@ -133,14 +148,43 @@ export class FindCursor extends AbstractCursor { } /** Execute the explain for the cursor */ - async explain(verbosity?: ExplainVerbosityLike | ExplainCommandOptions): Promise { + async explain(): Promise; + async explain(verbosity: ExplainVerbosityLike | ExplainCommandOptions): Promise; + async explain(options: { timeoutMS?: number }): Promise; + async explain( + verbosity: ExplainVerbosityLike | ExplainCommandOptions, + options: { timeoutMS?: number } + ): Promise; + async explain( + verbosity?: ExplainVerbosityLike | ExplainCommandOptions | { timeoutMS?: number }, + options?: { timeoutMS?: number } + ): Promise { + let explain: ExplainVerbosityLike | ExplainCommandOptions | undefined; + let timeout: { timeoutMS?: number } | undefined; + if (verbosity == null && options == null) { + explain = true; + timeout = undefined; + } else if (verbosity != null && options == null) { + explain = + typeof verbosity !== 'object' + ? verbosity + : 'timeoutMS' in verbosity + ? undefined + : (verbosity as ExplainCommandOptions); + timeout = typeof verbosity === 'object' && 'timeoutMS' in verbosity ? verbosity : undefined; + } else { + explain = verbosity as ExplainCommandOptions; + timeout = options; + } + return ( await executeOperation( this.client, new FindOperation(this.namespace, this.cursorFilter, { ...this.findOptions, // NOTE: order matters here, we may need to refine this ...this.cursorOptions, - explain: verbosity ?? true + ...timeout, + explain: explain ?? true }) ) ).shift(this.deserializationOptions); diff --git a/src/explain.ts b/src/explain.ts index 51f591efd47..e6d90286192 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -1,3 +1,7 @@ +import { type Document } from 'bson'; + +import { MongoAPIError } from './error'; + /** @public */ export const ExplainVerbosity = Object.freeze({ queryPlanner: 'queryPlanner', @@ -86,3 +90,36 @@ export class Explain { return new Explain(verbosity, maxTimeMS); } } + +export function validateExplainTimeoutOptions(options: Document, explain?: Explain) { + const { maxTimeMS, timeoutMS } = options; + if ((maxTimeMS != null && timeoutMS) || (explain?.maxTimeMS != null && timeoutMS != null)) { + throw new MongoAPIError('Cannot use maxTimeMS with timeoutMS for explain commands.'); + } +} + +/** + * Applies an explain to a given command. + * @internal + * + * @param command - the command on which to apply the explain + * @param options - the options containing the explain verbosity + */ +export function decorateWithExplain( + command: Document, + explain: Explain +): { + explain: Document; + verbosity: ExplainVerbosity; + maxTimeMS?: number; +} { + type ExplainCommand = ReturnType; + const { verbosity, maxTimeMS } = explain; + const baseCommand: ExplainCommand = { explain: command, verbosity }; + + if (typeof maxTimeMS === 'number') { + baseCommand.maxTimeMS = maxTimeMS; + } + + return baseCommand; +} diff --git a/src/operations/command.ts b/src/operations/command.ts index 5bd80f796d1..bcd3919017b 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -1,19 +1,19 @@ import type { BSONSerializeOptions, Document } from '../bson'; import { type MongoDBResponseConstructor } from '../cmap/wire_protocol/responses'; import { MongoInvalidArgumentError } from '../error'; -import { Explain, type ExplainOptions } from '../explain'; +import { + decorateWithExplain, + Explain, + type ExplainOptions, + validateExplainTimeoutOptions +} from '../explain'; import { ReadConcern } from '../read_concern'; import type { ReadPreference } from '../read_preference'; import type { Server } from '../sdam/server'; import { MIN_SECONDARY_WRITE_WIRE_VERSION } from '../sdam/server_selection'; import type { ClientSession } from '../sessions'; import { type TimeoutContext } from '../timeout'; -import { - commandSupportsReadConcern, - decorateWithExplain, - maxWireVersion, - MongoDBNamespace -} from '../utils'; +import { commandSupportsReadConcern, maxWireVersion, MongoDBNamespace } from '../utils'; import { WriteConcern, type WriteConcernOptions } from '../write_concern'; import type { ReadConcernLike } from './../read_concern'; import { AbstractOperation, Aspect, type OperationOptions } from './operation'; @@ -97,6 +97,7 @@ export abstract class CommandOperation extends AbstractOperation { if (this.hasAspect(Aspect.EXPLAINABLE)) { this.explain = Explain.fromOptions(options); + validateExplainTimeoutOptions(this.options, this.explain); } else if (options?.explain != null) { throw new MongoInvalidArgumentError(`Option "explain" is not supported on this command`); } diff --git a/src/operations/find.ts b/src/operations/find.ts index 348467acf75..d1ea3f3fddc 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -2,12 +2,13 @@ import type { Document } from '../bson'; import { CursorResponse, ExplainedCursorResponse } from '../cmap/wire_protocol/responses'; import { type AbstractCursorOptions, type CursorTimeoutMode } from '../cursor/abstract_cursor'; import { MongoInvalidArgumentError } from '../error'; +import { decorateWithExplain, validateExplainTimeoutOptions } from '../explain'; import { ReadConcern } from '../read_concern'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; import { formatSort, type Sort } from '../sort'; import { type TimeoutContext } from '../timeout'; -import { decorateWithExplain, type MongoDBNamespace, normalizeHintField } from '../utils'; +import { type MongoDBNamespace, normalizeHintField } from '../utils'; import { type CollationOptions, CommandOperation, type CommandOperationOptions } from './command'; import { Aspect, defineAspects, type Hint } from './operation'; @@ -112,6 +113,7 @@ export class FindOperation extends CommandOperation { let findCommand = makeFindCommand(this.ns, this.filter, options); if (this.explain) { + validateExplainTimeoutOptions(this.options, this.explain); findCommand = decorateWithExplain(findCommand, this.explain); } diff --git a/src/utils.ts b/src/utils.ts index 04174813c9c..76d5da891c6 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -25,7 +25,6 @@ import { MongoParseError, MongoRuntimeError } from './error'; -import type { Explain, ExplainVerbosity } from './explain'; import type { MongoClient } from './mongo_client'; import type { CommandOperationOptions, OperationParent } from './operations/command'; import type { Hint, OperationOptions } from './operations/operation'; @@ -244,32 +243,6 @@ export function decorateWithReadConcern( } } -/** - * Applies an explain to a given command. - * @internal - * - * @param command - the command on which to apply the explain - * @param options - the options containing the explain verbosity - */ -export function decorateWithExplain( - command: Document, - explain: Explain -): { - explain: Document; - verbosity: ExplainVerbosity; - maxTimeMS?: number; -} { - type ExplainCommand = ReturnType; - const { verbosity, maxTimeMS } = explain; - const baseCommand: ExplainCommand = { explain: command, verbosity }; - - if (typeof maxTimeMS === 'number') { - baseCommand.maxTimeMS = maxTimeMS; - } - - return baseCommand; -} - /** * @internal */ diff --git a/test/integration/crud/explain.test.ts b/test/integration/crud/explain.test.ts index 44fe381303a..079b57b57fd 100644 --- a/test/integration/crud/explain.test.ts +++ b/test/integration/crud/explain.test.ts @@ -5,9 +5,12 @@ import { type Collection, type CommandStartedEvent, type Db, + type Document, type MongoClient, + MongoOperationTimeoutError, MongoServerError } from '../../mongodb'; +import { clearFailPoint, configureFailPoint } from '../../tools/utils'; import { filterForCommands } from '../shared'; const explain = [true, false, 'queryPlanner', 'allPlansExecution', 'executionStats', 'invalid']; @@ -296,6 +299,269 @@ describe('CRUD API explain option', function () { }; } }); + + describe('explain with timeoutMS', function () { + let client: MongoClient; + type ExplainStartedEvent = CommandStartedEvent & { + command: { explain: Document & { maxTimeMS?: number }; maxTimeMS?: number }; + }; + const commands: ExplainStartedEvent[] = []; + + beforeEach(async function () { + client = this.configuration.newClient({}, { monitorCommands: true }); + client.on('commandStarted', filterForCommands('explain', commands)); + + await configureFailPoint(this.configuration, { + configureFailPoint: 'failCommand', + mode: { times: 1 }, + data: { + failCommands: ['explain'], + blockConnection: true, + blockTimeMS: 1000 + } + }); + }); + + afterEach(async function () { + await clearFailPoint(this.configuration); + await client?.close(); + commands.length = 0; + }); + + describe('when a cursor api is being explained', function () { + describe('when timeoutMS is provided', function () { + it('the explain command respects timeoutMS', async function () { + const cursor = client.db('foo').collection('bar').find({}, { timeoutMS: 1000 }); + const timeout = await cursor.explain({ verbosity: 'queryPlanner' }).catch(e => e); + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + }); + + it('the explain command has the calculated maxTimeMS value attached', async function () { + const cursor = client.db('foo').collection('bar').find({}, { timeoutMS: 1000 }); + const timeout = await cursor.explain({ verbosity: 'queryPlanner' }).catch(e => e); + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + + const [ + { + command: { maxTimeMS } + } + ] = commands; + + expect(maxTimeMS).to.be.a('number'); + }); + + it('the explained command does not have a maxTimeMS value attached', async function () { + const cursor = client.db('foo').collection('bar').find({}, { timeoutMS: 1000 }); + const timeout = await cursor.explain({ verbosity: 'queryPlanner' }).catch(e => e); + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + + const [ + { + command: { + explain: { maxTimeMS } + } + } + ] = commands; + + expect(maxTimeMS).not.to.exist; + }); + }); + + describe('when timeoutMS and maxTimeMS are both provided', function () { + it('an error is thrown indicating incompatibility of those options', async function () { + const cursor = client.db('foo').collection('bar').find({}, { timeoutMS: 1000 }); + const error = await cursor + .explain({ verbosity: 'queryPlanner', maxTimeMS: 1000 }) + .catch(e => e); + expect(error).to.match(/Cannot use maxTimeMS with timeoutMS for explain commands/); + }); + }); + }); + + describe('when a non-cursor api is being explained', function () { + describe('when timeoutMS is provided', function () { + it('the explain command respects timeoutMS', async function () { + const timeout = await client + .db('foo') + .collection('bar') + .deleteMany( + {}, + { + timeoutMS: 1000, + explain: { verbosity: 'queryPlanner' } + } + ) + .catch(e => e); + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + }); + + it('the explain command has the calculated maxTimeMS value attached', async function () { + const timeout = await client + .db('foo') + .collection('bar') + .deleteMany( + {}, + { + timeoutMS: 1000, + explain: { verbosity: 'queryPlanner' } + } + ) + .catch(e => e); + + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + + const [ + { + command: { maxTimeMS } + } + ] = commands; + + expect(maxTimeMS).to.be.a('number'); + }); + + it('the explained command does not have a maxTimeMS value attached', async function () { + const timeout = await client + .db('foo') + .collection('bar') + .deleteMany( + {}, + { + timeoutMS: 1000, + explain: { verbosity: 'queryPlanner' } + } + ) + .catch(e => e); + + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + + const [ + { + command: { + explain: { maxTimeMS } + } + } + ] = commands; + + expect(maxTimeMS).not.to.exist; + }); + }); + + describe('when timeoutMS and maxTimeMS are both provided', function () { + it('an error is thrown indicating incompatibility of those options', async function () { + const error = await client + .db('foo') + .collection('bar') + .deleteMany( + {}, + { + timeoutMS: 1000, + explain: { verbosity: 'queryPlanner', maxTimeMS: 1000 } + } + ) + .catch(e => e); + + expect(error).to.match(/Cannot use maxTimeMS with timeoutMS for explain commands/); + }); + }); + }); + + describe('when find({}, { explain: ...}) is used with timeoutMS', function () { + it('an error is thrown indicating that explain is not supported with timeoutMS for this API', async function () { + const error = await client + .db('foo') + .collection('bar') + .find( + {}, + { + timeoutMS: 1000, + explain: { verbosity: 'queryPlanner', maxTimeMS: 1000 } + } + ) + .toArray() + .catch(e => e); + + expect(error).to.match( + /timeoutMS cannot be used with explain when explain is specified in findOptions/ + ); + }); + }); + + describe('when aggregate({}, { explain: ...}) is used with timeoutMS', function () { + it('an error is thrown indicating that explain is not supported with timeoutMS for this API', async function () { + const error = await client + .db('foo') + .collection('bar') + .aggregate([], { + timeoutMS: 1000, + explain: { verbosity: 'queryPlanner', maxTimeMS: 1000 } + }) + .toArray() + .catch(e => e); + + expect(error).to.match( + /timeoutMS cannot be used with explain when explain is specified in aggregateOptions/ + ); + }); + }); + + describe('fluent api timeoutMS precedence and inheritance', function () { + describe('find({}, { timeoutMS }).explain()', function () { + it('respects the timeoutMS from the find options', async function () { + const cursor = client.db('foo').collection('bar').find({}, { timeoutMS: 1000 }); + const timeout = await cursor.explain({ verbosity: 'queryPlanner' }).catch(e => e); + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + }); + }); + + describe('find().explain({}, { timeoutMS })', function () { + it('respects the timeoutMS from the explain helper', async function () { + const cursor = client.db('foo').collection('bar').find(); + const timeout = await cursor + .explain({ verbosity: 'queryPlanner' }, { timeoutMS: 1000 }) + .catch(e => e); + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + }); + }); + + describe('find({}, { timeoutMS} ).explain({}, { timeoutMS })', function () { + it('the timeoutMS from the explain helper has precedence', async function () { + const cursor = client.db('foo').collection('bar').find({}, { timeoutMS: 2000 }); + const timeout = await cursor + .explain({ verbosity: 'queryPlanner' }, { timeoutMS: 1000 }) + .catch(e => e); + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + }); + }); + + describe('aggregate([], { timeoutMS }).explain()', function () { + it('respects the timeoutMS from the find options', async function () { + const cursor = client.db('foo').collection('bar').aggregate([], { timeoutMS: 1000 }); + const timeout = await cursor.explain({ verbosity: 'queryPlanner' }).catch(e => e); + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + }); + }); + + describe('aggregate([], { timeoutMS })', function () { + it('respects the timeoutMS from the explain helper', async function () { + const cursor = client.db('foo').collection('bar').aggregate(); + const timeout = await cursor + .explain({ verbosity: 'queryPlanner' }, { timeoutMS: 1000 }) + .catch(e => e); + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + }); + }); + + describe('aggregate([], { timeoutMS} ).explain({}, { timeoutMS })', function () { + it('the timeoutMS from the explain helper has precedence', async function () { + const cursor = client.db('foo').collection('bar').aggregate([], { timeoutMS: 2000 }); + const timeout = await cursor + .explain({ verbosity: 'queryPlanner' }, { timeoutMS: 1000 }) + .catch(e => e); + expect(timeout).to.be.instanceOf(MongoOperationTimeoutError); + }); + }); + }); + }); }); function explainValueToExpectation(explainValue: boolean | string) { diff --git a/test/tools/utils.ts b/test/tools/utils.ts index 8614bd7d64c..7f41f52ba28 100644 --- a/test/tools/utils.ts +++ b/test/tools/utils.ts @@ -18,6 +18,7 @@ import { Topology, type TopologyOptions } from '../mongodb'; +import { type TestConfiguration } from './runner/config'; import { runUnifiedSuite } from './unified-spec-runner/runner'; import { type CollectionData, @@ -598,3 +599,28 @@ export async function waitUntilPoolsFilled( await Promise.all([wait$(), client.connect()]); } + +export async function configureFailPoint(configuration: TestConfiguration, failPoint: FailPoint) { + const utilClient = configuration.newClient(); + await utilClient.connect(); + + try { + await utilClient.db('admin').command(failPoint); + } finally { + await utilClient.close(); + } +} + +export async function clearFailPoint(configuration: TestConfiguration) { + const utilClient = configuration.newClient(); + await utilClient.connect(); + + try { + await utilClient.db('admin').command({ + configureFailPoint: 'failCommand', + mode: 'off' + }); + } finally { + await utilClient.close(); + } +}