From b59141626e0e2d1b8f790b80e5c21096a35ad898 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Wed, 22 Jun 2016 12:06:07 -0500 Subject: [PATCH 1/2] Add preferNativeInt for unpack operation This forces database results to be read as language-native number values unless they are out of range, which reduces the code overhead for applications that do not need to worry about the extended range. --- src/v1/internal/connector.js | 6 +++--- src/v1/internal/packstream.js | 29 +++++++++++++++++++---------- test/internal/packstream.test.js | 11 +++++++---- test/v1/types.test.js | 8 +++++--- 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/v1/internal/connector.js b/src/v1/internal/connector.js index 3096076f0..a2894a1ae 100644 --- a/src/v1/internal/connector.js +++ b/src/v1/internal/connector.js @@ -161,7 +161,7 @@ class Connection { * @param channel - channel with a 'write' function and a 'onmessage' * callback property */ - constructor (channel) { + constructor (channel, config) { /** * An ordered queue of observers, each exchange response (zero or more * RECORD messages followed by a SUCCESS message) we recieve will be routed @@ -173,7 +173,7 @@ class Connection { this._dechunker = new chunking.Dechunker(); this._chunker = new chunking.Chunker( channel ); this._packer = new packstream.Packer( this._chunker ); - this._unpacker = new packstream.Unpacker(); + this._unpacker = new packstream.Unpacker( config ); this._isHandlingFailure = false; // Set to true on fatal errors, to get this out of session pool. @@ -428,7 +428,7 @@ function connect( url, config = {}) { trust : config.trust || (hasFeature("trust_on_first_use") ? "TRUST_ON_FIRST_USE" : "TRUST_SIGNED_CERTIFICATES"), trustedCertificates : config.trustedCertificates || [], knownHosts : config.knownHosts - })); + }), config); } export default { diff --git a/src/v1/internal/packstream.js b/src/v1/internal/packstream.js index de9ebc54f..66be36e5d 100644 --- a/src/v1/internal/packstream.js +++ b/src/v1/internal/packstream.js @@ -16,7 +16,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - + import {debug} from "./log"; import {alloc} from "./buf"; import utf8 from "./utf8"; @@ -244,7 +244,9 @@ class Packer { * @access private */ class Unpacker { - constructor () { + constructor (config) { + this._config = config || {}; + // Higher level layers can specify how to map structs to higher-level objects. // If we recieve a struct that has a signature that does not have a mapper, // we simply return a Structure object. @@ -255,7 +257,7 @@ class Unpacker { let value = []; for(let i = 0; i < size; i++) { value.push( this.unpack( buffer ) ); - } + } return value; } @@ -277,11 +279,19 @@ class Unpacker { let value = new Structure(signature, []); for(let i = 0; i < size; i++) { value.fields.push(this.unpack(buffer)); - } + } return value; } } + unpackSmallInt (value) { + if( this._config.preferNativeInt ) { + return value; + } else { + return int(value); + } + } + unpack ( buffer ) { let marker = buffer.readUInt8(); if (marker == NULL) { @@ -293,16 +303,15 @@ class Unpacker { } else if (marker == FLOAT_64) { return buffer.readFloat64(); } else if (marker >= 0 && marker < 128) { - return int(marker); + return this.unpackSmallInt(marker); } else if (marker >= 240 && marker < 256) { - return int(marker - 256); + return this.unpackSmallInt(marker - 256); } else if (marker == INT_8) { - return int(buffer.readInt8()); + return this.unpackSmallInt(buffer.readInt8()); } else if (marker == INT_16) { - return int(buffer.readInt16()); + return this.unpackSmallInt(buffer.readInt16()); } else if (marker == INT_32) { - let b = buffer.readInt32(); - return int(b); + return this.unpackSmallInt(buffer.readInt32()); } else if (marker == INT_64) { let high = buffer.readInt32(); let low = buffer.readInt32(); diff --git a/test/internal/packstream.test.js b/test/internal/packstream.test.js index 827daa19d..a3b49b998 100644 --- a/test/internal/packstream.test.js +++ b/test/internal/packstream.test.js @@ -16,7 +16,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - + var alloc = require('../../lib/v1/internal/buf').alloc, packstream = require("../../lib/v1/internal/packstream.js"), integer = require("../../lib/v1/integer.js"), @@ -32,16 +32,19 @@ describe('packstream', function() { for(n = -999; n <= 999; n += 1) { i = Integer.fromNumber(n); expect( packAndUnpack( i ).toString() ).toBe( i.toString() ); + expect( packAndUnpack( i, undefined, {preferNativeInt: true} ) ).toBe( i.toNumber() ); } // positive numbers for(n = 16; n <= 16 ; n += 1) { i = Integer.fromNumber(Math.pow(2, n)); expect( packAndUnpack( i ).toString() ).toBe( i.toString() ); + expect( packAndUnpack( i, undefined, {preferNativeInt: true} ) ).toBe( i.toNumber() ); } // negative numbers for(n = 0; n <= 63 ; n += 1) { i = Integer.fromNumber(-Math.pow(2, n)); expect( packAndUnpack( i ).toString() ).toBe( i.toString() ); + expect( packAndUnpack( i, undefined, {preferNativeInt: true} ).toString() ).toEqual( i.toString() ); } }); it('should pack strings', function() { @@ -51,7 +54,7 @@ describe('packstream', function() { expect( packAndUnpack(str, str.length + 8)).toBe(str); }); it('should pack structures', function() { - expect( packAndUnpack( new Structure(1, ["Hello, world!!!"] ) ).fields[0] ) + expect( packAndUnpack( new Structure(1, ["Hello, world!!!"] ) ).fields[0] ) .toBe( "Hello, world!!!" ); }); it('should pack lists', function() { @@ -73,10 +76,10 @@ describe('packstream', function() { }); }); -function packAndUnpack( val, bufferSize ) { +function packAndUnpack( val, bufferSize, options ) { bufferSize = bufferSize || 128; var buffer = alloc(bufferSize); new Packer( buffer ).pack( val ); buffer.reset(); - return new Unpacker().unpack( buffer ); + return new Unpacker(options).unpack( buffer ); } diff --git a/test/v1/types.test.js b/test/v1/types.test.js index e8876334c..d4560ebc8 100644 --- a/test/v1/types.test.js +++ b/test/v1/types.test.js @@ -34,6 +34,8 @@ describe('integer values', function() { it('should support integer -1 ', testVal( neo4j.int(-1) ) ); it('should support integer larger than JS Numbers can model', testVal( neo4j.int("0x7fffffffffffffff") ) ); it('should support integer smaller than JS Numbers can model', testVal( neo4j.int("0x8000000000000000") ) ); + + it('should support integer 1 casting to native ', testVal( neo4j.int(1), 1, {preferNativeInt: true} ) ); }); describe('boolean values', function() { @@ -131,14 +133,14 @@ describe('path values', function() { }); }); -function testVal( val ) { +function testVal( val, expected, config ) { return function( done ) { - var driver = neo4j.driver("bolt://localhost", neo4j.auth.basic("neo4j", "neo4j")); + var driver = neo4j.driver("bolt://localhost", neo4j.auth.basic("neo4j", "neo4j"), config); var session = driver.session(); session.run("RETURN {val} as v", {val: val}) .then( function( result ) { - expect( result.records[0].get('v') ).toEqual( val ); + expect( result.records[0].get('v') ).toEqual( expected || val ); driver.close(); done(); }).catch(function(err) { console.log(err); }); From d1ec40e36b0cdb6b0032e0a67d3e679c9c63f3b6 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Fri, 24 Jun 2016 01:02:30 -0500 Subject: [PATCH 2/2] Use native for 64bit numbers in scope as well --- src/v1/integer.js | 35 +++++++++++++++++++++++++++++++++++ src/v1/internal/packstream.js | 9 ++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/v1/integer.js b/src/v1/integer.js index 70b7250a9..2a30cb2af 100644 --- a/src/v1/integer.js +++ b/src/v1/integer.js @@ -718,6 +718,23 @@ Integer.fromValue = function(val) { return new Integer(val.low, val.high); }; +/** + * Returns a number or Integer for a given pair of low and high bits, preferring number when in scope. + * @access private + * @param {number} lowBits The low 32 bits + * @param {number} highBits The high 32 bits + * @returns {!Integer|number} The corresponding Integer value + * @expose + */ +Integer.asNative = function(lowBits, highBits) { + var val = new Integer(lowBits, highBits); + if (val.greaterThan(MIN_NATIVE) && val.lessThan(MAX_NATIVE)) { + return val.toNumber(); + } else { + return val; + } +}; + /** * @type {number} * @const @@ -801,6 +818,24 @@ Integer.MAX_VALUE = Integer.fromBits(0xFFFFFFFF|0, 0x7FFFFFFF|0, false); */ Integer.MIN_VALUE = Integer.fromBits(0, 0x80000000|0, false); +/** + * Minimum safe native value. + * @type {!Integer} + * @const + * @inner + * @private + */ +var MIN_NATIVE = Integer.fromValue(Number.MIN_SAFE_INTEGER); + +/** + * Maximum safe native value. + * @type {!Integer} + * @const + * @inner + * @private + */ +var MAX_NATIVE = Integer.fromValue(Number.MAX_SAFE_INTEGER); + /** * Cast value to Integer type. * @access public diff --git a/src/v1/internal/packstream.js b/src/v1/internal/packstream.js index 66be36e5d..aca29ac5d 100644 --- a/src/v1/internal/packstream.js +++ b/src/v1/internal/packstream.js @@ -291,6 +291,13 @@ class Unpacker { return int(value); } } + unpackLargeInt(low, high) { + if( this._config.preferNativeInt ) { + return Integer.asNative(low, high); + } else { + return Integer.fromBits(low, high); + } + } unpack ( buffer ) { let marker = buffer.readUInt8(); @@ -315,7 +322,7 @@ class Unpacker { } else if (marker == INT_64) { let high = buffer.readInt32(); let low = buffer.readInt32(); - return new Integer( low, high ); + return this.unpackLargeInt( low, high ); } else if (marker == STRING_8) { return utf8.decode( buffer, buffer.readUInt8()); } else if (marker == STRING_16) {