Skip to content

Commit 5ff7e43

Browse files
committed
feat(autocomplete): autocomplete collection names MONGOSH-335
Also implicitly address MONGOSH-291 by adding a 200ms timeout after which cached results are used (which may seem either high or low, so I’m happy to adjust it – see also the comment about that value).
1 parent 40855b7 commit 5ff7e43

File tree

11 files changed

+152
-20
lines changed

11 files changed

+152
-20
lines changed

packages/autocomplete/index.spec.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,24 @@ import { signatures as shellSignatures, Topologies } from '@mongosh/shell-api';
33

44
import { expect } from 'chai';
55

6+
let collections: string[];
67
const standalone440 = {
78
topology: () => Topologies.Standalone,
89
connectionInfo: () => ({
910
is_atlas: false,
1011
is_data_lake: false,
1112
server_version: '4.4.0'
12-
})
13+
}),
14+
getCollectionCompletionsForCurrentDb: () => collections
1315
};
1416
const sharded440 = {
1517
topology: () => Topologies.Sharded,
1618
connectionInfo: () => ({
1719
is_atlas: false,
1820
is_data_lake: false,
1921
server_version: '4.4.0'
20-
})
22+
}),
23+
getCollectionCompletionsForCurrentDb: () => collections
2124
};
2225

2326
const standalone300 = {
@@ -26,23 +29,30 @@ const standalone300 = {
2629
is_atlas: false,
2730
is_data_lake: false,
2831
server_version: '3.0.0'
29-
})
32+
}),
33+
getCollectionCompletionsForCurrentDb: () => collections
3034
};
3135
const datalake440 = {
3236
topology: () => Topologies.Sharded,
3337
connectionInfo: () => ({
3438
is_atlas: true,
3539
is_data_lake: true,
3640
server_version: '4.4.0'
37-
})
41+
}),
42+
getCollectionCompletionsForCurrentDb: () => collections
3843
};
3944

4045
const noParams = {
4146
topology: () => Topologies.Standalone,
42-
connectionInfo: () => undefined
47+
connectionInfo: () => undefined,
48+
getCollectionCompletionsForCurrentDb: () => collections
4349
};
4450

4551
describe('completer.completer', () => {
52+
beforeEach(() => {
53+
collections = [];
54+
});
55+
4656
context('when context is top level shell api', () => {
4757
it('matches shell completions', async() => {
4858
const i = 'u';
@@ -189,6 +199,12 @@ describe('completer.completer', () => {
189199
const i = 'db.shipw';
190200
expect(await completer(standalone440, i)).to.deep.equal([[], i]);
191201
});
202+
203+
it('includes collection names', async() => {
204+
collections = ['shipwrecks'];
205+
const i = 'db.shipw';
206+
expect(await completer(standalone440, i)).to.deep.equal([['db.shipwrecks'], i]);
207+
});
192208
});
193209

194210
context('when context is collections', () => {

packages/autocomplete/index.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ export interface AutocompleteParameters {
2020
is_atlas: boolean;
2121
is_data_lake: boolean;
2222
server_version: string;
23-
};
23+
},
24+
getCollectionCompletionsForCurrentDb: (collName: string) => string[] | Promise<string[]>;
2425
}
2526

2627
export const BASE_COMPLETIONS = EXPRESSION_OPERATORS.concat(
@@ -67,6 +68,8 @@ async function completer(params: AutocompleteParameters, line: string): Promise<
6768
return [hits.length ? hits : [], line];
6869
} else if (firstLineEl.includes('db') && splitLine.length === 2) {
6970
const hits = filterShellAPI(params, DB_COMPLETIONS, elToComplete, splitLine);
71+
const colls = await params.getCollectionCompletionsForCurrentDb(elToComplete.trim());
72+
hits.push(...colls.map(coll => `${splitLine[0]}.${coll}`));
7073
return [hits.length ? hits : [], line];
7174
} else if (firstLineEl.includes('db') && splitLine.length > 2) {
7275
if (splitLine.length > 3) {

packages/browser-runtime-core/src/autocompleter/shell-api-autocompleter.spec.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ const standalone440 = {
88
is_atlas: false,
99
is_data_lake: false,
1010
server_version: '4.4.0'
11-
})
11+
}),
12+
getCollectionCompletionsForCurrentDb: () => ['bananas']
1213
};
1314

1415
describe('Autocompleter', () => {
@@ -34,6 +35,14 @@ describe('Autocompleter', () => {
3435
completion: 'db.coll1.find'
3536
});
3637
});
38+
39+
it('returns collection names value with text after dot', async() => {
40+
const completions = await autocompleter.getCompletions('db.b');
41+
42+
expect(completions).to.deep.contain({
43+
completion: 'db.bananas'
44+
});
45+
});
3746
});
3847
});
3948

