Allow storing of Integer when 'disableLosslessIntegers=true'

`Integer` will now be allowed in query parameters even when native JS
numbers are configured on the driver level. This is done to not cut
away a valid neo4j integer type. It will also result in better
backwards compatibility - existing applications will not need to
change all `Integer` query parameters when they
set `disableLosslessIntegers=true`.

Author: lutovich

src/v1/internal/connector.js

@@ -164,8 +164,7 @@ class Connection {
    * @constructor
    * @param {NodeChannel|WebSocketChannel} channel - channel with a 'write' function and a 'onmessage' callback property.
    * @param {string} url - the hostname and port to connect to.
-   * @param {boolean} disableLosslessIntegers if this connection should convert all received integers to native JS numbers
-   * (including native {@link Number} type or our own {@link Integer}) as native {@link Number}.
+   * @param {boolean} disableLosslessIntegers if this connection should convert all received integers to native JS numbers.
    */
   constructor(channel, url, disableLosslessIntegers = false) {
     /**
@@ -181,7 +180,7 @@ class Connection {
     this._ch = channel;
     this._dechunker = new Dechunker();
     this._chunker = new Chunker( channel );
-    this._packer = new Packer(this._chunker, disableLosslessIntegers);
+    this._packer = new Packer(this._chunker);
     this._unpacker = new Unpacker(disableLosslessIntegers);
 
     this._isHandlingFailure = false;

src/v1/internal/packstream.js

@@ -80,13 +80,10 @@ class Packer {
   /**
    * @constructor
    * @param {Chunker} channel the chunker backed by a network channel.
-   * @param {boolean} disableLosslessIntegers if this packer should convert all received integers to native JS numbers
-   * (including native {@link Number} type or our own {@link Integer}) as native {@link Number}.
    */
-  constructor(channel, disableLosslessIntegers = false) {
+  constructor(channel) {
     this._ch = channel;
     this._byteArraysSupported = true;
-    this._disableLosslessIntegers = disableLosslessIntegers;
   }
 
   /**
@@ -107,7 +104,7 @@ class Packer {
     } else if (typeof(x) == "string") {
       return () => this.packString(x, onError);
     } else if (isInt(x)) {
-      return this._packableInteger(x, onError);
+      return () => this.packInteger(x);
     } else if (x instanceof Int8Array) {
       return () => this.packBytes(x, onError);
     } else if (x instanceof Array) {
@@ -150,28 +147,6 @@ class Packer {
     }
   }
 
-  /**
-   * Creates a packable function out of the provided {@link Integer} value.
-   * @param {Integer} x the value to pack.
-   * @param {function} onError the callback for the case when value cannot be packed.
-   * @return {function}
-   * @private
-   */
-  _packableInteger(x, onError) {
-    if (this._disableLosslessIntegers) {
-      // pack Integer objects only when native numbers are not used, fail otherwise
-      // Integer can't represent special values like Number.NEGATIVE_INFINITY
-      // and should not be used at all when native numbers are enabled
-      if (onError) {
-        onError(newError(`Cannot pack Integer value ${x} (${JSON.stringify(x)}) when native numbers are enabled. ` +
-          `Please use native Number instead or disable native number support on the driver level.`));
-      }
-      return () => undefined;
-    } else {
-      return () => this.packInteger(x);
-    }
-  }
-
   /**
    * Packs a struct
    * @param signature the signature of the struct
@@ -209,6 +184,7 @@ class Packer {
       this._ch.writeInt32(low);
     }
   }
+
   packFloat(x) {
     this._ch.writeUInt8(FLOAT_64);
     this._ch.writeFloat64(x);
@@ -343,8 +319,7 @@ class Unpacker {
 
   /**
    * @constructor
-   * @param {boolean} disableLosslessIntegers if this unpacker should convert all received integers to native JS numbers
-   * (including native {@link Number} type or our own {@link Integer}) as native {@link Number}.
+   * @param {boolean} disableLosslessIntegers if this unpacker should convert all received integers to native JS numbers.
    */
   constructor(disableLosslessIntegers = false) {
     // Higher level layers can specify how to map structs to higher-level objects.
@@ -368,9 +343,12 @@ class Unpacker {
       return boolean;
     }
 
-    const number = this._unpackNumber(marker, buffer);
-    if (number !== null) {
-      return number;
+    const numberOrInteger = this._unpackNumberOrInteger(marker, buffer);
+    if (numberOrInteger !== null) {
+      if (this._disableLosslessIntegers && isInt(numberOrInteger)) {
+        return numberOrInteger.toNumberOrInfinity();
+      }
+      return numberOrInteger;
     }
 
     const string = this._unpackString(marker, markerHigh, markerLow, buffer);
@@ -411,7 +389,7 @@ class Unpacker {
     }
   }
 
-  _unpackNumber(marker, buffer) {
+  _unpackNumberOrInteger(marker, buffer) {
     if (marker == FLOAT_64) {
       return buffer.readFloat64();
     } else if (marker >= 0 && marker < 128) {
@@ -428,8 +406,7 @@ class Unpacker {
     } else if (marker == INT_64) {
       const high = buffer.readInt32();
       const low = buffer.readInt32();
-      const integer = new Integer(low, high);
-      return this._disableLosslessIntegers ? integer.toNumberOrInfinity() : integer;
+      return new Integer(low, high);
     } else {
       return null;
     }

test/v1/driver.test.js

@@ -339,27 +339,32 @@ describe('driver', () => {
   nativeNumbers.forEach(number => {
 
     it(`should return native number ${number} when disableLosslessIntegers=true`, done => {
-      testNativeNumberInReturnedRecord(number, done);
+      testNumberInReturnedRecord(number, number, done);
     });
 
   });
 
-  it('should fail to pack Integer when disableLosslessIntegers=true', done => {
-    driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken, {disableLosslessIntegers: true});
-    const session = driver.session();
+  const integersWithNativeNumberEquivalent = [
+    [neo4j.int(0), 0],
+    [neo4j.int(42), 42],
+    [neo4j.int(-100), -100],
+    [neo4j.int(Number.MIN_SAFE_INTEGER), Number.MIN_SAFE_INTEGER],
+    [neo4j.int(Number.MAX_SAFE_INTEGER), Number.MAX_SAFE_INTEGER],
+    [neo4j.int('-9007199254740992'), Number.NEGATIVE_INFINITY], // Number.MIN_SAFE_INTEGER - 1
+    [neo4j.int('9007199254740992'), Number.POSITIVE_INFINITY], // Number.MAX_SAFE_INTEGER + 1
+    [neo4j.int('-9007199254749999'), Number.NEGATIVE_INFINITY], // Number.MIN_SAFE_INTEGER - 9007
+    [neo4j.int('9007199254749999'), Number.POSITIVE_INFINITY], // Number.MAX_SAFE_INTEGER + 9008
+  ];
+
+  integersWithNativeNumberEquivalent.forEach(integerWithNativeNumber => {
+
+    const integer = integerWithNativeNumber[0];
+    const nativeNumber = integerWithNativeNumber[1];
+
+    it(`should send Integer ${integer.toString()} and return native number ${nativeNumber} when disableLosslessIntegers=true`, done => {
+      testNumberInReturnedRecord(integer, nativeNumber, done);
+    });
 
-    session.run('RETURN $number', {number: neo4j.int(42)})
-      .then(result => {
-        done.fail(`Was somehow able to pack Integer and received result ${JSON.stringify(result)}`);
-      })
-      .catch(error => {
-        const message = error.message;
-        if (message.indexOf('Cannot pack Integer value') === -1) {
-          done.fail(`Unexpected error message: ${message}`);
-        } else {
-          done();
-        }
-      });
   });
 
   function testIPv6Connection(url, done) {
@@ -378,24 +383,24 @@ describe('driver', () => {
     });
   }
 
-  function testNativeNumberInReturnedRecord(number, done) {
+  function testNumberInReturnedRecord(inputNumber, expectedNumber, done) {
     driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken, {disableLosslessIntegers: true});
 
     const session = driver.session();
-    session.run('RETURN $number AS n0, $number AS n1', {number: number}).then(result => {
+    session.run('RETURN $number AS n0, $number AS n1', {number: inputNumber}).then(result => {
       session.close();
 
       const records = result.records;
       expect(records.length).toEqual(1);
       const record = records[0];
 
-      expect(record.get('n0')).toEqual(number);
-      expect(record.get('n1')).toEqual(number);
+      expect(record.get('n0')).toEqual(expectedNumber);
+      expect(record.get('n1')).toEqual(expectedNumber);
 
-      expect(record.get(0)).toEqual(number);
-      expect(record.get(1)).toEqual(number);
+      expect(record.get(0)).toEqual(expectedNumber);
+      expect(record.get(1)).toEqual(expectedNumber);
 
-      expect(record.toObject()).toEqual({n0: number, n1: number});
+      expect(record.toObject()).toEqual({n0: expectedNumber, n1: expectedNumber});
 
       done();
     });