fix(NODE-4905): double precision accuracy in canonical EJSON (#549)

mongodb · Jan 4, 2023 · d86bd52 · d86bd52
1 parent 853bbb0
commit d86bd52
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 17 deletions.
diff --git a/src/double.ts b/src/double.ts
@@ -52,23 +52,17 @@ export class Double {
       return this.value;
     }
 
-    // NOTE: JavaScript has +0 and -0, apparently to model limit calculations. If a user
-    // explicitly provided `-0` then we need to ensure the sign makes it into the output
     if (Object.is(Math.sign(this.value), -0)) {
-      return { $numberDouble: `-${this.value.toFixed(1)}` };
+      // NOTE: JavaScript has +0 and -0, apparently to model limit calculations. If a user
+      // explicitly provided `-0` then we need to ensure the sign makes it into the output
+      return { $numberDouble: '-0.0' };
     }
 
-    let $numberDouble: string;
     if (Number.isInteger(this.value)) {
-      $numberDouble = this.value.toFixed(1);
-      if ($numberDouble.length >= 13) {
-        $numberDouble = this.value.toExponential(13).toUpperCase();
-      }
+      return { $numberDouble: `${this.value}.0` };
     } else {
-      $numberDouble = this.value.toString();
+      return { $numberDouble: `${this.value}` };
     }
-
-    return { $numberDouble };
   }
 
   /** @internal */

diff --git a/test/mocha.opts b/test/mocha.opts
@@ -1,4 +1,3 @@
 --require ts-node/register
---require chai/register-expect
 --require source-map-support/register
 --timeout 10000
diff --git a/test/node/bson_corpus.spec.test.js b/test/node/bson_corpus.spec.test.js
@@ -181,8 +181,26 @@ describe('BSON Corpus', function () {
               // convert inputs to native Javascript objects
               const nativeFromCB = bsonToNative(cB);
 
-              // round tripped EJSON should match the original
-              expect(nativeToCEJSON(jsonToNative(cEJ))).to.equal(cEJ);
+              if (cEJ.includes('1.2345678921232E+18')) {
+                // The following is special test logic for a "Double type" bson corpus test that uses a different
+                // string format for the resulting double value
+                // The test does not have a loss in precision, just different exponential output
+                // We want to ensure that the stringified value when interpreted as a double is equal
+                // as opposed to the string being precisely the same
+                if (description !== 'Double type') {
+                  throw new Error('Unexpected test using 1.2345678921232E+18');
+                }
+                const eJSONParsedAsJSON = JSON.parse(cEJ);
+                const eJSONParsed = EJSON.parse(cEJ, { relaxed: false });
+                expect(eJSONParsedAsJSON).to.have.nested.property('d.$numberDouble');
+                expect(eJSONParsed).to.have.nested.property('d._bsontype', 'Double');
+                const testInputAsFloat = Number.parseFloat(eJSONParsedAsJSON.d.$numberDouble);
+                const ejsonOutputAsFloat = eJSONParsed.d.valueOf();
+                expect(ejsonOutputAsFloat).to.equal(testInputAsFloat);
+              } else {
+                // round tripped EJSON should match the original
+                expect(nativeToCEJSON(jsonToNative(cEJ))).to.equal(cEJ);
+              }
 
               // invalid, but still parseable, EJSON. if provided, make sure that we
               // properly convert it to canonical EJSON and BSON.
@@ -202,8 +220,22 @@ describe('BSON Corpus', function () {
                 expect(nativeToBson(jsonToNative(cEJ))).to.deep.equal(cB);
               }
 
-              // the reverse direction, BSON -> native -> EJSON, should match canonical EJSON.
-              expect(nativeToCEJSON(nativeFromCB)).to.equal(cEJ);
+              if (cEJ.includes('1.2345678921232E+18')) {
+                // The round tripped value should be equal in interpreted value, not in exact character match
+                const eJSONFromBSONAsJSON = JSON.parse(
+                  EJSON.stringify(BSON.deserialize(cB), { relaxed: false })
+                );
+                const eJSONParsed = EJSON.parse(cEJ, { relaxed: false });
+                // TODO(NODE-4377): EJSON transforms large doubles into longs
+                expect(eJSONFromBSONAsJSON).to.have.nested.property('d.$numberLong');
+                expect(eJSONParsed).to.have.nested.property('d._bsontype', 'Double');
+                const testInputAsFloat = Number.parseFloat(eJSONFromBSONAsJSON.d.$numberLong);
+                const ejsonOutputAsFloat = eJSONParsed.d.valueOf();
+                expect(ejsonOutputAsFloat).to.equal(testInputAsFloat);
+              } else {
+                // the reverse direction, BSON -> native -> EJSON, should match canonical EJSON.
+                expect(nativeToCEJSON(nativeFromCB)).to.equal(cEJ);
+              }
 
               if (v.relaxed_extjson) {
                 let rEJ = normalize(v.relaxed_extjson);

diff --git a/test/node/double_tests.js b/test/node/double_tests.js
@@ -2,6 +2,7 @@
 
 const BSON = require('../register-bson');
 const Double = BSON.Double;
+const inspect = require('util').inspect;
 
 describe('BSON Double Precision', function () {
   context('class Double', function () {
@@ -34,6 +35,32 @@ describe('BSON Double Precision', function () {
         });
       }
     });
+
+    describe('.toExtendedJSON()', () => {
+      const tests = [
+        { input: new Double(0), output: { $numberDouble: '0.0' } },
+        { input: new Double(-0), output: { $numberDouble: '-0.0' } },
+        { input: new Double(3), output: { $numberDouble: '3.0' } },
+        { input: new Double(-3), output: { $numberDouble: '-3.0' } },
+        { input: new Double(3.4), output: { $numberDouble: '3.4' } },
+        { input: new Double(Number.EPSILON), output: { $numberDouble: '2.220446049250313e-16' } },
+        { input: new Double(12345e7), output: { $numberDouble: '123450000000.0' } },
+        { input: new Double(12345e-1), output: { $numberDouble: '1234.5' } },
+        { input: new Double(-12345e-1), output: { $numberDouble: '-1234.5' } },
+        { input: new Double(Infinity), output: { $numberDouble: 'Infinity' } },
+        { input: new Double(-Infinity), output: { $numberDouble: '-Infinity' } },
+        { input: new Double(NaN), output: { $numberDouble: 'NaN' } }
+      ];
+
+      for (const test of tests) {
+        const input = test.input;
+        const output = test.output;
+        const title = `returns ${inspect(output)} when Double is ${input}`;
+        it(title, () => {
+          expect(output).to.deep.equal(input.toExtendedJSON({ relaxed: false }));
+        });
+      }
+    });
   });
 
   function serializeThenDeserialize(value) {

diff --git a/test/register-bson.js b/test/register-bson.js
@@ -7,7 +7,7 @@
 // and make sure you run mocha using our .mocharc.json or with --require ts-node/register
 
 // This should be done by mocha --require, but that isn't supported until mocha version 7+
-require('chai/register-expect');
+global.expect = require('chai').expect;
 require('array-includes/auto');
 require('object.entries/auto');
 require('array.prototype.flatmap/auto');