From e0dbb17d0a13f3aca9879f08cba3573b2a84fd8d Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 4 Jan 2023 15:21:24 -0500 Subject: [PATCH] fix(NODE-4905): double precision accuracy in canonical EJSON (#548) Co-authored-by: Durran Jordan --- src/double.ts | 16 ++++-------- test/node/bson_corpus.spec.test.js | 40 +++++++++++++++++++++++++++--- test/node/double.test.ts | 25 +++++++++++++++++++ 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/src/double.ts b/src/double.ts index 7a4feb64..5e790017 100644 --- 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/node/bson_corpus.spec.test.js b/test/node/bson_corpus.spec.test.js index 7b98bb50..36828bef 100644 --- a/test/node/bson_corpus.spec.test.js +++ b/test/node/bson_corpus.spec.test.js @@ -184,8 +184,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. @@ -205,8 +223,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.test.ts b/test/node/double.test.ts index 00ed30f8..450017a4 100644 --- a/test/node/double.test.ts +++ b/test/node/double.test.ts @@ -2,6 +2,7 @@ import { expect } from 'chai'; import { BSON, Double } from '../register-bson'; import { BSON_DATA_NUMBER, BSON_DATA_INT } from '../../src/constants'; +import { inspect } from 'node:util'; describe('BSON Double Precision', function () { context('class Double', function () { @@ -36,6 +37,30 @@ 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 { input, output } of tests) { + const title = `returns ${inspect(output)} when Double is ${input}`; + it(title, () => { + expect(output).to.deep.equal(input.toExtendedJSON({ relaxed: false })); + }); + } + }); }); function serializeThenDeserialize(value) {