Skip to content

Commit 968f2d4

Browse files
committed
fix(shell-api): use readPref for runCommand if explicitly set MONGOSH-508
If `readPreference` has been explicitly set, either through a) the connection string or b) by using `setReadPref()`, then pass it for `db.runCommand()` as well. The reason we’re not always using it is that the driver will report it back to use as `primary` by default, which would then break `db.runCommand()` for almost all calls when we’re connected to a secondary. As part of this, update the default `Mongo` instance to also report the connection string, like a programmatically created `Mongo` instance would (and like it worked in the old shell). Also addresses MONGOSH-557. Refs: https://docs.google.com/document/d/10FTlhliWtg_XquUGbEmZQ7ccUiImMLlBT5Qlm2O1ZxI/edit?pli=1
1 parent 99783aa commit 968f2d4

File tree

6 files changed

+64
-10
lines changed

6 files changed

+64
-10
lines changed

packages/cli-repl/test/e2e-direct.spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,31 @@ describe('e2e direct connection', () => {
6969
shell.assertContainsOutput(`me: '${await rs1.hostport()}'`);
7070
shell.assertContainsOutput(`setName: '${replSetId}'`);
7171
});
72+
73+
it('fails to list collections without explicit readPreference', async() => {
74+
const shell = TestShell.start({ args: [`${await rs1.connectionString()}`] });
75+
await shell.waitForPrompt();
76+
await shell.executeLine('use admin');
77+
await shell.executeLine('db.runCommand({ listCollections: 1 })');
78+
shell.assertContainsError('MongoError: not master');
79+
});
80+
81+
it('lists collections when readPreference is in the connection string', async() => {
82+
const shell = TestShell.start({ args: [`${await rs1.connectionString()}?readPreference=secondaryPreferred`] });
83+
await shell.waitForPrompt();
84+
await shell.executeLine('use admin');
85+
await shell.executeLine('db.runCommand({ listCollections: 1 })');
86+
shell.assertContainsOutput("name: 'system.version'");
87+
});
88+
89+
it('lists collections when readPreference is set via Mongo', async() => {
90+
const shell = TestShell.start({ args: [`${await rs1.connectionString()}`] });
91+
await shell.waitForPrompt();
92+
await shell.executeLine('use admin');
93+
await shell.executeLine('db.getMongo().setReadPref("secondaryPreferred")');
94+
await shell.executeLine('db.runCommand({ listCollections: 1 })');
95+
shell.assertContainsOutput("name: 'system.version'");
96+
});
7297
});
7398

7499
context('connecting to primary', () => {

packages/service-provider-core/src/admin.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ export default interface Admin {
4747
*/
4848
getNewConnection(uri: string, options: MongoClientOptions): Promise<any>; // returns the ServiceProvider instance
4949

50+
/**
51+
* Return the URI for the current connection, if this ServiceProvider is connected.
52+
*/
53+
getURI(): string | undefined;
54+
5055
/**
5156
* Return connection info
5257
*/

packages/service-provider-server/src/cli-service-provider.integration.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,4 +686,10 @@ describe('CliServiceProvider [integration]', function() {
686686
expect(serviceProvider.driverMetadata.driver.name).to.equal('nodejs');
687687
});
688688
});
689+
690+
describe('#getURI', () => {
691+
it('returns the current URI', () => {
692+
expect(serviceProvider.getURI()).to.equal(connectionString + '/');
693+
});
694+
});
689695
});

packages/service-provider-server/src/cli-service-provider.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,10 @@ class CliServiceProvider extends ServiceProviderCore implements ServiceProvider
11561156
getRawClient(): MongoClient {
11571157
return this.mongoClient;
11581158
}
1159+
1160+
getURI(): string | undefined {
1161+
return this.uri?.href;
1162+
}
11591163
}
11601164

11611165
export default CliServiceProvider;