packages/cli-repl/src/cli-repl.spec.ts

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { promisify } from 'util';
55
import { once } from 'events';
66
import CliRepl, { CliReplOptions } from './cli-repl';
77
import { startTestServer, MongodSetup } from '../../../testing/integration-testing-hooks';
8-
import { expect, useTmpdir, waitEval, waitBus, fakeTTYProps, readReplLogfile, tick } from '../test/repl-helpers';
8+
import { expect, useTmpdir, waitEval, waitBus, waitCompletion, fakeTTYProps, readReplLogfile } from '../test/repl-helpers';
99
import { CliReplErrors } from './error-codes';
1010
import { MongoshInternalError } from '@mongosh/errors';
1111
import http from 'http';
@@ -208,7 +208,8 @@ describe('CliRepl', () => {
208208
verifyAutocompletion({
209209
testServer: null,
210210
wantWatch: true,
211-
wantShardDistribution: true
211+
wantShardDistribution: true,
212+
hasCollectionNames: false
212213
});
213214
});
214215

@@ -353,7 +354,8 @@ describe('CliRepl', () => {
353354
verifyAutocompletion({
354355
testServer: testServer,
355356
wantWatch: false,
356-
wantShardDistribution: false
357+
wantShardDistribution: false,
358+
hasCollectionNames: true
357359
});
358360

359361
context('analytics integration', () => {
@@ -439,22 +441,25 @@ describe('CliRepl', () => {
439441
verifyAutocompletion({
440442
testServer: startTestServer('not-shared', '--replicaset', '--nodes', '1'),
441443
wantWatch: true,
442-
wantShardDistribution: false
444+
wantShardDistribution: false,
445+
hasCollectionNames: true
443446
});
444447
});
445448

446449
context('with a mongos', () => {
447450
verifyAutocompletion({
448451
testServer: startTestServer('not-shared', '--replicaset', '--sharded', '0'),
449452
wantWatch: true,
450-
wantShardDistribution: true
453+
wantShardDistribution: true,
454+
hasCollectionNames: false // We're only spinning up a mongos here
451455
});
452456
});
453457

454-
function verifyAutocompletion({ testServer, wantWatch, wantShardDistribution }: {
458+
function verifyAutocompletion({ testServer, wantWatch, wantShardDistribution, hasCollectionNames }: {
455459
testServer: MongodSetup | null,
456460
wantWatch: boolean,
457-
wantShardDistribution: boolean
461+
wantShardDistribution: boolean,
462+
hasCollectionNames: boolean
458463
}): void {
459464
describe('autocompletion', () => {
460465
let cliRepl: CliRepl;
@@ -475,7 +480,7 @@ describe('CliRepl', () => {
475480
it(`${wantWatch ? 'completes' : 'does not complete'} the watch method`, async() => {
476481
output = '';
477482
input.write('db.wat\u0009\u0009');
478-
await tick();
483+
await waitCompletion(cliRepl.bus);
479484
if (wantWatch) {
480485
expect(output).to.include('db.watch');
481486
} else {
@@ -486,13 +491,28 @@ describe('CliRepl', () => {
486491
it(`${wantShardDistribution ? 'completes' : 'does not complete'} the getShardDistribution method`, async() => {
487492
output = '';
488493
input.write('db.coll.getShardDis\u0009\u0009');
489-
await tick();
494+
await waitCompletion(cliRepl.bus);
490495
if (wantShardDistribution) {
491496
expect(output).to.include('db.coll.getShardDistribution');
492497
} else {
493498
expect(output).not.to.include('db.coll.getShardDistribution');
494499
}
495500
});
501+
502+
it('includes collection names', async() => {
503+
if (!hasCollectionNames) return;
504+
const collname = `testcollection${Date.now()}${(Math.random() * 1000) | 0}`;
505+
input.write(`db.${collname}.insertOne({});\n`);
506+
await waitEval(cliRepl.bus);
507+
508+
output = '';
509+
input.write('db.testcoll\u0009\u0009');
510+
await waitCompletion(cliRepl.bus);
511+
expect(output).to.include(collname);
512+
513+
input.write(`db.${collname}.drop()\n`);
514+
await waitEval(cliRepl.bus);
515+
});
496516
});
497517
}
498518
});

packages/cli-repl/src/mongosh-repl.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ class MongoshNodeRepl implements EvaluationListener {
127127
(async() => await origReplCompleter(text) || [[]])(),
128128
(async() => await mongoshCompleter(text))()
129129
]);
130+
this.bus.emit('mongosh:autocompletion-complete'); // For testing.
130131
// Remove duplicates, because shell API methods might otherwise show
131132
// up in both completions.
132133
const deduped = [...new Set([...replResults, ...mongoshResults])];

packages/cli-repl/test/repl-helpers.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ async function waitEval(bus: MongoshBus) {
5353
await tick();
5454
}
5555

56+
async function waitCompletion(bus: MongoshBus) {
57+
await waitBus(bus, 'mongosh:autocompletion-complete');
58+
await tick();
59+
}
60+
5661
const fakeTTYProps = {
5762
isTTY: true,
5863
isRaw: true,
@@ -74,6 +79,7 @@ export {
7479
tick,
7580
waitBus,
7681
waitEval,
82+
waitCompletion,
7783
fakeTTYProps,
7884
readReplLogfile
7985
};

packages/shell-api/src/database.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ export default class Database extends ShellApiClass {
5353
_collections: Record<string, Collection>;
5454
_baseOptions: CommandOperationOptions;
5555
_session: Session | undefined;
56+
_cachedCollectionNames: string[] = [];
5657

5758
constructor(mongo: Mongo, name: string, session?: Session) {
5859
super();
@@ -138,6 +139,30 @@ export default class Database extends ShellApiClass {
138139
);
139140
}
140141

142+
async _getCollectionNames(): Promise<string[]> {
143+
const infos = await this._listCollections({}, { nameOnly: true });
144+
this._cachedCollectionNames = infos.map((collection: any) => collection.name);
145+
return this._cachedCollectionNames;
146+
}
147+
148+
async _getCollectionNamesForCompletion(): Promise<string[]> {
149+
return await Promise.race([
150+
(async() => {
151+
return await this._getCollectionNames();
152+
})(),
153+
(async() => {
154+
// 200ms should be a good compromise between giving the server a chance
155+
// to reply and responsiveness for human perception. It's not the end
156+
// of the world if we end up using the cached results; usually, they
157+
// are not going to differ from fresh ones, and even if they do, a
158+
// subsequent autocompletion request will almost certainly have at least
159+
// the new cached results.
160+
await new Promise(resolve => setTimeout(resolve, 200).unref());
161+
return this._cachedCollectionNames;
162+
})()
163+
]);
164+
}
165+
141166
async _getLastErrorObj(w?: number|string, wTimeout?: number, j?: boolean): Promise<Document> {
142167
const cmd = { getlasterror: 1 } as any;
143168
if (w) {
@@ -177,8 +202,7 @@ export default class Database extends ShellApiClass {
177202
@returnsPromise
178203
async getCollectionNames(): Promise<string[]> {
179204
this._emitDatabaseApiCall('getCollectionNames');
180-
const infos = await this._listCollections({}, { nameOnly: true });
181-
return infos.map((collection: any) => collection.name);
205+
return this._getCollectionNames();
182206
}
183207

184208
/**

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1708,12 +1708,22 @@ describe('Shell API (integration)', function() {
17081708
});
17091709

17101710
describe('getAutocompleteParameters', () => {
1711+
beforeEach(async() => {
1712+
// Make sure the collection is present so it is included in autocompletion.
1713+
await collection.insertOne({});
1714+
// Make sure 'database' is the current db in the eyes of the internal state object.
1715+
internalState.setDbFunc(database);
1716+
});
1717+
17111718
it('returns information that is meaningful for autocompletion', async() => {
17121719
const params = await internalState.getAutocompleteParameters();
17131720
expect(params.topology()).to.equal(Topologies.Standalone);
17141721
expect(params.connectionInfo().uri).to.equal(await testServer.connectionString());
17151722
expect(params.connectionInfo().is_atlas).to.equal(false);
17161723
expect(params.connectionInfo().is_localhost).to.equal(true);
1724+
expect(await database._getCollectionNames()).to.deep.equal(['docs']);
1725+
expect(await params.getCollectionCompletionsForCurrentDb('d')).to.deep.equal(['docs']);
1726+
expect(await params.getCollectionCompletionsForCurrentDb('e')).to.deep.equal([]);
17171727
});
17181728
});
17191729
});

packages/shell-api/src/shell-internal-state.spec.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,11 @@ describe('ShellInternalState', () => {
4646
expect(() => run('db = 42')).to.throw("[COMMON-10002] Cannot reassign 'db' to non-Database type");
4747
});
4848

49-
it('allows setting db to a db', async() => {
49+
it('allows setting db to a db and causes prefetching', async() => {
50+
serviceProvider.listCollections
51+
.resolves([ { name: 'coll1' }, { name: 'coll2' } ]);
5052
expect(run('db = db.getSiblingDB("moo"); db.getName()')).to.equal('moo');
53+
expect(serviceProvider.listCollections.calledWith('moo', {}, { nameOnly: true })).to.equal(true);
5154
});
5255
});
5356
});

0 commit comments

Comments
 (0)