fix(VSCODE-118): Enable opening documents with binary _id from tree v…

…iew (#213)
mongodb-js · Nov 23, 2020 · 26f7eb7 · 26f7eb7
1 parent 3320243
commit 26f7eb7
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 28 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -827,7 +827,7 @@
     "@mongosh/service-provider-server": "^0.5.2",
     "@mongosh/shell-api": "^0.5.2",
     "analytics-node": "^3.4.0-beta.1",
-    "bson": "^4.0.3",
+    "bson": "^4.2.0",
     "classnames": "^2.2.6",
     "debug": "^4.1.1",
     "dotenv": "^8.2.0",

diff --git a/src/editors/documentProvider.ts b/src/editors/documentProvider.ts
@@ -36,7 +36,9 @@ export default class DocumentViewProvider implements vscode.TextDocumentContentP
       const documentIdEJSONString = decodeURIComponent(
         uriParams.get(DOCUMENT_ID_URI_IDENTIFIER) || ''
       );
-      const documentId = EJSON.parse(documentIdEJSONString).value;
+
+      const jsonObjectId = JSON.parse(documentIdEJSONString).value;
+      const documentId = EJSON.deserialize(jsonObjectId);
 
       // Ensure we're still connected to the correct connection.
       if (connectionId !== this._connectionController.getActiveConnectionId()) {
@@ -77,7 +79,7 @@ export default class DocumentViewProvider implements vscode.TextDocumentContentP
           }
 
           if (!documents || documents.length === 0) {
-            const errorMessage = `Unable to find document: ${documentId}`;
+            const errorMessage = `Unable to find document: ${JSON.stringify(jsonObjectId)}`;
             vscode.window.showErrorMessage(errorMessage);
             return reject(new Error(errorMessage));
           }

diff --git a/src/editors/editorsController.ts b/src/editors/editorsController.ts
@@ -99,15 +99,15 @@ export default class EditorsController {
     log.info('activated.');
   }
 
-  onViewDocument(namespace: string, documentId: any): Promise<boolean> {
+  onViewDocument(namespace: string, documentId: EJSON.SerializableTypes): Promise<boolean> {
     log.info('view document in editor', namespace);
 
     const connectionId = this._connectionController.getActiveConnectionId();
     const connectionIdUriQuery = `${CONNECTION_ID_URI_IDENTIFIER}=${connectionId}`;
     // Encode the _id field incase the document id is a custom string with
     // special characters.
-    const documentIdString = EJSON.stringify({
-      value: documentId
+    const documentIdString = JSON.stringify({
+      value: EJSON.serialize(documentId)
     });
 
     const documentIdUriQuery = `${DOCUMENT_ID_URI_IDENTIFIER}=${encodeURIComponent(

diff --git a/src/explorer/documentTreeItem.ts b/src/explorer/documentTreeItem.ts
@@ -1,3 +1,4 @@
+import { EJSON } from 'bson';
 import * as vscode from 'vscode';
 
 export const DOCUMENT_ITEM = 'documentTreeItem';
@@ -6,11 +7,9 @@ export default class DocumentTreeItem extends vscode.TreeItem
   implements vscode.TreeDataProvider<DocumentTreeItem> {
   contextValue = DOCUMENT_ITEM;
 
-  private _documentLabel: string;
-
   namespace: string;
   document: any;
-  documentId: string;
+  documentId: EJSON.SerializableTypes;
 
   constructor(document: any, namespace: string, documentIndexInTree: number) {
     // A document can not have a `_id` when it is in a view. In this instance
@@ -25,7 +24,6 @@ export default class DocumentTreeItem extends vscode.TreeItem
     const documentLabel = document._id
       ? JSON.stringify(document._id)
       : `Document ${documentIndexInTree + 1}`;
-    this._documentLabel = documentLabel;
 
     this.document = document;
     this.documentId = document._id;

diff --git a/src/test/suite/editors/documentProvider.test.ts b/src/test/suite/editors/documentProvider.test.ts
@@ -16,7 +16,9 @@ import {
 } from '../dbTestHelper';
 import {
   documentWithAllBSONTypes,
-  documentWithAllBsonTypesJsonified
+  documentWithAllBsonTypesJsonified,
+  documentWithBinaryId,
+  documentWithBinaryIdString
 } from './documentStringFixtures';
 
 const mockDocumentAsJsonString = `{
@@ -139,7 +141,7 @@ suite('Document Provider Test Suite', () => {
       .then((document) => {
         assert(
           document === mockDocumentAsJsonString,
-          `Expected provideTextDocumentContent to return ejson stringified string, found ${document}`
+          `Expected provideTextDocumentContent to return json stringified string, found ${document}`
         );
         done();
       })
@@ -212,7 +214,9 @@ suite('Document Provider Test Suite', () => {
     });
 
     test('handles displaying a document with all bson types', (done) => {
-      seedDataAndCreateDataService('ramen', [documentWithAllBSONTypes]).then(
+      seedDataAndCreateDataService('ramen', [
+        documentWithAllBSONTypes
+      ]).then(
         (dataService) => {
           const mockExtensionContext = new TestExtensionContext();
           const mockStorageController = new StorageController(
@@ -236,7 +240,7 @@ suite('Document Provider Test Suite', () => {
           );
 
           const documentId = EJSON.stringify({
-            value: documentWithAllBSONTypes._id
+            value: (documentWithAllBSONTypes as any)._id
           });
           const uri = vscode.Uri.parse(
             `scheme:Results: filename.json?namespace=${TEST_DB_NAME}.ramen&documentId=${documentId}`
@@ -247,7 +251,7 @@ suite('Document Provider Test Suite', () => {
             .then((document) => {
               assert(
                 document === documentWithAllBsonTypesJsonified,
-                `Expected provideTextDocumentContent to return ejson stringified string, found ${document}`
+                `Expected provideTextDocumentContent to return json stringified string, found ${document}`
               );
               done();
             })
@@ -256,6 +260,46 @@ suite('Document Provider Test Suite', () => {
       );
     });
 
+    test('can find a doc with a binary _id', async () => {
+      const dataService = await seedDataAndCreateDataService('ramen', [
+        documentWithBinaryId
+      ]);
+      const mockExtensionContext = new TestExtensionContext();
+      const mockStorageController = new StorageController(
+        mockExtensionContext
+      );
+      const testTelemetryController = new TelemetryController(
+        mockStorageController,
+        mockExtensionContext
+      );
+      const mockConnectionController = new ConnectionController(
+        new StatusView(mockExtensionContext),
+        mockStorageController,
+        testTelemetryController
+      );
+
+      mockConnectionController.setActiveConnection(dataService);
+
+      const testCollectionViewProvider = new DocumentProvider(
+        mockConnectionController,
+        new StatusView(mockExtensionContext)
+      );
+
+      const documentId = EJSON.stringify({
+        value: documentWithBinaryId._id
+      });
+      const uri = vscode.Uri.parse(
+        `scheme:Results: filename.json?namespace=${TEST_DB_NAME}.ramen&documentId=${documentId}`
+      );
+
+      const document = await testCollectionViewProvider
+        .provideTextDocumentContent(uri);
+      assert(
+        document === documentWithBinaryIdString,
+        `Expected provideTextDocumentContent to return json stringified string ${documentWithBinaryIdString}, found ${document}`
+      );
+    });
+
     test('expected provideTextDocumentContent to handle an id that is an object id', (done) => {
       const mockDocument = {
         _id: new ObjectId('5e32b4d67bf47f4525f2f8ab'),
@@ -297,7 +341,7 @@ suite('Document Provider Test Suite', () => {
             .then((document) => {
               assert(
                 document === docAsString2,
-                `Expected provideTextDocumentContent to return ejson stringified string, found ${document}`
+                `Expected provideTextDocumentContent to return json stringified string, found ${document}`
               );
               done();
             })
@@ -347,7 +391,7 @@ suite('Document Provider Test Suite', () => {
             .then((document) => {
               assert(
                 document === docAsString3,
-                `Expected provideTextDocumentContent to return ejson stringified string, found ${document}`
+                `Expected provideTextDocumentContent to return json stringified string, found ${document}`
               );
               done();
             })

diff --git a/src/test/suite/editors/documentStringFixtures.ts b/src/test/suite/editors/documentStringFixtures.ts
@@ -1,4 +1,4 @@
-import { EJSON } from 'bson';
+import { Binary, EJSON } from 'bson';
 
 const docString =
   '{"_id":{"$oid":"57e193d7a9cc81b4027498b5"},"Symbol":"symbol","String":"string","Int32":{"$numberInt":"42"},"Int64":{"$numberLong":"42"},"Double":{"$numberDouble":"-1"},"Binary":{"$binary":{"base64":"o0w498Or7cijeBSpkquNtg==","subType":"03"}},"BinaryUserDefined":{"$binary":{"base64":"AQIDBAU=","subType":"80"}},"Code":{"$code":"function() {}"},"CodeWithScope":{"$code":"function() {}","$scope":{}},"Subdocument":{"foo":"bar"},"Array":[{"$numberInt":"1"},{"$numberInt":"2"},{"$numberInt":"3"},{"$numberInt":"4"},{"$numberInt":"5"}],"Timestamp":{"$timestamp":{"t":42,"i":1}},"Regex":{"$regularExpression":{"pattern":"pattern","options":""}},"DatetimeEpoch":{"$date":{"$numberLong":"0"}},"DatetimePositive":{"$date":{"$numberLong":"2147483647"}},"DatetimeNegative":{"$date":{"$numberLong":"-2147483648"}},"True":true,"False":false,"DBPointer":{"$ref":"collection","$id":{"$oid":"57e193d7a9cc81b4027498b1"}},"DBRef":{"$ref":"collection","$id":{"$oid":"57fd71e96e32ab4225b723fb"},"$db":"database"},"Minkey":{"$minKey":1},"Maxkey":{"$maxKey":1},"Null":null,"Undefined":null}';
@@ -56,3 +56,12 @@ export const documentWithAllBsonTypesJsonified = `{
   "Null": null,
   "Undefined": null
 }`;
+
+export const documentWithBinaryId = {
+  _id: new Binary('aaa')
+};
+export const documentWithBinaryIdString = JSON.stringify(
+  documentWithBinaryId,
+  null,
+  2
+);