Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(NODE-4932): remove .0 suffix from double extended json values #554

Merged
merged 4 commits into from
Jan 10, 2023
Merged

fix(NODE-4932): remove .0 suffix from double extended json values #554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
51c790e
fix(NODE-4932): remove .0 suffix from double extended json values
nbbeeken Jan 6, 2023
4870431
use toFixed and add test
nbbeeken Jan 9, 2023
c6f1709
sync changes
nbbeeken Jan 9, 2023
127b9af
fix: lint
nbbeeken Jan 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions src/double.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,9 @@ export class Double {
return { $numberDouble: '-0.0' };
}

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 */
Expand Down
43 changes: 34 additions & 9 deletions test/node/bson_corpus.spec.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,22 +184,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);
Expand All @@ -223,18 +229,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) &&
!Object.is(testInputAsFloat, -0)
? testInputAsFloat <= 0x7fffffff && testInputAsFloat >= -0x80000000
? '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);
Expand Down
165 changes: 149 additions & 16 deletions test/node/double.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ 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 () {
Expand Down Expand Up @@ -40,24 +39,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 { input, output } of tests) {
const title = `returns ${inspect(output)} when Double is ${input}`;
for (const test of tests) {
const input = test.input;
const output = test.output;
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);
});
}
});
Expand Down