Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 29 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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