Skip to content

Commit

Permalink
feat(NODE-5207): deprecate unsupported runCommand options and add spe…
Browse files Browse the repository at this point in the history
…c tests (#3643)
  • Loading branch information
nbbeeken authored Apr 26, 2023
1 parent a16bdfa commit d6d76b4
Show file tree
Hide file tree
Showing 11 changed files with 892 additions and 24 deletions.
16 changes: 16 additions & 0 deletions src/admin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
25 changes: 13 additions & 12 deletions src/cmap/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}
Expand All @@ -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()
};
}
Expand All @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions src/cmap/wire_protocol/shared.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { Document } from '../../bson';
import { MongoInvalidArgumentError } from '../../error';
import type { ReadPreferenceLike } from '../../read_preference';
import { ReadPreference } from '../../read_preference';
Expand All @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions src/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
43 changes: 40 additions & 3 deletions src/operations/run_command.ts
Original file line number Diff line number Diff line change
@@ -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> {
Expand Down
Empty file.
6 changes: 6 additions & 0 deletions test/integration/run-command/run_command.spec.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { loadSpecTests } from '../../spec';
import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';

describe('RunCommand spec', () => {
runUnifiedSuite(loadSpecTests('run-command'));
});
58 changes: 58 additions & 0 deletions test/integration/run-command/run_command.test.ts
Original file line number Diff line number Diff line change
@@ -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';
});
Loading

0 comments on commit d6d76b4

Please sign in to comment.