From 4957f27c167525b1d276b02f2cb2ca4777c61402 Mon Sep 17 00:00:00 2001 From: lutovich Date: Wed, 11 Apr 2018 13:02:37 +0200 Subject: [PATCH] Support number props in spatial and temporal types By default, driver exposes integer values as instances of `Integer` class. This class can represent integer values larger than `Number.MAX_SAFE_INTEGER` without loss of precision. Driver can be forced to return native numbers using `disableLosslessIntegers` config option. This commit makes spatial and temporal types support `disableLosslessIntegers`. All exposed integer properties will be of type `number` when this config option is set to `true`. Internal conversions for temporal types still use `Integer` to not lose precision. --- src/v1/internal/packstream-v1.js | 17 +++++- src/v1/internal/packstream-v2.js | 95 ++++++++++++++++++++------------ src/v1/spatial-types.js | 6 +- test/v1/spatial-types.test.js | 24 ++++++++ test/v1/temporal-types.test.js | 74 ++++++++++++++++++++++++- 5 files changed, 174 insertions(+), 42 deletions(-) diff --git a/src/v1/internal/packstream-v1.js b/src/v1/internal/packstream-v1.js index 80e3b3c97..e773fc26f 100644 --- a/src/v1/internal/packstream-v1.js +++ b/src/v1/internal/packstream-v1.js @@ -400,6 +400,15 @@ class Unpacker { throw newError('Unknown packed value with marker ' + marker.toString(16)); } + unpackInteger(buffer) { + const marker = buffer.readUInt8(); + const result = this._unpackInteger(marker, buffer); + if (result == null) { + throw newError('Unable to unpack integer value with marker ' + marker.toString(16)); + } + return result; + } + _unpackBoolean(marker) { if (marker == TRUE) { return true; @@ -413,7 +422,13 @@ class Unpacker { _unpackNumberOrInteger(marker, buffer) { if (marker == FLOAT_64) { return buffer.readFloat64(); - } else if (marker >= 0 && marker < 128) { + } else { + return this._unpackInteger(marker, buffer); + } + } + + _unpackInteger(marker, buffer) { + if (marker >= 0 && marker < 128) { return int(marker); } else if (marker >= 240 && marker < 256) { return int(marker - 256); diff --git a/src/v1/internal/packstream-v2.js b/src/v1/internal/packstream-v2.js index 6aea28d81..af16eb0e6 100644 --- a/src/v1/internal/packstream-v2.js +++ b/src/v1/internal/packstream-v2.js @@ -33,7 +33,7 @@ import { isTime, Time } from '../temporal-types'; -import {int} from '../integer'; +import {int, isInt} from '../integer'; import { dateToEpochDay, epochDayToDate, @@ -126,19 +126,19 @@ export class Unpacker extends v1.Unpacker { } else if (signature == DURATION) { return unpackDuration(this, structSize, buffer); } else if (signature == LOCAL_TIME) { - return unpackLocalTime(this, structSize, buffer); + return unpackLocalTime(this, structSize, buffer, this._disableLosslessIntegers); } else if (signature == TIME) { - return unpackTime(this, structSize, buffer); + return unpackTime(this, structSize, buffer, this._disableLosslessIntegers); } else if (signature == DATE) { - return unpackDate(this, structSize, buffer); + return unpackDate(this, structSize, buffer, this._disableLosslessIntegers); } else if (signature == LOCAL_DATE_TIME) { - return unpackLocalDateTime(this, structSize, buffer); + return unpackLocalDateTime(this, structSize, buffer, this._disableLosslessIntegers); } else if (signature == DATE_TIME_WITH_ZONE_OFFSET) { - return unpackDateTimeWithZoneOffset(this, structSize, buffer); + return unpackDateTimeWithZoneOffset(this, structSize, buffer, this._disableLosslessIntegers); } else if (signature == DATE_TIME_WITH_ZONE_ID) { - return unpackDateTimeWithZoneId(this, structSize, buffer); + return unpackDateTimeWithZoneId(this, structSize, buffer, this._disableLosslessIntegers); } else { - return super._unpackUnknownStruct(signature, structSize, buffer); + return super._unpackUnknownStruct(signature, structSize, buffer, this._disableLosslessIntegers); } } } @@ -284,13 +284,15 @@ function packLocalTime(value, packer, onError) { * @param {Unpacker} unpacker the unpacker to use. * @param {number} structSize the retrieved struct size. * @param {BaseBuffer} buffer the buffer to unpack from. + * @param {boolean} disableLosslessIntegers if integer properties in the result local time should be native JS numbers. * @return {LocalTime} the unpacked local time value. */ -function unpackLocalTime(unpacker, structSize, buffer) { +function unpackLocalTime(unpacker, structSize, buffer, disableLosslessIntegers) { unpacker._verifyStructSize('LocalTime', LOCAL_TIME_STRUCT_SIZE, structSize); - const nanoOfDay = unpacker.unpack(buffer); - return nanoOfDayToLocalTime(nanoOfDay); + const nanoOfDay = unpacker.unpackInteger(buffer); + const result = nanoOfDayToLocalTime(nanoOfDay); + return convertIntegerPropsIfNeeded(result, disableLosslessIntegers); } /** @@ -315,16 +317,18 @@ function packTime(value, packer, onError) { * @param {Unpacker} unpacker the unpacker to use. * @param {number} structSize the retrieved struct size. * @param {BaseBuffer} buffer the buffer to unpack from. + * @param {boolean} disableLosslessIntegers if integer properties in the result time should be native JS numbers. * @return {Time} the unpacked time value. */ -function unpackTime(unpacker, structSize, buffer) { +function unpackTime(unpacker, structSize, buffer, disableLosslessIntegers) { unpacker._verifyStructSize('Time', TIME_STRUCT_SIZE, structSize); - const nanoOfDay = unpacker.unpack(buffer); - const offsetSeconds = unpacker.unpack(buffer); + const nanoOfDay = unpacker.unpackInteger(buffer); + const offsetSeconds = unpacker.unpackInteger(buffer); const localTime = nanoOfDayToLocalTime(nanoOfDay); - return new Time(localTime.hour, localTime.minute, localTime.second, localTime.nanosecond, offsetSeconds); + const result = new Time(localTime.hour, localTime.minute, localTime.second, localTime.nanosecond, offsetSeconds); + return convertIntegerPropsIfNeeded(result, disableLosslessIntegers); } /** @@ -347,13 +351,15 @@ function packDate(value, packer, onError) { * @param {Unpacker} unpacker the unpacker to use. * @param {number} structSize the retrieved struct size. * @param {BaseBuffer} buffer the buffer to unpack from. + * @param {boolean} disableLosslessIntegers if integer properties in the result date should be native JS numbers. * @return {Date} the unpacked neo4j date value. */ -function unpackDate(unpacker, structSize, buffer) { +function unpackDate(unpacker, structSize, buffer, disableLosslessIntegers) { unpacker._verifyStructSize('Date', DATE_STRUCT_SIZE, structSize); - const epochDay = unpacker.unpack(buffer); - return epochDayToDate(epochDay); + const epochDay = unpacker.unpackInteger(buffer); + const result = epochDayToDate(epochDay); + return convertIntegerPropsIfNeeded(result, disableLosslessIntegers); } /** @@ -378,15 +384,16 @@ function packLocalDateTime(value, packer, onError) { * @param {Unpacker} unpacker the unpacker to use. * @param {number} structSize the retrieved struct size. * @param {BaseBuffer} buffer the buffer to unpack from. + * @param {boolean} disableLosslessIntegers if integer properties in the result local date-time should be native JS numbers. * @return {LocalDateTime} the unpacked local date time value. */ -function unpackLocalDateTime(unpacker, structSize, buffer) { +function unpackLocalDateTime(unpacker, structSize, buffer, disableLosslessIntegers) { unpacker._verifyStructSize('LocalDateTime', LOCAL_DATE_TIME_STRUCT_SIZE, structSize); - const epochSecond = unpacker.unpack(buffer); - const nano = unpacker.unpack(buffer); - - return epochSecondAndNanoToLocalDateTime(epochSecond, nano); + const epochSecond = unpacker.unpackInteger(buffer); + const nano = unpacker.unpackInteger(buffer); + const result = epochSecondAndNanoToLocalDateTime(epochSecond, nano); + return convertIntegerPropsIfNeeded(result, disableLosslessIntegers); } /** @@ -413,18 +420,20 @@ function packDateTimeWithZoneOffset(value, packer, onError) { * @param {Unpacker} unpacker the unpacker to use. * @param {number} structSize the retrieved struct size. * @param {BaseBuffer} buffer the buffer to unpack from. + * @param {boolean} disableLosslessIntegers if integer properties in the result date-time should be native JS numbers. * @return {DateTimeWithZoneOffset} the unpacked date time with zone offset value. */ -function unpackDateTimeWithZoneOffset(unpacker, structSize, buffer) { +function unpackDateTimeWithZoneOffset(unpacker, structSize, buffer, disableLosslessIntegers) { unpacker._verifyStructSize('DateTimeWithZoneOffset', DATE_TIME_WITH_ZONE_OFFSET_STRUCT_SIZE, structSize); - const epochSecond = unpacker.unpack(buffer); - const nano = unpacker.unpack(buffer); - const offsetSeconds = unpacker.unpack(buffer); + const epochSecond = unpacker.unpackInteger(buffer); + const nano = unpacker.unpackInteger(buffer); + const offsetSeconds = unpacker.unpackInteger(buffer); const localDateTime = epochSecondAndNanoToLocalDateTime(epochSecond, nano); - return new DateTimeWithZoneOffset(localDateTime.year, localDateTime.month, localDateTime.day, localDateTime.hour, - localDateTime.minute, localDateTime.second, localDateTime.nanosecond, offsetSeconds); + const result = new DateTimeWithZoneOffset(localDateTime.year, localDateTime.month, localDateTime.day, + localDateTime.hour, localDateTime.minute, localDateTime.second, localDateTime.nanosecond, offsetSeconds); + return convertIntegerPropsIfNeeded(result, disableLosslessIntegers); } /** @@ -451,16 +460,34 @@ function packDateTimeWithZoneId(value, packer, onError) { * @param {Unpacker} unpacker the unpacker to use. * @param {number} structSize the retrieved struct size. * @param {BaseBuffer} buffer the buffer to unpack from. + * @param {boolean} disableLosslessIntegers if integer properties in the result date-time should be native JS numbers. * @return {DateTimeWithZoneId} the unpacked date time with zone id value. */ -function unpackDateTimeWithZoneId(unpacker, structSize, buffer) { +function unpackDateTimeWithZoneId(unpacker, structSize, buffer, disableLosslessIntegers) { unpacker._verifyStructSize('DateTimeWithZoneId', DATE_TIME_WITH_ZONE_ID_STRUCT_SIZE, structSize); - const epochSecond = unpacker.unpack(buffer); - const nano = unpacker.unpack(buffer); + const epochSecond = unpacker.unpackInteger(buffer); + const nano = unpacker.unpackInteger(buffer); const zoneId = unpacker.unpack(buffer); const localDateTime = epochSecondAndNanoToLocalDateTime(epochSecond, nano); - return new DateTimeWithZoneId(localDateTime.year, localDateTime.month, localDateTime.day, localDateTime.hour, - localDateTime.minute, localDateTime.second, localDateTime.nanosecond, zoneId); + const result = new DateTimeWithZoneId(localDateTime.year, localDateTime.month, localDateTime.day, + localDateTime.hour, localDateTime.minute, localDateTime.second, localDateTime.nanosecond, zoneId); + return convertIntegerPropsIfNeeded(result, disableLosslessIntegers); +} + +function convertIntegerPropsIfNeeded(obj, disableLosslessIntegers) { + if (!disableLosslessIntegers) { + return obj; + } + + const clone = Object.create(Object.getPrototypeOf(obj)); + for (let prop in obj) { + if (obj.hasOwnProperty(prop)) { + const value = obj[prop]; + clone[prop] = isInt(value) ? value.toNumberOrInfinity() : value; + } + } + Object.freeze(clone); + return clone; } diff --git a/src/v1/spatial-types.js b/src/v1/spatial-types.js index 6c382802c..56592cda7 100644 --- a/src/v1/spatial-types.js +++ b/src/v1/spatial-types.js @@ -17,8 +17,6 @@ * limitations under the License. */ -import {int} from './integer'; - const POINT_IDENTIFIER_PROPERTY = '__isPoint__'; /** @@ -29,13 +27,13 @@ export class Point { /** * @constructor - * @param {number|Integer} srid the coordinate reference system identifier. + * @param {Integer|number} srid the coordinate reference system identifier. * @param {number} x the x coordinate of the point. * @param {number} y the y coordinate of the point. * @param {number} [z=undefined] the y coordinate of the point or undefined if point has 2 dimensions. */ constructor(srid, x, y, z) { - this.srid = int(srid); + this.srid = srid; this.x = x; this.y = y; this.z = z; diff --git a/test/v1/spatial-types.test.js b/test/v1/spatial-types.test.js index b874aa587..698326e3c 100644 --- a/test/v1/spatial-types.test.js +++ b/test/v1/spatial-types.test.js @@ -21,6 +21,7 @@ import neo4j from '../../src/v1'; import sharedNeo4j from '../internal/shared-neo4j'; import {ServerVersion, VERSION_3_4_0} from '../../src/v1/internal/server-version'; import {isPoint, Point} from '../../src/v1/spatial-types'; +import _ from 'lodash'; const WGS_84_2D_CRS_CODE = neo4j.int(4326); const CARTESIAN_2D_CRS_CODE = neo4j.int(7203); @@ -31,11 +32,13 @@ const CARTESIAN_3D_CRS_CODE = neo4j.int(9157); describe('spatial-types', () => { let driver; + let driverWithNativeNumbers; let session; let serverVersion; beforeAll(done => { driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + driverWithNativeNumbers = neo4j.driver('bolt://localhost', sharedNeo4j.authToken, {disableLosslessIntegers: true}); ServerVersion.fromDriver(driver).then(version => { serverVersion = version; done(); @@ -47,6 +50,11 @@ describe('spatial-types', () => { driver.close(); driver = null; } + + if (driverWithNativeNumbers) { + driverWithNativeNumbers.close(); + driverWithNativeNumbers = null; + } }); beforeEach(done => { @@ -166,6 +174,22 @@ describe('spatial-types', () => { testSendingAndReceivingOfPoints(done, arrayOfPoints); }); + it('should receive point with number srid when disableLosslessIntegers=true', done => { + session = driverWithNativeNumbers.session(); + + testReceivingOfPoints(done, 'RETURN point({x: 42.231, y: 176.938123})', point => { + expect(isPoint(point)).toBeTruthy(); + expect(_.isNumber(point.srid)).toBeTruthy(); + expect(point.srid).toEqual(CARTESIAN_2D_CRS_CODE.toNumber()); + }); + }); + + it('should send and receive point with number srid when disableLosslessIntegers=true', done => { + session = driverWithNativeNumbers.session(); + + testSendingAndReceivingOfPoints(done, new Point(CARTESIAN_3D_CRS_CODE.toNumber(), 12.87, 13.89, 14.901)); + }); + function testReceivingOfPoints(done, query, pointChecker) { if (neo4jDoesNotSupportPoints(done)) { return; diff --git a/test/v1/temporal-types.test.js b/test/v1/temporal-types.test.js index 83095a6ad..2d5a9de48 100644 --- a/test/v1/temporal-types.test.js +++ b/test/v1/temporal-types.test.js @@ -40,6 +40,7 @@ describe('temporal-types', () => { let originalTimeout; let driver; + let driverWithNativeNumbers; let session; let serverVersion; @@ -48,6 +49,8 @@ describe('temporal-types', () => { jasmine.DEFAULT_TIMEOUT_INTERVAL = 30000; driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken); + driverWithNativeNumbers = neo4j.driver('bolt://localhost', sharedNeo4j.authToken, {disableLosslessIntegers: true}); + ServerVersion.fromDriver(driver).then(version => { serverVersion = version; done(); @@ -61,6 +64,11 @@ describe('temporal-types', () => { driver.close(); driver = null; } + + if (driverWithNativeNumbers) { + driverWithNativeNumbers.close(); + driverWithNativeNumbers = null; + } }); beforeEach(done => { @@ -96,6 +104,15 @@ describe('temporal-types', () => { testSendAndReceiveRandomTemporalValues(() => randomDuration(), done); }); + it('should send and receive Duration when disableLosslessIntegers=true', done => { + if (neo4jDoesNotSupportTemporalTypes(done)) { + return; + } + session = driverWithNativeNumbers.session(); + + testSendReceiveTemporalValue(new neo4j.Duration(4, 15, 931, 99953), done); + }); + it('should send and receive array of Duration', done => { if (neo4jDoesNotSupportTemporalTypes(done)) { return; @@ -131,6 +148,15 @@ describe('temporal-types', () => { testSendReceiveTemporalValue(minLocalTime, done); }); + it('should send and receive LocalTime when disableLosslessIntegers=true', done => { + if (neo4jDoesNotSupportTemporalTypes(done)) { + return; + } + session = driverWithNativeNumbers.session(); + + testSendReceiveTemporalValue(new neo4j.LocalTime(12, 32, 56, 12345), done); + }); + it('should send and receive random LocalTime', done => { if (neo4jDoesNotSupportTemporalTypes(done)) { return; @@ -174,6 +200,15 @@ describe('temporal-types', () => { testSendReceiveTemporalValue(minTime, done); }); + it('should send and receive Time when disableLosslessIntegers=true', done => { + if (neo4jDoesNotSupportTemporalTypes(done)) { + return; + } + session = driverWithNativeNumbers.session(); + + testSendReceiveTemporalValue(new neo4j.Time(22, 19, 32, 18381, MAX_TIME_ZONE_OFFSET), done); + }); + it('should send and receive random Time', done => { if (neo4jDoesNotSupportTemporalTypes(done)) { return; @@ -217,6 +252,15 @@ describe('temporal-types', () => { testSendReceiveTemporalValue(minDate, done); }); + it('should send and receive Date when disableLosslessIntegers=true', done => { + if (neo4jDoesNotSupportTemporalTypes(done)) { + return; + } + session = driverWithNativeNumbers.session(); + + testSendReceiveTemporalValue(new neo4j.Date(1923, 8, 14), done); + }); + it('should send and receive random Date', done => { if (neo4jDoesNotSupportTemporalTypes(done)) { return; @@ -260,6 +304,15 @@ describe('temporal-types', () => { testSendReceiveTemporalValue(minLocalDateTime, done); }); + it('should send and receive LocalDateTime when disableLosslessIntegers=true', done => { + if (neo4jDoesNotSupportTemporalTypes(done)) { + return; + } + session = driverWithNativeNumbers.session(); + + testSendReceiveTemporalValue(new neo4j.LocalDateTime(2045, 9, 1, 11, 25, 25, 911), done); + }); + it('should send and receive random LocalDateTime', done => { if (neo4jDoesNotSupportTemporalTypes(done)) { return; @@ -303,6 +356,15 @@ describe('temporal-types', () => { testSendReceiveTemporalValue(minDateTime, done); }); + it('should send and receive DateTimeWithZoneOffset when disableLosslessIntegers=true', done => { + if (neo4jDoesNotSupportTemporalTypes(done)) { + return; + } + session = driverWithNativeNumbers.session(); + + testSendReceiveTemporalValue(new neo4j.DateTimeWithZoneOffset(2022, 2, 7, 17, 15, 59, 12399, MAX_TIME_ZONE_OFFSET), done); + }); + it('should send and receive random DateTimeWithZoneOffset', done => { if (neo4jDoesNotSupportTemporalTypes(done)) { return; @@ -346,6 +408,15 @@ describe('temporal-types', () => { testSendReceiveTemporalValue(minDateTime, done); }); + it('should send and receive DateTimeWithZoneId when disableLosslessIntegers=true', done => { + if (neo4jDoesNotSupportTemporalTypes(done)) { + return; + } + session = driverWithNativeNumbers.session(); + + testSendReceiveTemporalValue(new neo4j.DateTimeWithZoneId(2011, 11, 25, 23, 59, 59, 192378, 'Europe/Stockholm'), done); + }); + it('should send and receive random DateTimeWithZoneId', done => { if (neo4jDoesNotSupportTemporalTypes(done)) { return; @@ -475,9 +546,6 @@ describe('temporal-types', () => { function testSendAndReceiveArrayOfRandomTemporalValues(valueGenerator, done) { const arrayLength = _.random(MIN_TEMPORAL_ARRAY_LENGTH, MAX_TEMPORAL_ARRAY_LENGTH); const values = _.range(arrayLength).map(() => valueGenerator()); - - console.log('Generated: ' + values); - testSendReceiveTemporalValue(values, done); }