From fcb377b9dae3054bca68eff923ed5a3a1064c42e Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 28 Nov 2022 16:40:19 -0500 Subject: [PATCH 1/2] feat!(NODE-4464): stringify negative zero to -0.0 --- src/double.ts | 2 +- src/extended_json.ts | 13 +++++++------ test/node/bson_corpus.spec.test.js | 2 ++ test/node/{double_tests.js => double.test.ts} | 16 +++++++++++++--- test/node/extended_json_tests.js | 3 ++- 5 files changed, 25 insertions(+), 11 deletions(-) rename test/node/{double_tests.js => double.test.ts} (88%) diff --git a/src/double.ts b/src/double.ts index 7a4feb64..920ad018 100644 --- a/src/double.ts +++ b/src/double.ts @@ -43,7 +43,7 @@ export class Double { } toString(radix?: number): string { - return this.value.toString(radix); + return radix == null && Object.is(this.value, -0) ? '-0' : this.value.toString(radix); } /** @internal */ diff --git a/src/extended_json.ts b/src/extended_json.ts index 90269623..ec5c035c 100644 --- a/src/extended_json.ts +++ b/src/extended_json.ts @@ -75,8 +75,9 @@ function deserializeValue(value: any, options: EJSON.Options = {}) { // if it's an integer, should interpret as smallest BSON integer // that can represent it exactly. (if out of range, interpret as double.) - if (Math.floor(value) === value) { + if (Number.isInteger(value) && !Object.is(value, -0)) { if (value >= BSON_INT32_MIN && value <= BSON_INT32_MAX) return new Int32(value); + // TODO(NODE-4377): EJSON js number handling diverges from BSON if (value >= BSON_INT64_MIN && value <= BSON_INT64_MAX) return Long.fromNumber(value); } @@ -216,16 +217,16 @@ function serializeValue(value: any, options: EJSONSerializeOptions): any { } if (typeof value === 'number' && (!options.relaxed || !isFinite(value))) { - // it's an integer - if (Math.floor(value) === value) { - const int32Range = value >= BSON_INT32_MIN && value <= BSON_INT32_MAX, - int64Range = value >= BSON_INT64_MIN && value <= BSON_INT64_MAX; + if (Number.isInteger(value) && !Object.is(value, -0)) { + const int32Range = value >= BSON_INT32_MIN && value <= BSON_INT32_MAX; + const int64Range = value >= BSON_INT64_MIN && value <= BSON_INT64_MAX; // interpret as being of the smallest BSON integer type that can represent the number exactly if (int32Range) return { $numberInt: value.toString() }; + // TODO(NODE-4377): EJSON js number handling diverges from BSON if (int64Range) return { $numberLong: value.toString() }; } - return { $numberDouble: value.toString() }; + return { $numberDouble: Object.is(value, -0) ? '-0.0' : value.toString() }; } if (value instanceof RegExp || isRegExp(value)) { diff --git a/test/node/bson_corpus.spec.test.js b/test/node/bson_corpus.spec.test.js index 62dbf94b..730a6cb3 100644 --- a/test/node/bson_corpus.spec.test.js +++ b/test/node/bson_corpus.spec.test.js @@ -47,6 +47,8 @@ function nativeToREJSON(native) { } function normalize(cEJ) { + // TODO(NODE-3396): loses information about the original input + // ex. parse will preserve -0 but stringify will output +0 return JSON.stringify(JSON.parse(cEJ)); } diff --git a/test/node/double_tests.js b/test/node/double.test.ts similarity index 88% rename from test/node/double_tests.js rename to test/node/double.test.ts index 5346f6cf..1c8b14a4 100644 --- a/test/node/double_tests.js +++ b/test/node/double.test.ts @@ -1,6 +1,4 @@ -'use strict'; - -const BSON = require('../register-bson'); +import * as BSON from '../register-bson'; const Double = BSON.Double; describe('BSON Double Precision', function () { @@ -89,4 +87,16 @@ describe('BSON Double Precision', function () { expect(type).to.not.equal(BSON.BSON_DATA_NUMBER); expect(type).to.equal(BSON.BSON_DATA_INT); }); + + describe('extended JSON', () => { + it('preserves negative zero in canonical format', () => { + const result = BSON.EJSON.stringify({ a: -0.0 }, { relaxed: false }); + expect(result).to.equal(`{"a":{"$numberDouble":"-0.0"}}`); + }); + + it('loses negative zero in relaxed format', () => { + const result = BSON.EJSON.stringify({ a: -0.0 }, { relaxed: true }); + expect(result).to.equal(`{"a":0}`); + }); + }); }); diff --git a/test/node/extended_json_tests.js b/test/node/extended_json_tests.js index 551b0737..fd5bae48 100644 --- a/test/node/extended_json_tests.js +++ b/test/node/extended_json_tests.js @@ -79,7 +79,7 @@ describe('Extended JSON', function () { }); it('should correctly extend an existing mongodb module', function () { - // Serialize the document + // TODO(NODE-4377): doubleNumberIntFit should be a double not a $numberLong var json = '{"_id":{"$numberInt":"100"},"gh":{"$numberInt":"1"},"binary":{"$binary":{"base64":"AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+Pw==","subType":"00"}},"date":{"$date":{"$numberLong":"1488372056737"}},"code":{"$code":"function() {}","$scope":{"a":{"$numberInt":"1"}}},"dbRef":{"$ref":"tests","$id":{"$numberInt":"1"},"$db":"test"},"decimal":{"$numberDecimal":"100"},"double":{"$numberDouble":"10.1"},"int32":{"$numberInt":"10"},"long":{"$numberLong":"200"},"maxKey":{"$maxKey":1},"minKey":{"$minKey":1},"objectId":{"$oid":"111111111111111111111111"},"objectID":{"$oid":"111111111111111111111111"},"oldObjectID":{"$oid":"111111111111111111111111"},"regexp":{"$regularExpression":{"pattern":"hello world","options":"i"}},"symbol":{"$symbol":"symbol"},"timestamp":{"$timestamp":{"t":0,"i":1000}},"int32Number":{"$numberInt":"300"},"doubleNumber":{"$numberDouble":"200.2"},"longNumberIntFit":{"$numberLong":"7036874417766400"},"doubleNumberIntFit":{"$numberLong":"19007199250000000"}}'; @@ -103,6 +103,7 @@ describe('Extended JSON', function () { expect(doc1.int32Number._bsontype).to.equal('Int32'); expect(doc1.doubleNumber._bsontype).to.equal('Double'); expect(doc1.longNumberIntFit._bsontype).to.equal('Long'); + // TODO(NODE-4377): EJSON should not try to make Longs from large ints expect(doc1.doubleNumberIntFit._bsontype).to.equal('Long'); }); From 8e6746be8ac59210a7303a5edc2de40e011d993c Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 1 Dec 2022 13:19:29 -0500 Subject: [PATCH 2/2] address comments --- src/double.ts | 2 +- src/extended_json.ts | 26 +++++++++++++++----------- test/node/double.test.ts | 26 ++++++++++++++++++++------ 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/double.ts b/src/double.ts index 920ad018..7a4feb64 100644 --- a/src/double.ts +++ b/src/double.ts @@ -43,7 +43,7 @@ export class Double { } toString(radix?: number): string { - return radix == null && Object.is(this.value, -0) ? '-0' : this.value.toString(radix); + return this.value.toString(radix); } /** @internal */ diff --git a/src/extended_json.ts b/src/extended_json.ts index ec5c035c..c2a9671b 100644 --- a/src/extended_json.ts +++ b/src/extended_json.ts @@ -73,12 +73,15 @@ function deserializeValue(value: any, options: EJSON.Options = {}) { return value; } - // if it's an integer, should interpret as smallest BSON integer - // that can represent it exactly. (if out of range, interpret as double.) if (Number.isInteger(value) && !Object.is(value, -0)) { - if (value >= BSON_INT32_MIN && value <= BSON_INT32_MAX) return new Int32(value); - // TODO(NODE-4377): EJSON js number handling diverges from BSON - if (value >= BSON_INT64_MIN && value <= BSON_INT64_MAX) return Long.fromNumber(value); + // interpret as being of the smallest BSON integer type that can represent the number exactly + if (value >= BSON_INT32_MIN && value <= BSON_INT32_MAX) { + return new Int32(value); + } + if (value >= BSON_INT64_MIN && value <= BSON_INT64_MAX) { + // TODO(NODE-4377): EJSON js number handling diverges from BSON + return Long.fromNumber(value); + } } // If the number is a non-integer or out of integer range, should interpret as BSON Double. @@ -218,13 +221,14 @@ function serializeValue(value: any, options: EJSONSerializeOptions): any { if (typeof value === 'number' && (!options.relaxed || !isFinite(value))) { if (Number.isInteger(value) && !Object.is(value, -0)) { - const int32Range = value >= BSON_INT32_MIN && value <= BSON_INT32_MAX; - const int64Range = value >= BSON_INT64_MIN && value <= BSON_INT64_MAX; - // interpret as being of the smallest BSON integer type that can represent the number exactly - if (int32Range) return { $numberInt: value.toString() }; - // TODO(NODE-4377): EJSON js number handling diverges from BSON - if (int64Range) return { $numberLong: value.toString() }; + if (value >= BSON_INT32_MIN && value <= BSON_INT32_MAX) { + return { $numberInt: value.toString() }; + } + if (value >= BSON_INT64_MIN && value <= BSON_INT64_MAX) { + // TODO(NODE-4377): EJSON js number handling diverges from BSON + return { $numberLong: value.toString() }; + } } return { $numberDouble: Object.is(value, -0) ? '-0.0' : value.toString() }; } diff --git a/test/node/double.test.ts b/test/node/double.test.ts index 1c8b14a4..b6fcb37b 100644 --- a/test/node/double.test.ts +++ b/test/node/double.test.ts @@ -89,14 +89,28 @@ describe('BSON Double Precision', function () { }); describe('extended JSON', () => { - it('preserves negative zero in canonical format', () => { - const result = BSON.EJSON.stringify({ a: -0.0 }, { relaxed: false }); - expect(result).to.equal(`{"a":{"$numberDouble":"-0.0"}}`); + describe('stringify()', () => { + it('preserves negative zero in canonical format', () => { + const result = BSON.EJSON.stringify({ a: -0.0 }, { relaxed: false }); + expect(result).to.equal(`{"a":{"$numberDouble":"-0.0"}}`); + }); + + it('loses negative zero in relaxed format', () => { + const result = BSON.EJSON.stringify({ a: -0.0 }, { relaxed: true }); + expect(result).to.equal(`{"a":0}`); + }); }); - it('loses negative zero in relaxed format', () => { - const result = BSON.EJSON.stringify({ a: -0.0 }, { relaxed: true }); - expect(result).to.equal(`{"a":0}`); + describe('parse()', () => { + it('preserves negative zero in deserialization with relaxed false', () => { + const result = BSON.EJSON.parse(`{ "a": -0.0 }`, { relaxed: false }); + expect(result.a).to.have.property('_bsontype', 'Double'); + }); + + it('preserves negative zero in deserialization with relaxed true', () => { + const result = BSON.EJSON.parse(`{ "a": -0.0 }`, { relaxed: true }); + expect(Object.is(result.a, -0), 'expected prop a to be negative zero').to.be.true; + }); }); }); });