feat(NODE-5207): deprecate unsupported runCommand options and add spec tests (#3643)

Author: nbbeeken

src/admin.ts

@@ -54,6 +54,22 @@ export class Admin {
   /**
    * Execute a command
    *
+   * The driver will ensure the following fields are attached to the command sent to the server:
+   * - `lsid` - sourced from an implicit session or options.session
+   * - `$readPreference` - defaults to primary or can be configured by options.readPreference
+   * - `$db` - sourced from the name of this database
+   *
+   * If the client has a serverApi setting:
+   * - `apiVersion`
+   * - `apiStrict`
+   * - `apiDeprecationErrors`
+   *
+   * When in a transaction:
+   * - `readConcern` - sourced from readConcern set on the TransactionOptions
+   * - `writeConcern` - sourced from writeConcern set on the TransactionOptions
+   *
+   * Attaching any of the above fields to the command will have no effect as the driver will overwrite the value.
+   *
    * @param command - The command to execute
    * @param options - Optional settings for the command
    */

src/cmap/connection.ts

@@ -484,22 +484,23 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
 
   command(
     ns: MongoDBNamespace,
-    cmd: Document,
+    command: Document,
     options: CommandOptions | undefined,
     callback: Callback
   ): void {
-    const readPreference = getReadPreference(cmd, options);
+    let cmd = { ...command };
+
+    const readPreference = getReadPreference(options);
     const shouldUseOpMsg = supportsOpMsg(this);
     const session = options?.session;
 
     let clusterTime = this.clusterTime;
-    let finalCmd = Object.assign({}, cmd);
 
     if (this.serverApi) {
       const { version, strict, deprecationErrors } = this.serverApi;
-      finalCmd.apiVersion = version;
-      if (strict != null) finalCmd.apiStrict = strict;
-      if (deprecationErrors != null) finalCmd.apiDeprecationErrors = deprecationErrors;
+      cmd.apiVersion = version;
+      if (strict != null) cmd.apiStrict = strict;
+      if (deprecationErrors != null) cmd.apiDeprecationErrors = deprecationErrors;
     }
 
     if (hasSessionSupport(this) && session) {
@@ -511,7 +512,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
         clusterTime = session.clusterTime;
       }
 
-      const err = applySession(session, finalCmd, options);
+      const err = applySession(session, cmd, options);
       if (err) {
         return callback(err);
       }
@@ -521,12 +522,12 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
 
     // if we have a known cluster time, gossip it
     if (clusterTime) {
-      finalCmd.$clusterTime = clusterTime;
+      cmd.$clusterTime = clusterTime;
     }
 
     if (isSharded(this) && !shouldUseOpMsg && readPreference && readPreference.mode !== 'primary') {
-      finalCmd = {
-        $query: finalCmd,
+      cmd = {
+        $query: cmd,
         $readPreference: readPreference.toJSON()
       };
     }
@@ -544,8 +545,8 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
 
     const cmdNs = `${ns.db}.$cmd`;
     const message = shouldUseOpMsg
-      ? new Msg(cmdNs, finalCmd, commandOptions)
-      : new Query(cmdNs, finalCmd, commandOptions);
+      ? new Msg(cmdNs, cmd, commandOptions)
+      : new Query(cmdNs, cmd, commandOptions);
 
     try {
       write(this, message, commandOptions, callback);

src/cmap/wire_protocol/shared.ts

@@ -1,4 +1,3 @@
-import type { Document } from '../../bson';
 import { MongoInvalidArgumentError } from '../../error';
 import type { ReadPreferenceLike } from '../../read_preference';
 import { ReadPreference } from '../../read_preference';
@@ -13,9 +12,9 @@ export interface ReadPreferenceOption {
   readPreference?: ReadPreferenceLike;
 }
 
-export function getReadPreference(cmd: Document, options?: ReadPreferenceOption): ReadPreference {
+export function getReadPreference(options?: ReadPreferenceOption): ReadPreference {
   // Default to command version of the readPreference
-  let readPreference = cmd.readPreference || ReadPreference.primary;
+  let readPreference = options?.readPreference ?? ReadPreference.primary;
   // If we have an option readPreference override the command one
   if (options?.readPreference) {
     readPreference = options.readPreference;

src/db.ts

@@ -232,6 +232,22 @@ export class Db {
    * @remarks
    * This command does not inherit options from the MongoClient.
    *
+   * The driver will ensure the following fields are attached to the command sent to the server:
+   * - `lsid` - sourced from an implicit session or options.session
+   * - `$readPreference` - defaults to primary or can be configured by options.readPreference
+   * - `$db` - sourced from the name of this database
+   *
+   * If the client has a serverApi setting:
+   * - `apiVersion`
+   * - `apiStrict`
+   * - `apiDeprecationErrors`
+   *
+   * When in a transaction:
+   * - `readConcern` - sourced from readConcern set on the TransactionOptions
+   * - `writeConcern` - sourced from writeConcern set on the TransactionOptions
+   *
+   * Attaching any of the above fields to the command will have no effect as the driver will overwrite the value.
+   *
    * @param command - The command to run
    * @param options - Optional settings for the command
    */

src/operations/run_command.ts

@@ -1,11 +1,48 @@
-import type { Document } from '../bson';
+import type { BSONSerializeOptions, Document } from '../bson';
+import type { ReadPreferenceLike } from '../read_preference';
 import type { Server } from '../sdam/server';
 import type { ClientSession } from '../sessions';
 import { Callback, MongoDBNamespace } from '../utils';
-import { CommandOperation, CommandOperationOptions, OperationParent } from './command';
+import { CommandOperation, OperationParent } from './command';
 
 /** @public */
-export type RunCommandOptions = CommandOperationOptions;
+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 CommandOperation<T> {

test/integration/run-command/.gitkeep

@@ -0,0 +1,6 @@
+import { loadSpecTests } from '../../spec';
+import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';
+
+describe('RunCommand spec', () => {
+  runUnifiedSuite(loadSpecTests('run-command'));
+});

test/integration/run-command/run_command.spec.test.ts

@@ -0,0 +1,58 @@
+import { expect } from 'chai';
+
+import {
+  CommandStartedEvent,
+  Db,
+  MongoClient,
+  ReadConcern,
+  ReadPreference,
+  WriteConcern
+} from '../../mongodb';
+
+describe('RunCommand API', () => {
+  let client: MongoClient;
+  let db: Db;
+  let commandsStarted: CommandStartedEvent[];
+  beforeEach(async function () {
+    const options = {
+      serverApi: { version: '1', strict: true, deprecationErrors: false },
+      monitorCommands: true
+    };
+    client = this.configuration.newClient({}, options);
+    db = client.db();
+    commandsStarted = [];
+    client.on('commandStarted', started => commandsStarted.push(started));
+  });
+
+  afterEach(async function () {
+    commandsStarted = [];
+    await client.close();
+  });
+
+  it('does not modify user input', { requires: { mongodb: '>=5.0' } }, async () => {
+    const command = Object.freeze({ ping: 1 });
+    // will throw if it tries to modify command
+    await db.command(command, { readPreference: ReadPreference.nearest });
+  });
+
+  it('does not support writeConcern in options', { requires: { mongodb: '>=5.0' } }, async () => {
+    const command = Object.freeze({ insert: 'test', documents: [{ x: 1 }] });
+    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';
+});