Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(playground): Add clusteredIndex option to createCollection playground template VSCODE-330 #409

Merged
merged 2 commits into from
Apr 27, 2022
Merged

feat(playground): Add clusteredIndex option to createCollection playground template VSCODE-330 #409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d66bffe
Add clusteredIndex to createCollection playground template, remove ti…
Anemy Apr 13, 2022
00adebb
Add docs link to createCollection template
Anemy Apr 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions src/editors/playgroundController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as vscode from 'vscode';
import vm from 'vm';
import semver from 'semver';

import ActiveConnectionCodeLensProvider from './activeConnectionCodeLensProvider';
import CodeActionProvider from './codeActionProvider';
Expand All @@ -15,7 +14,6 @@ import { LanguageServerController } from '../language';
import { OutputChannel, ProgressLocation, TextEditor } from 'vscode';
import playgroundCreateIndexTemplate from '../templates/playgroundCreateIndexTemplate';
import playgroundCreateCollectionTemplate from '../templates/playgroundCreateCollectionTemplate';
import playgroundCreateCollectionWithTSTemplate from '../templates/playgroundCreateCollectionWithTSTemplate';
import {
PlaygroundResult,
ShellExecuteAllResult,
Expand All @@ -35,16 +33,6 @@ import TelemetryService from '../telemetry/telemetryService';
const log = createLogger('playground controller');
const transpiler = require('bson-transpilers');

const MIN_TIME_SERIES_SERVER_VERSION = '5.0.0-alpha0';

const hasTimeSeriesSupport = (serverVersion) => {
try {
return semver.gte(serverVersion, MIN_TIME_SERIES_SERVER_VERSION);
} catch (e) {
return true;
}
};

interface ToCompile {
filter?: string;
aggregation?: string;
Expand Down Expand Up @@ -241,17 +229,8 @@ export default class PlaygroundController {
async createPlaygroundForCreateCollection(
element: ConnectionTreeItem | DatabaseTreeItem
): Promise<boolean> {
const dataService = this._connectionController.getActiveDataService();
let content = playgroundCreateCollectionTemplate;

if (dataService) {
const instance = await dataService.instance();

if (hasTimeSeriesSupport(instance.build.version)) {
content = playgroundCreateCollectionWithTSTemplate;
}
}

element.cacheIsUpToDate = false;

if (element instanceof DatabaseTreeItem) {
Expand Down
12 changes: 11 additions & 1 deletion src/templates/playgroundCreateCollectionTemplate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,19 @@ db.createCollection(collection);
viewOn: <string>,
pipeline: <pipeline>,
collation: <document>,
writeConcern: <document>
writeConcern: <document>,
timeseries: { // Added in MongoDB 5.0
timeField: <string>, // required for time series collections
metaField: <string>,
granularity: <string>
},
expireAfterSeconds: <number>,
clusteredIndex: <document>, // Added in MongoDB 5.3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see for timeseries we list some options but for clusteredIndex and a few others we just say <document>. Wondering if we should put more stuff in there? Maybe best like this so they have to read the documentation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're following the docs there: https://www.mongodb.com/docs/v5.3/reference/method/db.createCollection/
Which also only expands the time-series document. I'm not totally sure on the reasoning there. I think it's alright though, since these are more complex configurations I think expanding all of them might make us have to update this more frequently, and could possibly lead some users astray on differing mongodb server versions.

}
)*/
// More information on the \`createCollection\` command can be found at:
// https://www.mongodb.com/docs/manual/reference/method/db.createCollection/
`;

export default template;
45 changes: 0 additions & 45 deletions src/templates/playgroundCreateCollectionWithTSTemplate.ts
View file
Open in desktop

This file was deleted.

146 changes: 3 additions & 143 deletions src/test/suite/mdbExtensionController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ suite('MDBExtensionController Test Suite', function () {
.then(done, done);
});

test('mdb.addDatabase should create a MongoDB playground with create collection template without time-series', async () => {
test('mdb.addDatabase should create a MongoDB playground with create collection template', async () => {
const mockTreeItem = new ConnectionTreeItem(
'tasty_sandwhich',
vscode.TreeItemCollapsibleState.None,
Expand All @@ -592,74 +592,6 @@ suite('MDBExtensionController Test Suite', function () {
{}
);

const mockGetActiveDataService: any = sinon.fake.returns({
instance: () => Promise.resolve({
dataLake: {},
build: { version: '4.9.7' },
genuineMongoDB: {},
host: {}
}),
createCollection: (namespace, options, callback) => {
callback(null);
}
});
sinon.replace(
mdbTestExtension.testExtensionController._connectionController,
'getActiveDataService',
mockGetActiveDataService
);
const mockActiveConnectionId: any = sinon.fake.returns('tasty_sandwhich');
sinon.replace(
mdbTestExtension.testExtensionController._connectionController,
'getActiveConnectionId',
mockActiveConnectionId
);

const mockOpenTextDocument: any = sinon.fake.resolves('untitled');
sinon.replace(vscode.workspace, 'openTextDocument', mockOpenTextDocument);

const mockShowTextDocument: any = sinon.fake();
sinon.replace(vscode.window, 'showTextDocument', mockShowTextDocument);

await vscode.commands.executeCommand('mdb.addDatabase', mockTreeItem);

assert(mockOpenTextDocument.firstArg.language === 'mongodb');
assert(
mockOpenTextDocument.firstArg.content.includes(
'// Create a new database.'
)
);
assert(mockOpenTextDocument.firstArg.content.includes('NEW_DATABASE_NAME'));
assert(mockOpenTextDocument.firstArg.content.includes('NEW_COLLECTION_NAME'));
assert(!mockOpenTextDocument.firstArg.content.includes('time-series'));
});

test('mdb.addDatabase should create a MongoDB playground with create collection template with time-series', async () => {
const mockTreeItem = new ConnectionTreeItem(
'tasty_sandwhich',
vscode.TreeItemCollapsibleState.None,
false,
mdbTestExtension.testExtensionController._connectionController,
false,
{}
);

const mockGetActiveDataService: any = sinon.fake.returns({
instance: () => Promise.resolve({
dataLake: {},
build: { version: '5.0.1' },
genuineMongoDB: {},
host: {}
}),
createCollection: (namespace, options, callback) => {
callback(null);
}
});
sinon.replace(
mdbTestExtension.testExtensionController._connectionController,
'getActiveDataService',
mockGetActiveDataService
);
const mockActiveConnectionId: any = sinon.fake.returns('tasty_sandwhich');
sinon.replace(
mdbTestExtension.testExtensionController._connectionController,
Expand All @@ -683,7 +615,6 @@ suite('MDBExtensionController Test Suite', function () {
);
assert(mockOpenTextDocument.firstArg.content.includes('NEW_DATABASE_NAME'));
assert(mockOpenTextDocument.firstArg.content.includes('NEW_COLLECTION_NAME'));
assert(mockOpenTextDocument.firstArg.content.includes('time-series'));
});

test('mdb.addDatabase command fails when disconnecting', (done) => {
Expand Down Expand Up @@ -782,31 +713,15 @@ suite('MDBExtensionController Test Suite', function () {
.then(done, done);
});

test('mdb.addCollection should create a MongoDB playground with create collection template without time-series', async () => {
const mockGetActiveDataService: any = sinon.fake.returns({
instance: () => Promise.resolve({
dataLake: {},
build: { version: '4.9.7' },
genuineMongoDB: {},
host: {}
}),
createCollection: (namespace, options, callback) => {
callback(null);
}
});
test('mdb.addCollection should create a MongoDB playground with create collection template', async () => {
const mockTreeItem = new DatabaseTreeItem(
'iceCreamDB',
mockGetActiveDataService,
{},
false,
false,
{}
);

sinon.replace(
mdbTestExtension.testExtensionController._connectionController,
'getActiveDataService',
mockGetActiveDataService
);
const mockActiveConnectionId: any = sinon.fake.returns('tasty_sandwhich');
sinon.replace(
mdbTestExtension.testExtensionController._connectionController,
Expand All @@ -833,61 +748,6 @@ suite('MDBExtensionController Test Suite', function () {
assert(!mockOpenTextDocument.firstArg.content.includes('time-series'));
});

test('mdb.addCollection should create a MongoDB playground with create collection template with time-series', async () => {
const mockTreeItem = new DatabaseTreeItem(
'iceCreamDB',
{
createCollection: (namespace, options, callback): void => {
callback(null);
}
},
false,
false,
{}
);

const mockGetActiveDataService: any = sinon.fake.returns({
instance: () => Promise.resolve({
dataLake: {},
build: { version: '5.0.1' },
genuineMongoDB: {},
host: {}
}),
createCollection: (namespace, options, callback) => {
callback(null);
}
});
sinon.replace(
mdbTestExtension.testExtensionController._connectionController,
'getActiveDataService',
mockGetActiveDataService
);
const mockActiveConnectionId: any = sinon.fake.returns('tasty_sandwhich');
sinon.replace(
mdbTestExtension.testExtensionController._connectionController,
'getActiveConnectionId',
mockActiveConnectionId
);

const mockOpenTextDocument: any = sinon.fake.resolves('untitled');
sinon.replace(vscode.workspace, 'openTextDocument', mockOpenTextDocument);

const mockShowTextDocument: any = sinon.fake();
sinon.replace(vscode.window, 'showTextDocument', mockShowTextDocument);

await vscode.commands.executeCommand('mdb.addCollection', mockTreeItem);

assert(mockOpenTextDocument.firstArg.language === 'mongodb');
assert(
mockOpenTextDocument.firstArg.content.includes(
'// The current database to use.'
)
);
assert(mockOpenTextDocument.firstArg.content.includes('iceCreamDB'));
assert(mockOpenTextDocument.firstArg.content.includes('NEW_COLLECTION_NAME'));
assert(mockOpenTextDocument.firstArg.content.includes('time-series'));
});

test('mdb.addCollection command fails when disconnecting', (done) => {
const mockTreeItem = new DatabaseTreeItem(
'iceCreamDB',
Expand Down