packages/shell-api/src/database.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ import type {
2828
CommandOperationOptions,
2929
CreateCollectionOptions,
3030
Document,
31-
WriteConcern
31+
WriteConcern,
32+
ListCollectionsOptions
3233
} from '@mongosh/service-provider-core';
3334
import { AggregationCursor, CommandResult } from './index';
3435
import {
@@ -116,27 +117,27 @@ export default class Database extends ShellApiClass {
116117

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

119-
public async _runCommand(cmd: Document, options = {}): Promise<Document> {
120+
public async _runCommand(cmd: Document, options: CommandOperationOptions = {}): Promise<Document> {
120121
return this._mongo._serviceProvider.runCommandWithCheck(
121122
this._name,
122123
cmd,
123-
{ ...this._baseOptions, ...options }
124+
{ ...this._mongo._getExplicitlyRequestedReadPref(), ...this._baseOptions, ...options }
124125
);
125126
}
126127

127-
public async _runAdminCommand(cmd: Document, options = {}): Promise<Document> {
128+
public async _runAdminCommand(cmd: Document, options: CommandOperationOptions = {}): Promise<Document> {
128129
return this._mongo._serviceProvider.runCommandWithCheck(
129130
ADMIN_DB,
130131
cmd,
131-
{ ...this._baseOptions, ...options }
132+
{ ...this._mongo._getExplicitlyRequestedReadPref(), ...this._baseOptions, ...options }
132133
);
133134
}
134135

135-
private async _listCollections(filter: Document, options: Document): Promise<Document[]> {
136+
private async _listCollections(filter: Document, options: ListCollectionsOptions): Promise<Document[]> {
136137
return await this._mongo._serviceProvider.listCollections(
137138
this._name,
138139
filter,
139-
{ ...this._baseOptions, ...options }
140+
{ ...this._mongo._getExplicitlyRequestedReadPref(), ...this._baseOptions, ...options }
140141
) || [];
141142
}
142143

@@ -216,7 +217,7 @@ export default class Database extends ShellApiClass {
216217
*/
217218
@returnsPromise
218219
@serverVersions(['3.0.0', ServerVersions.latest])
219-
async getCollectionInfos(filter: Document = {}, options: Document = {}): Promise<Document[]> {
220+
async getCollectionInfos(filter: Document = {}, options: ListCollectionsOptions = {}): Promise<Document[]> {
220221
this._emitDatabaseApiCall('getCollectionInfos', { filter, options });
221222
return await this._listCollections(
222223
filter,

packages/shell-api/src/mongo.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,23 @@ export default class Mongo extends ShellApiClass {
5959
public _fleOptions: SPAutoEncryption | undefined;
6060
private _keyVault: KeyVault | undefined; // need to keep it around so that the ShellApi ClientEncryption class can access it
6161
private _clientEncryption: ClientEncryption | undefined;
62+
private _readPreferenceWasExplicitlyRequested = false;
6263

6364
constructor(
6465
internalState: ShellInternalState,
65-
uri = 'mongodb://localhost/',
66+
uri?: string,
6667
fleOptions?: ClientSideFieldLevelEncryptionOptions
6768
) {
6869
super();
6970
this._internalState = internalState;
7071
this._databases = {};
71-
this._uri = generateUri({ _: [uri] });
7272
this._serviceProvider = this._internalState.initialServiceProvider;
73+
if (typeof uri === 'string') {
74+
this._uri = generateUri({ _: [uri] });
75+
} else {
76+
this._uri = this._serviceProvider?.getURI?.() ?? generateUri({ _: ['mongodb://localhost/'] });
77+
}
78+
this._readPreferenceWasExplicitlyRequested = /\breadPreference=/.test(this._uri);
7379
if (fleOptions) {
7480
this._fleOptions = processFLEOptions(fleOptions, this._serviceProvider);
7581
const { mongocryptdSpawnPath } = this._internalState;
@@ -203,6 +209,12 @@ export default class Mongo extends ShellApiClass {
203209
return this._serviceProvider.getReadPreference();
204210
}
205211

212+
_getExplicitlyRequestedReadPref(): { readPreference: ReadPreference } | undefined {
213+
return this._readPreferenceWasExplicitlyRequested ?
214+
{ readPreference: this.getReadPref() } :
215+
undefined;
216+
}
217+
206218
getReadConcern(): string | undefined {
207219
try {
208220
const rc = this._serviceProvider.getReadConcern();
@@ -217,6 +229,7 @@ export default class Mongo extends ShellApiClass {
217229
await this._serviceProvider.resetConnectionOptions({
218230
readPreference: { mode: mode, tagSet: tagSet, hedgeOptions: hedgeOptions }
219231
});
232+
this._readPreferenceWasExplicitlyRequested = true;
220233
}
221234

222235
@returnsPromise

0 commit comments

Comments
 (0)