fix(NODE-3630): remove float parser and test edge cases for Double (#502)

Author: aditi-khare-mongoDB

etc/benchmarks/main.mjs

@@ -9,7 +9,7 @@ console.log();
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 await runner({
-  skip: false,
+  skip: true,
   name: 'deserialize({ oid, string }, { validation: { utf8: false } })',
   iterations,
   setup(libs) {
@@ -58,6 +58,46 @@ await runner({
   }
 });
 
+await runner({
+  skip: true,
+  name: 'Double Serialization',
+  iterations,
+  run(i, bson) {
+    bson.lib.serialize({ d: 2.3 });
+  }
+});
+
+await runner({
+  skip: false,
+  name: 'Double Deserialization',
+  iterations,
+  setup(libs) {
+    const bson = getCurrentLocalBSON(libs);
+    return bson.lib.serialize({ d: 2.3 });
+  },
+  run(i, bson, serialized_double) {
+    bson.lib.deserialize(serialized_double);
+  }
+});
+
+await runner({
+  skip: false,
+  name: 'Many Doubles Deserialization',
+  iterations,
+  setup(libs) {
+    const bson = getCurrentLocalBSON(libs);
+    let doubles = Object.fromEntries(
+      Array.from({ length: 1000 }, i => {
+        return [`a_${i}`, 2.3];
+      })
+    );
+    return bson.lib.serialize(doubles);
+  },
+  run(i, bson, serialized_doubles) {
+    bson.lib.deserialize(serialized_doubles);
+  }
+});
+
 // End
 console.log(
   'Total time taken to benchmark:',

src/float_parser.ts

@@ -197,6 +197,7 @@ function deserializeObject(
   let isPossibleDBRef = isArray ? false : null;
 
   // While we have more left data left keep parsing
+  const dataview = new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength);
   while (!done) {
     // Read the type
     const elementType = buffer[index++];
@@ -263,10 +264,10 @@ function deserializeObject(
         (buffer[index++] << 16) |
         (buffer[index++] << 24);
     } else if (elementType === constants.BSON_DATA_NUMBER && promoteValues === false) {
-      value = new Double(buffer.readDoubleLE(index));
+      value = new Double(dataview.getFloat64(index, true));
       index = index + 8;
     } else if (elementType === constants.BSON_DATA_NUMBER) {
-      value = buffer.readDoubleLE(index);
+      value = dataview.getFloat64(index, true);
       index = index + 8;
     } else if (elementType === constants.BSON_DATA_DATE) {
       const lowBits =

src/parser/deserializer.ts

@@ -9,7 +9,6 @@ import type { Double } from '../double';
 import { ensureBuffer } from '../ensure_buffer';
 import { BSONError, BSONTypeError } from '../error';
 import { isBSONType } from '../extended_json';
-import { writeIEEE754 } from '../float_parser';
 import type { Int32 } from '../int_32';
 import { Long } from '../long';
 import { Map } from '../map';
@@ -79,6 +78,12 @@ function serializeString(
   return index;
 }
 
+const SPACE_FOR_FLOAT64 = new Uint8Array(8);
+const DV_FOR_FLOAT64 = new DataView(
+  SPACE_FOR_FLOAT64.buffer,
+  SPACE_FOR_FLOAT64.byteOffset,
+  SPACE_FOR_FLOAT64.byteLength
+);
 function serializeNumber(
   buffer: Buffer,
   key: string,
@@ -119,7 +124,8 @@ function serializeNumber(
     index = index + numberOfWrittenBytes;
     buffer[index++] = 0;
     // Write float
-    writeIEEE754(buffer, value, index, 'little', 52, 8);
+    DV_FOR_FLOAT64.setFloat64(0, value, true);
+    buffer.set(SPACE_FOR_FLOAT64, index);
     // Adjust index
     index = index + 8;
   }
@@ -487,7 +493,8 @@ function serializeDouble(
   buffer[index++] = 0;
 
   // Write float
-  writeIEEE754(buffer, value.value, index, 'little', 52, 8);
+  DV_FOR_FLOAT64.setFloat64(0, value.value, true);
+  buffer.set(SPACE_FOR_FLOAT64, index);
 
   // Adjust index
   index = index + 8;

src/parser/serializer.ts

@@ -121,11 +121,6 @@ describe('BSON Corpus', function () {
         describe('valid-bson', function () {
           for (const v of valid) {
             it(v.description, function () {
-              if (v.description === 'NaN with payload') {
-                // TODO(NODE-3630): remove custom float parser so we can handle the NaN payload data
-                this.skip();
-              }
-
               if (
                 v.description === 'All BSON types' &&
                 scenario._filename === 'multi-type-deprecated'

test/node/bson_corpus.spec.test.js

@@ -3,36 +3,90 @@
 const BSON = require('../register-bson');
 const Double = BSON.Double;
 
-describe('Double', function () {
-  describe('Constructor', function () {
-    var value = 42.3456;
+describe('BSON Double Precision', function () {
+  context('class Double', function () {
+    describe('constructor()', function () {
+      const value = 42.3456;
 
-    it('Primitive number', function (done) {
-      expect(new Double(value).valueOf()).to.equal(value);
-      done();
+      it('Primitive number', function () {
+        expect(new Double(value).valueOf()).to.equal(value);
+      });
+
+      it('Number object', function () {
+        expect(new Double(new Number(value)).valueOf()).to.equal(value);
+      });
     });
 
-    it('Number object', function (done) {
-      expect(new Double(new Number(value)).valueOf()).to.equal(value);
-      done();
+    describe('#toString()', () => {
+      it('should serialize to a string', () => {
+        const testNumber = Math.random() * Number.MAX_VALUE;
+        const double = new Double(testNumber);
+        expect(double.toString()).to.equal(testNumber.toString());
+      });
+
+      const testRadices = [2, 8, 10, 16, 22];
+
+      for (const radix of testRadices) {
+        it(`should support radix argument: ${radix}`, () => {
+          const testNumber = Math.random() * Number.MAX_VALUE;
+          const double = new Double(testNumber);
+          expect(double.toString(radix)).to.equal(testNumber.toString(radix));
+        });
+      }
     });
   });
 
-  describe('toString', () => {
-    it('should serialize to a string', () => {
-      const testNumber = Math.random() * Number.MAX_VALUE;
-      const double = new Double(testNumber);
-      expect(double.toString()).to.equal(testNumber.toString());
+  function serializeThenDeserialize(value) {
+    const serializedDouble = BSON.serialize({ d: value });
+    const deserializedDouble = BSON.deserialize(serializedDouble, { promoteValues: false });
+    return deserializedDouble.d;
+  }
+
+  const testCases = [
+    { name: 'Infinity', doubleVal: new Double(Infinity), testVal: Infinity },
+    { name: '-Infinity', doubleVal: new Double(-Infinity), testVal: -Infinity },
+    { name: 'Number.EPSILON', doubleVal: new Double(Number.EPSILON), testVal: Number.EPSILON },
+    { name: 'Zero', doubleVal: new Double(0), testVal: 0 },
+    { name: 'Negative Zero', doubleVal: new Double(-0), testVal: -0 },
+    { name: 'NaN', doubleVal: new Double(NaN), testVal: NaN }
+  ];
+
+  for (const { name, doubleVal, testVal } of testCases) {
+    it(`should preserve the input value ${name} in Double serialize-deserialize roundtrip`, () => {
+      const roundTrippedVal = serializeThenDeserialize(doubleVal);
+      expect(Object.is(doubleVal.value, testVal)).to.be.true;
+      expect(Object.is(roundTrippedVal.value, doubleVal.value)).to.be.true;
     });
+  }
 
-    const testRadices = [2, 8, 10, 16, 22];
+  context('NaN with Payload', function () {
+    const NanPayloadBuffer = Buffer.from('120000000000F87F', 'hex');
+    const NanPayloadDV = new DataView(
+      NanPayloadBuffer.buffer,
+      NanPayloadBuffer.byteOffset,
+      NanPayloadBuffer.byteLength
+    );
+    const NanPayloadDouble = NanPayloadDV.getFloat64(0, true);
+    // Using promoteValues: false (returning raw BSON) in order to be able to check that payload remains intact
+    const serializedNanPayloadDouble = BSON.serialize({ d: NanPayloadDouble });
 
-    for (const radix of testRadices) {
-      it(`should support radix argument: ${radix}`, () => {
-        const testNumber = Math.random() * Number.MAX_VALUE;
-        const double = new Double(testNumber);
-        expect(double.toString(radix)).to.equal(testNumber.toString(radix));
-      });
-    }
+    it('should keep payload in serialize-deserialize roundtrip', function () {
+      expect(serializedNanPayloadDouble.subarray(7, 15)).to.deep.equal(NanPayloadBuffer);
+    });
+
+    it('should preserve NaN value in serialize-deserialize roundtrip', function () {
+      const { d: newVal } = BSON.deserialize(serializedNanPayloadDouble, { promoteValues: true });
+      expect(newVal).to.be.NaN;
+    });
+  });
+
+  it('NODE-4335: does not preserve -0 in serialize-deserialize roundtrip if JS number is used', function () {
+    // TODO (NODE-4335): -0 should be serialized as double
+    // This test is demonstrating the behavior of -0 being serialized as an int32 something we do NOT want to unintentionally change, but may want to change in the future, which the above ticket serves to track.
+    const value = -0;
+    const serializedDouble = BSON.serialize({ d: value });
+    const type = serializedDouble[4];
+    expect(type).to.not.equal(BSON.BSON_DATA_NUMBER);
+    expect(type).to.equal(BSON.BSON_DATA_INT);
   });
 });