From a8dfabef967bbd3f089b41f1f246b1d00ab86f88 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Fri, 30 Oct 2020 11:57:41 -0400 Subject: [PATCH 01/23] FindOneOperation extends CommandOperation --- src/operations/find_one.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/operations/find_one.ts b/src/operations/find_one.ts index 2c8b89e470..393e031431 100644 --- a/src/operations/find_one.ts +++ b/src/operations/find_one.ts @@ -1,24 +1,21 @@ -import { OperationBase } from './operation'; import type { Callback } from '../utils'; -import { Document, resolveBSONOptions } from '../bson'; +import type { Document } from '../bson'; import type { Collection } from '../collection'; import type { FindOptions } from './find'; import { MongoError } from '../error'; import type { Server } from '../sdam/server'; +import { CommandOperation } from './command'; /** @internal */ -export class FindOneOperation extends OperationBase { +export class FindOneOperation extends CommandOperation { collection: Collection; query: Document; constructor(collection: Collection, query: Document, options: FindOptions) { - super(options); + super(collection, options); this.collection = collection; this.query = query; - - // Assign BSON serialize options to OperationBase, preferring options over collection options - this.bsonOptions = resolveBSONOptions(options, collection); } execute(server: Server, callback: Callback): void { From 5bded70f4d118794016fb70ee41f6075a7e8dfc2 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Tue, 27 Oct 2020 17:04:31 -0400 Subject: [PATCH 02/23] implement explain for write commands --- src/cmap/wire_protocol/write_command.ts | 11 +- src/cursor/cursor.ts | 9 +- src/explain.ts | 66 +++++++ src/operations/delete.ts | 9 +- src/operations/operation.ts | 2 - src/operations/update.ts | 9 +- src/sdam/server.ts | 13 ++ src/utils.ts | 19 ++ test/functional/aggregation.test.js | 4 +- test/functional/explain.test.js | 249 ++++++++++++++++++++++++ 10 files changed, 379 insertions(+), 12 deletions(-) create mode 100644 src/explain.ts create mode 100644 test/functional/explain.test.js diff --git a/src/cmap/wire_protocol/write_command.ts b/src/cmap/wire_protocol/write_command.ts index 18fbcecf3d..5eb12a9c68 100644 --- a/src/cmap/wire_protocol/write_command.ts +++ b/src/cmap/wire_protocol/write_command.ts @@ -1,9 +1,10 @@ import { MongoError } from '../../error'; -import { collectionNamespace, Callback } from '../../utils'; +import { collectionNamespace, Callback, decorateWithExplain } from '../../utils'; import { command, CommandOptions } from './command'; import type { Server } from '../../sdam/server'; import type { Document, BSONSerializeOptions } from '../../bson'; import type { WriteConcern } from '../../write_concern'; +import type { ExplainOptions } from '../../explain'; /** @public */ export interface CollationOptions { @@ -18,7 +19,7 @@ export interface CollationOptions { } /** @internal */ -export interface WriteCommandOptions extends BSONSerializeOptions, CommandOptions { +export interface WriteCommandOptions extends BSONSerializeOptions, CommandOptions, ExplainOptions { ordered?: boolean; writeConcern?: WriteConcern; collation?: CollationOptions; @@ -43,7 +44,7 @@ export function writeCommand( options = options || {}; const ordered = typeof options.ordered === 'boolean' ? options.ordered : true; const writeConcern = options.writeConcern; - const writeCommand: Document = {}; + let writeCommand: Document = {}; writeCommand[type] = collectionNamespace(ns); writeCommand[opsField] = ops; writeCommand.ordered = ordered; @@ -64,6 +65,10 @@ export function writeCommand( writeCommand.bypassDocumentValidation = options.bypassDocumentValidation; } + if (options.explain) { + writeCommand = decorateWithExplain(writeCommand, options); + } + const commandOptions = Object.assign( { checkKeys: type === 'insert', diff --git a/src/cursor/cursor.ts b/src/cursor/cursor.ts index 2529b43a04..61bfefcec5 100644 --- a/src/cursor/cursor.ts +++ b/src/cursor/cursor.ts @@ -1307,10 +1307,11 @@ export class Cursor< explain(callback?: Callback): Promise | void { // NOTE: the next line includes a special case for operations which do not // subclass `CommandOperationV2`. To be removed asap. - if (this.operation && this.operation.cmd == null) { - this.operation.options.explain = true; - return executeOperation(this.topology, this.operation as any, callback); - } + // TODO: uncomment/fix this when cursor explain is re-implemented + // if (this.operation && this.operation.cmd == null) { + // this.operation.options.explain = true; + // return executeOperation(this.topology, this.operation as any, callback); + // } this.cmd.explain = true; diff --git a/src/explain.ts b/src/explain.ts new file mode 100644 index 0000000000..4f7c28eaf2 --- /dev/null +++ b/src/explain.ts @@ -0,0 +1,66 @@ +import type { Server } from './sdam/server'; +import { maxWireVersion } from './utils'; + +enum VerbosityStrings { + queryPlanner = 'queryPlanner', + queryPlannerExtended = 'queryPlannerExtended', + executionStats = 'executionStats', + allPlansExecution = 'allPlansExecution' +} + +export type Verbosity = + | boolean + | VerbosityStrings.queryPlannerExtended + | VerbosityStrings.queryPlannerExtended + | VerbosityStrings.executionStats + | VerbosityStrings.allPlansExecution; + +/** @public */ +export interface ExplainOptions { + // The requested verbosity of the explain. + explain?: Verbosity; +} + +export const SUPPORTS_EXPLAIN_WITH_REMOVE = 3; +export const SUPPORTS_EXPLAIN_WITH_UPDATE = 3; +export const SUPPORTS_EXPLAIN_WITH_DISTINCT = 3.2; +export const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 3.2; +export const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 4.4; + +/** + * Checks that the server supports explain on the given operation. + * @internal + * + * @param server - to check against + * @param op - the operation to explain + */ +export function explainSupported(server: Server, op: string): boolean { + const wireVersion = maxWireVersion(server); + if ( + (op === 'remove' && wireVersion >= SUPPORTS_EXPLAIN_WITH_REMOVE) || + (op === 'update' && wireVersion >= SUPPORTS_EXPLAIN_WITH_UPDATE) || + (op === 'distinct' && wireVersion >= SUPPORTS_EXPLAIN_WITH_DISTINCT) || + (op === 'findAndModify' && wireVersion >= SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY) || + (op === 'mapReduce' && wireVersion >= SUPPORTS_EXPLAIN_WITH_MAP_REDUCE) + ) { + return true; + } + + return false; +} + +/** + * Checks that the provided verbosity level is valid. + * @internal + */ +export function validExplainVerbosity(verbosity: boolean | string): boolean { + if (typeof verbosity === 'string') { + return ( + verbosity === VerbosityStrings.queryPlanner || + verbosity === VerbosityStrings.queryPlannerExtended || + verbosity === VerbosityStrings.allPlansExecution || + verbosity === VerbosityStrings.executionStats + ); + } + return true; +} diff --git a/src/operations/delete.ts b/src/operations/delete.ts index 5ed6b750b1..4b89dd737b 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -8,9 +8,10 @@ import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { Connection } from '../cmap/connection'; +import type { ExplainOptions } from '../explain'; /** @public */ -export interface DeleteOptions extends CommandOperationOptions { +export interface DeleteOptions extends CommandOperationOptions, ExplainOptions { single?: boolean; hint?: Hint; } @@ -75,6 +76,9 @@ export class DeleteOneOperation extends CommandOperation { if (err || !r) return callback(err); + // If an explain option was executed, don't process the server results + if (options.explain) return callback(undefined, r); + const result: UpdateResult = { modifiedCount: r.nModified != null ? r.nModified : r.n, upsertedId: @@ -136,6 +140,9 @@ export class UpdateManyOperation extends CommandOperation { if (err || !r) return callback(err); + // If an explain option was executed, don't process the server results + if (options.explain) return callback(undefined, r); + const result: UpdateResult = { modifiedCount: r.nModified != null ? r.nModified : r.n, upsertedId: diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 5e63ca351b..10be455022 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -45,6 +45,7 @@ import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { Document, Long } from '../bson'; import type { AutoEncrypter } from '../deps'; import type { FindOptions } from '../operations/find'; +import { explainSupported, validExplainVerbosity } from '../explain'; // Used for filtering out fields for logging const DEBUG_FIELDS = [ @@ -457,6 +458,18 @@ function executeWriteOperation( } } + if (options.explain) { + if (!validExplainVerbosity(options.explain)) { + callback(new MongoError(`${options.explain} is an invalid explain verbosity`)); + return; + } + + if (!explainSupported(server, op)) { + callback(new MongoError(`server ${server.name} does not support explain on ${op}`)); + return; + } + } + server.s.pool.withConnection((err, conn, cb) => { if (err || !conn) { markServerUnknown(server, err); diff --git a/src/utils.ts b/src/utils.ts index 1f81eb121c..3fa0577b33 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -16,6 +16,7 @@ import { readFileSync } from 'fs'; import { resolve } from 'path'; import type { Document } from './bson'; import type { IndexSpecification, IndexDirection } from './operations/indexes'; +import type { ExplainOptions } from './explain'; /** * MongoDB Driver style callback @@ -486,6 +487,24 @@ export function decorateWithReadConcern( } } +/** + * Applies an explain to a given command. + * @internal + * + * @param command - the command on which to apply the read concern + * @param options - the options containing the explain verbosity + */ +export function decorateWithExplain(command: Document, options: ExplainOptions): Document { + if (options.explain) { + // Reformat command into explain command + command = { explain: command }; + if (typeof options.explain === 'string') { + command.verbosity = options.explain; + } + } + return command; +} + /** @internal */ export function emitDeprecationWarning(msg: string): void { return process.emitWarning(msg, 'DeprecationWarning'); diff --git a/test/functional/aggregation.test.js b/test/functional/aggregation.test.js index 534665fdc4..566576ae3b 100644 --- a/test/functional/aggregation.test.js +++ b/test/functional/aggregation.test.js @@ -386,7 +386,9 @@ describe('Aggregation', function () { * @example-class Collection * @example-method aggregate */ - it('should correctly return a cursor and call explain', { + it.skip('should correctly return a cursor and call explain', { + // TODO: add back this test when cursor explain is fully implemented + // Add a tag that our runner can trigger on // in this case we are setting that node needs to be higher than 0.10.X to run metadata: { diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js new file mode 100644 index 0000000000..d4386e38aa --- /dev/null +++ b/test/functional/explain.test.js @@ -0,0 +1,249 @@ +'use strict'; +const { setupDatabase, withClient } = require('./shared'); +const chai = require('chai'); +const expect = chai.expect; +chai.use(require('chai-subset')); + +describe('Explain', function () { + before(function () { + return setupDatabase(this.configuration); + }); + + it('shouldHonorBooleanExplainWithRemoveOne', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithRemoveOne'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + collection.removeOne({ a: 1 }, { explain: true }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + done(); + }); + }); + }) + }); + + it('shouldHonorBooleanExplainWithRemoveMany', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithRemoveMany'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + collection.removeMany({ a: 1 }, { explain: true }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + done(); + }); + }); + }) + }); + + it('shouldHonorBooleanExplainWithUpdateOne', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithUpdateOne'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + collection.updateOne( + { a: 1 }, + { $inc: { a: 2 } }, + { explain: true }, + (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; // nested result is how updateOne is returned, unfortunately + done(); + } + ); + }); + }) + }); + + it('shouldHonorBooleanExplainWithUpdateMany', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithUpdateMany'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + collection.updateMany( + { a: 1 }, + { $inc: { a: 2 } }, + { explain: true }, + (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).nested.property('queryPlanner').to.exist; + done(); + } + ); + }); + }) + }); + + it('shouldUseAllPlansExecutionAsDefaultExplainVerbosity', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldUseAllPlansExecutionAsDefaultExplainVerbosity'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + // Verify explanation result contains properties of allPlansExecution output + collection.removeOne({ a: 1 }, { explain: true }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + expect(explanation).nested.property('executionStats.allPlansExecution').to.exist; + done(); + }); + }); + }) + }); + + it('shouldHonorQueryPlannerStringExplainWithRemoveOne', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorQueryPlannerStringExplainWithRemoveOne'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + // Verify explanation result contains properties of queryPlanner output + collection.removeOne({ a: 1 }, { explain: 'queryPlanner' }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + expect(explanation).to.not.have.property('executionStats'); + done(); + }); + }); + }) + }); + + it('shouldHonorExecutionStatsStringExplainWithRemoveOne', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorExecutionStatsStringExplainWithRemoveOne'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + // Verify explanation result contains properties of executionStats output + collection.removeOne({ a: 1 }, { explain: 'executionStats' }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + expect(explanation).property('executionStats').to.exist; + expect(explanation.executionStats).to.not.have.property('allPlansExecution'); + done(); + }); + }); + }) + }); + + it('shouldHonorAllPlansStringExplainWithRemoveOne', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorAllPlansStringExplainWithRemoveOne'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + // Verify explanation result contains properties of allPlansExecution output + collection.removeOne({ a: 1 }, { explain: 'allPlansExecution' }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + expect(explanation).nested.property('executionStats.allPlansExecution').to.exist; + done(); + }); + }); + }) + }); + + // have to skip this Or move it to a data lake testing file + // it('shouldHonorQueryPlannerExtendedStringExplainWithRemoveOne', { + // metadata: { + // requires: { + // mongodb: '>3.0' + // } + // }, + // test: withClient(function (client, done) { + // var db = client.db('shouldHonorQueryPlannerExtendedStringExplainWithRemoveOne'); + // var collection = db.collection('test'); + + // collection.insertOne({ a: 1 }, (err, res) => { + // expect(err).to.not.exist; + // expect(res).to.exist; + + // // Verify explanation result contains properties of queryPlannerExtended (treated like allPlansExecution) + // collection.removeOne({ a: 1 }, { explain: 'queryPlannerExtended' }, (err, explanation) => { + // expect(err).to.not.exist; + // expect(explanation).to.exist; + // expect(explanation).property('queryPlanner').to.exist; + // expect(explanation).nested.property('executionStats.allPlansExecution').to.exist; + // done(); + // }); + // }); + // }) + // }); +}); From 4374b6b4a29bbc65564a809519502182057ea0b2 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Tue, 27 Oct 2020 17:23:38 -0400 Subject: [PATCH 03/23] implement explain for distinct command --- src/operations/command.ts | 12 +++++++++--- src/operations/distinct.ts | 23 ++++++++++++++++++++--- test/functional/explain.test.js | 24 ++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/operations/command.ts b/src/operations/command.ts index 818610402a..31bcba3776 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -1,7 +1,7 @@ import { Aspect, OperationBase, OperationOptions } from './operation'; import { ReadConcern } from '../read_concern'; import { WriteConcern, WriteConcernOptions } from '../write_concern'; -import { maxWireVersion, MongoDBNamespace, Callback } from '../utils'; +import { maxWireVersion, MongoDBNamespace, Callback, decorateWithExplain } from '../utils'; import { ReadPreference, ReadPreferenceLike } from '../read_preference'; import { commandSupportsReadConcern } from '../sessions'; import { MongoError } from '../error'; @@ -10,6 +10,7 @@ import type { Server } from '../sdam/server'; import { BSONSerializeOptions, Document, resolveBSONOptions } from '../bson'; import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import type { ReadConcernLike } from './../read_concern'; +import type { Verbosity } from '../explain'; const SUPPORTS_WRITE_CONCERN_AND_COLLATION = 5; @@ -54,7 +55,7 @@ export abstract class CommandOperation< readPreference: ReadPreference; readConcern?: ReadConcern; writeConcern?: WriteConcern; - explain: boolean; + explain?: Verbosity; fullResponse?: boolean; logger?: Logger; @@ -79,7 +80,6 @@ export abstract class CommandOperation< : ReadPreference.resolve(propertyProvider, this.options); this.readConcern = resolveReadConcern(propertyProvider, this.options); this.writeConcern = resolveWriteConcern(propertyProvider, this.options); - this.explain = false; this.fullResponse = options && typeof options.fullResponse === 'boolean' ? options.fullResponse : false; @@ -137,6 +137,12 @@ export abstract class CommandOperation< cmd.comment = options.comment; } + // If we have reached this point, then the explain verbosity must be valid + // Note: explain inherits any comment from its command + if (this.explain) { + cmd = decorateWithExplain(cmd, { explain: this.explain }); + } + if (this.logger && this.logger.isDebug()) { this.logger.debug(`executing command ${JSON.stringify(cmd)} against ${this.ns}`); } diff --git a/src/operations/distinct.ts b/src/operations/distinct.ts index 7bc4159c95..9e6d29372e 100644 --- a/src/operations/distinct.ts +++ b/src/operations/distinct.ts @@ -1,12 +1,14 @@ import { Aspect, defineAspects } from './operation'; import { CommandOperation, CommandOperationOptions } from './command'; -import { decorateWithCollation, decorateWithReadConcern, Callback } from '../utils'; +import { decorateWithCollation, decorateWithReadConcern, Callback, maxWireVersion } from '../utils'; import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; +import { ExplainOptions, SUPPORTS_EXPLAIN_WITH_DISTINCT, validExplainVerbosity } from '../explain'; +import { MongoError } from '../error'; /** @public */ -export type DistinctOptions = CommandOperationOptions; +export interface DistinctOptions extends CommandOperationOptions, ExplainOptions {} /** * Return a list of distinct values for the given key across a collection. @@ -33,6 +35,7 @@ export class DistinctOperation extends CommandOperation): void { @@ -63,13 +66,27 @@ export class DistinctOperation extends CommandOperation { if (err) { callback(err); return; } - callback(undefined, this.options.fullResponse ? result : result.values); + callback(undefined, this.options.fullResponse || this.explain ? result : result.values); }); } } diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index d4386e38aa..08833c49fd 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -247,3 +247,27 @@ describe('Explain', function () { // }) // }); }); + +it('shouldHonorBooleanExplainWithDistinct', { + metadata: { + requires: { + mongodb: '>3.2' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithDistinct'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + collection.distinct('a', {}, { explain: true }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + done(); + }); + }); + }) +}); From f00fbfe457cb5945f91acfa988f85448eadf288f Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Tue, 27 Oct 2020 17:28:12 -0400 Subject: [PATCH 04/23] implement explain for findAndModify commands --- src/operations/find_and_modify.ts | 22 +++++- test/functional/explain.test.js | 119 ++++++++++++++++++++++++------ 2 files changed, 119 insertions(+), 22 deletions(-) diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 58c2185699..6f280fd200 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -14,9 +14,14 @@ import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import { Sort, formatSort } from '../sort'; +import { + ExplainOptions, + SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY, + validExplainVerbosity +} from '../explain'; /** @public */ -export interface FindAndModifyOptions extends CommandOperationOptions { +export interface FindAndModifyOptions extends CommandOperationOptions, ExplainOptions { /** When false, returns the updated document rather than the original. The default is true. */ returnOriginal?: boolean; /** Upsert the document if it does not exist. */ @@ -63,6 +68,7 @@ export class FindAndModifyOperation extends CommandOperation): void { @@ -143,6 +149,20 @@ export class FindAndModifyOperation extends CommandOperation { if (err) return callback(err); diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index 08833c49fd..981221fdec 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -246,28 +246,105 @@ describe('Explain', function () { // }); // }) // }); -}); -it('shouldHonorBooleanExplainWithDistinct', { - metadata: { - requires: { - mongodb: '>3.2' - } - }, - test: withClient(function (client, done) { - var db = client.db('shouldHonorBooleanExplainWithDistinct'); - var collection = db.collection('test'); - - collection.insertOne({ a: 1 }, (err, res) => { - expect(err).to.not.exist; - expect(res).to.exist; - - collection.distinct('a', {}, { explain: true }, (err, explanation) => { + it('shouldHonorBooleanExplainWithDistinct', { + metadata: { + requires: { + mongodb: '>3.2' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithDistinct'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + collection.distinct('a', {}, { explain: true }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + done(); + }); + }); + }) + }); + + it('shouldHonorBooleanExplainWithFindOneAndDelete', { + metadata: { + requires: { + mongodb: '>3.2' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithFindOneAndDelete'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + collection.findOneAndDelete({ a: 1 }, { explain: true }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + done(); + }); + }); + }) + }); + + it('shouldHonorBooleanExplainWithFindOneAndReplace', { + metadata: { + requires: { + mongodb: '>3.2' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithFindOneAndReplace'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; - expect(explanation).to.exist; - expect(explanation).property('queryPlanner').to.exist; - done(); + expect(res).to.exist; + + collection.findOneAndReplace({ a: 1 }, { a: 2 }, { explain: true }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + done(); + }); }); - }); - }) + }) + }); + + it('shouldHonorBooleanExplainWithFindOneAndUpdate', { + metadata: { + requires: { + mongodb: '>3.2' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithFindOneAndUpdate'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + collection.findOneAndUpdate( + { a: 1 }, + { $inc: { a: 2 } }, + { explain: true }, + (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + done(); + } + ); + }); + }) + }); }); From 1d937a816c50f868a1a506f8d9810107d5199686 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Tue, 27 Oct 2020 17:35:40 -0400 Subject: [PATCH 05/23] implement explain for mapReduce command --- src/operations/map_reduce.ts | 29 +++++++++++++++++++++++++++-- test/functional/explain.test.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/operations/map_reduce.ts b/src/operations/map_reduce.ts index 46fb88c287..875ceb7b98 100644 --- a/src/operations/map_reduce.ts +++ b/src/operations/map_reduce.ts @@ -5,7 +5,8 @@ import { decorateWithCollation, decorateWithReadConcern, isObject, - Callback + Callback, + maxWireVersion } from '../utils'; import { ReadPreference, ReadPreferenceMode } from '../read_preference'; import { CommandOperation, CommandOperationOptions } from './command'; @@ -14,8 +15,14 @@ import type { Collection } from '../collection'; import type { Sort } from '../sort'; import { MongoError } from '../error'; import type { ObjectId } from '../bson'; +import { + ExplainOptions, + SUPPORTS_EXPLAIN_WITH_MAP_REDUCE, + validExplainVerbosity +} from '../explain'; const exclusionList = [ + 'explain', 'readPreference', 'session', 'bypassDocumentValidation', @@ -34,7 +41,7 @@ export type ReduceFunction = (key: string, values: Document[]) => Document; export type FinalizeFunction = (key: string, reducedValue: Document) => Document; /** @public */ -export interface MapReduceOptions extends CommandOperationOptions { +export interface MapReduceOptions extends CommandOperationOptions, ExplainOptions { /** Sets the output target for the map reduce job. */ out?: 'inline' | { inline: 1 } | { replace: string } | { merge: string } | { reduce: string }; /** Query filter object. */ @@ -93,6 +100,7 @@ export class MapReduceOperation extends CommandOperation): void { @@ -152,6 +160,20 @@ export class MapReduceOperation extends CommandOperation { if (err) return callback(err); @@ -160,6 +182,9 @@ export class MapReduceOperation extends CommandOperation4.4' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithMapReduce'); + var collection = db.collection('test'); + + collection.insertMany([{ user_id: 1 }, { user_id: 2 }], (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + var map = 'function() { emit(this.user_id, 1); }'; + var reduce = 'function(k,vals) { return 1; }'; + + collection.mapReduce( + map, + reduce, + { out: { replace: 'tempCollection' }, explain: true }, + (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('stages').to.exist; + done(); + } + ); + }); + }) + }); }); From 2a9b1be5fbda269f716afb891603b96b78912d57 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Wed, 28 Oct 2020 11:03:47 -0400 Subject: [PATCH 06/23] first attempt: handle removing sessions for write operations --- src/sessions.ts | 2 +- test/functional/explain.test.js | 109 +++++++++++++++++++------------- 2 files changed, 66 insertions(+), 45 deletions(-) diff --git a/src/sessions.ts b/src/sessions.ts index abff68c0bb..59912ca4b1 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -777,7 +777,7 @@ function applySession( // first apply non-transaction-specific sessions data const inTransaction = session.inTransaction() || isTransactionCommand(command); - const isRetryableWrite = options?.willRetryWrite || false; + const isRetryableWrite = (!command.explain && options?.willRetryWrite) || false; const shouldApplyReadConcern = commandSupportsReadConcern(command, options); if (serverSession.txnNumber && (isRetryableWrite || inTransaction)) { diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index f3768c341a..803eab2df0 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -9,21 +9,21 @@ describe('Explain', function () { return setupDatabase(this.configuration); }); - it('shouldHonorBooleanExplainWithRemoveOne', { + it('shouldHonorBooleanExplainWithDeleteOne', { metadata: { requires: { mongodb: '>3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorBooleanExplainWithRemoveOne'); + var db = client.db('shouldHonorBooleanExplainWithDeleteOne'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - collection.removeOne({ a: 1 }, { explain: true }, (err, explanation) => { + collection.deleteOne({ a: 1 }, { explain: true }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; @@ -33,21 +33,21 @@ describe('Explain', function () { }) }); - it('shouldHonorBooleanExplainWithRemoveMany', { + it('shouldHonorBooleanExplainWithDeleteMany', { metadata: { requires: { mongodb: '>3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorBooleanExplainWithRemoveMany'); + var db = client.db('shouldHonorBooleanExplainWithDeleteMany'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - collection.removeMany({ a: 1 }, { explain: true }, (err, explanation) => { + collection.deleteMany({ a: 1 }, { explain: true }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; @@ -78,7 +78,7 @@ describe('Explain', function () { (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; - expect(explanation).property('queryPlanner').to.exist; // nested result is how updateOne is returned, unfortunately + expect(explanation).property('queryPlanner').to.exist; done(); } ); @@ -115,6 +115,54 @@ describe('Explain', function () { }) }); + it('shouldHonorBooleanExplainWithRemoveOne', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithRemoveOne'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + collection.removeOne({ a: 1 }, { explain: true }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + done(); + }); + }); + }) + }); + + it('shouldHonorBooleanExplainWithRemoveMany', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithRemoveMany'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + collection.removeMany({ a: 1 }, { explain: true }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + done(); + }); + }); + }) + }); + it('shouldUseAllPlansExecutionAsDefaultExplainVerbosity', { metadata: { requires: { @@ -130,7 +178,7 @@ describe('Explain', function () { expect(res).to.exist; // Verify explanation result contains properties of allPlansExecution output - collection.removeOne({ a: 1 }, { explain: true }, (err, explanation) => { + collection.deleteOne({ a: 1 }, { explain: true }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; @@ -141,14 +189,14 @@ describe('Explain', function () { }) }); - it('shouldHonorQueryPlannerStringExplainWithRemoveOne', { + it('shouldHonorQueryPlannerStringExplain', { metadata: { requires: { mongodb: '>3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorQueryPlannerStringExplainWithRemoveOne'); + var db = client.db('shouldHonorQueryPlannerStringExplain'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { @@ -156,7 +204,7 @@ describe('Explain', function () { expect(res).to.exist; // Verify explanation result contains properties of queryPlanner output - collection.removeOne({ a: 1 }, { explain: 'queryPlanner' }, (err, explanation) => { + collection.deleteOne({ a: 1 }, { explain: 'queryPlanner' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; @@ -167,14 +215,14 @@ describe('Explain', function () { }) }); - it('shouldHonorExecutionStatsStringExplainWithRemoveOne', { + it('shouldHonorExecutionStatsStringExplain', { metadata: { requires: { mongodb: '>3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorExecutionStatsStringExplainWithRemoveOne'); + var db = client.db('shouldHonorExecutionStatsStringExplain'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { @@ -182,7 +230,7 @@ describe('Explain', function () { expect(res).to.exist; // Verify explanation result contains properties of executionStats output - collection.removeOne({ a: 1 }, { explain: 'executionStats' }, (err, explanation) => { + collection.deleteOne({ a: 1 }, { explain: 'executionStats' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; @@ -194,14 +242,14 @@ describe('Explain', function () { }) }); - it('shouldHonorAllPlansStringExplainWithRemoveOne', { + it('shouldHonorAllPlansStringExplain', { metadata: { requires: { mongodb: '>3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorAllPlansStringExplainWithRemoveOne'); + var db = client.db('shouldHonorAllPlansStringExplain'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { @@ -209,7 +257,7 @@ describe('Explain', function () { expect(res).to.exist; // Verify explanation result contains properties of allPlansExecution output - collection.removeOne({ a: 1 }, { explain: 'allPlansExecution' }, (err, explanation) => { + collection.deleteOne({ a: 1 }, { explain: 'allPlansExecution' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; @@ -220,33 +268,6 @@ describe('Explain', function () { }) }); - // have to skip this Or move it to a data lake testing file - // it('shouldHonorQueryPlannerExtendedStringExplainWithRemoveOne', { - // metadata: { - // requires: { - // mongodb: '>3.0' - // } - // }, - // test: withClient(function (client, done) { - // var db = client.db('shouldHonorQueryPlannerExtendedStringExplainWithRemoveOne'); - // var collection = db.collection('test'); - - // collection.insertOne({ a: 1 }, (err, res) => { - // expect(err).to.not.exist; - // expect(res).to.exist; - - // // Verify explanation result contains properties of queryPlannerExtended (treated like allPlansExecution) - // collection.removeOne({ a: 1 }, { explain: 'queryPlannerExtended' }, (err, explanation) => { - // expect(err).to.not.exist; - // expect(explanation).to.exist; - // expect(explanation).property('queryPlanner').to.exist; - // expect(explanation).nested.property('executionStats.allPlansExecution').to.exist; - // done(); - // }); - // }); - // }) - // }); - it('shouldHonorBooleanExplainWithDistinct', { metadata: { requires: { From 237a70ff3751d9c95eec07c2886f90c95c757eeb Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Wed, 28 Oct 2020 12:07:36 -0400 Subject: [PATCH 07/23] consider explain during canRetryWrite --- src/operations/command.ts | 4 ++++ src/operations/delete.ts | 6 ++++-- src/operations/update.ts | 6 ++++-- src/sessions.ts | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/operations/command.ts b/src/operations/command.ts index 31bcba3776..3c95571aaa 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -96,6 +96,10 @@ export abstract class CommandOperation< this.bsonOptions = resolveBSONOptions(options, parent); } + get canRetryWrite(): boolean { + return !this.explain; + } + abstract execute(server: Server, callback: Callback): void; executeCommand(server: Server, cmd: Document, callback: Callback): void { diff --git a/src/operations/delete.ts b/src/operations/delete.ts index 4b89dd737b..277d710e11 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -61,6 +61,7 @@ export class DeleteOneOperation extends CommandOperation): void { @@ -77,7 +78,7 @@ export class DeleteOneOperation extends CommandOperation): void { @@ -118,7 +120,7 @@ export class DeleteManyOperation extends CommandOperation): void { @@ -96,7 +97,7 @@ export class UpdateOneOperation extends CommandOperation): void { @@ -141,7 +143,7 @@ export class UpdateManyOperation extends CommandOperation Date: Wed, 28 Oct 2020 17:14:51 -0400 Subject: [PATCH 08/23] small enum cleanup --- src/explain.ts | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/explain.ts b/src/explain.ts index 4f7c28eaf2..2126bea959 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -8,12 +8,7 @@ enum VerbosityStrings { allPlansExecution = 'allPlansExecution' } -export type Verbosity = - | boolean - | VerbosityStrings.queryPlannerExtended - | VerbosityStrings.queryPlannerExtended - | VerbosityStrings.executionStats - | VerbosityStrings.allPlansExecution; +export type Verbosity = boolean | VerbosityStrings; /** @public */ export interface ExplainOptions { @@ -55,12 +50,7 @@ export function explainSupported(server: Server, op: string): boolean { */ export function validExplainVerbosity(verbosity: boolean | string): boolean { if (typeof verbosity === 'string') { - return ( - verbosity === VerbosityStrings.queryPlanner || - verbosity === VerbosityStrings.queryPlannerExtended || - verbosity === VerbosityStrings.allPlansExecution || - verbosity === VerbosityStrings.executionStats - ); + return verbosity in VerbosityStrings; } return true; } From 0126381b6d232e22b59cb1a6849f9a48e2b59376 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Thu, 29 Oct 2020 11:43:35 -0400 Subject: [PATCH 09/23] model explain after read concern --- src/cmap/wire_protocol/write_command.ts | 2 +- src/explain.ts | 88 ++++++++++++++----------- src/operations/command.ts | 8 +-- src/operations/delete.ts | 6 +- src/operations/distinct.ts | 9 +-- src/operations/find_and_modify.ts | 13 +--- src/operations/map_reduce.ts | 13 +--- src/operations/update.ts | 6 +- src/sdam/server.ts | 15 ++--- src/utils.ts | 12 ++-- test/functional/explain.test.js | 30 ++++++++- 11 files changed, 102 insertions(+), 100 deletions(-) diff --git a/src/cmap/wire_protocol/write_command.ts b/src/cmap/wire_protocol/write_command.ts index 5eb12a9c68..14cfced8fc 100644 --- a/src/cmap/wire_protocol/write_command.ts +++ b/src/cmap/wire_protocol/write_command.ts @@ -65,7 +65,7 @@ export function writeCommand( writeCommand.bypassDocumentValidation = options.bypassDocumentValidation; } - if (options.explain) { + if (options.explain !== undefined) { writeCommand = decorateWithExplain(writeCommand, options); } diff --git a/src/explain.ts b/src/explain.ts index 2126bea959..847038936e 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -1,56 +1,66 @@ import type { Server } from './sdam/server'; import { maxWireVersion } from './utils'; -enum VerbosityStrings { +export const SUPPORTS_EXPLAIN_WITH_REMOVE = 3; +export const SUPPORTS_EXPLAIN_WITH_UPDATE = 3; +export const SUPPORTS_EXPLAIN_WITH_DISTINCT = 3.2; +export const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 3.2; +export const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 4.4; + +/** @public */ +export interface ExplainOptions { + explain?: VerbosityLike; +} + +export enum Verbosity { queryPlanner = 'queryPlanner', queryPlannerExtended = 'queryPlannerExtended', executionStats = 'executionStats', allPlansExecution = 'allPlansExecution' } -export type Verbosity = boolean | VerbosityStrings; - /** @public */ -export interface ExplainOptions { - // The requested verbosity of the explain. - explain?: Verbosity; -} +export type VerbosityLike = Verbosity | boolean; -export const SUPPORTS_EXPLAIN_WITH_REMOVE = 3; -export const SUPPORTS_EXPLAIN_WITH_UPDATE = 3; -export const SUPPORTS_EXPLAIN_WITH_DISTINCT = 3.2; -export const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 3.2; -export const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 4.4; +export class Explain { + explain: Verbosity; -/** - * Checks that the server supports explain on the given operation. - * @internal - * - * @param server - to check against - * @param op - the operation to explain - */ -export function explainSupported(server: Server, op: string): boolean { - const wireVersion = maxWireVersion(server); - if ( - (op === 'remove' && wireVersion >= SUPPORTS_EXPLAIN_WITH_REMOVE) || - (op === 'update' && wireVersion >= SUPPORTS_EXPLAIN_WITH_UPDATE) || - (op === 'distinct' && wireVersion >= SUPPORTS_EXPLAIN_WITH_DISTINCT) || - (op === 'findAndModify' && wireVersion >= SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY) || - (op === 'mapReduce' && wireVersion >= SUPPORTS_EXPLAIN_WITH_MAP_REDUCE) - ) { - return true; + constructor(explain: VerbosityLike) { + if (typeof explain === 'boolean') { + // For backwards compatibility, true is interpreted as + // "allPlansExecution" and false as "queryPlanner". + this.explain = explain ? Verbosity.allPlansExecution : Verbosity.queryPlanner; + } else { + this.explain = Verbosity[explain]; + } } - return false; -} + static fromOptions(options?: ExplainOptions): Explain | undefined { + if (options == null || options.explain === undefined) { + return; + } + return new Explain(options.explain); + } + + /** + * Checks that the server supports explain on the given operation. + * @internal + * + * @param server - to check against + * @param op - the operation to explain + */ + static explainSupported(server: Server, op: string): boolean { + const wireVersion = maxWireVersion(server); + if ( + (op === 'remove' && wireVersion >= SUPPORTS_EXPLAIN_WITH_REMOVE) || + (op === 'update' && wireVersion >= SUPPORTS_EXPLAIN_WITH_UPDATE) || + (op === 'distinct' && wireVersion >= SUPPORTS_EXPLAIN_WITH_DISTINCT) || + (op === 'findAndModify' && wireVersion >= SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY) || + (op === 'mapReduce' && wireVersion >= SUPPORTS_EXPLAIN_WITH_MAP_REDUCE) + ) { + return true; + } -/** - * Checks that the provided verbosity level is valid. - * @internal - */ -export function validExplainVerbosity(verbosity: boolean | string): boolean { - if (typeof verbosity === 'string') { - return verbosity in VerbosityStrings; + return false; } - return true; } diff --git a/src/operations/command.ts b/src/operations/command.ts index 3c95571aaa..f2d54f997e 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -10,7 +10,7 @@ import type { Server } from '../sdam/server'; import { BSONSerializeOptions, Document, resolveBSONOptions } from '../bson'; import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import type { ReadConcernLike } from './../read_concern'; -import type { Verbosity } from '../explain'; +import type { Explain } from '../explain'; const SUPPORTS_WRITE_CONCERN_AND_COLLATION = 5; @@ -55,7 +55,7 @@ export abstract class CommandOperation< readPreference: ReadPreference; readConcern?: ReadConcern; writeConcern?: WriteConcern; - explain?: Verbosity; + explain?: Explain; fullResponse?: boolean; logger?: Logger; @@ -97,7 +97,7 @@ export abstract class CommandOperation< } get canRetryWrite(): boolean { - return !this.explain; + return this.explain === undefined; } abstract execute(server: Server, callback: Callback): void; @@ -144,7 +144,7 @@ export abstract class CommandOperation< // If we have reached this point, then the explain verbosity must be valid // Note: explain inherits any comment from its command if (this.explain) { - cmd = decorateWithExplain(cmd, { explain: this.explain }); + cmd = decorateWithExplain(cmd, { explain: this.explain.explain }); // TODO! } if (this.logger && this.logger.isDebug()) { diff --git a/src/operations/delete.ts b/src/operations/delete.ts index 277d710e11..2f661fc235 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -8,7 +8,7 @@ import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { Connection } from '../cmap/connection'; -import type { ExplainOptions } from '../explain'; +import { Explain, ExplainOptions } from '../explain'; /** @public */ export interface DeleteOptions extends CommandOperationOptions, ExplainOptions { @@ -61,7 +61,7 @@ export class DeleteOneOperation extends CommandOperation): void { @@ -99,7 +99,7 @@ export class DeleteManyOperation extends CommandOperation): void { diff --git a/src/operations/distinct.ts b/src/operations/distinct.ts index 9e6d29372e..4c3a218c01 100644 --- a/src/operations/distinct.ts +++ b/src/operations/distinct.ts @@ -4,7 +4,7 @@ import { decorateWithCollation, decorateWithReadConcern, Callback, maxWireVersio import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; -import { ExplainOptions, SUPPORTS_EXPLAIN_WITH_DISTINCT, validExplainVerbosity } from '../explain'; +import { Explain, ExplainOptions, SUPPORTS_EXPLAIN_WITH_DISTINCT } from '../explain'; import { MongoError } from '../error'; /** @public */ @@ -35,7 +35,7 @@ export class DistinctOperation extends CommandOperation): void { @@ -67,11 +67,6 @@ export class DistinctOperation extends CommandOperation): void { @@ -150,11 +146,6 @@ export class FindAndModifyOperation extends CommandOperation): void { @@ -161,11 +157,6 @@ export class MapReduceOperation extends CommandOperation): void { @@ -127,7 +127,7 @@ export class UpdateManyOperation extends CommandOperation): void { diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 10be455022..79bfd0bd5d 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -45,7 +45,7 @@ import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { Document, Long } from '../bson'; import type { AutoEncrypter } from '../deps'; import type { FindOptions } from '../operations/find'; -import { explainSupported, validExplainVerbosity } from '../explain'; +import { Explain } from '../explain'; // Used for filtering out fields for logging const DEBUG_FIELDS = [ @@ -458,16 +458,9 @@ function executeWriteOperation( } } - if (options.explain) { - if (!validExplainVerbosity(options.explain)) { - callback(new MongoError(`${options.explain} is an invalid explain verbosity`)); - return; - } - - if (!explainSupported(server, op)) { - callback(new MongoError(`server ${server.name} does not support explain on ${op}`)); - return; - } + if (options.explain !== undefined && !Explain.explainSupported(server, op)) { + callback(new MongoError(`server ${server.name} does not support explain on ${op}`)); + return; } server.s.pool.withConnection((err, conn, cb) => { diff --git a/src/utils.ts b/src/utils.ts index 3fa0577b33..472fd17408 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -16,7 +16,7 @@ import { readFileSync } from 'fs'; import { resolve } from 'path'; import type { Document } from './bson'; import type { IndexSpecification, IndexDirection } from './operations/indexes'; -import type { ExplainOptions } from './explain'; +import { Explain, ExplainOptions } from './explain'; /** * MongoDB Driver style callback @@ -495,13 +495,9 @@ export function decorateWithReadConcern( * @param options - the options containing the explain verbosity */ export function decorateWithExplain(command: Document, options: ExplainOptions): Document { - if (options.explain) { - // Reformat command into explain command - command = { explain: command }; - if (typeof options.explain === 'string') { - command.verbosity = options.explain; - } - } + const explain = Explain.fromOptions(options); + if (explain === undefined) return command; + command = { explain: command, verbosity: explain.explain }; // todo at this point, this could be invalid? return command; } diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index 803eab2df0..f7e127449b 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -163,14 +163,14 @@ describe('Explain', function () { }) }); - it('shouldUseAllPlansExecutionAsDefaultExplainVerbosity', { + it('shouldUseAllPlansExecutionAsTrueExplainVerbosity', { metadata: { requires: { mongodb: '>3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldUseAllPlansExecutionAsDefaultExplainVerbosity'); + var db = client.db('shouldUseAllPlansExecutionAsTrueExplainVerbosity'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { @@ -189,6 +189,32 @@ describe('Explain', function () { }) }); + it('shouldUseQueryPlannerAsFalseExplainVerbosity', { + metadata: { + requires: { + mongodb: '>3.0' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldUseQueryPlannerAsFalseExplainVerbosity'); + var collection = db.collection('test'); + + collection.insertOne({ a: 1 }, (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + // Verify explanation result contains properties of queryPlanner output + collection.deleteOne({ a: 1 }, { explain: false }, (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('queryPlanner').to.exist; + expect(explanation).to.not.have.property('executionStats'); + done(); + }); + }); + }) + }); + it('shouldHonorQueryPlannerStringExplain', { metadata: { requires: { From 82715bd6128ceaf75b80b2d6cf69c3c627372f67 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Fri, 30 Oct 2020 14:29:09 -0400 Subject: [PATCH 10/23] create ExplainableCommand class --- src/cmap/wire_protocol/command.ts | 6 ++ src/cmap/wire_protocol/write_command.ts | 8 +-- src/explain.ts | 83 +++++++++++++++++++++---- src/operations/command.ts | 14 +---- src/operations/delete.ts | 17 ++--- src/operations/distinct.ts | 20 ++---- src/operations/find_and_modify.ts | 17 +---- src/operations/map_reduce.ts | 23 +++---- src/operations/update.ts | 11 ++-- src/utils.ts | 15 ----- 10 files changed, 105 insertions(+), 109 deletions(-) diff --git a/src/cmap/wire_protocol/command.ts b/src/cmap/wire_protocol/command.ts index a165cba9f3..b0cc54d1e5 100644 --- a/src/cmap/wire_protocol/command.ts +++ b/src/cmap/wire_protocol/command.ts @@ -10,6 +10,7 @@ import type { Topology } from '../../sdam/topology'; import type { ReadPreferenceLike } from '../../read_preference'; import type { WriteConcernOptions, WriteConcern, W } from '../../write_concern'; import type { WriteCommandOptions } from './write_command'; +import { decorateWithExplain } from '../../explain'; /** @internal */ export interface CommandOptions extends BSONSerializeOptions { @@ -68,6 +69,11 @@ export function command( return callback(new MongoError(`command ${JSON.stringify(cmd)} does not return a cursor`)); } + // TODO: should not modify the command here + if (cmd.explain !== undefined) { + cmd = decorateWithExplain(cmd, cmd.explain); + } + if (!isClientEncryptionEnabled(server)) { _command(server, ns, cmd, options, callback); return; diff --git a/src/cmap/wire_protocol/write_command.ts b/src/cmap/wire_protocol/write_command.ts index 14cfced8fc..eb7585f9aa 100644 --- a/src/cmap/wire_protocol/write_command.ts +++ b/src/cmap/wire_protocol/write_command.ts @@ -1,10 +1,10 @@ import { MongoError } from '../../error'; -import { collectionNamespace, Callback, decorateWithExplain } from '../../utils'; +import { collectionNamespace, Callback } from '../../utils'; import { command, CommandOptions } from './command'; import type { Server } from '../../sdam/server'; import type { Document, BSONSerializeOptions } from '../../bson'; import type { WriteConcern } from '../../write_concern'; -import type { ExplainOptions } from '../../explain'; +import { Explain, ExplainOptions } from '../../explain'; /** @public */ export interface CollationOptions { @@ -44,7 +44,7 @@ export function writeCommand( options = options || {}; const ordered = typeof options.ordered === 'boolean' ? options.ordered : true; const writeConcern = options.writeConcern; - let writeCommand: Document = {}; + const writeCommand: Document = {}; writeCommand[type] = collectionNamespace(ns); writeCommand[opsField] = ops; writeCommand.ordered = ordered; @@ -66,7 +66,7 @@ export function writeCommand( } if (options.explain !== undefined) { - writeCommand = decorateWithExplain(writeCommand, options); + writeCommand.explain = Explain.fromOptions(options); } const commandOptions = Object.assign( diff --git a/src/explain.ts b/src/explain.ts index 847038936e..469630fff0 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -1,17 +1,50 @@ +import type { Callback, Document } from '.'; +import { MongoError } from './error'; +import { CommandOperation, CommandOperationOptions, OperationParent } from './operations/command'; import type { Server } from './sdam/server'; import { maxWireVersion } from './utils'; -export const SUPPORTS_EXPLAIN_WITH_REMOVE = 3; -export const SUPPORTS_EXPLAIN_WITH_UPDATE = 3; -export const SUPPORTS_EXPLAIN_WITH_DISTINCT = 3.2; -export const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 3.2; -export const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 4.4; +const SUPPORTS_EXPLAIN_WITH_REMOVE = 3; +const SUPPORTS_EXPLAIN_WITH_UPDATE = 3; +const SUPPORTS_EXPLAIN_WITH_DISTINCT = 3.2; +const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 3.2; +const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 4.4; + +/** @internal */ +export abstract class ExplainableCommand< + T extends ExplainOptions = ExplainOptions, + TResult = Document +> extends CommandOperation { + explain?: Explain; + + constructor(parent?: OperationParent, options?: T) { + super(parent, options); + this.explain = Explain.fromOptions(options); + } + + get canRetryWrite(): boolean { + return this.explain === undefined; + } + + executeCommand(server: Server, cmd: Document, callback: Callback): void { + if (this.explain) { + if (!Explain.explainSupportedOnCmd(server, cmd)) { + callback(new MongoError(`server ${server.name} does not support explain on this command`)); + return; + } + + cmd.explain = this.explain; + } + super.executeCommand(server, cmd, callback); + } +} /** @public */ -export interface ExplainOptions { +export interface ExplainOptions extends CommandOperationOptions { explain?: VerbosityLike; } +/** @public */ export enum Verbosity { queryPlanner = 'queryPlanner', queryPlannerExtended = 'queryPlannerExtended', @@ -22,6 +55,7 @@ export enum Verbosity { /** @public */ export type VerbosityLike = Verbosity | boolean; +/** @internal */ export class Explain { explain: Verbosity; @@ -51,16 +85,43 @@ export class Explain { */ static explainSupported(server: Server, op: string): boolean { const wireVersion = maxWireVersion(server); - if ( + return ( (op === 'remove' && wireVersion >= SUPPORTS_EXPLAIN_WITH_REMOVE) || (op === 'update' && wireVersion >= SUPPORTS_EXPLAIN_WITH_UPDATE) || (op === 'distinct' && wireVersion >= SUPPORTS_EXPLAIN_WITH_DISTINCT) || (op === 'findAndModify' && wireVersion >= SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY) || (op === 'mapReduce' && wireVersion >= SUPPORTS_EXPLAIN_WITH_MAP_REDUCE) - ) { - return true; - } + ); + } - return false; + static explainSupportedOnCmd(server: Server, cmd: Document): boolean { + const wireVersion = maxWireVersion(server); + return ( + (cmd.remove && wireVersion >= SUPPORTS_EXPLAIN_WITH_REMOVE) || + (cmd.update && wireVersion >= SUPPORTS_EXPLAIN_WITH_UPDATE) || + (cmd.distinct && wireVersion >= SUPPORTS_EXPLAIN_WITH_DISTINCT) || + (cmd.findAndModify && wireVersion >= SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY) || + (cmd.mapReduce && wireVersion >= SUPPORTS_EXPLAIN_WITH_MAP_REDUCE) + ); } } + +/** + * Applies an explain to a given command. + * @internal + * + * @param command - the command on which to apply the read concern + * @param options - the options containing the explain verbosity + */ +export function decorateWithExplain(command: Document, options: ExplainOptions): Document { + const explain = Explain.fromOptions(options); + if (explain === undefined) return command; + + // A command being explained may not have an explain field directly on it + if (command.explain !== undefined) { + delete command.explain; + } + + command = { explain: command, verbosity: explain.explain }; + return command; +} diff --git a/src/operations/command.ts b/src/operations/command.ts index f2d54f997e..050eae00a6 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -1,7 +1,7 @@ import { Aspect, OperationBase, OperationOptions } from './operation'; import { ReadConcern } from '../read_concern'; import { WriteConcern, WriteConcernOptions } from '../write_concern'; -import { maxWireVersion, MongoDBNamespace, Callback, decorateWithExplain } from '../utils'; +import { maxWireVersion, MongoDBNamespace, Callback } from '../utils'; import { ReadPreference, ReadPreferenceLike } from '../read_preference'; import { commandSupportsReadConcern } from '../sessions'; import { MongoError } from '../error'; @@ -10,7 +10,6 @@ import type { Server } from '../sdam/server'; import { BSONSerializeOptions, Document, resolveBSONOptions } from '../bson'; import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import type { ReadConcernLike } from './../read_concern'; -import type { Explain } from '../explain'; const SUPPORTS_WRITE_CONCERN_AND_COLLATION = 5; @@ -55,7 +54,6 @@ export abstract class CommandOperation< readPreference: ReadPreference; readConcern?: ReadConcern; writeConcern?: WriteConcern; - explain?: Explain; fullResponse?: boolean; logger?: Logger; @@ -96,10 +94,6 @@ export abstract class CommandOperation< this.bsonOptions = resolveBSONOptions(options, parent); } - get canRetryWrite(): boolean { - return this.explain === undefined; - } - abstract execute(server: Server, callback: Callback): void; executeCommand(server: Server, cmd: Document, callback: Callback): void { @@ -141,12 +135,6 @@ export abstract class CommandOperation< cmd.comment = options.comment; } - // If we have reached this point, then the explain verbosity must be valid - // Note: explain inherits any comment from its command - if (this.explain) { - cmd = decorateWithExplain(cmd, { explain: this.explain.explain }); // TODO! - } - if (this.logger && this.logger.isDebug()) { this.logger.debug(`executing command ${JSON.stringify(cmd)} against ${this.ns}`); } diff --git a/src/operations/delete.ts b/src/operations/delete.ts index 2f661fc235..1994f4f231 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -1,6 +1,5 @@ import { defineAspects, Aspect, OperationBase, Hint } from './operation'; import { removeDocuments } from './common_functions'; -import { CommandOperation, CommandOperationOptions } from './command'; import { isObject } from 'util'; import type { Callback, MongoDBNamespace } from '../utils'; import type { Document } from '../bson'; @@ -8,10 +7,10 @@ import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { Connection } from '../cmap/connection'; -import { Explain, ExplainOptions } from '../explain'; +import { ExplainableCommand, ExplainOptions } from '../explain'; /** @public */ -export interface DeleteOptions extends CommandOperationOptions, ExplainOptions { +export interface DeleteOptions extends ExplainOptions { single?: boolean; hint?: Hint; } @@ -52,7 +51,7 @@ export class DeleteOperation extends OperationBase { } } -export class DeleteOneOperation extends CommandOperation { +export class DeleteOneOperation extends ExplainableCommand { collection: Collection; filter: Document; @@ -61,7 +60,6 @@ export class DeleteOneOperation extends CommandOperation): void { @@ -77,16 +75,13 @@ export class DeleteOneOperation extends CommandOperation { +export class DeleteManyOperation extends ExplainableCommand { collection: Collection; filter: Document; @@ -99,7 +94,6 @@ export class DeleteManyOperation extends CommandOperation): void { @@ -119,9 +113,6 @@ export class DeleteManyOperation extends CommandOperation { +export class DistinctOperation extends ExplainableCommand { collection: Collection; /** Field of the document to find distinct values for. */ key: string; @@ -35,7 +33,6 @@ export class DistinctOperation extends CommandOperation): void { @@ -66,15 +63,6 @@ export class DistinctOperation extends CommandOperation { if (err) { callback(err); diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index a1c0cce41d..404b99728a 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -8,16 +8,15 @@ import { Callback } from '../utils'; import { MongoError } from '../error'; -import { CommandOperation, CommandOperationOptions } from './command'; import { defineAspects, Aspect } from './operation'; import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import { Sort, formatSort } from '../sort'; -import { Explain, ExplainOptions, SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY } from '../explain'; +import { ExplainableCommand, ExplainOptions } from '../explain'; /** @public */ -export interface FindAndModifyOptions extends CommandOperationOptions, ExplainOptions { +export interface FindAndModifyOptions extends ExplainOptions { /** When false, returns the updated document rather than the original. The default is true. */ returnOriginal?: boolean; /** Upsert the document if it does not exist. */ @@ -42,7 +41,7 @@ export interface FindAndModifyOptions extends CommandOperationOptions, ExplainOp } /** @internal */ -export class FindAndModifyOperation extends CommandOperation { +export class FindAndModifyOperation extends ExplainableCommand { collection: Collection; query: Document; sort?: Sort; @@ -64,7 +63,6 @@ export class FindAndModifyOperation extends CommandOperation): void { @@ -145,15 +143,6 @@ export class FindAndModifyOperation extends CommandOperation { if (err) return callback(err); diff --git a/src/operations/map_reduce.ts b/src/operations/map_reduce.ts index 800996808e..1d1bd7fac1 100644 --- a/src/operations/map_reduce.ts +++ b/src/operations/map_reduce.ts @@ -5,17 +5,15 @@ import { decorateWithCollation, decorateWithReadConcern, isObject, - Callback, - maxWireVersion + Callback } from '../utils'; import { ReadPreference, ReadPreferenceMode } from '../read_preference'; -import { CommandOperation, CommandOperationOptions } from './command'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { Sort } from '../sort'; import { MongoError } from '../error'; import type { ObjectId } from '../bson'; -import { Explain, ExplainOptions, SUPPORTS_EXPLAIN_WITH_MAP_REDUCE } from '../explain'; +import { ExplainableCommand, ExplainOptions } from '../explain'; const exclusionList = [ 'explain', @@ -37,7 +35,7 @@ export type ReduceFunction = (key: string, values: Document[]) => Document; export type FinalizeFunction = (key: string, reducedValue: Document) => Document; /** @public */ -export interface MapReduceOptions extends CommandOperationOptions, ExplainOptions { +export interface MapReduceOptions extends ExplainOptions { /** Sets the output target for the map reduce job. */ out?: 'inline' | { inline: 1 } | { replace: string } | { merge: string } | { reduce: string }; /** Query filter object. */ @@ -70,7 +68,10 @@ interface MapReduceStats { * Run Map Reduce across a collection. Be aware that the inline option for out will return an array of results not a collection. * @internal */ -export class MapReduceOperation extends CommandOperation { +export class MapReduceOperation extends ExplainableCommand< + MapReduceOptions, + Document | Document[] +> { collection: Collection; /** The mapping function. */ map: MapFunction | string; @@ -96,7 +97,6 @@ export class MapReduceOperation extends CommandOperation): void { @@ -156,15 +156,6 @@ export class MapReduceOperation extends CommandOperation { if (err) return callback(err); diff --git a/src/operations/update.ts b/src/operations/update.ts index a564f2c147..e59374fb9c 100644 --- a/src/operations/update.ts +++ b/src/operations/update.ts @@ -1,15 +1,14 @@ import { defineAspects, Aspect, OperationBase } from './operation'; import { updateDocuments } from './common_functions'; import { hasAtomicOperators, MongoDBNamespace, Callback } from '../utils'; -import { CommandOperation, CommandOperationOptions } from './command'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { CollationOptions, WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { ObjectId, Document } from '../bson'; -import { Explain, ExplainOptions } from '../explain'; +import { ExplainableCommand, ExplainOptions } from '../explain'; /** @public */ -export interface UpdateOptions extends CommandOperationOptions, ExplainOptions { +export interface UpdateOptions extends ExplainOptions { /** A set of filters specifying to which array elements an update should apply */ arrayFilters?: Document[]; /** If true, allows the write to opt-out of document level validation */ @@ -66,7 +65,7 @@ export class UpdateOperation extends OperationBase { } /** @internal */ -export class UpdateOneOperation extends CommandOperation { +export class UpdateOneOperation extends ExplainableCommand { collection: Collection; filter: Document; update: Document; @@ -81,7 +80,6 @@ export class UpdateOneOperation extends CommandOperation): void { @@ -116,7 +114,7 @@ export class UpdateOneOperation extends CommandOperation { +export class UpdateManyOperation extends ExplainableCommand { collection: Collection; filter: Document; update: Document; @@ -127,7 +125,6 @@ export class UpdateManyOperation extends CommandOperation): void { diff --git a/src/utils.ts b/src/utils.ts index 472fd17408..1f81eb121c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -16,7 +16,6 @@ import { readFileSync } from 'fs'; import { resolve } from 'path'; import type { Document } from './bson'; import type { IndexSpecification, IndexDirection } from './operations/indexes'; -import { Explain, ExplainOptions } from './explain'; /** * MongoDB Driver style callback @@ -487,20 +486,6 @@ export function decorateWithReadConcern( } } -/** - * Applies an explain to a given command. - * @internal - * - * @param command - the command on which to apply the read concern - * @param options - the options containing the explain verbosity - */ -export function decorateWithExplain(command: Document, options: ExplainOptions): Document { - const explain = Explain.fromOptions(options); - if (explain === undefined) return command; - command = { explain: command, verbosity: explain.explain }; // todo at this point, this could be invalid? - return command; -} - /** @internal */ export function emitDeprecationWarning(msg: string): void { return process.emitWarning(msg, 'DeprecationWarning'); From 3bf0d782226c14fb57d8428e116bc21c188536d6 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Mon, 2 Nov 2020 13:59:04 -0500 Subject: [PATCH 11/23] check explain value in explain command constructor --- src/explain.ts | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/explain.ts b/src/explain.ts index 469630fff0..2b3954fd1b 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -19,6 +19,11 @@ export abstract class ExplainableCommand< constructor(parent?: OperationParent, options?: T) { super(parent, options); + + if (!Explain.explainOptionsValid(options)) { + throw new MongoError(`explain must be one of ${Object.keys(Verbosity)} or a boolean`); + } + this.explain = Explain.fromOptions(options); } @@ -57,15 +62,15 @@ export type VerbosityLike = Verbosity | boolean; /** @internal */ export class Explain { - explain: Verbosity; + verbosity: Verbosity; - constructor(explain: VerbosityLike) { - if (typeof explain === 'boolean') { + constructor(verbosity: VerbosityLike) { + if (typeof verbosity === 'boolean') { // For backwards compatibility, true is interpreted as // "allPlansExecution" and false as "queryPlanner". - this.explain = explain ? Verbosity.allPlansExecution : Verbosity.queryPlanner; + this.verbosity = verbosity ? Verbosity.allPlansExecution : Verbosity.queryPlanner; } else { - this.explain = Verbosity[explain]; + this.verbosity = Verbosity[verbosity]; } } @@ -76,6 +81,14 @@ export class Explain { return new Explain(options.explain); } + static explainOptionsValid(options?: ExplainOptions): boolean { + if (options == null || options.explain === undefined) { + return true; + } + const explain = options.explain; + return typeof explain === 'boolean' || explain in Verbosity; + } + /** * Checks that the server supports explain on the given operation. * @internal @@ -113,15 +126,12 @@ export class Explain { * @param command - the command on which to apply the read concern * @param options - the options containing the explain verbosity */ -export function decorateWithExplain(command: Document, options: ExplainOptions): Document { - const explain = Explain.fromOptions(options); - if (explain === undefined) return command; - +export function decorateWithExplain(command: Document, explain: Explain): Document { // A command being explained may not have an explain field directly on it if (command.explain !== undefined) { delete command.explain; } - command = { explain: command, verbosity: explain.explain }; + command = { explain: command, verbosity: explain.verbosity }; return command; } From 7090c393655e59c5f91e707e164d437febca96dc Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Mon, 2 Nov 2020 15:28:46 -0500 Subject: [PATCH 12/23] quick cursor fix --- src/cmap/wire_protocol/query.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/cmap/wire_protocol/query.ts b/src/cmap/wire_protocol/query.ts index 798f79fc29..9291be0f2d 100644 --- a/src/cmap/wire_protocol/query.ts +++ b/src/cmap/wire_protocol/query.ts @@ -7,6 +7,7 @@ import { Document, pluckBSONSerializeOptions } from '../../bson'; import type { Server } from '../../sdam/server'; import type { ReadPreferenceLike } from '../../read_preference'; import type { FindOptions } from '../../operations/find'; +import { Explain } from '../../explain'; /** @internal */ export interface QueryOptions extends CommandOptions { @@ -62,7 +63,7 @@ export function query( } function prepareFindCommand(server: Server, ns: string, cmd: Document) { - let findCmd: Document = { + const findCmd: Document = { find: collectionNamespace(ns) }; @@ -146,12 +147,9 @@ function prepareFindCommand(server: Server, ns: string, cmd: Document) { if (cmd.collation) findCmd.collation = cmd.collation; if (cmd.readConcern) findCmd.readConcern = cmd.readConcern; - // If we have explain, we need to rewrite the find command - // to wrap it in the explain command - if (cmd.explain) { - findCmd = { - explain: findCmd - }; + // TODO: Quick fix to make tests pass; will be updated during NODE-2853 + if (cmd.explain !== undefined) { + findCmd.explain = Explain.fromOptions({ explain: cmd.explain }); } return findCmd; From 82f0e608658344dc9d28cca1357a18b3ff979a5f Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Mon, 2 Nov 2020 17:21:33 -0500 Subject: [PATCH 13/23] move explain cmd/options to separate file --- src/cmap/wire_protocol/command.ts | 6 --- src/cmap/wire_protocol/query.ts | 12 +++-- src/cmap/wire_protocol/write_command.ts | 14 ++++-- src/explain.ts | 63 ++----------------------- src/operations/command.ts | 8 +++- src/operations/delete.ts | 5 +- src/operations/distinct.ts | 5 +- src/operations/explainable_command.ts | 42 +++++++++++++++++ src/operations/find_and_modify.ts | 5 +- src/operations/map_reduce.ts | 5 +- src/operations/update.ts | 5 +- src/utils.ts | 18 +++++++ test/functional/explain.test.js | 4 +- 13 files changed, 104 insertions(+), 88 deletions(-) create mode 100644 src/operations/explainable_command.ts diff --git a/src/cmap/wire_protocol/command.ts b/src/cmap/wire_protocol/command.ts index b0cc54d1e5..a165cba9f3 100644 --- a/src/cmap/wire_protocol/command.ts +++ b/src/cmap/wire_protocol/command.ts @@ -10,7 +10,6 @@ import type { Topology } from '../../sdam/topology'; import type { ReadPreferenceLike } from '../../read_preference'; import type { WriteConcernOptions, WriteConcern, W } from '../../write_concern'; import type { WriteCommandOptions } from './write_command'; -import { decorateWithExplain } from '../../explain'; /** @internal */ export interface CommandOptions extends BSONSerializeOptions { @@ -69,11 +68,6 @@ export function command( return callback(new MongoError(`command ${JSON.stringify(cmd)} does not return a cursor`)); } - // TODO: should not modify the command here - if (cmd.explain !== undefined) { - cmd = decorateWithExplain(cmd, cmd.explain); - } - if (!isClientEncryptionEnabled(server)) { _command(server, ns, cmd, options, callback); return; diff --git a/src/cmap/wire_protocol/query.ts b/src/cmap/wire_protocol/query.ts index 9291be0f2d..798f79fc29 100644 --- a/src/cmap/wire_protocol/query.ts +++ b/src/cmap/wire_protocol/query.ts @@ -7,7 +7,6 @@ import { Document, pluckBSONSerializeOptions } from '../../bson'; import type { Server } from '../../sdam/server'; import type { ReadPreferenceLike } from '../../read_preference'; import type { FindOptions } from '../../operations/find'; -import { Explain } from '../../explain'; /** @internal */ export interface QueryOptions extends CommandOptions { @@ -63,7 +62,7 @@ export function query( } function prepareFindCommand(server: Server, ns: string, cmd: Document) { - const findCmd: Document = { + let findCmd: Document = { find: collectionNamespace(ns) }; @@ -147,9 +146,12 @@ function prepareFindCommand(server: Server, ns: string, cmd: Document) { if (cmd.collation) findCmd.collation = cmd.collation; if (cmd.readConcern) findCmd.readConcern = cmd.readConcern; - // TODO: Quick fix to make tests pass; will be updated during NODE-2853 - if (cmd.explain !== undefined) { - findCmd.explain = Explain.fromOptions({ explain: cmd.explain }); + // If we have explain, we need to rewrite the find command + // to wrap it in the explain command + if (cmd.explain) { + findCmd = { + explain: findCmd + }; } return findCmd; diff --git a/src/cmap/wire_protocol/write_command.ts b/src/cmap/wire_protocol/write_command.ts index eb7585f9aa..dacfbda6f9 100644 --- a/src/cmap/wire_protocol/write_command.ts +++ b/src/cmap/wire_protocol/write_command.ts @@ -1,10 +1,11 @@ import { MongoError } from '../../error'; -import { collectionNamespace, Callback } from '../../utils'; +import { collectionNamespace, Callback, decorateWithExplain } from '../../utils'; import { command, CommandOptions } from './command'; import type { Server } from '../../sdam/server'; import type { Document, BSONSerializeOptions } from '../../bson'; import type { WriteConcern } from '../../write_concern'; -import { Explain, ExplainOptions } from '../../explain'; +import { Explain } from '../../explain'; +import type { ExplainOptions } from '../../operations/explainable_command'; /** @public */ export interface CollationOptions { @@ -44,7 +45,7 @@ export function writeCommand( options = options || {}; const ordered = typeof options.ordered === 'boolean' ? options.ordered : true; const writeConcern = options.writeConcern; - const writeCommand: Document = {}; + let writeCommand: Document = {}; writeCommand[type] = collectionNamespace(ns); writeCommand[opsField] = ops; writeCommand.ordered = ordered; @@ -65,8 +66,13 @@ export function writeCommand( writeCommand.bypassDocumentValidation = options.bypassDocumentValidation; } + // If a command is to be explained, we need to reformat the command after + // the other command properties are specified. if (options.explain !== undefined) { - writeCommand.explain = Explain.fromOptions(options); + const explain = Explain.fromOptions(options); + if (explain) { + writeCommand = decorateWithExplain(writeCommand, explain); + } } const commandOptions = Object.assign( diff --git a/src/explain.ts b/src/explain.ts index 2b3954fd1b..d3f5dac301 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -1,6 +1,5 @@ -import type { Callback, Document } from '.'; -import { MongoError } from './error'; -import { CommandOperation, CommandOperationOptions, OperationParent } from './operations/command'; +import type { Document } from '.'; +import type { ExplainOptions } from './operations/explainable_command'; import type { Server } from './sdam/server'; import { maxWireVersion } from './utils'; @@ -10,45 +9,6 @@ const SUPPORTS_EXPLAIN_WITH_DISTINCT = 3.2; const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 3.2; const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 4.4; -/** @internal */ -export abstract class ExplainableCommand< - T extends ExplainOptions = ExplainOptions, - TResult = Document -> extends CommandOperation { - explain?: Explain; - - constructor(parent?: OperationParent, options?: T) { - super(parent, options); - - if (!Explain.explainOptionsValid(options)) { - throw new MongoError(`explain must be one of ${Object.keys(Verbosity)} or a boolean`); - } - - this.explain = Explain.fromOptions(options); - } - - get canRetryWrite(): boolean { - return this.explain === undefined; - } - - executeCommand(server: Server, cmd: Document, callback: Callback): void { - if (this.explain) { - if (!Explain.explainSupportedOnCmd(server, cmd)) { - callback(new MongoError(`server ${server.name} does not support explain on this command`)); - return; - } - - cmd.explain = this.explain; - } - super.executeCommand(server, cmd, callback); - } -} - -/** @public */ -export interface ExplainOptions extends CommandOperationOptions { - explain?: VerbosityLike; -} - /** @public */ export enum Verbosity { queryPlanner = 'queryPlanner', @@ -81,7 +41,7 @@ export class Explain { return new Explain(options.explain); } - static explainOptionsValid(options?: ExplainOptions): boolean { + static valid(options?: ExplainOptions): boolean { if (options == null || options.explain === undefined) { return true; } @@ -118,20 +78,3 @@ export class Explain { ); } } - -/** - * Applies an explain to a given command. - * @internal - * - * @param command - the command on which to apply the read concern - * @param options - the options containing the explain verbosity - */ -export function decorateWithExplain(command: Document, explain: Explain): Document { - // A command being explained may not have an explain field directly on it - if (command.explain !== undefined) { - delete command.explain; - } - - command = { explain: command, verbosity: explain.verbosity }; - return command; -} diff --git a/src/operations/command.ts b/src/operations/command.ts index 050eae00a6..c80ebf1b77 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -1,7 +1,7 @@ import { Aspect, OperationBase, OperationOptions } from './operation'; import { ReadConcern } from '../read_concern'; import { WriteConcern, WriteConcernOptions } from '../write_concern'; -import { maxWireVersion, MongoDBNamespace, Callback } from '../utils'; +import { maxWireVersion, MongoDBNamespace, Callback, decorateWithExplain } from '../utils'; import { ReadPreference, ReadPreferenceLike } from '../read_preference'; import { commandSupportsReadConcern } from '../sessions'; import { MongoError } from '../error'; @@ -139,6 +139,12 @@ export abstract class CommandOperation< this.logger.debug(`executing command ${JSON.stringify(cmd)} against ${this.ns}`); } + // If a command is to be explained, we need to reformat the command after + // the other command properties are specified. + if (cmd.explain) { + cmd = decorateWithExplain(cmd, cmd.explain); + } + server.command( this.ns.toString(), cmd, diff --git a/src/operations/delete.ts b/src/operations/delete.ts index 1994f4f231..a991ca73c3 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -7,10 +7,11 @@ import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { Connection } from '../cmap/connection'; -import { ExplainableCommand, ExplainOptions } from '../explain'; +import { ExplainableCommand, ExplainOptions } from '../operations/explainable_command'; +import type { CommandOperationOptions } from './command'; /** @public */ -export interface DeleteOptions extends ExplainOptions { +export interface DeleteOptions extends CommandOperationOptions, ExplainOptions { single?: boolean; hint?: Hint; } diff --git a/src/operations/distinct.ts b/src/operations/distinct.ts index 0d3c2e7e1e..4b20fa93a5 100644 --- a/src/operations/distinct.ts +++ b/src/operations/distinct.ts @@ -3,10 +3,11 @@ import { decorateWithCollation, decorateWithReadConcern, Callback } from '../uti import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; -import { ExplainableCommand, ExplainOptions } from '../explain'; +import { ExplainableCommand, ExplainOptions } from '../operations/explainable_command'; +import type { CommandOperationOptions } from './command'; /** @public */ -export type DistinctOptions = ExplainOptions; +export interface DistinctOptions extends CommandOperationOptions, ExplainOptions {} /** * Return a list of distinct values for the given key across a collection. diff --git a/src/operations/explainable_command.ts b/src/operations/explainable_command.ts new file mode 100644 index 0000000000..afa7d4ce3f --- /dev/null +++ b/src/operations/explainable_command.ts @@ -0,0 +1,42 @@ +import { CommandOperation, OperationParent, CommandOperationOptions } from './command'; +import { Explain, Verbosity, VerbosityLike } from '../explain'; +import { Callback, Document, MongoError, Server } from '..'; + +/** @public */ +export interface ExplainOptions { + explain?: VerbosityLike; +} + +/** @internal */ +export abstract class ExplainableCommand< + T extends ExplainOptions & CommandOperationOptions, + TResult = Document +> extends CommandOperation { + explain?: Explain; + + constructor(parent?: OperationParent, options?: T) { + super(parent, options); + + if (!Explain.valid(options)) { + throw new MongoError(`explain must be one of ${Object.keys(Verbosity)} or a boolean`); + } + + this.explain = Explain.fromOptions(options); + } + + get canRetryWrite(): boolean { + return this.explain === undefined; + } + + executeCommand(server: Server, cmd: Document, callback: Callback): void { + if (this.explain) { + if (!Explain.explainSupportedOnCmd(server, cmd)) { + callback(new MongoError(`server ${server.name} does not support explain on this command`)); + return; + } + + cmd.explain = this.explain; + } + super.executeCommand(server, cmd, callback); + } +} diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 404b99728a..7c9e1a5a51 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -13,10 +13,11 @@ import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import { Sort, formatSort } from '../sort'; -import { ExplainableCommand, ExplainOptions } from '../explain'; +import { ExplainableCommand, ExplainOptions } from '../operations/explainable_command'; +import type { CommandOperationOptions } from './command'; /** @public */ -export interface FindAndModifyOptions extends ExplainOptions { +export interface FindAndModifyOptions extends CommandOperationOptions, ExplainOptions { /** When false, returns the updated document rather than the original. The default is true. */ returnOriginal?: boolean; /** Upsert the document if it does not exist. */ diff --git a/src/operations/map_reduce.ts b/src/operations/map_reduce.ts index 1d1bd7fac1..54dd636b9a 100644 --- a/src/operations/map_reduce.ts +++ b/src/operations/map_reduce.ts @@ -13,7 +13,8 @@ import type { Collection } from '../collection'; import type { Sort } from '../sort'; import { MongoError } from '../error'; import type { ObjectId } from '../bson'; -import { ExplainableCommand, ExplainOptions } from '../explain'; +import { ExplainableCommand, ExplainOptions } from '../operations/explainable_command'; +import type { CommandOperationOptions } from './command'; const exclusionList = [ 'explain', @@ -35,7 +36,7 @@ export type ReduceFunction = (key: string, values: Document[]) => Document; export type FinalizeFunction = (key: string, reducedValue: Document) => Document; /** @public */ -export interface MapReduceOptions extends ExplainOptions { +export interface MapReduceOptions extends CommandOperationOptions, ExplainOptions { /** Sets the output target for the map reduce job. */ out?: 'inline' | { inline: 1 } | { replace: string } | { merge: string } | { reduce: string }; /** Query filter object. */ diff --git a/src/operations/update.ts b/src/operations/update.ts index e59374fb9c..a89ee4f33c 100644 --- a/src/operations/update.ts +++ b/src/operations/update.ts @@ -5,10 +5,11 @@ import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { CollationOptions, WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { ObjectId, Document } from '../bson'; -import { ExplainableCommand, ExplainOptions } from '../explain'; +import { ExplainableCommand, ExplainOptions } from '../operations/explainable_command'; +import type { CommandOperationOptions } from './command'; /** @public */ -export interface UpdateOptions extends ExplainOptions { +export interface UpdateOptions extends CommandOperationOptions, ExplainOptions { /** A set of filters specifying to which array elements an update should apply */ arrayFilters?: Document[]; /** If true, allows the write to opt-out of document level validation */ diff --git a/src/utils.ts b/src/utils.ts index 1f81eb121c..5c4dd3e42b 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -16,6 +16,7 @@ import { readFileSync } from 'fs'; import { resolve } from 'path'; import type { Document } from './bson'; import type { IndexSpecification, IndexDirection } from './operations/indexes'; +import type { Explain } from './explain'; /** * MongoDB Driver style callback @@ -486,6 +487,23 @@ export function decorateWithReadConcern( } } +/** + * Applies an explain to a given command. + * @internal + * + * @param command - the command on which to apply the read concern + * @param options - the options containing the explain verbosity + */ +export function decorateWithExplain(command: Document, explain: Explain): Document { + // A command being explained may not have an explain field directly on it + if (command.explain !== undefined) { + delete command.explain; + } + + command = { explain: command, verbosity: explain.verbosity }; + return command; +} + /** @internal */ export function emitDeprecationWarning(msg: string): void { return process.emitWarning(msg, 'DeprecationWarning'); diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index f7e127449b..48942ced1f 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -256,7 +256,7 @@ describe('Explain', function () { expect(res).to.exist; // Verify explanation result contains properties of executionStats output - collection.deleteOne({ a: 1 }, { explain: 'executionStats' }, (err, explanation) => { + collection.findOneAndDelete({ a: 1 }, { explain: 'executionStats' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; @@ -283,7 +283,7 @@ describe('Explain', function () { expect(res).to.exist; // Verify explanation result contains properties of allPlansExecution output - collection.deleteOne({ a: 1 }, { explain: 'allPlansExecution' }, (err, explanation) => { + collection.distinct('a', {}, { explain: 'allPlansExecution' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; From f5e65ed81fcfc581594be43f8e221284c365804a Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Tue, 3 Nov 2020 09:24:53 -0500 Subject: [PATCH 14/23] some commenting --- src/explain.ts | 30 +++++++++++++-------------- src/operations/explainable_command.ts | 2 ++ src/utils.ts | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/explain.ts b/src/explain.ts index d3f5dac301..52606ab41f 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -3,12 +3,6 @@ import type { ExplainOptions } from './operations/explainable_command'; import type { Server } from './sdam/server'; import { maxWireVersion } from './utils'; -const SUPPORTS_EXPLAIN_WITH_REMOVE = 3; -const SUPPORTS_EXPLAIN_WITH_UPDATE = 3; -const SUPPORTS_EXPLAIN_WITH_DISTINCT = 3.2; -const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 3.2; -const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 4.4; - /** @public */ export enum Verbosity { queryPlanner = 'queryPlanner', @@ -17,17 +11,26 @@ export enum Verbosity { allPlansExecution = 'allPlansExecution' } -/** @public */ +/** + * For backwards compatibility, true is interpreted as + * "allPlansExecution" and false as "queryPlanner". + * @public + */ export type VerbosityLike = Verbosity | boolean; +// Minimum server versions which support explain with specific operations +const SUPPORTS_EXPLAIN_WITH_REMOVE = 3; +const SUPPORTS_EXPLAIN_WITH_UPDATE = 3; +const SUPPORTS_EXPLAIN_WITH_DISTINCT = 3.2; +const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 3.2; +const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 4.4; + /** @internal */ export class Explain { verbosity: Verbosity; constructor(verbosity: VerbosityLike) { if (typeof verbosity === 'boolean') { - // For backwards compatibility, true is interpreted as - // "allPlansExecution" and false as "queryPlanner". this.verbosity = verbosity ? Verbosity.allPlansExecution : Verbosity.queryPlanner; } else { this.verbosity = Verbosity[verbosity]; @@ -49,13 +52,7 @@ export class Explain { return typeof explain === 'boolean' || explain in Verbosity; } - /** - * Checks that the server supports explain on the given operation. - * @internal - * - * @param server - to check against - * @param op - the operation to explain - */ + /** Checks that the server supports explain on the given operation.*/ static explainSupported(server: Server, op: string): boolean { const wireVersion = maxWireVersion(server); return ( @@ -67,6 +64,7 @@ export class Explain { ); } + /** Checks that the server supports explain on the given command.*/ static explainSupportedOnCmd(server: Server, cmd: Document): boolean { const wireVersion = maxWireVersion(server); return ( diff --git a/src/operations/explainable_command.ts b/src/operations/explainable_command.ts index afa7d4ce3f..42c0144302 100644 --- a/src/operations/explainable_command.ts +++ b/src/operations/explainable_command.ts @@ -35,6 +35,8 @@ export abstract class ExplainableCommand< return; } + // For now, tag the command with the explain; after cmd is finalized in the super class, + // it will be refactored into the required shape using the explain. cmd.explain = this.explain; } super.executeCommand(server, cmd, callback); diff --git a/src/utils.ts b/src/utils.ts index 5c4dd3e42b..0a7c676978 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -495,7 +495,7 @@ export function decorateWithReadConcern( * @param options - the options containing the explain verbosity */ export function decorateWithExplain(command: Document, explain: Explain): Document { - // A command being explained may not have an explain field directly on it + // A command being explained cannot have an explain field directly on it if (command.explain !== undefined) { delete command.explain; } From 4bdf50311aace6ca69860f7e20703a454cc260f5 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Thu, 5 Nov 2020 12:42:01 -0500 Subject: [PATCH 15/23] respond to comments --- src/cursor/cursor.ts | 3 +- src/explain.ts | 61 ++++++++++++--------------- src/operations/command.ts | 6 +-- src/operations/explainable_command.ts | 2 +- src/utils.ts | 3 +- test/functional/aggregation.test.js | 3 +- test/functional/explain.test.js | 32 +++++++------- 7 files changed, 50 insertions(+), 60 deletions(-) diff --git a/src/cursor/cursor.ts b/src/cursor/cursor.ts index 61bfefcec5..077ee5befa 100644 --- a/src/cursor/cursor.ts +++ b/src/cursor/cursor.ts @@ -1307,7 +1307,8 @@ export class Cursor< explain(callback?: Callback): Promise | void { // NOTE: the next line includes a special case for operations which do not // subclass `CommandOperationV2`. To be removed asap. - // TODO: uncomment/fix this when cursor explain is re-implemented + // TODO NODE-2853: This had to be removed during NODE-2852; fix while re-implementing + // cursor explain // if (this.operation && this.operation.cmd == null) { // this.operation.options.explain = true; // return executeOperation(this.topology, this.operation as any, callback); diff --git a/src/explain.ts b/src/explain.ts index 52606ab41f..d5729faf27 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -3,31 +3,30 @@ import type { ExplainOptions } from './operations/explainable_command'; import type { Server } from './sdam/server'; import { maxWireVersion } from './utils'; -/** @public */ -export enum Verbosity { - queryPlanner = 'queryPlanner', - queryPlannerExtended = 'queryPlannerExtended', - executionStats = 'executionStats', - allPlansExecution = 'allPlansExecution' -} +export const Verbosity = { + queryPlanner: 'queryPlanner', + queryPlannerExtended: 'queryPlannerExtended', + executionStats: 'executionStats', + allPlansExecution: 'allPlansExecution' +} as const; /** * For backwards compatibility, true is interpreted as * "allPlansExecution" and false as "queryPlanner". * @public */ -export type VerbosityLike = Verbosity | boolean; +export type VerbosityLike = keyof typeof Verbosity | boolean; // Minimum server versions which support explain with specific operations const SUPPORTS_EXPLAIN_WITH_REMOVE = 3; const SUPPORTS_EXPLAIN_WITH_UPDATE = 3; -const SUPPORTS_EXPLAIN_WITH_DISTINCT = 3.2; -const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 3.2; -const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 4.4; +const SUPPORTS_EXPLAIN_WITH_DISTINCT = 4; +const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 4; +const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 9; /** @internal */ export class Explain { - verbosity: Verbosity; + verbosity: keyof typeof Verbosity; constructor(verbosity: VerbosityLike) { if (typeof verbosity === 'boolean') { @@ -38,41 +37,35 @@ export class Explain { } static fromOptions(options?: ExplainOptions): Explain | undefined { - if (options == null || options.explain === undefined) { + if (options?.explain === undefined) { return; } return new Explain(options.explain); } static valid(options?: ExplainOptions): boolean { - if (options == null || options.explain === undefined) { + if (options?.explain === undefined) { return true; } const explain = options.explain; return typeof explain === 'boolean' || explain in Verbosity; } - /** Checks that the server supports explain on the given operation.*/ - static explainSupported(server: Server, op: string): boolean { + /** Checks that the server supports explain on the given operation or command.*/ + static explainSupported(server: Server, op: string | Document): boolean { const wireVersion = maxWireVersion(server); - return ( - (op === 'remove' && wireVersion >= SUPPORTS_EXPLAIN_WITH_REMOVE) || - (op === 'update' && wireVersion >= SUPPORTS_EXPLAIN_WITH_UPDATE) || - (op === 'distinct' && wireVersion >= SUPPORTS_EXPLAIN_WITH_DISTINCT) || - (op === 'findAndModify' && wireVersion >= SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY) || - (op === 'mapReduce' && wireVersion >= SUPPORTS_EXPLAIN_WITH_MAP_REDUCE) - ); - } + if (op === 'remove' || (typeof op === 'object' && op.remove)) { + return wireVersion >= SUPPORTS_EXPLAIN_WITH_REMOVE; + } else if (op === 'update' || (typeof op === 'object' && op.update)) { + return wireVersion >= SUPPORTS_EXPLAIN_WITH_UPDATE; + } else if (op === 'distinct' || (typeof op === 'object' && op.distinct)) { + return wireVersion >= SUPPORTS_EXPLAIN_WITH_DISTINCT; + } else if (op === 'findAndModify' || (typeof op === 'object' && op.findAndModify)) { + return wireVersion >= SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY; + } else if (op === 'mapReduce' || (typeof op === 'object' && op.mapReduce)) { + return wireVersion >= SUPPORTS_EXPLAIN_WITH_MAP_REDUCE; + } - /** Checks that the server supports explain on the given command.*/ - static explainSupportedOnCmd(server: Server, cmd: Document): boolean { - const wireVersion = maxWireVersion(server); - return ( - (cmd.remove && wireVersion >= SUPPORTS_EXPLAIN_WITH_REMOVE) || - (cmd.update && wireVersion >= SUPPORTS_EXPLAIN_WITH_UPDATE) || - (cmd.distinct && wireVersion >= SUPPORTS_EXPLAIN_WITH_DISTINCT) || - (cmd.findAndModify && wireVersion >= SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY) || - (cmd.mapReduce && wireVersion >= SUPPORTS_EXPLAIN_WITH_MAP_REDUCE) - ); + return false; } } diff --git a/src/operations/command.ts b/src/operations/command.ts index c80ebf1b77..4e197348a6 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -141,13 +141,9 @@ export abstract class CommandOperation< // If a command is to be explained, we need to reformat the command after // the other command properties are specified. - if (cmd.explain) { - cmd = decorateWithExplain(cmd, cmd.explain); - } - server.command( this.ns.toString(), - cmd, + cmd.explain ? decorateWithExplain(cmd, cmd.explain) : cmd, { fullResult: !!this.fullResponse, ...this.options }, callback ); diff --git a/src/operations/explainable_command.ts b/src/operations/explainable_command.ts index 42c0144302..bfa3ec938b 100644 --- a/src/operations/explainable_command.ts +++ b/src/operations/explainable_command.ts @@ -30,7 +30,7 @@ export abstract class ExplainableCommand< executeCommand(server: Server, cmd: Document, callback: Callback): void { if (this.explain) { - if (!Explain.explainSupportedOnCmd(server, cmd)) { + if (!Explain.explainSupported(server, cmd)) { callback(new MongoError(`server ${server.name} does not support explain on this command`)); return; } diff --git a/src/utils.ts b/src/utils.ts index 0a7c676978..1f99680ed9 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -500,8 +500,7 @@ export function decorateWithExplain(command: Document, explain: Explain): Docume delete command.explain; } - command = { explain: command, verbosity: explain.verbosity }; - return command; + return { explain: command, verbosity: explain.verbosity }; } /** @internal */ diff --git a/test/functional/aggregation.test.js b/test/functional/aggregation.test.js index 566576ae3b..a89d09dd5d 100644 --- a/test/functional/aggregation.test.js +++ b/test/functional/aggregation.test.js @@ -387,7 +387,8 @@ describe('Aggregation', function () { * @example-method aggregate */ it.skip('should correctly return a cursor and call explain', { - // TODO: add back this test when cursor explain is fully implemented + // TODO NODE-2853: This had to be skipped during NODE-2852; un-skip while re-implementing + // cursor explain // Add a tag that our runner can trigger on // in this case we are setting that node needs to be higher than 0.10.X to run diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index 48942ced1f..871e867440 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -12,7 +12,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithDeleteOne', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -36,7 +36,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithDeleteMany', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -60,7 +60,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithUpdateOne', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -89,7 +89,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithUpdateMany', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -118,7 +118,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithRemoveOne', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -142,7 +142,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithRemoveMany', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -166,7 +166,7 @@ describe('Explain', function () { it('shouldUseAllPlansExecutionAsTrueExplainVerbosity', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -192,7 +192,7 @@ describe('Explain', function () { it('shouldUseQueryPlannerAsFalseExplainVerbosity', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -218,7 +218,7 @@ describe('Explain', function () { it('shouldHonorQueryPlannerStringExplain', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -244,7 +244,7 @@ describe('Explain', function () { it('shouldHonorExecutionStatsStringExplain', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -271,7 +271,7 @@ describe('Explain', function () { it('shouldHonorAllPlansStringExplain', { metadata: { requires: { - mongodb: '>3.0' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { @@ -297,7 +297,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithDistinct', { metadata: { requires: { - mongodb: '>3.2' + mongodb: '>=3.2' } }, test: withClient(function (client, done) { @@ -321,7 +321,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithFindOneAndDelete', { metadata: { requires: { - mongodb: '>3.2' + mongodb: '>=3.2' } }, test: withClient(function (client, done) { @@ -345,7 +345,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithFindOneAndReplace', { metadata: { requires: { - mongodb: '>3.2' + mongodb: '>=3.2' } }, test: withClient(function (client, done) { @@ -369,7 +369,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithFindOneAndUpdate', { metadata: { requires: { - mongodb: '>3.2' + mongodb: '>=3.2' } }, test: withClient(function (client, done) { @@ -398,7 +398,7 @@ describe('Explain', function () { it('shouldHonorBooleanExplainWithMapReduce', { metadata: { requires: { - mongodb: '>4.4' + mongodb: '>=4.4' } }, test: withClient(function (client, done) { From 03333d7db5260258a5a5b0bc3d99feac9db4d808 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Mon, 9 Nov 2020 09:16:29 -0500 Subject: [PATCH 16/23] test bug fix --- test/functional/explain.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index 871e867440..87986eec58 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -244,7 +244,7 @@ describe('Explain', function () { it('shouldHonorExecutionStatsStringExplain', { metadata: { requires: { - mongodb: '>=3.0' + mongodb: '>3.2' } }, test: withClient(function (client, done) { @@ -271,7 +271,7 @@ describe('Explain', function () { it('shouldHonorAllPlansStringExplain', { metadata: { requires: { - mongodb: '>=3.0' + mongodb: '>3.2' } }, test: withClient(function (client, done) { From c1a4ff92c816fc404e2817fa0a3add3f6d4efbad Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Wed, 11 Nov 2020 13:24:32 -0500 Subject: [PATCH 17/23] add explain-related exports to index --- src/index.ts | 2 ++ src/operations/explainable_command.ts | 1 + test/functional/explain.test.js | 4 ++-- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 4316800f53..13466fb668 100644 --- a/src/index.ts +++ b/src/index.ts @@ -163,6 +163,7 @@ export type { export type { DbPrivate, DbOptions } from './db'; export type { AutoEncryptionOptions, AutoEncryptionLoggerLevels, AutoEncrypter } from './deps'; export type { AnyError, ErrorDescription } from './error'; +export type { Verbosity, VerbosityLike } from './explain'; export type { GridFSBucketReadStream, GridFSBucketReadStreamOptions, @@ -207,6 +208,7 @@ export type { DistinctOptions } from './operations/distinct'; export type { DropCollectionOptions, DropDatabaseOptions } from './operations/drop'; export type { EstimatedDocumentCountOptions } from './operations/estimated_document_count'; export type { EvalOptions } from './operations/eval'; +export type { ExplainOptions } from './operations/explainable_command'; export type { FindOptions } from './operations/find'; export type { Sort, SortDirection } from './sort'; export type { FindAndModifyOptions } from './operations/find_and_modify'; diff --git a/src/operations/explainable_command.ts b/src/operations/explainable_command.ts index bfa3ec938b..9452bb15bb 100644 --- a/src/operations/explainable_command.ts +++ b/src/operations/explainable_command.ts @@ -4,6 +4,7 @@ import { Callback, Document, MongoError, Server } from '..'; /** @public */ export interface ExplainOptions { + /** Specifies the verbosity mode for the explain output. */ explain?: VerbosityLike; } diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index 87986eec58..68e169e0d8 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -244,7 +244,7 @@ describe('Explain', function () { it('shouldHonorExecutionStatsStringExplain', { metadata: { requires: { - mongodb: '>3.2' + mongodb: '>=3.2' } }, test: withClient(function (client, done) { @@ -271,7 +271,7 @@ describe('Explain', function () { it('shouldHonorAllPlansStringExplain', { metadata: { requires: { - mongodb: '>3.2' + mongodb: '>=3.2' } }, test: withClient(function (client, done) { From 2c7334b69d1099759988021c987d3b6f85dce9e8 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Thu, 12 Nov 2020 09:09:40 -0500 Subject: [PATCH 18/23] use aspects and throw from fromOptions --- src/cmap/wire_protocol/write_command.ts | 3 +- src/explain.ts | 25 ++++++++------ src/index.ts | 3 +- src/operations/aggregate.ts | 2 -- src/operations/command.ts | 33 +++++++++++++++--- src/operations/delete.ts | 13 ++++--- src/operations/distinct.ts | 9 +++-- src/operations/explainable_command.ts | 45 ------------------------- src/operations/find.ts | 2 -- src/operations/find_and_modify.ts | 13 ++++--- src/operations/map_reduce.ts | 13 ++++--- src/operations/operation.ts | 3 +- src/operations/update.ts | 13 ++++--- src/utils.ts | 5 ++- 14 files changed, 79 insertions(+), 103 deletions(-) delete mode 100644 src/operations/explainable_command.ts diff --git a/src/cmap/wire_protocol/write_command.ts b/src/cmap/wire_protocol/write_command.ts index dacfbda6f9..2f135d0761 100644 --- a/src/cmap/wire_protocol/write_command.ts +++ b/src/cmap/wire_protocol/write_command.ts @@ -4,8 +4,7 @@ import { command, CommandOptions } from './command'; import type { Server } from '../../sdam/server'; import type { Document, BSONSerializeOptions } from '../../bson'; import type { WriteConcern } from '../../write_concern'; -import { Explain } from '../../explain'; -import type { ExplainOptions } from '../../operations/explainable_command'; +import { Explain, ExplainOptions } from '../../explain'; /** @public */ export interface CollationOptions { diff --git a/src/explain.ts b/src/explain.ts index d5729faf27..7f1096b9a9 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -1,8 +1,9 @@ import type { Document } from '.'; -import type { ExplainOptions } from './operations/explainable_command'; +import { MongoError } from './error'; import type { Server } from './sdam/server'; import { maxWireVersion } from './utils'; +/** @public */ export const Verbosity = { queryPlanner: 'queryPlanner', queryPlannerExtended: 'queryPlannerExtended', @@ -17,6 +18,12 @@ export const Verbosity = { */ export type VerbosityLike = keyof typeof Verbosity | boolean; +/** @public */ +export interface ExplainOptions { + /** Specifies the verbosity mode for the explain output. */ + explain?: VerbosityLike; +} + // Minimum server versions which support explain with specific operations const SUPPORTS_EXPLAIN_WITH_REMOVE = 3; const SUPPORTS_EXPLAIN_WITH_UPDATE = 3; @@ -37,18 +44,14 @@ export class Explain { } static fromOptions(options?: ExplainOptions): Explain | undefined { - if (options?.explain === undefined) { - return; - } - return new Explain(options.explain); - } + if (options?.explain === undefined) return; - static valid(options?: ExplainOptions): boolean { - if (options?.explain === undefined) { - return true; - } const explain = options.explain; - return typeof explain === 'boolean' || explain in Verbosity; + if (typeof explain === 'boolean' || explain in Verbosity) { + return new Explain(explain); + } + + throw new MongoError(`explain must be one of ${Object.keys(Verbosity)} or a boolean`); } /** Checks that the server supports explain on the given operation or command.*/ diff --git a/src/index.ts b/src/index.ts index 13466fb668..2eee916184 100644 --- a/src/index.ts +++ b/src/index.ts @@ -163,7 +163,7 @@ export type { export type { DbPrivate, DbOptions } from './db'; export type { AutoEncryptionOptions, AutoEncryptionLoggerLevels, AutoEncrypter } from './deps'; export type { AnyError, ErrorDescription } from './error'; -export type { Verbosity, VerbosityLike } from './explain'; +export type { ExplainOptions, Verbosity, VerbosityLike } from './explain'; export type { GridFSBucketReadStream, GridFSBucketReadStreamOptions, @@ -208,7 +208,6 @@ export type { DistinctOptions } from './operations/distinct'; export type { DropCollectionOptions, DropDatabaseOptions } from './operations/drop'; export type { EstimatedDocumentCountOptions } from './operations/estimated_document_count'; export type { EvalOptions } from './operations/eval'; -export type { ExplainOptions } from './operations/explainable_command'; export type { FindOptions } from './operations/find'; export type { Sort, SortDirection } from './sort'; export type { FindAndModifyOptions } from './operations/find_and_modify'; diff --git a/src/operations/aggregate.ts b/src/operations/aggregate.ts index fb779de121..e8ed3ef3bc 100644 --- a/src/operations/aggregate.ts +++ b/src/operations/aggregate.ts @@ -22,8 +22,6 @@ export interface AggregateOptions extends CommandOperationOptions { bypassDocumentValidation?: boolean; /** Return the query as cursor, on 2.6 \> it returns as a real cursor on pre 2.6 it returns as an emulated cursor. */ cursor?: Document; - /** Explain returns the aggregation execution plan (requires mongodb 2.6 \>) */ - explain?: boolean; /** specifies a cumulative time limit in milliseconds for processing operations on the cursor. MongoDB interrupts the operation at the earliest following interrupt point. */ maxTimeMS?: number; /** The maximum amount of time for the server to wait on new documents to satisfy a tailable cursor query. */ diff --git a/src/operations/command.ts b/src/operations/command.ts index 4e197348a6..6a885f3a91 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -10,11 +10,15 @@ import type { Server } from '../sdam/server'; import { BSONSerializeOptions, Document, resolveBSONOptions } from '../bson'; import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import type { ReadConcernLike } from './../read_concern'; +import { Explain, ExplainOptions } from '../explain'; const SUPPORTS_WRITE_CONCERN_AND_COLLATION = 5; /** @public */ -export interface CommandOperationOptions extends OperationOptions, WriteConcernOptions { +export interface CommandOperationOptions + extends OperationOptions, + WriteConcernOptions, + ExplainOptions { /** Return the full server response for the command */ fullResponse?: boolean; /** Specify a read concern and level for the collection. (only MongoDB 3.2 or higher supported) */ @@ -56,6 +60,7 @@ export abstract class CommandOperation< writeConcern?: WriteConcern; fullResponse?: boolean; logger?: Logger; + explain?: Explain; constructor(parent?: OperationParent, options?: T) { super(options); @@ -92,6 +97,19 @@ export abstract class CommandOperation< // Assign BSON serialize options to OperationBase, preferring options over parent options. this.bsonOptions = resolveBSONOptions(options, parent); + + if (this.hasAspect(Aspect.EXPLAINABLE)) { + this.explain = Explain.fromOptions(options); + } else if (options?.explain !== undefined) { + throw new MongoError(`explain is not supported on this command`); + } + } + + get canRetryWrite(): boolean { + if (this.hasAspect(Aspect.EXPLAINABLE)) { + return this.explain === undefined; + } + return true; } abstract execute(server: Server, callback: Callback): void; @@ -139,11 +157,18 @@ export abstract class CommandOperation< this.logger.debug(`executing command ${JSON.stringify(cmd)} against ${this.ns}`); } - // If a command is to be explained, we need to reformat the command after - // the other command properties are specified. + if (this.hasAspect(Aspect.EXPLAINABLE) && this.explain) { + if (!Explain.explainSupported(server, cmd)) { + callback(new MongoError(`server ${server.name} does not support explain on this command`)); + return; + } + + cmd = decorateWithExplain(cmd, this.explain); + } + server.command( this.ns.toString(), - cmd.explain ? decorateWithExplain(cmd, cmd.explain) : cmd, + cmd, { fullResult: !!this.fullResponse, ...this.options }, callback ); diff --git a/src/operations/delete.ts b/src/operations/delete.ts index a991ca73c3..381b6aa9de 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -7,11 +7,10 @@ import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { Connection } from '../cmap/connection'; -import { ExplainableCommand, ExplainOptions } from '../operations/explainable_command'; -import type { CommandOperationOptions } from './command'; +import { CommandOperation, CommandOperationOptions } from './command'; /** @public */ -export interface DeleteOptions extends CommandOperationOptions, ExplainOptions { +export interface DeleteOptions extends CommandOperationOptions { single?: boolean; hint?: Hint; } @@ -52,7 +51,7 @@ export class DeleteOperation extends OperationBase { } } -export class DeleteOneOperation extends ExplainableCommand { +export class DeleteOneOperation extends CommandOperation { collection: Collection; filter: Document; @@ -82,7 +81,7 @@ export class DeleteOneOperation extends ExplainableCommand { +export class DeleteManyOperation extends CommandOperation { collection: Collection; filter: Document; @@ -121,5 +120,5 @@ export class DeleteManyOperation extends ExplainableCommand { +export class DistinctOperation extends CommandOperation { collection: Collection; /** Field of the document to find distinct values for. */ key: string; @@ -75,4 +74,4 @@ export class DistinctOperation extends ExplainableCommand extends CommandOperation { - explain?: Explain; - - constructor(parent?: OperationParent, options?: T) { - super(parent, options); - - if (!Explain.valid(options)) { - throw new MongoError(`explain must be one of ${Object.keys(Verbosity)} or a boolean`); - } - - this.explain = Explain.fromOptions(options); - } - - get canRetryWrite(): boolean { - return this.explain === undefined; - } - - executeCommand(server: Server, cmd: Document, callback: Callback): void { - if (this.explain) { - if (!Explain.explainSupported(server, cmd)) { - callback(new MongoError(`server ${server.name} does not support explain on this command`)); - return; - } - - // For now, tag the command with the explain; after cmd is finalized in the super class, - // it will be refactored into the required shape using the explain. - cmd.explain = this.explain; - } - super.executeCommand(server, cmd, callback); - } -} diff --git a/src/operations/find.ts b/src/operations/find.ts index 9f0a39b3ad..2716b1c98d 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -22,8 +22,6 @@ export interface FindOptions extends QueryOptions, CommandOperationOptions { skip?: number; /** Tell the query to use specific indexes in the query. Object of indexes to use, `{'_id':1}` */ hint?: Hint; - /** Explain the query instead of returning the data. */ - explain?: boolean; /** Specify if the cursor can timeout. */ timeout?: boolean; /** Specify if the cursor is tailable. */ diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 7c9e1a5a51..dd5bb0faf3 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -13,11 +13,10 @@ import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import { Sort, formatSort } from '../sort'; -import { ExplainableCommand, ExplainOptions } from '../operations/explainable_command'; -import type { CommandOperationOptions } from './command'; +import { CommandOperation, CommandOperationOptions } from './command'; /** @public */ -export interface FindAndModifyOptions extends CommandOperationOptions, ExplainOptions { +export interface FindAndModifyOptions extends CommandOperationOptions { /** When false, returns the updated document rather than the original. The default is true. */ returnOriginal?: boolean; /** Upsert the document if it does not exist. */ @@ -42,7 +41,7 @@ export interface FindAndModifyOptions extends CommandOperationOptions, ExplainOp } /** @internal */ -export class FindAndModifyOperation extends ExplainableCommand { +export class FindAndModifyOperation extends CommandOperation { collection: Collection; query: Document; sort?: Sort; @@ -232,4 +231,8 @@ export class FindOneAndUpdateOperation extends FindAndModifyOperation { } } -defineAspects(FindAndModifyOperation, [Aspect.WRITE_OPERATION, Aspect.RETRYABLE]); +defineAspects(FindAndModifyOperation, [ + Aspect.WRITE_OPERATION, + Aspect.RETRYABLE, + Aspect.EXPLAINABLE +]); diff --git a/src/operations/map_reduce.ts b/src/operations/map_reduce.ts index cc6c3d03c8..611c4d250d 100644 --- a/src/operations/map_reduce.ts +++ b/src/operations/map_reduce.ts @@ -13,8 +13,8 @@ import type { Collection } from '../collection'; import type { Sort } from '../sort'; import { MongoError } from '../error'; import type { ObjectId } from '../bson'; -import { ExplainableCommand, ExplainOptions } from '../operations/explainable_command'; -import type { CommandOperationOptions } from './command'; +import { CommandOperation, CommandOperationOptions } from './command'; +import { Aspect, defineAspects } from './operation'; const exclusionList = [ 'explain', @@ -36,7 +36,7 @@ export type ReduceFunction = (key: string, values: Document[]) => Document; export type FinalizeFunction = (key: string, reducedValue: Document) => Document; /** @public */ -export interface MapReduceOptions extends CommandOperationOptions, ExplainOptions { +export interface MapReduceOptions extends CommandOperationOptions { /** Sets the output target for the map reduce job. */ out?: 'inline' | { inline: 1 } | { replace: string } | { merge: string } | { reduce: string }; /** Query filter object. */ @@ -69,10 +69,7 @@ interface MapReduceStats { * Run Map Reduce across a collection. Be aware that the inline option for out will return an array of results not a collection. * @internal */ -export class MapReduceOperation extends ExplainableCommand< - MapReduceOptions, - Document | Document[] -> { +export class MapReduceOperation extends CommandOperation { collection: Collection; /** The mapping function. */ map: MapFunction | string; @@ -231,3 +228,5 @@ function processScope(scope: Document | ObjectId) { return newScope; } + +defineAspects(MapReduceOperation, [Aspect.EXPLAINABLE]); diff --git a/src/operations/operation.ts b/src/operations/operation.ts index 3d5c862590..239b28d5fd 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -8,7 +8,8 @@ export const Aspect = { READ_OPERATION: Symbol('READ_OPERATION'), WRITE_OPERATION: Symbol('WRITE_OPERATION'), RETRYABLE: Symbol('RETRYABLE'), - NO_INHERIT_OPTIONS: Symbol('NO_INHERIT_OPTIONS') + NO_INHERIT_OPTIONS: Symbol('NO_INHERIT_OPTIONS'), + EXPLAINABLE: Symbol('EXPLAINABLE') } as const; /** @public */ diff --git a/src/operations/update.ts b/src/operations/update.ts index a89ee4f33c..c253c78b6f 100644 --- a/src/operations/update.ts +++ b/src/operations/update.ts @@ -5,11 +5,10 @@ import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { CollationOptions, WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { ObjectId, Document } from '../bson'; -import { ExplainableCommand, ExplainOptions } from '../operations/explainable_command'; -import type { CommandOperationOptions } from './command'; +import { CommandOperation, CommandOperationOptions } from './command'; /** @public */ -export interface UpdateOptions extends CommandOperationOptions, ExplainOptions { +export interface UpdateOptions extends CommandOperationOptions { /** A set of filters specifying to which array elements an update should apply */ arrayFilters?: Document[]; /** If true, allows the write to opt-out of document level validation */ @@ -66,7 +65,7 @@ export class UpdateOperation extends OperationBase { } /** @internal */ -export class UpdateOneOperation extends ExplainableCommand { +export class UpdateOneOperation extends CommandOperation { collection: Collection; filter: Document; update: Document; @@ -115,7 +114,7 @@ export class UpdateOneOperation extends ExplainableCommand { +export class UpdateManyOperation extends CommandOperation { collection: Collection; filter: Document; update: Document; @@ -160,5 +159,5 @@ export class UpdateManyOperation extends ExplainableCommand Date: Thu, 12 Nov 2020 09:23:04 -0500 Subject: [PATCH 19/23] use expanded explain types for clarity --- src/explain.ts | 20 +++++++++++--------- src/index.ts | 6 +++++- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/explain.ts b/src/explain.ts index 7f1096b9a9..0530290276 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -4,7 +4,7 @@ import type { Server } from './sdam/server'; import { maxWireVersion } from './utils'; /** @public */ -export const Verbosity = { +export const ExplainVerbosity = { queryPlanner: 'queryPlanner', queryPlannerExtended: 'queryPlannerExtended', executionStats: 'executionStats', @@ -16,12 +16,12 @@ export const Verbosity = { * "allPlansExecution" and false as "queryPlanner". * @public */ -export type VerbosityLike = keyof typeof Verbosity | boolean; +export type ExplainVerbosityLike = keyof typeof ExplainVerbosity | boolean; /** @public */ export interface ExplainOptions { /** Specifies the verbosity mode for the explain output. */ - explain?: VerbosityLike; + explain?: ExplainVerbosityLike; } // Minimum server versions which support explain with specific operations @@ -33,13 +33,15 @@ const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 9; /** @internal */ export class Explain { - verbosity: keyof typeof Verbosity; + verbosity: keyof typeof ExplainVerbosity; - constructor(verbosity: VerbosityLike) { + constructor(verbosity: ExplainVerbosityLike) { if (typeof verbosity === 'boolean') { - this.verbosity = verbosity ? Verbosity.allPlansExecution : Verbosity.queryPlanner; + this.verbosity = verbosity + ? ExplainVerbosity.allPlansExecution + : ExplainVerbosity.queryPlanner; } else { - this.verbosity = Verbosity[verbosity]; + this.verbosity = ExplainVerbosity[verbosity]; } } @@ -47,11 +49,11 @@ export class Explain { if (options?.explain === undefined) return; const explain = options.explain; - if (typeof explain === 'boolean' || explain in Verbosity) { + if (typeof explain === 'boolean' || explain in ExplainVerbosity) { return new Explain(explain); } - throw new MongoError(`explain must be one of ${Object.keys(Verbosity)} or a boolean`); + throw new MongoError(`explain must be one of ${Object.keys(ExplainVerbosity)} or a boolean`); } /** Checks that the server supports explain on the given operation or command.*/ diff --git a/src/index.ts b/src/index.ts index 2eee916184..7d1caef966 100644 --- a/src/index.ts +++ b/src/index.ts @@ -163,7 +163,11 @@ export type { export type { DbPrivate, DbOptions } from './db'; export type { AutoEncryptionOptions, AutoEncryptionLoggerLevels, AutoEncrypter } from './deps'; export type { AnyError, ErrorDescription } from './error'; -export type { ExplainOptions, Verbosity, VerbosityLike } from './explain'; +export type { + ExplainOptions, + ExplainVerbosity as Verbosity, + ExplainVerbosityLike as VerbosityLike +} from './explain'; export type { GridFSBucketReadStream, GridFSBucketReadStreamOptions, From ddc982657b5a3c1e4ae7fe13da4fef45bae4c39e Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Thu, 12 Nov 2020 09:37:11 -0500 Subject: [PATCH 20/23] change test names and ordering --- test/functional/explain.test.js | 143 ++++++++++++++++++++------------ 1 file changed, 88 insertions(+), 55 deletions(-) diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index 68e169e0d8..ae5810a6f0 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -9,7 +9,7 @@ describe('Explain', function () { return setupDatabase(this.configuration); }); - it('shouldHonorBooleanExplainWithDeleteOne', { + it('should honor boolean explain with delete one', { metadata: { requires: { mongodb: '>=3.0' @@ -33,7 +33,7 @@ describe('Explain', function () { }) }); - it('shouldHonorBooleanExplainWithDeleteMany', { + it('should honor boolean explain with delete many', { metadata: { requires: { mongodb: '>=3.0' @@ -57,7 +57,7 @@ describe('Explain', function () { }) }); - it('shouldHonorBooleanExplainWithUpdateOne', { + it('should honor boolean explain with update one', { metadata: { requires: { mongodb: '>=3.0' @@ -86,7 +86,7 @@ describe('Explain', function () { }) }); - it('shouldHonorBooleanExplainWithUpdateMany', { + it('should honor boolean explain with update many', { metadata: { requires: { mongodb: '>=3.0' @@ -115,7 +115,7 @@ describe('Explain', function () { }) }); - it('shouldHonorBooleanExplainWithRemoveOne', { + it('should honor boolean explain with remove one', { metadata: { requires: { mongodb: '>=3.0' @@ -139,7 +139,7 @@ describe('Explain', function () { }) }); - it('shouldHonorBooleanExplainWithRemoveMany', { + it('should honor boolean explain with remove many', { metadata: { requires: { mongodb: '>=3.0' @@ -163,227 +163,260 @@ describe('Explain', function () { }) }); - it('shouldUseAllPlansExecutionAsTrueExplainVerbosity', { + it('should honor boolean explain with distinct', { metadata: { requires: { - mongodb: '>=3.0' + mongodb: '>=3.2' } }, test: withClient(function (client, done) { - var db = client.db('shouldUseAllPlansExecutionAsTrueExplainVerbosity'); + var db = client.db('shouldHonorBooleanExplainWithDistinct'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - // Verify explanation result contains properties of allPlansExecution output - collection.deleteOne({ a: 1 }, { explain: true }, (err, explanation) => { + collection.distinct('a', {}, { explain: true }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; - expect(explanation).nested.property('executionStats.allPlansExecution').to.exist; done(); }); }); }) }); - it('shouldUseQueryPlannerAsFalseExplainVerbosity', { + it('should honor boolean explain with findOneAndModify', { metadata: { requires: { - mongodb: '>=3.0' + mongodb: '>=3.2' } }, test: withClient(function (client, done) { - var db = client.db('shouldUseQueryPlannerAsFalseExplainVerbosity'); + var db = client.db('shouldHonorBooleanExplainWithFindOneAndModify'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - // Verify explanation result contains properties of queryPlanner output - collection.deleteOne({ a: 1 }, { explain: false }, (err, explanation) => { + collection.findOneAndDelete({ a: 1 }, { explain: true }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; - expect(explanation).to.not.have.property('executionStats'); done(); }); }); }) }); - it('shouldHonorQueryPlannerStringExplain', { + it('should honor boolean explain with mapReduce', { + metadata: { + requires: { + mongodb: '>=4.4' + } + }, + test: withClient(function (client, done) { + var db = client.db('shouldHonorBooleanExplainWithMapReduce'); + var collection = db.collection('test'); + + collection.insertMany([{ user_id: 1 }, { user_id: 2 }], (err, res) => { + expect(err).to.not.exist; + expect(res).to.exist; + + var map = 'function() { emit(this.user_id, 1); }'; + var reduce = 'function(k,vals) { return 1; }'; + + collection.mapReduce( + map, + reduce, + { out: { replace: 'tempCollection' }, explain: true }, + (err, explanation) => { + expect(err).to.not.exist; + expect(explanation).to.exist; + expect(explanation).property('stages').to.exist; + done(); + } + ); + }); + }) + }); + + it('should use allPlansExecution as true explain verbosity', { metadata: { requires: { mongodb: '>=3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorQueryPlannerStringExplain'); + var db = client.db('shouldUseAllPlansExecutionAsTrueExplainVerbosity'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - // Verify explanation result contains properties of queryPlanner output - collection.deleteOne({ a: 1 }, { explain: 'queryPlanner' }, (err, explanation) => { + // Verify explanation result contains properties of allPlansExecution output + collection.deleteOne({ a: 1 }, { explain: true }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; - expect(explanation).to.not.have.property('executionStats'); + expect(explanation).nested.property('executionStats.allPlansExecution').to.exist; done(); }); }); }) }); - it('shouldHonorExecutionStatsStringExplain', { + it('should use queryPlanner as false explain verbosity', { metadata: { requires: { - mongodb: '>=3.2' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorExecutionStatsStringExplain'); + var db = client.db('shouldUseQueryPlannerAsFalseExplainVerbosity'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - // Verify explanation result contains properties of executionStats output - collection.findOneAndDelete({ a: 1 }, { explain: 'executionStats' }, (err, explanation) => { + // Verify explanation result contains properties of queryPlanner output + collection.deleteOne({ a: 1 }, { explain: false }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; - expect(explanation).property('executionStats').to.exist; - expect(explanation.executionStats).to.not.have.property('allPlansExecution'); + expect(explanation).to.not.have.property('executionStats'); done(); }); }); }) }); - it('shouldHonorAllPlansStringExplain', { + it('should honor queryPlanner string explain', { metadata: { requires: { - mongodb: '>=3.2' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorAllPlansStringExplain'); + var db = client.db('shouldHonorQueryPlannerStringExplain'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - // Verify explanation result contains properties of allPlansExecution output - collection.distinct('a', {}, { explain: 'allPlansExecution' }, (err, explanation) => { + // Verify explanation result contains properties of queryPlanner output + collection.deleteOne({ a: 1 }, { explain: 'queryPlanner' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; - expect(explanation).nested.property('executionStats.allPlansExecution').to.exist; + expect(explanation).to.not.have.property('executionStats'); done(); }); }); }) }); - it('shouldHonorBooleanExplainWithDistinct', { + it('should honor executionStats string explain', { metadata: { requires: { - mongodb: '>=3.2' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorBooleanExplainWithDistinct'); + var db = client.db('shouldHonorExecutionStatsStringExplain'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - collection.distinct('a', {}, { explain: true }, (err, explanation) => { + // Verify explanation result contains properties of executionStats output + collection.deleteMany({ a: 1 }, { explain: 'executionStats' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; + expect(explanation).property('executionStats').to.exist; + expect(explanation.executionStats).to.not.have.property('allPlansExecution'); done(); }); }); }) }); - it('shouldHonorBooleanExplainWithFindOneAndDelete', { + it('should honor allPlansExecution string explain', { metadata: { requires: { - mongodb: '>=3.2' + mongodb: '>=3.0' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorBooleanExplainWithFindOneAndDelete'); + var db = client.db('shouldHonorAllPlansStringExplain'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - collection.findOneAndDelete({ a: 1 }, { explain: true }, (err, explanation) => { + // Verify explanation result contains properties of allPlansExecution output + collection.removeOne({ a: 1 }, { explain: 'allPlansExecution' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; + expect(explanation).nested.property('executionStats.allPlansExecution').to.exist; done(); }); }); }) }); - it('shouldHonorBooleanExplainWithFindOneAndReplace', { + it('should honor string explain with distinct', { metadata: { requires: { mongodb: '>=3.2' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorBooleanExplainWithFindOneAndReplace'); + var db = client.db('shouldHonorStringExplainWithDistinct'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - collection.findOneAndReplace({ a: 1 }, { a: 2 }, { explain: true }, (err, explanation) => { + collection.distinct('a', {}, { explain: 'executionStats' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; expect(explanation).property('queryPlanner').to.exist; + expect(explanation).property('executionStats').to.exist; done(); }); }); }) }); - it('shouldHonorBooleanExplainWithFindOneAndUpdate', { + it('should honor string explain with findOneAndModify', { metadata: { requires: { mongodb: '>=3.2' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorBooleanExplainWithFindOneAndUpdate'); + var db = client.db('shouldHonorStringExplainWithFindOneAndModify'); var collection = db.collection('test'); collection.insertOne({ a: 1 }, (err, res) => { expect(err).to.not.exist; expect(res).to.exist; - collection.findOneAndUpdate( + collection.findOneAndReplace( { a: 1 }, - { $inc: { a: 2 } }, - { explain: true }, + { a: 2 }, + { explain: 'queryPlanner' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; @@ -395,14 +428,14 @@ describe('Explain', function () { }) }); - it('shouldHonorBooleanExplainWithMapReduce', { + it('should honor string explain with mapReduce', { metadata: { requires: { mongodb: '>=4.4' } }, test: withClient(function (client, done) { - var db = client.db('shouldHonorBooleanExplainWithMapReduce'); + var db = client.db('shouldHonorStringExplainWithMapReduce'); var collection = db.collection('test'); collection.insertMany([{ user_id: 1 }, { user_id: 2 }], (err, res) => { @@ -415,7 +448,7 @@ describe('Explain', function () { collection.mapReduce( map, reduce, - { out: { replace: 'tempCollection' }, explain: true }, + { out: { replace: 'tempCollection' }, explain: 'executionStats' }, (err, explanation) => { expect(err).to.not.exist; expect(explanation).to.exist; From 6a17656e020eedb5aef81f460934364bd31aa414 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Thu, 12 Nov 2020 09:49:01 -0500 Subject: [PATCH 21/23] clean up --- src/operations/command.ts | 2 +- src/operations/delete.ts | 2 +- src/operations/distinct.ts | 2 +- src/operations/find_and_modify.ts | 2 +- src/operations/map_reduce.ts | 2 +- src/operations/update.ts | 2 +- test/functional/explain.test.js | 1 - 7 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/operations/command.ts b/src/operations/command.ts index 6a885f3a91..1bb639fd16 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -58,9 +58,9 @@ export abstract class CommandOperation< readPreference: ReadPreference; readConcern?: ReadConcern; writeConcern?: WriteConcern; + explain?: Explain; fullResponse?: boolean; logger?: Logger; - explain?: Explain; constructor(parent?: OperationParent, options?: T) { super(options); diff --git a/src/operations/delete.ts b/src/operations/delete.ts index 381b6aa9de..27e71cfc48 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -1,5 +1,6 @@ import { defineAspects, Aspect, OperationBase, Hint } from './operation'; import { removeDocuments } from './common_functions'; +import { CommandOperation, CommandOperationOptions } from './command'; import { isObject } from 'util'; import type { Callback, MongoDBNamespace } from '../utils'; import type { Document } from '../bson'; @@ -7,7 +8,6 @@ import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { Connection } from '../cmap/connection'; -import { CommandOperation, CommandOperationOptions } from './command'; /** @public */ export interface DeleteOptions extends CommandOperationOptions { diff --git a/src/operations/distinct.ts b/src/operations/distinct.ts index 69dcd8fc57..15c5d07a0a 100644 --- a/src/operations/distinct.ts +++ b/src/operations/distinct.ts @@ -1,9 +1,9 @@ import { Aspect, defineAspects } from './operation'; +import { CommandOperation, CommandOperationOptions } from './command'; import { decorateWithCollation, decorateWithReadConcern, Callback } from '../utils'; import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; -import { CommandOperation, CommandOperationOptions } from './command'; /** @public */ export type DistinctOptions = CommandOperationOptions; diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index dd5bb0faf3..85e7824807 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -8,12 +8,12 @@ import { Callback } from '../utils'; import { MongoError } from '../error'; +import { CommandOperation, CommandOperationOptions } from './command'; import { defineAspects, Aspect } from './operation'; import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import { Sort, formatSort } from '../sort'; -import { CommandOperation, CommandOperationOptions } from './command'; /** @public */ export interface FindAndModifyOptions extends CommandOperationOptions { diff --git a/src/operations/map_reduce.ts b/src/operations/map_reduce.ts index 611c4d250d..11ce678a38 100644 --- a/src/operations/map_reduce.ts +++ b/src/operations/map_reduce.ts @@ -8,12 +8,12 @@ import { Callback } from '../utils'; import { ReadPreference, ReadPreferenceMode } from '../read_preference'; +import { CommandOperation, CommandOperationOptions } from './command'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { Sort } from '../sort'; import { MongoError } from '../error'; import type { ObjectId } from '../bson'; -import { CommandOperation, CommandOperationOptions } from './command'; import { Aspect, defineAspects } from './operation'; const exclusionList = [ diff --git a/src/operations/update.ts b/src/operations/update.ts index c253c78b6f..35c8664e5c 100644 --- a/src/operations/update.ts +++ b/src/operations/update.ts @@ -1,11 +1,11 @@ import { defineAspects, Aspect, OperationBase } from './operation'; import { updateDocuments } from './common_functions'; import { hasAtomicOperators, MongoDBNamespace, Callback } from '../utils'; +import { CommandOperation, CommandOperationOptions } from './command'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; import type { CollationOptions, WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { ObjectId, Document } from '../bson'; -import { CommandOperation, CommandOperationOptions } from './command'; /** @public */ export interface UpdateOptions extends CommandOperationOptions { diff --git a/test/functional/explain.test.js b/test/functional/explain.test.js index ae5810a6f0..152cbd15ee 100644 --- a/test/functional/explain.test.js +++ b/test/functional/explain.test.js @@ -2,7 +2,6 @@ const { setupDatabase, withClient } = require('./shared'); const chai = require('chai'); const expect = chai.expect; -chai.use(require('chai-subset')); describe('Explain', function () { before(function () { From 001ec2e88084b348282780c426d40aff7c97f13a Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Thu, 12 Nov 2020 11:55:33 -0500 Subject: [PATCH 22/23] fix explain export in index --- src/index.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/index.ts b/src/index.ts index 7d1caef966..8fbedd3f49 100644 --- a/src/index.ts +++ b/src/index.ts @@ -163,11 +163,7 @@ export type { export type { DbPrivate, DbOptions } from './db'; export type { AutoEncryptionOptions, AutoEncryptionLoggerLevels, AutoEncrypter } from './deps'; export type { AnyError, ErrorDescription } from './error'; -export type { - ExplainOptions, - ExplainVerbosity as Verbosity, - ExplainVerbosityLike as VerbosityLike -} from './explain'; +export type { ExplainOptions, ExplainVerbosity, ExplainVerbosityLike } from './explain'; export type { GridFSBucketReadStream, GridFSBucketReadStreamOptions, From 0b37761f1404d0629da42b88c71b68af9e4b4094 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Fri, 13 Nov 2020 15:55:47 -0500 Subject: [PATCH 23/23] check explain supported in individual op classes --- src/cmap/wire_protocol/write_command.ts | 8 +++---- src/explain.ts | 28 ------------------------- src/operations/command.ts | 5 ----- src/operations/common_functions.ts | 15 ++++++++++++- src/operations/distinct.ts | 8 ++++++- src/operations/find_and_modify.ts | 5 +++++ src/operations/map_reduce.ts | 8 ++++++- src/sdam/server.ts | 6 ------ src/utils.ts | 2 +- 9 files changed, 37 insertions(+), 48 deletions(-) diff --git a/src/cmap/wire_protocol/write_command.ts b/src/cmap/wire_protocol/write_command.ts index 2f135d0761..7ec75b5d50 100644 --- a/src/cmap/wire_protocol/write_command.ts +++ b/src/cmap/wire_protocol/write_command.ts @@ -67,11 +67,9 @@ export function writeCommand( // If a command is to be explained, we need to reformat the command after // the other command properties are specified. - if (options.explain !== undefined) { - const explain = Explain.fromOptions(options); - if (explain) { - writeCommand = decorateWithExplain(writeCommand, explain); - } + const explain = Explain.fromOptions(options); + if (explain) { + writeCommand = decorateWithExplain(writeCommand, explain); } const commandOptions = Object.assign( diff --git a/src/explain.ts b/src/explain.ts index 0530290276..790efbd600 100644 --- a/src/explain.ts +++ b/src/explain.ts @@ -1,7 +1,4 @@ -import type { Document } from '.'; import { MongoError } from './error'; -import type { Server } from './sdam/server'; -import { maxWireVersion } from './utils'; /** @public */ export const ExplainVerbosity = { @@ -24,13 +21,6 @@ export interface ExplainOptions { explain?: ExplainVerbosityLike; } -// Minimum server versions which support explain with specific operations -const SUPPORTS_EXPLAIN_WITH_REMOVE = 3; -const SUPPORTS_EXPLAIN_WITH_UPDATE = 3; -const SUPPORTS_EXPLAIN_WITH_DISTINCT = 4; -const SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY = 4; -const SUPPORTS_EXPLAIN_WITH_MAP_REDUCE = 9; - /** @internal */ export class Explain { verbosity: keyof typeof ExplainVerbosity; @@ -55,22 +45,4 @@ export class Explain { throw new MongoError(`explain must be one of ${Object.keys(ExplainVerbosity)} or a boolean`); } - - /** Checks that the server supports explain on the given operation or command.*/ - static explainSupported(server: Server, op: string | Document): boolean { - const wireVersion = maxWireVersion(server); - if (op === 'remove' || (typeof op === 'object' && op.remove)) { - return wireVersion >= SUPPORTS_EXPLAIN_WITH_REMOVE; - } else if (op === 'update' || (typeof op === 'object' && op.update)) { - return wireVersion >= SUPPORTS_EXPLAIN_WITH_UPDATE; - } else if (op === 'distinct' || (typeof op === 'object' && op.distinct)) { - return wireVersion >= SUPPORTS_EXPLAIN_WITH_DISTINCT; - } else if (op === 'findAndModify' || (typeof op === 'object' && op.findAndModify)) { - return wireVersion >= SUPPORTS_EXPLAIN_WITH_FIND_AND_MODIFY; - } else if (op === 'mapReduce' || (typeof op === 'object' && op.mapReduce)) { - return wireVersion >= SUPPORTS_EXPLAIN_WITH_MAP_REDUCE; - } - - return false; - } } diff --git a/src/operations/command.ts b/src/operations/command.ts index ea09db9659..e9a4020224 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -156,11 +156,6 @@ export abstract class CommandOperation< } if (this.hasAspect(Aspect.EXPLAINABLE) && this.explain) { - if (!Explain.explainSupported(server, cmd)) { - callback(new MongoError(`server ${server.name} does not support explain on this command`)); - return; - } - cmd = decorateWithExplain(cmd, this.explain); } diff --git a/src/operations/common_functions.ts b/src/operations/common_functions.ts index 69b1befc0d..2fbb102fd7 100644 --- a/src/operations/common_functions.ts +++ b/src/operations/common_functions.ts @@ -4,7 +4,8 @@ import { applyWriteConcern, decorateWithCollation, Callback, - getTopology + getTopology, + maxWireVersion } from '../utils'; import type { Document } from '../bson'; import type { Db } from '../db'; @@ -162,6 +163,12 @@ export function removeDocuments( return callback ? callback(err, null) : undefined; } + if (options.explain !== undefined && maxWireVersion(server) < 3) { + return callback + ? callback(new MongoError(`server ${server.name} does not support explain on remove`)) + : undefined; + } + // Execute the remove server.remove( coll.s.namespace.toString(), @@ -254,6 +261,12 @@ export function updateDocuments( return callback(err, null); } + if (options.explain !== undefined && maxWireVersion(server) < 3) { + return callback + ? callback(new MongoError(`server ${server.name} does not support explain on update`)) + : undefined; + } + // Update options server.update( coll.s.namespace.toString(), diff --git a/src/operations/distinct.ts b/src/operations/distinct.ts index 15c5d07a0a..93d996481a 100644 --- a/src/operations/distinct.ts +++ b/src/operations/distinct.ts @@ -1,9 +1,10 @@ import { Aspect, defineAspects } from './operation'; import { CommandOperation, CommandOperationOptions } from './command'; -import { decorateWithCollation, decorateWithReadConcern, Callback } from '../utils'; +import { decorateWithCollation, decorateWithReadConcern, Callback, maxWireVersion } from '../utils'; import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; +import { MongoError } from '../error'; /** @public */ export type DistinctOptions = CommandOperationOptions; @@ -63,6 +64,11 @@ export class DistinctOperation extends CommandOperation { if (err) { callback(err); diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 85e7824807..1b3715a99e 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -143,6 +143,11 @@ export class FindAndModifyOperation extends CommandOperation { if (err) return callback(err); diff --git a/src/operations/map_reduce.ts b/src/operations/map_reduce.ts index 03c4078808..c6d36cfc8f 100644 --- a/src/operations/map_reduce.ts +++ b/src/operations/map_reduce.ts @@ -5,7 +5,8 @@ import { decorateWithCollation, decorateWithReadConcern, isObject, - Callback + Callback, + maxWireVersion } from '../utils'; import { ReadPreference, ReadPreferenceMode } from '../read_preference'; import { CommandOperation, CommandOperationOptions } from './command'; @@ -158,6 +159,11 @@ export class MapReduceOperation extends CommandOperation { if (err) return callback(err); diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 79bfd0bd5d..5e63ca351b 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -45,7 +45,6 @@ import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command'; import type { Document, Long } from '../bson'; import type { AutoEncrypter } from '../deps'; import type { FindOptions } from '../operations/find'; -import { Explain } from '../explain'; // Used for filtering out fields for logging const DEBUG_FIELDS = [ @@ -458,11 +457,6 @@ function executeWriteOperation( } } - if (options.explain !== undefined && !Explain.explainSupported(server, op)) { - callback(new MongoError(`server ${server.name} does not support explain on ${op}`)); - return; - } - server.s.pool.withConnection((err, conn, cb) => { if (err || !conn) { markServerUnknown(server, err); diff --git a/src/utils.ts b/src/utils.ts index 83f0d0935a..0c146835ae 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -447,7 +447,7 @@ export function decorateWithReadConcern( * Applies an explain to a given command. * @internal * - * @param command - the command on which to apply the read concern + * @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): Document {