Skip to content

Commit ef3bfd4

Browse files
committed
fix(shell-api): use primaryPreferred for show commands
People don’t necessarily expect `show` to follow the same rules as other shell calls, and expect it to work even when connected to a secondary. Use `primaryPreferred` as the read preference for `show db`/`show collections` to address this. (This is a deviation from the old shell’s behavior.)
1 parent 968f2d4 commit ef3bfd4

File tree

8 files changed

+36
-15
lines changed

8 files changed

+36
-15
lines changed

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { startTestCluster } from '../../../testing/integration-testing-hooks';
22
import { eventually } from './helpers';
3+
import { expect } from 'chai';
34
import { TestShell } from './test-shell';
45

56
describe('e2e direct connection', () => {
@@ -94,6 +95,14 @@ describe('e2e direct connection', () => {
9495
await shell.executeLine('db.runCommand({ listCollections: 1 })');
9596
shell.assertContainsOutput("name: 'system.version'");
9697
});
98+
99+
it('lists collections and dbs using show by default', async() => {
100+
const shell = TestShell.start({ args: [`${await rs1.connectionString()}`] });
101+
await shell.waitForPrompt();
102+
await shell.executeLine('use admin');
103+
expect(await shell.executeLine('show collections')).to.include('system.version');
104+
expect(await shell.executeLine('show dbs')).to.include('admin');
105+
});
97106
});
98107

99108
context('connecting to primary', () => {
@@ -122,6 +131,14 @@ describe('e2e direct connection', () => {
122131
shell.assertContainsOutput(`me: '${await rs0.hostport()}'`);
123132
shell.assertContainsOutput(`setName: '${replSetId}'`);
124133
});
134+
135+
it('lists collections and dbs using show by default', async() => {
136+
const shell = TestShell.start({ args: [`${await rs1.connectionString()}`] });
137+
await shell.waitForPrompt();
138+
await shell.executeLine('use admin');
139+
expect(await shell.executeLine('show collections')).to.include('system.version');
140+
expect(await shell.executeLine('show dbs')).to.include('admin');
141+
});
125142
});
126143
});
127144
});

