fix(NODE-4936)!: remove unsupported options from db.command and admin.command (mongodb#3775)

Author: nbbeeken

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<Document> {
     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
+      })
     );
   }
 

src/db.ts

@@ -259,7 +259,14 @@ export class Db {
    */
   async command(command: Document, options?: RunCommandOptions): Promise<Document> {
     // 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
+      })
+    );
   }
 
   /**

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<MongoClientEvents> {
     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;
       }
     }
 

src/operations/operation.ts

@@ -49,7 +49,6 @@ const kSession = Symbol('session');
  */
 export abstract class AbstractOperation<TResult = any> {
   ns!: MongoDBNamespace;
-  cmd!: Document;
   readPreference: ReadPreference;
   server!: Server;
   bypassPinningCheck: boolean;

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<Document> {
+  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<Collection> {
-    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<Document> = 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.');
   }
 }
 

src/operations/run_command.ts

@@ -1,73 +1,55 @@
 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 = {
   /** Specify ClientSession for this command */
   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<T = Document> extends CommandCallbackOperation<T> {
-  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<T = Document> extends AbstractOperation<T> {
+  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<T>
-  ): void {
-    const command = this.command;
-    this.executeCommandCallback(server, session, command, callback);
+  override async execute(server: Server, session: ClientSession | undefined): Promise<T> {
+    this.server = server;
+    return server.commandAsync(this.ns, this.command, {
+      ...this.options,
+      readPreference: this.readPreference,
+      session
+    }) as TODO_NODE_3286;
   }
 }
 
-export class RunAdminCommandOperation<T = Document> extends RunCommandOperation<T> {
-  constructor(parent: OperationParent | undefined, command: Document, options?: RunCommandOptions) {
-    super(parent, command, options);
-    this.ns = new MongoDBNamespace('admin');
+export class RunAdminCommandOperation<T = Document> extends AbstractOperation<T> {
+  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<T> {
+    this.server = server;
+    return server.commandAsync(this.ns, this.command, {
+      ...this.options,
+      readPreference: this.readPreference,
+      session
+    }) as TODO_NODE_3286;
   }
 }

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 });

src/utils.ts

@@ -1155,20 +1155,11 @@ export function shuffle<T>(sequence: Iterable<T>, limit = 0): Array<T> {
 
 // 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;
 }
 

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');
+  });
 });