Skip to content

Commit 1c09841

Browse files
AnemyAnemy
authored
fix(data-explorer): opening documents with _ids with slashes VSCODE-276 (#342)
1 parent e966091 commit 1c09841

File tree

5 files changed

+187
-58
lines changed

5 files changed

+187
-58
lines changed

‎playgrounds/different-id-types.mongodb

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Playground for seeding the `mongodbVSCodePlaygroundDB.idTypeTesting` with
2+
// documents that have different kinds of ids.
3+
4+
const databaseName = 'mongodbVSCodePlaygroundDB';
5+
const collectionName = 'idTypeTesting';
6+
7+
use(databaseName);
8+
9+
db[collectionName].insertOne({
10+
description: 'auto generated default object id'
11+
});
12+
13+
db[collectionName].insertOne({
14+
_id: ObjectId(),
15+
description: 'object id'
16+
});
17+
18+
db[collectionName].insertOne({
19+
_id: 'testString',
20+
description: 'string'
21+
});
22+
23+
db[collectionName].insertOne({
24+
_id: 123,
25+
description: 'number'
26+
});
27+
28+
db[collectionName].insertOne({
29+
_id: {
30+
name: 'aaa'
31+
},
32+
description: 'object'
33+
});
34+
35+
db[collectionName].insertOne({
36+
_id: 'abc//\\\nab c$%@1s df',
37+
description: 'string with special characters'
38+
});
39+
40+
db[collectionName].insertOne({
41+
_id: {
42+
name: 'abc//\\\nab c$%@1s df',
43+
2: 3
44+
},
45+
description: 'object with a string with special characters'
46+
});
47+
48+
db[collectionName].insertOne({
49+
_id: new Date(),
50+
description: 'date'
51+
});
52+
53+
db[collectionName].insertOne({
54+
_id: Binary('pineapple'),
55+
description: 'binary'
56+
});

‎playgrounds/index-types.mongodb

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,12 @@
1-
// MongoDB Playground
2-
// To disable this template go to Settings | MongoDB | Use Default Template For Playground.
3-
// Make sure you are connected to enable completions and to be able to run a playground.
4-
// Use Ctrl+Space inside a snippet or a string literal to trigger completions.
1+
// Playground for seeding the `mongodbVSCodePlaygroundDB.index-testing` with
2+
// different types of indexes.
53

6-
// Select the database to use.
74
use('mongodbVSCodePlaygroundDB');
85

96
db['index-testing'].insertOne({
107
'title': 'there and back again'
118
});
129

13-
// Insert a few documents into the sales collection.
1410
db['index-testing'].createIndex({
1511
'fieldAscending': 1,
1612
'fieldDescending': -1

‎src/editors/editorsController.ts

Lines changed: 61 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,51 @@ import TelemetryService from '../telemetry/telemetryService';
3232

3333
const log = createLogger('editors controller');
3434

35+
export function getFileDisplayNameForDocument(
36+
documentId: EJSON.SerializableTypes,
37+
namespace: string
38+
) {
39+
let displayName = `${namespace}:${EJSON.stringify(documentId)}`;
40+
41+
// Encode special file uri characters to ensure VSCode handles
42+
// it correctly in a uri while avoiding collisions.
43+
displayName = displayName.replace(/[\\/%]/gi, function(c) {
44+
return `%${c.charCodeAt(0).toString(16)}`;
45+
});
46+
47+
displayName = displayName.length > 200
48+
? displayName.substring(0, 200)
49+
: displayName;
50+
51+
return displayName;
52+
}
53+
54+
export function getViewCollectionDocumentsUri(
55+
operationId: string,
56+
namespace: string,
57+
connectionId: string
58+
): vscode.Uri {
59+
// We attach a unique id to the query so that it creates a new file in
60+
// the editor and so that we can virtually manage the amount of docs shown.
61+
const operationIdUriQuery = `${OPERATION_ID_URI_IDENTIFIER}=${operationId}`;
62+
const connectionIdUriQuery = `${CONNECTION_ID_URI_IDENTIFIER}=${connectionId}`;
63+
const namespaceUriQuery = `${NAMESPACE_URI_IDENTIFIER}=${namespace}`;
64+
const uriQuery = `?${namespaceUriQuery}&${connectionIdUriQuery}&${operationIdUriQuery}`;
65+
66+
// Encode special file uri characters to ensure VSCode handles
67+
// it correctly in a uri while avoiding collisions.
68+
const namespaceDisplayName = encodeURIComponent(
69+
namespace.replace(/[\\/%]/gi, function(c) {
70+
return `%${c.charCodeAt(0).toString(16)}`;
71+
})
72+
);
73+
74+
// The part of the URI after the scheme and before the query is the file name.
75+
return vscode.Uri.parse(
76+
`${VIEW_COLLECTION_SCHEME}:Results: ${namespaceDisplayName}.json${uriQuery}`
77+
);
78+
}
79+
3580
/**
3681
* This controller manages when our extension needs to open
3782
* new editors and the data they need. It also manages active editors.
@@ -108,14 +153,6 @@ export default class EditorsController {
108153

109154
async openMongoDBDocument(data: EditDocumentInfo): Promise<boolean> {
110155
try {
111-
let fileDocumentId = EJSON.stringify(data.documentId);
112-
113-
fileDocumentId =
114-
fileDocumentId.length > 50
115-
? fileDocumentId.substring(0, 50)
116-
: fileDocumentId;
117-
118-
const fileName = `${VIEW_DOCUMENT_SCHEME}:/${data.namespace}:${fileDocumentId}.json`;
119156
const mdbDocument = (await this._mongoDBDocumentService.fetchDocument(
120157
data
121158
)) as EJSON.SerializableTypes;
@@ -128,19 +165,27 @@ export default class EditorsController {
128165
return false;
129166
}
130167

131-
this._saveDocumentToMemoryFileSystem(fileName, mdbDocument);
132-
133168
const activeConnectionId =
134169
this._connectionController.getActiveConnectionId() || '';
135170
const namespaceUriQuery = `${NAMESPACE_URI_IDENTIFIER}=${data.namespace}`;
136171
const connectionIdUriQuery = `${CONNECTION_ID_URI_IDENTIFIER}=${activeConnectionId}`;
137172
const documentIdReference = this._documentIdStore.add(data.documentId);
138173
const documentIdUriQuery = `${DOCUMENT_ID_URI_IDENTIFIER}=${documentIdReference}`;
139174
const documentSourceUriQuery = `${DOCUMENT_SOURCE_URI_IDENTIFIER}=${data.source}`;
140-
const uri: vscode.Uri = vscode.Uri.parse(fileName).with({
175+
176+
const fileTitle = encodeURIComponent(getFileDisplayNameForDocument(
177+
data.documentId,
178+
data.namespace
179+
));
180+
const fileName = `${VIEW_DOCUMENT_SCHEME}:/${fileTitle}.json`;
181+
182+
const fileUri = vscode.Uri.parse(fileName, true).with({
141183
query: `?${namespaceUriQuery}&${connectionIdUriQuery}&${documentIdUriQuery}&${documentSourceUriQuery}`
142184
});
143-
const document = await vscode.workspace.openTextDocument(uri);
185+
186+
this._saveDocumentToMemoryFileSystem(fileUri, mdbDocument);
187+
188+
const document = await vscode.workspace.openTextDocument(fileUri);
144189

145190
await vscode.window.showTextDocument(document, { preview: false });
146191

@@ -212,31 +257,13 @@ export default class EditorsController {
212257
}
213258
}
214259

215-
static getViewCollectionDocumentsUri(
216-
operationId: string,
217-
namespace: string,
218-
connectionId: string
219-
): vscode.Uri {
220-
// We attach a unique id to the query so that it creates a new file in
221-
// the editor and so that we can virtually manage the amount of docs shown.
222-
const operationIdUriQuery = `${OPERATION_ID_URI_IDENTIFIER}=${operationId}`;
223-
const connectionIdUriQuery = `${CONNECTION_ID_URI_IDENTIFIER}=${connectionId}`;
224-
const namespaceUriQuery = `${NAMESPACE_URI_IDENTIFIER}=${namespace}`;
225-
const uriQuery = `?${namespaceUriQuery}&${connectionIdUriQuery}&${operationIdUriQuery}`;
226-
227-
// The part of the URI after the scheme and before the query is the file name.
228-
return vscode.Uri.parse(
229-
`${VIEW_COLLECTION_SCHEME}:Results: ${namespace}.json${uriQuery}`
230-
);
231-
}
232-
233260
async onViewCollectionDocuments(namespace: string): Promise<boolean> {
234261
log.info('view collection documents', namespace);
235262

236263
const operationId = this._collectionDocumentsOperationsStore.createNewOperation();
237264
const activeConnectionId =
238265
this._connectionController.getActiveConnectionId() || '';
239-
const uri = EditorsController.getViewCollectionDocumentsUri(
266+
const uri = getViewCollectionDocumentsUri(
240267
operationId,
241268
namespace,
242269
activeConnectionId
@@ -294,7 +321,7 @@ export default class EditorsController {
294321
);
295322
}
296323

297-
const uri = EditorsController.getViewCollectionDocumentsUri(
324+
const uri = getViewCollectionDocumentsUri(
298325
operationId,
299326
namespace,
300327
connectionId
@@ -311,11 +338,11 @@ export default class EditorsController {
311338
}
312339

313340
_saveDocumentToMemoryFileSystem(
314-
fileName: string,
341+
fileUri: vscode.Uri,
315342
document: EJSON.SerializableTypes
316343
): void {
317344
this._memoryFileSystemProvider.writeFile(
318-
vscode.Uri.parse(fileName),
345+
fileUri,
319346
Buffer.from(JSON.stringify(document, null, 2)),
320347
{ create: true, overwrite: true }
321348
);

‎src/test/suite/editors/editorsController.test.ts

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,12 @@ import assert from 'assert';
44
import chai from 'chai';
55
import { mockTextEditor } from '../stubs';
66
import sinon from 'sinon';
7+
import { ObjectId } from 'bson';
78

8-
import { EditorsController } from '../../../editors';
9+
import {
10+
getFileDisplayNameForDocument,
11+
getViewCollectionDocumentsUri
12+
} from '../../../editors/editorsController';
913

1014
const expect = chai.expect;
1115

@@ -17,28 +21,72 @@ suite('Editors Controller Test Suite', () => {
1721
sinon.restore();
1822
});
1923

24+
suite('#getFileDisplayNameForDocumentId', () => {
25+
test('it strips special characters from the document id', () => {
26+
const str = 'abc//\\\nab c"$%%..@1s df""';
27+
const result = getFileDisplayNameForDocument(str, 'a.b');
28+
const expected = 'a.b:"abc%2f%2f%5c%5c%5cnab c%5c"$%25%25..@1s df%5c"%5c""';
29+
assert.strictEqual(result, expected);
30+
});
31+
32+
test('it trims the string to 200 characters', () => {
33+
const str = '123sdfhadfbnjiekbfdakjsdbfkjsabdfkjasbdfkjsvasdjvbskdafdf123sdfhadfbnjiekbfdakjsdbfkjsabdfkjasbdfkjsvasdjvbskdafdffbnjiekbfdakjsdbfkjsabdfkjasbfbnjiekbfdakjsdbfkjsabdfkjasbkjasbfbnjiekbfdakjsdbfkjsabdfkjasb';
34+
const result = getFileDisplayNameForDocument(str, 'db.col');
35+
const expected = 'db.col:"123sdfhadfbnjiekbfdakjsdbfkjsabdfkjasbdfkjsvasdjvbskdafdf123sdfhadfbnjiekbfdakjsdbfkjsabdfkjasbdfkjsvasdjvbskdafdffbnjiekbfdakjsdbfkjsabdfkjasbfbnjiekbfdakjsdbfkjsabdfkjasbkjasbfbnjiekbfdakjsd';
36+
assert.strictEqual(result, expected);
37+
});
38+
39+
test('it handles ids that are objects', () => {
40+
const str = {
41+
str: 'abc//\\\nab c$%%..@1s df"',
42+
b: new ObjectId('5d973ae744376d2aae72a160')
43+
};
44+
const result = getFileDisplayNameForDocument(str, 'db.col');
45+
const expected = 'db.col:{"str":"abc%2f%2f%5c%5c%5cnab c$%25%25..@1s df%5c"","b":{"$oid":"5d973ae744376d2aae72a160"}}';
46+
assert.strictEqual(result, expected);
47+
});
48+
49+
test('has the namespace at the start of the display name', () => {
50+
const str = 'pineapples';
51+
const result = getFileDisplayNameForDocument(str, 'grilled');
52+
const expected = 'grilled:"pineapples"';
53+
assert.strictEqual(result, expected);
54+
});
55+
});
56+
2057
test('getViewCollectionDocumentsUri builds a uri from the namespace and connection info', () => {
2158
const testOpId = '100011011101110011';
2259
const testNamespace = 'myFavoriteNamespace';
2360
const testConnectionId = 'alienSateliteConnection';
24-
const testUri = EditorsController.getViewCollectionDocumentsUri(
61+
const testUri = getViewCollectionDocumentsUri(
2562
testOpId,
2663
testNamespace,
2764
testConnectionId
2865
);
2966

30-
assert(
31-
testUri.path === 'Results: myFavoriteNamespace.json',
32-
`Expected uri path ${testUri.path} to equal 'Results: myFavoriteNamespace.json'.`
67+
assert.strictEqual(testUri.path, 'Results: myFavoriteNamespace.json');
68+
assert.strictEqual(testUri.scheme, 'VIEW_COLLECTION_SCHEME');
69+
assert.strictEqual(
70+
testUri.query,
71+
'namespace=myFavoriteNamespace&connectionId=alienSateliteConnection&operationId=100011011101110011',
3372
);
34-
assert(
35-
testUri.scheme === 'VIEW_COLLECTION_SCHEME',
36-
`Expected uri scheme ${testUri.scheme} to equal 'VIEW_COLLECTION_SCHEME'.`
73+
});
74+
75+
test('getViewCollectionDocumentsUri handles / \\ and % in the namespace', () => {
76+
const testOpId = '100011011101110011';
77+
const testNamespace = 'myFa%%\\\\///\\%vorite%Namespace';
78+
const testConnectionId = 'alienSateliteConnection';
79+
const testUri = getViewCollectionDocumentsUri(
80+
testOpId,
81+
testNamespace,
82+
testConnectionId
3783
);
38-
assert(
39-
testUri.query ===
40-
'namespace=myFavoriteNamespace&connectionId=alienSateliteConnection&operationId=100011011101110011',
41-
`Expected uri query ${testUri.query} to equal 'namespace=myFavoriteNamespace&connectionId=alienSateliteConnection&operationId=100011011101110011'.`
84+
85+
assert.strictEqual(testUri.path, 'Results: myFa%25%25%5c%5c%2f%2f%2f%5c%25vorite%25Namespace.json');
86+
assert.strictEqual(testUri.scheme, 'VIEW_COLLECTION_SCHEME');
87+
assert.strictEqual(
88+
testUri.query,
89+
'namespace=myFa%%\\\\///\\%vorite%Namespace&connectionId=alienSateliteConnection&operationId=100011011101110011',
4290
);
4391
});
4492

‎src/test/suite/explorer/playgroundsExplorer.test.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,18 +54,20 @@ suite('Playgrounds Controller Test Suite', function () {
5454
try {
5555
const children = await treeController.getPlaygrounds(rootUri);
5656

57-
assert(
58-
Object.keys(children).length === 4,
59-
`Tree playgrounds should have 4 child, found ${children.length}`
57+
assert.strictEqual(
58+
Object.keys(children).length,
59+
5,
60+
`Tree playgrounds should have 5 child, found ${children.length}`
6061
);
6162

6263
const playgrounds = Object.values(children).filter(
6364
(item: any) => item.label && item.label.split('.').pop() === 'mongodb'
6465
);
6566

66-
assert(
67-
Object.keys(playgrounds).length === 4,
68-
`Tree playgrounds should have 4 playgrounds with mongodb extension, found ${children.length}`
67+
assert.strictEqual(
68+
Object.keys(playgrounds).length,
69+
5,
70+
`Tree playgrounds should have 5 playgrounds with mongodb extension, found ${children.length}`
6971
);
7072
} catch (error) {
7173
assert(false, error);

0 commit comments

Comments
 (0)