From a298d22173e7e07789d86051f95f76153f29e10a Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 10 Jan 2023 14:46:07 -0500 Subject: [PATCH] fix(NODE-4932): remove .0 suffix from double extended json values (#553) --- src/double.ts | 10 +- test/node/bson_corpus.spec.test.js | 43 ++++++-- test/node/double_tests.js | 161 ++++++++++++++++++++++++++--- 3 files changed, 184 insertions(+), 30 deletions(-) diff --git a/src/double.ts b/src/double.ts index c302eb8c..96e5fcc6 100644 --- a/src/double.ts +++ b/src/double.ts @@ -55,14 +55,12 @@ export class Double { if (Object.is(Math.sign(this.value), -0)) { // 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' }; + return { $numberDouble: `-${this.value.toFixed(1)}` }; } - if (Number.isInteger(this.value)) { - return { $numberDouble: `${this.value}.0` }; - } else { - return { $numberDouble: `${this.value}` }; - } + return { + $numberDouble: Number.isInteger(this.value) ? this.value.toFixed(1) : this.value.toString() + }; } /** @internal */ diff --git a/test/node/bson_corpus.spec.test.js b/test/node/bson_corpus.spec.test.js index 82d37977..9b6d4634 100644 --- a/test/node/bson_corpus.spec.test.js +++ b/test/node/bson_corpus.spec.test.js @@ -181,22 +181,28 @@ describe('BSON Corpus', function () { // convert inputs to native Javascript objects const nativeFromCB = bsonToNative(cB); - if (cEJ.includes('1.2345678921232E+18')) { + if (description === 'Double type') { // 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 testInputAsNumber = Number(eJSONParsedAsJSON.d.$numberDouble); const ejsonOutputAsFloat = eJSONParsed.d.valueOf(); - expect(ejsonOutputAsFloat).to.equal(testInputAsFloat); + if (eJSONParsedAsJSON.d.$numberDouble === 'NaN') { + expect(ejsonOutputAsFloat).to.be.NaN; + } else { + if (eJSONParsedAsJSON.d.$numberDouble === '-0.0') { + expect(Object.is(ejsonOutputAsFloat, -0)).to.be.true; + } + expect(ejsonOutputAsFloat).to.equal(testInputAsFloat); + expect(ejsonOutputAsFloat).to.equal(testInputAsNumber); + } } else { // round tripped EJSON should match the original expect(nativeToCEJSON(jsonToNative(cEJ))).to.equal(cEJ); @@ -220,18 +226,37 @@ describe('BSON Corpus', function () { expect(nativeToBson(jsonToNative(cEJ))).to.deep.equal(cB); } - if (cEJ.includes('1.2345678921232E+18')) { + if (description === 'Double type') { // 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 }); + const stringValueKey = Object.keys(eJSONFromBSONAsJSON.d)[0]; + const testInputAsFloat = Number.parseFloat(eJSONFromBSONAsJSON.d[stringValueKey]); + const testInputAsNumber = Number(eJSONFromBSONAsJSON.d[stringValueKey]); + // TODO(NODE-4377): EJSON transforms large doubles into longs - expect(eJSONFromBSONAsJSON).to.have.nested.property('d.$numberLong'); + expect(eJSONFromBSONAsJSON).to.have.nested.property( + Number.isFinite(testInputAsFloat) && Number.isInteger(testInputAsFloat) + ? testInputAsFloat <= 0x7fffffff && + testInputAsFloat >= -0x80000000 && + !Object.is(testInputAsFloat, -0) + ? 'd.$numberInt' + : 'd.$numberLong' + : 'd.$numberDouble' + ); 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); + if (eJSONFromBSONAsJSON.d.$numberDouble === 'NaN') { + expect(ejsonOutputAsFloat).to.be.NaN; + } else { + if (eJSONFromBSONAsJSON.d.$numberDouble === '-0.0') { + expect(Object.is(ejsonOutputAsFloat, -0)).to.be.true; + } + expect(ejsonOutputAsFloat).to.equal(testInputAsFloat); + expect(ejsonOutputAsFloat).to.equal(testInputAsNumber); + } } else { // the reverse direction, BSON -> native -> EJSON, should match canonical EJSON. expect(nativeToCEJSON(nativeFromCB)).to.equal(cEJ); diff --git a/test/node/double_tests.js b/test/node/double_tests.js index 73dfa7d7..6adf3d16 100644 --- a/test/node/double_tests.js +++ b/test/node/double_tests.js @@ -2,7 +2,6 @@ const BSON = require('../register-bson'); const Double = BSON.Double; -const inspect = require('util').inspect; describe('BSON Double Precision', function () { context('class Double', function () { @@ -38,26 +37,158 @@ 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' } } + { + title: 'returns "0.0" when input is a number 0', + input: 0, + output: { $numberDouble: '0.0' } + }, + { + title: 'returns "-0.0" when input is a number -0', + input: -0, + output: { $numberDouble: '-0.0' } + }, + { + title: 'returns "0.0" when input is a string "-0.0"', + input: '-0.0', + output: { $numberDouble: '-0.0' } + }, + { + title: 'returns "3.0" when input is a number 3', + input: 3, + output: { $numberDouble: '3.0' } + }, + { + title: 'returns "-3.0" when input is a number -3', + input: -3, + output: { $numberDouble: '-3.0' } + }, + { + title: 'returns "3.4" when input is a number 3.4', + input: 3.4, + output: { $numberDouble: '3.4' } + }, + { + title: 'returns "2.220446049250313e-16" when input is Number.EPSILON', + input: Number.EPSILON, + output: { $numberDouble: '2.220446049250313e-16' } + }, + { + title: 'returns "123450000000.0" when input is a number 12345e7', + input: 12345e7, + output: { $numberDouble: '123450000000.0' } + }, + { + title: 'returns "1234.5" when input is a number 12345e-1', + input: 12345e-1, + output: { $numberDouble: '1234.5' } + }, + { + title: 'returns "-1234.5" when input is a number -12345e-1', + input: -12345e-1, + output: { $numberDouble: '-1234.5' } + }, + { + title: 'returns "Infinity" when input is a number Infinity', + input: Infinity, + output: { $numberDouble: 'Infinity' } + }, + { + title: 'returns "-Infinity" when input is a number -Infinity', + input: -Infinity, + output: { $numberDouble: '-Infinity' } + }, + { + title: 'returns "NaN" when input is a number NaN', + input: NaN, + output: { $numberDouble: 'NaN' } + }, + { + title: 'returns "1.7976931348623157e+308" when input is a number Number.MAX_VALUE', + input: Number.MAX_VALUE, + output: { $numberDouble: '1.7976931348623157e+308' } + }, + { + title: 'returns "5e-324" when input is a number Number.MIN_VALUE', + input: Number.MIN_VALUE, + output: { $numberDouble: '5e-324' } + }, + { + title: 'returns "-1.7976931348623157e+308" when input is a number -Number.MAX_VALUE', + input: -Number.MAX_VALUE, + output: { $numberDouble: '-1.7976931348623157e+308' } + }, + { + title: 'returns "-5e-324" when input is a number -Number.MIN_VALUE', + input: -Number.MIN_VALUE, + output: { $numberDouble: '-5e-324' } + }, + { + // Reference: https://docs.oracle.com/cd/E19957-01/806-3568/ncg_math.html + // min positive normal number + title: + 'returns "2.2250738585072014e-308" when input is a number the minimum positive normal value', + input: '2.2250738585072014e-308', + output: { $numberDouble: '2.2250738585072014e-308' } + }, + { + // max subnormal number + title: + 'returns "2.225073858507201e-308" when input is a number the maximum positive subnormal value', + input: '2.225073858507201e-308', + output: { $numberDouble: '2.225073858507201e-308' } + }, + { + // min positive subnormal number (NOTE: JS does not output same input string, but numeric values are equal) + title: 'returns "5e-324" when input is a number the minimum positive subnormal value', + input: '4.9406564584124654e-324', + output: { $numberDouble: '5e-324' } + }, + { + // https://262.ecma-international.org/13.0/#sec-number.prototype.tofixed + // Note: calling toString on this integer returns 1000000000000000100, so toFixed is more precise + // This test asserts we do not change _current_ behavior, however preserving this value is not + // something that is possible in BSON, if a future version of this library were to emit + // "1000000000000000100.0" instead, it would not be incorrect from a BSON/MongoDB/Double precision perspective, + // it would just constrain the string output to what is possible with 8 bytes of floating point precision + title: + 'returns "1000000000000000128.0" when input is an int-like number beyond 8-byte double precision', + input: '1000000000000000128', + output: { $numberDouble: '1000000000000000128.0' } + } ]; for (const test of tests) { const input = test.input; const output = test.output; - const title = `returns ${inspect(output)} when Double is ${input}`; + const title = test.title; it(title, () => { - expect(output).to.deep.equal(input.toExtendedJSON({ relaxed: false })); + const inputAsDouble = new Double(input); + expect(inputAsDouble.toExtendedJSON({ relaxed: false })).to.deep.equal(output); + if (!Number.isNaN(inputAsDouble.value)) { + expect(Number(inputAsDouble.toExtendedJSON({ relaxed: false }).$numberDouble)).to.equal( + inputAsDouble.value + ); + } + }); + + it(`preserves the byte wise value of ${input} (${typeof input}) after stringification`, () => { + // Asserts the same bytes can be reconstructed from the generated string, + // sometimes the string changes "4.9406564584124654e-324" -> "5e-324" + // but both represent the same ieee754 double bytes + const ejsonDoubleString = new Double(input).toExtendedJSON().$numberDouble; + const bytesFromInput = (() => { + const b = Buffer.alloc(8); + b.writeDoubleBE(Number(input)); + return b.toString('hex'); + })(); + + const bytesFromOutput = (() => { + const b = Buffer.alloc(8); + b.writeDoubleBE(Number(ejsonDoubleString)); + return b.toString('hex'); + })(); + + expect(bytesFromOutput).to.equal(bytesFromInput); }); } });