fix: prefer half-relaxed EJSON mode

Fix various EJSON handling bugs by aligning our EJSON handling a bit more closely. This fixes regressions introduced by me in e654e1d (COMPASS-5710, COMPASS-5711 aka #2960) and another long-standing export bug (COMPASS-4694). Specifically, use `{ relaxed: false }` when parsing values into `HadronDocument`, and use `{ relaxed: false }` but undo the strict conversion for `$numberInt` and `$numberDouble`. This is a mode that we may eventually want to upstream into the Node.js driver’s `bson` package.
mongodb-js · Apr 11, 2022 · f7e519d · f7e519d
1 parent 7c86421
commit f7e519d
Show file tree

Hide file tree

Showing 23 changed files with 272 additions and 37 deletions.
diff --git a/.../compass-aggregations/src/components/pipeline-results-workspace/pipeline-results-list.tsx b/.../compass-aggregations/src/components/pipeline-results-workspace/pipeline-results-list.tsx
@@ -10,7 +10,6 @@ import {
 import { DocumentListView, DocumentJsonView } from '@mongodb-js/compass-crud';
 import { useId } from '@react-aria/utils';
 import HadronDocument from 'hadron-document';
-import { EJSON } from 'bson';
 
 const viewTypeContainer = css({
   display: 'flex',
@@ -93,12 +92,7 @@ export const PipelineResultsList: React.FunctionComponent<
       docs: documents.map((doc) => new HadronDocument(doc)),
       isEditable: false,
       copyToClipboard(doc) {
-        const obj = doc.generateObject();
-        const str = EJSON.stringify(
-          obj as EJSON.SerializableTypes,
-          undefined,
-          2
-        );
+        const str = doc.toEJSON();
         void navigator.clipboard.writeText(str);
       },
     }),

diff --git a/packages/compass-crud/src/components/json-editor.jsx b/packages/compass-crud/src/components/json-editor.jsx
@@ -1,8 +1,8 @@
-import { EJSON } from 'bson';
 import React from 'react';
 import PropTypes from 'prop-types';
 import jsonParse from 'fast-json-parse';
 import { DocumentList } from '@mongodb-js/compass-components';
+import HadronDocument from 'hadron-document';
 import UpdateDocumentFooter from './document-footer';
 import RemoveDocumentFooter from './remove-document-footer';
 
@@ -96,7 +96,7 @@ class EditableJson extends React.Component {
   }
 
   _getObjectAsString() {
-    return EJSON.stringify(this.props.doc.generateObject(), null, 2);
+    return this.props.doc.toEJSON();
   }
 
   /**
@@ -290,7 +290,7 @@ class EditableJson extends React.Component {
         <UpdateDocumentFooter
           doc={this.props.doc}
           replaceDocument={() => {
-            this.props.doc.apply(EJSON.parse(this.state.value));
+            this.props.doc.apply(HadronDocument.FromEJSON(this.state.value));
             this.props.replaceDocument(this.props.doc);
           }}
           cancelHandler={this.handleCancel.bind(this)}

diff --git a/packages/compass-crud/src/stores/crud-store.js b/packages/compass-crud/src/stores/crud-store.js
@@ -1,4 +1,3 @@
-import { EJSON } from 'bson';
 import Reflux from 'reflux';
 import toNS from 'mongodb-ns';
 import { findIndex, isEmpty } from 'lodash';
@@ -379,7 +378,7 @@ const configureStore = (options = {}) => {
      */
     copyToClipboard(doc) {
       track('Document Copied', { mode: this.modeForTelemetry() });
-      const documentJSON = EJSON.stringify(doc.generateObject(), null, 2);
+      const documentJSON = doc.toEJSON();
       let input = document.createElement(INPUT);
       input.type = TYPE;
       input.setAttribute(STYLES, DISPLAY);
@@ -654,7 +653,7 @@ const configureStore = (options = {}) => {
         }
       }
 
-      const jsonDoc = EJSON.stringify(hadronDoc.generateObject(), null, 2);
+      const jsonDoc = hadronDoc.toEJSON();
 
       this.setState({
         insert: {
@@ -700,7 +699,7 @@ const configureStore = (options = {}) => {
      */
     toggleInsertDocument(view) {
       if (view === 'JSON') {
-        const jsonDoc = EJSON.stringify(this.state.insert.doc.generateObject(), null, 2);
+        const jsonDoc = this.state.insert.toEJSON();
 
         this.setState({
           insert: {
@@ -719,7 +718,7 @@ const configureStore = (options = {}) => {
         if (this.state.insert.jsonDoc === '') {
           hadronDoc = this.state.insert.doc;
         } else {
-          hadronDoc = new HadronDocument(EJSON.parse(this.state.insert.jsonDoc), false);
+          hadronDoc = HadronDocument.FromEJSON(this.state.insert.jsonDoc);
         }
 
         this.setState({
@@ -780,7 +779,9 @@ const configureStore = (options = {}) => {
      * Insert a single document.
      */
     insertMany() {
-      const docs = EJSON.parse(this.state.insert.jsonDoc);
+      const docs =
+        HadronDocument.FromEJSONArray(this.state.insert.jsonDoc)
+          .map(doc => doc.generateObject());
       track('Document Inserted', {
         mode: this.state.insert.jsonView ? 'json' : 'field-by-field',
         multiple: docs.length > 1
@@ -833,7 +834,7 @@ const configureStore = (options = {}) => {
       let doc;
 
       if (this.state.insert.jsonView) {
-        doc = EJSON.parse(this.state.insert.jsonDoc);
+        doc = HadronDocument.FromEJSON(this.state.insert.jsonDoc).generateObject();
       } else {
         doc = this.state.insert.doc.generateObject();
       }

diff --git a/packages/compass-e2e-tests/tests/collection-documents-tab.test.ts b/packages/compass-e2e-tests/tests/collection-documents-tab.test.ts
@@ -398,6 +398,53 @@ FindIterable<Document> result = collection.find(filter);`);
     );
   });
 
+  it('supports view/edit for Int64 values via json view', async function () {
+    await browser.runFindOperation('Documents', '{ i: 32 }');
+    await browser.clickVisible(Selectors.SelectJSONView);
+
+    const document = await browser.$(Selectors.DocumentJSONEntry);
+    await document.waitForDisplayed();
+
+    await waitForJSON(browser, document);
+
+    const json = await document.getText();
+    expect(json.replace(/\s+/g, ' ')).to.match(
+      /^\{ "_id": \{ "\$oid": "[a-f0-9]{24}" \}, "i": 32, "j": 0 \}$/
+    );
+
+    await browser.hover('[data-test-id="editable-json"]');
+    await browser.clickVisible('[data-testid="edit-document-button"]');
+
+    const newjson = JSON.stringify({
+      ...JSON.parse(json),
+      j: { $numberLong: '12345' },
+    });
+
+    await browser.setAceValue(
+      '[data-test-id="editable-json"] #ace-editor',
+      newjson
+    );
+
+    const footer = await document.$(Selectors.DocumentFooterMessage);
+    expect(await footer.getText()).to.equal('Document Modified.');
+
+    const button = await document.$('[data-test-id="update-document-button"]');
+    await button.click();
+    await footer.waitForDisplayed({ reverse: true });
+
+    await browser.runFindOperation('Documents', '{ i: 32 }');
+    await browser.clickVisible(Selectors.SelectJSONView);
+
+    const modifiedDocument = await browser.$(Selectors.DocumentJSONEntry);
+    await modifiedDocument.waitForDisplayed();
+
+    await waitForJSON(browser, modifiedDocument);
+
+    expect((await modifiedDocument.getText()).replace(/\s+/g, ' ')).to.match(
+      /^\{ "_id": \{ "\$oid": "[a-f0-9]{24}" \}, "i": 32, "j": \{\.\.\.\} \}$/
+    );
+  });
+
   it('supports view/edit via table view', async function () {
     await browser.runFindOperation('Documents', '{ i: 33 }');
     await browser.clickVisible(Selectors.SelectTableView);

diff --git a/packages/compass-e2e-tests/tests/collection-import.test.ts b/packages/compass-e2e-tests/tests/collection-import.test.ts
@@ -125,7 +125,10 @@ describe('Collection import', function () {
     await insertDialog.waitForDisplayed();
 
     // set the text in the editor
-    await browser.setAceValue(Selectors.InsertJSONEditor, '{ "foo": 10 }');
+    await browser.setAceValue(
+      Selectors.InsertJSONEditor,
+      '{ "foo": 10, "long": { "$numberLong": "99" } }'
+    );
 
     // confirm
     const insertConfirm = await browser.$(Selectors.InsertConfirm);
@@ -154,6 +157,7 @@ describe('Collection import', function () {
 
     expect(result).to.deep.equal({
       foo: '10',
+      long: '99',
     });
   });
 

diff --git a/packages/compass-import-export/package.json b/packages/compass-import-export/package.json
@@ -71,6 +71,7 @@
     "electron": "^13.5.1",
     "fast-csv": "^3.4.0",
     "flat": "cipacda/flat",
+    "hadron-document": "^7.9.0",
     "hadron-react-buttons": "^5.7.0",
     "hadron-react-components": "^5.13.0",
     "lodash": "^4.17.21",

diff --git a/packages/compass-import-export/src/utils/formatters.js b/packages/compass-import-export/src/utils/formatters.js
@@ -3,7 +3,7 @@
 /* eslint-disable complexity */
 
 import * as csv from 'fast-csv';
-import { EJSON } from 'bson';
+import HadronDocument from 'hadron-document';
 import { serialize as flatten } from './bson-csv';
 import { Transform } from 'stream';
 import { EOL } from 'os';
@@ -22,7 +22,9 @@ export const createJSONFormatter = function ({ brackets = true } = {}) {
           this.push(EOL);
         }
       }
-      const s = EJSON.stringify(doc, null, brackets ? 2 : null);
+      const s = new HadronDocument(doc).toEJSON('current', {
+        indent: brackets ? 2 : undefined,
+      });
       if (this._counter === undefined) {
         this._counter = 0;
         if (brackets) {

diff --git a/packages/compass-import-export/src/utils/formatters.spec.js b/packages/compass-import-export/src/utils/formatters.spec.js
@@ -1,6 +1,6 @@
 import { createJSONFormatter, createCSVFormatter } from './formatters';
 import stream from 'stream';
-import { EJSON, ObjectID, Binary } from 'bson';
+import { EJSON, ObjectID, Binary, Long } from 'bson';
 import { createCSVParser } from './import-parser';
 import fs from 'fs';
 import { promisify } from 'util';
@@ -90,6 +90,36 @@ describe('formatters', function () {
       });
     });
   });
+  describe('should format Longs correctly', function () {
+    afterEach(async function () {
+      try {
+        await rm(FIXTURES.JSON_MULTI_SMALL_DOCS);
+      } catch (e) {
+        // noop
+      }
+    });
+
+    it('works for input with Long data', async function () {
+      const long = Long.fromString('9007199254740993');
+
+      const docs = [
+        {
+          _id: new ObjectID('5e5ea7558d35931a05eafec0'),
+          test: long,
+        },
+      ];
+      const source = stream.Readable.from(docs);
+      const formatter = createJSONFormatter({ brackets: true });
+      const dest = fs.createWriteStream(FIXTURES.JSON_MULTI_SMALL_DOCS);
+
+      await pipeline(source, formatter, dest);
+
+      const contents = await readFile(FIXTURES.JSON_MULTI_SMALL_DOCS);
+      const parsed = EJSON.parse(contents, { relaxed: false });
+
+      expect(parsed).to.deep.equal(docs);
+    });
+  });
   describe('jsonl', function () {
     it('should support newline delimited ejson', function () {
       const docs = [

diff --git a/packages/hadron-document/src/document.ts b/packages/hadron-document/src/document.ts
@@ -2,9 +2,11 @@
 import type { Element } from './element';
 import { LinkedList, Events as ElementEvents } from './element';
 import EventEmitter from 'eventemitter3';
-import { UUID } from 'bson';
+import { EJSON, UUID } from 'bson';
 import ObjectGenerator from './object-generator';
 import type { BSONObject, BSONValue } from './utils';
+import { objectToIdiomaticEJSON } from './utils';
+import type { HadronEJSONOptions } from './utils';
 
 /**
  * The event constant.
@@ -59,7 +61,10 @@ export class Document extends EventEmitter {
     this.currentType = 'Document';
   }
 
-  apply(doc: BSONObject): void {
+  apply(doc: BSONObject | Document): void {
+    if (typeof doc?.generateObject === 'function') {
+      doc = doc.generateObject();
+    }
     const updatedKeys: (string | number)[] = [];
     let prevKey = null;
     for (const [key, value] of Object.entries(doc)) {
@@ -461,6 +466,41 @@ export class Document extends EventEmitter {
   static get Events(): typeof Events {
     return Events;
   }
+
+  /**
+   * Parse a new Document from extended JSON input.
+   */
+  static FromEJSON(input: string): Document {
+    const parsed = EJSON.parse(input, { relaxed: false });
+    return new Document(parsed as BSONObject);
+  }
+
+  /**
+   * Parse multiple Document from extended JSON input.
+   * If the input consists of only a single document without
+   * `[array]` brackets, return an array consisting of only
+   * that document.
+   */
+  static FromEJSONArray(input: string): Document[] {
+    const parsed = EJSON.parse(input, { relaxed: false });
+    return Array.isArray(parsed)
+      ? parsed.map((doc) => new Document(doc as BSONObject))
+      : [new Document(parsed as BSONObject)];
+  }
+
+  /**
+   * Convert this Document instance into a human-readable EJSON string.
+   */
+  toEJSON(
+    source: 'original' | 'current' = 'current',
+    options: HadronEJSONOptions = {}
+  ): string {
+    const obj =
+      source === 'original'
+        ? this.generateOriginalObject()
+        : this.generateObject();
+    return objectToIdiomaticEJSON(obj, options);
+  }
 }
 
 export default Document;
diff --git a/packages/hadron-document/src/utils.ts b/packages/hadron-document/src/utils.ts
@@ -1,3 +1,4 @@
+import { EJSON } from 'bson';
 import type { TypeCastMap, TypeCastTypes } from 'hadron-type-checker';
 
 export function fieldStringLen(value: unknown): number {
@@ -8,3 +9,60 @@ export function fieldStringLen(value: unknown): number {
 export type BSONObject = TypeCastMap['Object'];
 export type BSONArray = TypeCastMap['Array'];
 export type BSONValue = TypeCastMap[TypeCastTypes];
+
+export interface HadronEJSONOptions {
+  indent?: number | string;
+}
+
+/**
+ * Turn a BSON value into what we consider idiomatic extended JSON.
+ *
+ * This differs from both the relaxed and strict mode of the 'bson'
+ * package's EJSON class: We preserve the type information for longs
+ * via $numberLong, but redact it for $numberInt and $numberDouble.
+ *
+ * This may seem inconsistent, but considering that the latter two
+ * types are exactly representable in JS and $numberLong is not,
+ * in addition to the fact that this has been historic behavior
+ * in Compass for a long time, this seems like a reasonable choice.
+ *
+ * @param value Any BSON value.
+ * @returns A serialized, human-readable and human-editable string.
+ */
+export function objectToIdiomaticEJSON(
+  value: Readonly<EJSON.SerializableTypes>,
+  options: HadronEJSONOptions = {}
+): string {
+  const serialized = EJSON.serialize(value, {
+    relaxed: false,
+  });
+
+  makeEJSONIdiomatic(serialized);
+
+  return JSON.stringify(
+    serialized,
+    null,
+    'indent' in options ? options.indent : 2
+  );
+}
+
+function makeEJSONIdiomatic(value: EJSON.SerializableTypes): void {
+  if (!value || typeof value !== 'object') return;
+
+  for (const key of Object.keys(value)) {
+    const entry = (value as any)[key];
+    if (entry.$numberInt) {
+      (value as any)[key] = +entry.$numberInt;
+      continue;
+    }
+    if (entry.$numberDouble) {
+      (value as any)[key] = +entry.$numberDouble;
+      continue;
+    }
+    if (key.startsWith('$')) {
+      // Do not recurse into other EJSON-specific keys.
+      continue;
+    }
+    makeEJSONIdiomatic(entry);
+  }
+}