Skip to content

Commit

Permalink
fix(NODE-4936)!: remove unsupported options from db.command and admin…
Browse files Browse the repository at this point in the history
….command (#3775)
  • Loading branch information
nbbeeken authored Jul 21, 2023
1 parent c7c5dae commit 52cd649
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 118 deletions.
10 changes: 7 additions & 3 deletions src/admin.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
Expand Down Expand Up @@ -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
})
);
}

Expand Down
9 changes: 8 additions & 1 deletion src/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
);
}

/**
Expand Down
9 changes: 6 additions & 3 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
}

Expand Down
1 change: 0 additions & 1 deletion src/operations/operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const kSession = Symbol('session');
*/
export abstract class AbstractOperation<TResult = any> {
ns!: MongoDBNamespace;
cmd!: Document;
readPreference: ReadPreference;
server!: Server;
bypassPinningCheck: boolean;
Expand Down
59 changes: 27 additions & 32 deletions src/operations/rename.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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.');
}
}

Expand Down
86 changes: 34 additions & 52 deletions src/operations/run_command.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
6 changes: 3 additions & 3 deletions src/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -778,7 +778,7 @@ function endTransaction(

return executeOperation(
session.client,
new RunAdminCommandOperation(undefined, command, {
new RunAdminCommandOperation(command, {
session,
readPreference: ReadPreference.primary,
bypassPinningCheck: true
Expand Down Expand Up @@ -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 });
Expand Down
11 changes: 1 addition & 10 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
22 changes: 9 additions & 13 deletions test/integration/run-command/run_command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});

0 comments on commit 52cd649

Please sign in to comment.