packages/java-shell/src/main/kotlin/com/mongodb/mongosh/service/AdminServiceProvider.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import org.graalvm.polyglot.Value
66
* see service-provider-core/src/admin.ts
77
*/
88
internal interface AdminServiceProvider {
9-
fun listDatabases(database: String): Value
9+
fun listDatabases(database: String, options: Value?): Value
1010
fun getNewConnection(uri: String, options: Value?): Value
1111
fun getConnectionInfo(): Value
1212
fun authenticate(authDoc: Value): Value

packages/java-shell/src/main/kotlin/com/mongodb/mongosh/service/JavaServiceProvider.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ internal class JavaServiceProvider(private val client: MongoClient,
438438
}
439439

440440
@HostAccess.Export
441-
override fun listDatabases(database: String): Value = promise {
441+
override fun listDatabases(database: String, options: Value?): Value = promise {
442442
Right(mapOf("databases" to client.listDatabases()))
443443
}
444444

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import type {
88
CreateCollectionOptions,
99
ClientSession,
1010
DbOptions,
11-
ClientSessionOptions
11+
ClientSessionOptions,
12+
ListDatabasesOptions
1213
} from './all-transport-types';
1314
import type { bson as BSON } from './index';
1415
import { ReplPlatform } from './platform';
@@ -37,7 +38,7 @@ export default interface Admin {
3738
*
3839
* @returns {Promise} The promise of command Documents.
3940
*/
40-
listDatabases(database: string): Promise<Document>;
41+
listDatabases(database: string, options?: ListDatabasesOptions): Promise<Document>;
4142

4243
/**
4344
* create a new service provider with a new connection.

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -841,8 +841,9 @@ class CliServiceProvider extends ServiceProviderCore implements ServiceProvider
841841
*
842842
* @returns {Promise} The promise of command results.
843843
*/
844-
listDatabases(database: string): Promise<Document> {
845-
return this.db(database).admin().listDatabases(this.baseCmdOptions as ListDatabasesOptions);
844+
listDatabases(database: string, options: ListDatabasesOptions = {}): Promise<Document> {
845+
options = { ...this.baseCmdOptions, ...options };
846+
return this.db(database).admin().listDatabases(options);
846847
}
847848

848849
/**

packages/shell-api/src/database.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ export default class Database extends ShellApiClass {
141141
) || [];
142142
}
143143

144-
async _getCollectionNames(): Promise<string[]> {
145-
const infos = await this._listCollections({}, { nameOnly: true });
144+
async _getCollectionNames(options?: ListCollectionsOptions): Promise<string[]> {
145+
const infos = await this._listCollections({}, { ...options, nameOnly: true });
146146
this._cachedCollectionNames = infos.map((collection: any) => collection.name);
147147
return this._cachedCollectionNames;
148148
}

packages/shell-api/src/mongo.spec.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,22 +116,24 @@ describe('Mongo', () => {
116116
describe(t, () => {
117117
it('calls database.getCollectionNames', async() => {
118118
const expectedResult = ['a', 'b'];
119-
database.getCollectionNames.resolves(expectedResult);
119+
database._getCollectionNames.resolves(expectedResult);
120120
await mongo.show(t);
121-
expect(database.getCollectionNames).to.have.been.calledWith();
121+
expect(database._getCollectionNames).to.have.been.calledWith({
122+
readPreference: 'primaryPreferred'
123+
});
122124
});
123125

124126
it('returns ShowCollectionsResult CommandResult', async() => {
125127
const expectedResult = ['a', 'b'];
126-
database.getCollectionNames.resolves(expectedResult);
128+
database._getCollectionNames.resolves(expectedResult);
127129
const result = await mongo.show(t);
128130
expect(result.value).to.deep.equal(expectedResult);
129131
expect(result.type).to.equal('ShowCollectionsResult');
130132
});
131133

132134
it('throws if database.getCollectionNames rejects', async() => {
133135
const expectedError = new Error();
134-
database.getCollectionNames.rejects(expectedError);
136+
database._getCollectionNames.rejects(expectedError);
135137
const catchedError = await mongo.show(t)
136138
.catch(e => e);
137139
expect(catchedError).to.equal(expectedError);

packages/shell-api/src/mongo.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,12 @@ export default class Mongo extends ShellApiClass {
145145

146146
@returnsPromise
147147
async show(cmd: string, arg?: string): Promise<CommandResult> {
148-
this._internalState.messageBus.emit( 'mongosh:show', { method: `show ${cmd}` });
148+
this._internalState.messageBus.emit('mongosh:show', { method: `show ${cmd}` });
149149

150150
switch (cmd) {
151151
case 'databases':
152152
case 'dbs':
153-
const result = await this._serviceProvider.listDatabases('admin');
153+
const result = await this._serviceProvider.listDatabases('admin', { readPreference: 'primaryPreferred' });
154154
if (!('databases' in result)) {
155155
const err = new MongoshRuntimeError('Got invalid result from "listDatabases"', CommonErrors.CommandFailed);
156156
this._internalState.messageBus.emit('mongosh:error', err);
@@ -160,7 +160,7 @@ export default class Mongo extends ShellApiClass {
160160
return new CommandResult('ShowDatabasesResult', result.databases);
161161
case 'collections':
162162
case 'tables':
163-
const collectionNames = await this._internalState.currentDb.getCollectionNames();
163+
const collectionNames = await this._internalState.currentDb._getCollectionNames({ readPreference: 'primaryPreferred' });
164164
return new CommandResult('ShowCollectionsResult', collectionNames);
165165
case 'profile':
166166
const sysprof = this._internalState.currentDb.getCollection('system.profile');

0 commit comments

Comments
 (0)