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.

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

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

Merged
merged 3 commits into from
Jan 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 5 additions & 11 deletions src/double.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
40 changes: 36 additions & 4 deletions test/node/bson_corpus.spec.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
durran marked this conversation as resolved.
Show resolved Hide resolved
// 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.
Expand All @@ -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);
Expand Down
25 changes: 25 additions & 0 deletions test/node/double.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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) {
Expand Down