Skip to content

Commit

Permalink
fix: dropIndexes so it run on the primary regardless the read prefere…
Browse files Browse the repository at this point in the history
…nce MONGOSH-1478 (#1566)

* chore: fix dropIndexes so it run on the primary

* chore: fix test for dropIndexes as now it forces to use readPreference: priamryPreferred

* chore: do not provide a default readPreference

* chore: update error documentation (#1585)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* chore: fix read preference on runCommand

* chore: propagate readPreference options from the REPL

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Anna Henningsen <anna.henningsen@mongodb.com>
  • Loading branch information
3 people authored Aug 16, 2023
1 parent 336e649 commit 949c2b5
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 25 deletions.
15 changes: 13 additions & 2 deletions packages/cli-repl/test/e2e-direct.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ describe('e2e direct connection', function () {
await shell.waitForPrompt();
await shell.executeLine('use admin');
await shell.executeLine('db.runCommand({ listCollections: 1 })');
shell.assertContainsOutput("name: 'system.version'");
shell.assertContainsOutput('MongoServerError: not primary');
});

it('lists collections when readPreference is set via Mongo', async function () {
Expand All @@ -142,7 +142,7 @@ describe('e2e direct connection', function () {
'db.getMongo().setReadPref("secondaryPreferred")'
);
await shell.executeLine('db.runCommand({ listCollections: 1 })');
shell.assertContainsOutput("name: 'system.version'");
shell.assertContainsOutput('MongoServerError: not primary');
});

it('fails to list databases without explicit readPreference', async function () {
Expand Down Expand Up @@ -398,6 +398,17 @@ describe('e2e direct connection', function () {
await shell.executeLine('db.runCommand({connectionStatus: 1})');
shell.assertContainsOutput("user: 'anna'");
});
it('drops indexes even if a read preference is specified in the connection url', async function () {
const shell = TestShell.start({
args: [await rs0.connectionString({ readPreference: 'secondary' })],
});

await shell.waitForPrompt();
await shell.executeLine('db.mydb.test.createIndex({ field: 1 })');
await shell.executeLine('db.mydb.test.dropIndexes({ field: 1 })');
shell.assertContainsOutput('nIndexesWas: 2');
shell.assertContainsOutput('ok: 1');
});
});

describe('fail-fast connections', function () {
Expand Down
4 changes: 2 additions & 2 deletions packages/shell-api/src/collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1612,7 +1612,7 @@ export default class Collection extends ShellApiWithMongoClass {
* This function provides the deprecated fallback in those instances.
*/
async _getLegacyCollStats(scale: number) {
const result = await this._database._runCommand({
const result = await this._database._runReadCommand({
collStats: this._name,
scale: scale || 1,
});
Expand Down Expand Up @@ -2047,7 +2047,7 @@ export default class Collection extends ShellApiWithMongoClass {
if (typeof options === 'boolean') {
options = { full: options };
}
return await this._database._runCommand({
return await this._database._runReadCommand({
validate: this._name,
...options,
});
Expand Down
58 changes: 38 additions & 20 deletions packages/shell-api/src/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ import {
getBadge,
} from './helpers';

import type {
ChangeStreamOptions,
CommandOperationOptions,
CreateCollectionOptions,
Document,
WriteConcern,
ListCollectionsOptions,
import {
type ChangeStreamOptions,
type CommandOperationOptions,
type CreateCollectionOptions,
type Document,
type WriteConcern,
type ListCollectionsOptions,
} from '@mongosh/service-provider-core';
import { AggregationCursor, RunCommandCursor, CommandResult } from './index';
import {
Expand All @@ -50,6 +50,7 @@ import { ShellApiErrors } from './error-codes';
import type {
CreateEncryptedCollectionOptions,
CheckMetadataConsistencyOptions,
RunCommandOptions,
} from '@mongosh/service-provider-core';

export type CollectionNamesWithTypes = {
Expand Down Expand Up @@ -149,10 +150,24 @@ export default class Database extends ShellApiWithMongoClass {
}

// Private helpers to avoid sending telemetry events for internal calls. Public so rs/sh can use them

public async _runCommand(
cmd: Document,
options: CommandOperationOptions = {}
): Promise<Document> {
return this._mongo._serviceProvider.runCommandWithCheck(
this._name,
adjustRunCommand(cmd, this._instanceState.shellBson),
{
...(await this._baseOptions()),
...options,
}
);
}

// Private helpers to avoid sending telemetry events for internal calls. Public so rs/sh can use them
public async _runReadCommand(
cmd: Document,
options: CommandOperationOptions = {}
): Promise<Document> {
return this._mongo._serviceProvider.runCommandWithCheck(
this._name,
Expand Down Expand Up @@ -341,17 +356,20 @@ export default class Database extends ShellApiWithMongoClass {
*/
@returnsPromise
@apiVersions([1])
async runCommand(cmd: string | Document): Promise<Document> {
async runCommand(
cmd: string | Document,
options?: RunCommandOptions
): Promise<Document> {
assertArgsDefinedType([cmd], [['string', 'object']], 'Database.runCommand');
if (typeof cmd === 'string') {
cmd = { [cmd]: 1 };
}

const hiddenCommands = new RegExp(HIDDEN_COMMANDS);
if (!Object.keys(cmd).some((k) => hiddenCommands.test(k))) {
this._emitDatabaseApiCall('runCommand', { cmd });
this._emitDatabaseApiCall('runCommand', { cmd, options });
}
return this._runCommand(cmd);
return this._runCommand(cmd, options);
}

/**
Expand Down Expand Up @@ -711,7 +729,7 @@ export default class Database extends ShellApiWithMongoClass {
{ usersInfo: { user: username, db: this._name } },
options
);
const result = await this._runCommand(command);
const result = await this._runReadCommand(command);
if (result.users === undefined) {
throw new MongoshInternalError(
'No users were returned from the userInfo command'
Expand All @@ -730,7 +748,7 @@ export default class Database extends ShellApiWithMongoClass {
async getUsers(options: Document = {}): Promise<Document> {
this._emitDatabaseApiCall('getUsers', { options: options });
const command = adaptOptions({}, { usersInfo: 1 }, options);
return await this._runCommand(command);
return await this._runReadCommand(command);
}

@returnsPromise
Expand Down Expand Up @@ -973,7 +991,7 @@ export default class Database extends ShellApiWithMongoClass {
{ rolesInfo: { role: rolename, db: this._name } },
options
);
const result = await this._runCommand(command);
const result = await this._runReadCommand(command);
if (result.roles === undefined) {
throw new MongoshInternalError(
'No roles returned from rolesInfo command'
Expand All @@ -992,7 +1010,7 @@ export default class Database extends ShellApiWithMongoClass {
async getRoles(options: Document = {}): Promise<Document> {
this._emitDatabaseApiCall('getRoles', { options: options });
const command = adaptOptions({}, { rolesInfo: 1 }, options);
return await this._runCommand(command);
return await this._runReadCommand(command);
}

async _getCurrentOperations(opts: Document | boolean): Promise<Document[]> {
Expand Down Expand Up @@ -1133,7 +1151,7 @@ export default class Database extends ShellApiWithMongoClass {
@apiVersions([])
async isMaster(): Promise<Document> {
this._emitDatabaseApiCall('isMaster', {});
const result = await this._runCommand({
const result = await this._runReadCommand({
isMaster: 1,
});
result.isWritablePrimary = result.ismaster;
Expand All @@ -1146,7 +1164,7 @@ export default class Database extends ShellApiWithMongoClass {
async hello(): Promise<Document> {
this._emitDatabaseApiCall('hello', {});
try {
this._cachedHello = await this._runCommand({
this._cachedHello = await this._runReadCommand({
hello: 1,
});
return this._cachedHello;
Expand Down Expand Up @@ -1192,7 +1210,7 @@ export default class Database extends ShellApiWithMongoClass {
scaleOrOptions = { scale: scaleOrOptions };
}
this._emitDatabaseApiCall('stats', { scale: scaleOrOptions.scale });
return await this._runCommand({
return await this._runReadCommand({
dbStats: 1,
scale: 1,
...scaleOrOptions,
Expand Down Expand Up @@ -1254,7 +1272,7 @@ export default class Database extends ShellApiWithMongoClass {
@apiVersions([])
async getProfilingStatus(): Promise<Document> {
this._emitDatabaseApiCall('getProfilingStatus', {});
return await this._runCommand({
return await this._runReadCommand({
profile: -1,
});
}
Expand Down Expand Up @@ -1377,7 +1395,7 @@ export default class Database extends ShellApiWithMongoClass {
@apiVersions([])
async listCommands(): Promise<CommandResult> {
this._emitDatabaseApiCall('listCommands', {});
const result = await this._runCommand({
const result = await this._runReadCommand({
listCommands: 1,
});
if (!result || result.commands === undefined) {
Expand Down
2 changes: 1 addition & 1 deletion packages/shell-api/src/explainable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export default class Explainable extends ShellApiWithMongoClass {
this._emitExplainableApiCall('count', { query, options });
// This is the only one that currently lacks explicit driver support.
return markAsExplainOutput(
await this._collection._database._runCommand({
await this._collection._database._runReadCommand({
explain: {
count: `${this._collection._name}`,
query,
Expand Down

0 comments on commit 949c2b5

Please sign in to comment.