From 52cd649caf2e64aef6d3984c5f2d24af03db4c51 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 21 Jul 2023 15:27:35 -0400 Subject: [PATCH] fix(NODE-4936)!: remove unsupported options from db.command and admin.command (#3775) --- src/admin.ts | 10 ++- src/db.ts | 9 +- src/mongo_client.ts | 9 +- src/operations/operation.ts | 1 - src/operations/rename.ts | 59 ++++++------- src/operations/run_command.ts | 86 ++++++++----------- src/sessions.ts | 6 +- src/utils.ts | 11 +-- .../run-command/run_command.test.ts | 22 ++--- 9 files changed, 95 insertions(+), 118 deletions(-) diff --git a/src/admin.ts b/src/admin.ts index 61d67dbc50..0ec2e47763 100644 --- a/src/admin.ts +++ b/src/admin.ts @@ -1,4 +1,4 @@ -import type { Document } from './bson'; +import { type Document, resolveBSONOptions } from './bson'; import type { Db } from './db'; import { AddUserOperation, type AddUserOptions } from './operations/add_user'; import type { CommandOperationOptions } from './operations/command'; @@ -9,7 +9,7 @@ import { type ListDatabasesResult } from './operations/list_databases'; import { RemoveUserOperation, type RemoveUserOptions } from './operations/remove_user'; -import { RunCommandOperation, type RunCommandOptions } from './operations/run_command'; +import { RunAdminCommandOperation, type RunCommandOptions } from './operations/run_command'; import { ValidateCollectionOperation, type ValidateCollectionOptions @@ -76,7 +76,11 @@ export class Admin { async command(command: Document, options?: RunCommandOptions): Promise { return executeOperation( this.s.db.client, - new RunCommandOperation(this.s.db, command, { dbName: 'admin', ...options }) + new RunAdminCommandOperation(command, { + ...resolveBSONOptions(options), + session: options?.session, + readPreference: options?.readPreference + }) ); } diff --git a/src/db.ts b/src/db.ts index 4b3c2d22f5..08b9195444 100644 --- a/src/db.ts +++ b/src/db.ts @@ -259,7 +259,14 @@ export class Db { */ async command(command: Document, options?: RunCommandOptions): Promise { // Intentionally, we do not inherit options from parent for this operation. - return executeOperation(this.client, new RunCommandOperation(this, command, options)); + return executeOperation( + this.client, + new RunCommandOperation(this, command, { + ...resolveBSONOptions(options), + session: options?.session, + readPreference: options?.readPreference + }) + ); } /** diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 0c071086c1..0de3e8e079 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -23,6 +23,8 @@ import type { Encrypter } from './encrypter'; import { MongoInvalidArgumentError } from './error'; import { MongoLogger, type MongoLoggerOptions } from './mongo_logger'; import { TypedEventEmitter } from './mongo_types'; +import { executeOperation } from './operations/execute_operation'; +import { RunAdminCommandOperation } from './operations/run_command'; import type { ReadConcern, ReadConcernLevel, ReadConcernLike } from './read_concern'; import { ReadPreference, type ReadPreferenceMode } from './read_preference'; import type { TagSet } from './sdam/server_description'; @@ -521,12 +523,13 @@ export class MongoClient extends TypedEventEmitter { if (servers.length !== 0) { const endSessions = Array.from(this.s.sessionPool.sessions, ({ id }) => id); if (endSessions.length !== 0) { - await this.db('admin') - .command( + await executeOperation( + this, + new RunAdminCommandOperation( { endSessions }, { readPreference: ReadPreference.primaryPreferred, noResponse: true } ) - .catch(() => null); // outcome does not matter + ).catch(() => null); // outcome does not matter; } } diff --git a/src/operations/operation.ts b/src/operations/operation.ts index 43d84c2cdc..7a86f392a1 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -49,7 +49,6 @@ const kSession = Symbol('session'); */ export abstract class AbstractOperation { ns!: MongoDBNamespace; - cmd!: Document; readPreference: ReadPreference; server!: Server; bypassPinningCheck: boolean; diff --git a/src/operations/rename.ts b/src/operations/rename.ts index bb51398282..d0de350e61 100644 --- a/src/operations/rename.ts +++ b/src/operations/rename.ts @@ -1,12 +1,10 @@ import type { Document } from '../bson'; import { Collection } from '../collection'; -import { MongoServerError } from '../error'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; -import { checkCollectionName } from '../utils'; -import type { CommandOperationOptions } from './command'; +import { checkCollectionName, MongoDBNamespace } from '../utils'; +import { CommandOperation, type CommandOperationOptions } from './command'; import { Aspect, defineAspects } from './operation'; -import { RunAdminCommandOperation } from './run_command'; /** @public */ export interface RenameOptions extends CommandOperationOptions { @@ -17,39 +15,36 @@ export interface RenameOptions extends CommandOperationOptions { } /** @internal */ -export class RenameOperation extends RunAdminCommandOperation { - override options: RenameOptions; - collection: Collection; - newName: string; - - constructor(collection: Collection, newName: string, options: RenameOptions) { - // Check the collection name +export class RenameOperation extends CommandOperation { + constructor( + public collection: Collection, + public newName: string, + public override options: RenameOptions + ) { checkCollectionName(newName); - - // Build the command - const renameCollection = collection.namespace; - const toCollection = collection.s.namespace.withCollection(newName).toString(); - const dropTarget = typeof options.dropTarget === 'boolean' ? options.dropTarget : false; - const cmd = { renameCollection: renameCollection, to: toCollection, dropTarget: dropTarget }; - - super(collection, cmd, options); - this.options = options; - this.collection = collection; - this.newName = newName; + super(collection, options); + this.ns = new MongoDBNamespace('admin', '$cmd'); } override async execute(server: Server, session: ClientSession | undefined): Promise { - const coll = this.collection; - - const doc = await super.execute(server, session); - // We have an error - if (doc?.errmsg) { - throw new MongoServerError(doc); - } - - const newColl: Collection = new Collection(coll.s.db, this.newName, coll.s.options); + // Build the command + const renameCollection = this.collection.namespace; + const toCollection = this.collection.s.namespace.withCollection(this.newName).toString(); + const dropTarget = + typeof this.options.dropTarget === 'boolean' ? this.options.dropTarget : false; + + const command = { + renameCollection: renameCollection, + to: toCollection, + dropTarget: dropTarget + }; + + await super.executeCommand(server, session, command); + return new Collection(this.collection.s.db, this.newName, this.collection.s.options); + } - return newColl; + protected executeCallback(_server: any, _session: any, _callback: any): void { + throw new Error('Method not implemented.'); } } diff --git a/src/operations/run_command.ts b/src/operations/run_command.ts index 352965e836..a0e306e6c6 100644 --- a/src/operations/run_command.ts +++ b/src/operations/run_command.ts @@ -1,9 +1,11 @@ import type { BSONSerializeOptions, Document } from '../bson'; +import { type Db } from '../db'; +import { type TODO_NODE_3286 } from '../mongo_types'; import type { ReadPreferenceLike } from '../read_preference'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; -import { type Callback, MongoDBNamespace } from '../utils'; -import { CommandCallbackOperation, type OperationParent } from './command'; +import { MongoDBNamespace } from '../utils'; +import { AbstractOperation } from './operation'; /** @public */ export type RunCommandOptions = { @@ -11,63 +13,43 @@ export type RunCommandOptions = { session?: ClientSession; /** The read preference */ readPreference?: ReadPreferenceLike; - - // The following options were "accidentally" supported - // Since the options are generally supported through inheritance - - /** @deprecated This is an internal option that has undefined behavior for this API */ - willRetryWrite?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - omitReadPreference?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - writeConcern?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - explain?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - readConcern?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - collation?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - maxTimeMS?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - comment?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - retryWrites?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - dbName?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - authdb?: any; - /** @deprecated This is an internal option that has undefined behavior for this API */ - noResponse?: any; - - /** @internal Used for transaction commands */ - bypassPinningCheck?: boolean; } & BSONSerializeOptions; /** @internal */ -export class RunCommandOperation extends CommandCallbackOperation { - override options: RunCommandOptions; - command: Document; - - constructor(parent: OperationParent | undefined, command: Document, options?: RunCommandOptions) { - super(parent, options); - this.options = options ?? {}; - this.command = command; +export class RunCommandOperation extends AbstractOperation { + constructor(parent: Db, public command: Document, public override options: RunCommandOptions) { + super(options); + this.ns = parent.s.namespace.withCollection('$cmd'); } - override executeCallback( - server: Server, - session: ClientSession | undefined, - callback: Callback - ): void { - const command = this.command; - this.executeCommandCallback(server, session, command, callback); + override async execute(server: Server, session: ClientSession | undefined): Promise { + this.server = server; + return server.commandAsync(this.ns, this.command, { + ...this.options, + readPreference: this.readPreference, + session + }) as TODO_NODE_3286; } } -export class RunAdminCommandOperation extends RunCommandOperation { - constructor(parent: OperationParent | undefined, command: Document, options?: RunCommandOptions) { - super(parent, command, options); - this.ns = new MongoDBNamespace('admin'); +export class RunAdminCommandOperation extends AbstractOperation { + constructor( + public command: Document, + public override options: RunCommandOptions & { + noResponse?: boolean; + bypassPinningCheck?: boolean; + } + ) { + super(options); + this.ns = new MongoDBNamespace('admin', '$cmd'); + } + + override async execute(server: Server, session: ClientSession | undefined): Promise { + this.server = server; + return server.commandAsync(this.ns, this.command, { + ...this.options, + readPreference: this.readPreference, + session + }) as TODO_NODE_3286; } } diff --git a/src/sessions.ts b/src/sessions.ts index 544ab53936..9b4d106352 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -754,7 +754,7 @@ function endTransaction( // send the command executeOperation( session.client, - new RunAdminCommandOperation(undefined, command, { + new RunAdminCommandOperation(command, { session, readPreference: ReadPreference.primary, bypassPinningCheck: true @@ -778,7 +778,7 @@ function endTransaction( return executeOperation( session.client, - new RunAdminCommandOperation(undefined, command, { + new RunAdminCommandOperation(command, { session, readPreference: ReadPreference.primary, bypassPinningCheck: true @@ -989,7 +989,7 @@ export function applySession( if ( session.supports.causalConsistency && session.operationTime && - commandSupportsReadConcern(command, options) + commandSupportsReadConcern(command) ) { command.readConcern = command.readConcern || {}; Object.assign(command.readConcern, { afterClusterTime: session.operationTime }); diff --git a/src/utils.ts b/src/utils.ts index 6dd512d1cf..aa9afc51a2 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1155,20 +1155,11 @@ export function shuffle(sequence: Iterable, limit = 0): Array { // TODO(NODE-4936): read concern eligibility for commands should be codified in command construction // @see https://github.com/mongodb/specifications/blob/master/source/read-write-concern/read-write-concern.rst#read-concern -export function commandSupportsReadConcern(command: Document, options?: Document): boolean { +export function commandSupportsReadConcern(command: Document): boolean { if (command.aggregate || command.count || command.distinct || command.find || command.geoNear) { return true; } - if ( - command.mapReduce && - options && - options.out && - (options.out.inline === 1 || options.out === 'inline') - ) { - return true; - } - return false; } diff --git a/test/integration/run-command/run_command.test.ts b/test/integration/run-command/run_command.test.ts index acca3b5fe6..0b657e48f1 100644 --- a/test/integration/run-command/run_command.test.ts +++ b/test/integration/run-command/run_command.test.ts @@ -37,22 +37,18 @@ describe('RunCommand API', () => { it('does not support writeConcern in options', { requires: { mongodb: '>=5.0' } }, async () => { const command = Object.freeze({ insert: 'test', documents: [{ x: 1 }] }); + //@ts-expect-error: Testing WC is not supported await db.command(command, { writeConcern: new WriteConcern('majority') }); expect(commandsStarted).to.not.have.nested.property('[0].command.writeConcern'); expect(command).to.not.have.property('writeConcern'); }); - // TODO(NODE-4936): We do support readConcern in options, the spec forbids this - it.skip( - 'does not support readConcern in options', - { requires: { mongodb: '>=5.0' } }, - async () => { - const command = Object.freeze({ find: 'test', filter: {} }); - const res = await db.command(command, { readConcern: ReadConcern.MAJORITY }); - expect(res).to.have.property('ok', 1); - expect(commandsStarted).to.not.have.nested.property('[0].command.readConcern'); - expect(command).to.not.have.property('readConcern'); - } - ).skipReason = - 'TODO(NODE-4936): Enable this test when readConcern support has been removed from runCommand'; + it('does not support readConcern in options', { requires: { mongodb: '>=5.0' } }, async () => { + const command = Object.freeze({ find: 'test', filter: {} }); + //@ts-expect-error: Testing RC is not supported + const res = await db.command(command, { readConcern: ReadConcern.MAJORITY }); + expect(res).to.have.property('ok', 1); + expect(commandsStarted).to.not.have.nested.property('[0].command.readConcern'); + expect(command).to.not.have.property('readConcern'); + }); });