From 60f301ee0499a8e4531b9608f76b53e3b71a0e33 Mon Sep 17 00:00:00 2001 From: lutovich Date: Wed, 14 Nov 2018 20:52:28 +0100 Subject: [PATCH] Fix conversion from standard Date to Time & DateTime Time zone offset was not converted correctly. Offset exposed by the standard JavaScript `Date` is the difference, in minutes, from local time to UTC. So positive offset means local time is behind UTC and negative means it's ahead. This is different from Neo4j temporal types that support time zone offsets - `Time` and `DateTime`. They define offset as the difference, in seconds, from UTC to local time. Previous code converted offset in minutes to offset in seconds but did not change the sign of the value. --- src/v1/internal/temporal-util.js | 13 ++++++- src/v1/temporal-types.js | 7 +++- test/internal/temporal-util.test.js | 17 ++++----- test/internal/test-utils.js | 7 ++++ test/v1/temporal-types.test.js | 58 +++++++++++++++++++++++++++-- 5 files changed, 86 insertions(+), 16 deletions(-) diff --git a/src/v1/internal/temporal-util.js b/src/v1/internal/temporal-util.js index ef3047a95..cd32de911 100644 --- a/src/v1/internal/temporal-util.js +++ b/src/v1/internal/temporal-util.js @@ -311,11 +311,22 @@ export function totalNanoseconds(standardDate, nanoseconds) { /** * Get the time zone offset in seconds from the given standard JavaScript date. + * + * Implementation note: + * Time zone offset returned by the standard JavaScript date is the difference, in minutes, from local time to UTC. + * So positive value means offset is behind UTC and negative value means it is ahead. + * For Neo4j temporal types, like `Time` or `DateTime` offset is in seconds and represents difference from UTC to local time. + * This is different from standard JavaScript dates and that's why implementation negates the returned value. + * * @param {global.Date} standardDate the standard JavaScript date. * @return {number} the time zone offset in seconds. */ export function timeZoneOffsetInSeconds(standardDate) { - return standardDate.getTimezoneOffset() * SECONDS_PER_MINUTE; + let offsetInMinutes = standardDate.getTimezoneOffset(); + if (offsetInMinutes === 0) { + return 0; + } + return -1 * offsetInMinutes * SECONDS_PER_MINUTE; } /** diff --git a/src/v1/temporal-types.js b/src/v1/temporal-types.js index 13fe242d6..4afa0bee9 100644 --- a/src/v1/temporal-types.js +++ b/src/v1/temporal-types.js @@ -139,7 +139,8 @@ export class Time { * @param {Integer|number} minute the minute for the new local time. * @param {Integer|number} second the second for the new local time. * @param {Integer|number} nanosecond the nanosecond for the new local time. - * @param {Integer|number} timeZoneOffsetSeconds the time zone offset in seconds. + * @param {Integer|number} timeZoneOffsetSeconds the time zone offset in seconds. Value represents the difference, in seconds, from UTC to local time. + * This is different from standard JavaScript `Date.getTimezoneOffset()` which is the difference, in minutes, from local time to UTC. */ constructor(hour, minute, second, nanosecond, timeZoneOffsetSeconds) { this.hour = util.assertValidHour(hour); @@ -312,7 +313,9 @@ export class DateTime { * @param {Integer|number} minute the minute for the new date-time. * @param {Integer|number} second the second for the new date-time. * @param {Integer|number} nanosecond the nanosecond for the new date-time. - * @param {Integer|number|null} timeZoneOffsetSeconds the total time zone offset in seconds for the new date-time. Either this argument or `timeZoneId` should be defined. + * @param {Integer|number} timeZoneOffsetSeconds the time zone offset in seconds. Either this argument or `timeZoneId` should be defined. + * Value represents the difference, in seconds, from UTC to local time. + * This is different from standard JavaScript `Date.getTimezoneOffset()` which is the difference, in minutes, from local time to UTC. * @param {string|null} timeZoneId the time zone id for the new date-time. Either this argument or `timeZoneOffsetSeconds` should be defined. */ constructor(year, month, day, hour, minute, second, nanosecond, timeZoneOffsetSeconds, timeZoneId) { diff --git a/test/internal/temporal-util.test.js b/test/internal/temporal-util.test.js index ee3610a4b..de25f839c 100644 --- a/test/internal/temporal-util.test.js +++ b/test/internal/temporal-util.test.js @@ -20,6 +20,7 @@ import {int} from '../../src/v1/integer'; import * as util from '../../src/v1/internal/temporal-util'; import {types} from '../../src/v1'; +import testUtils from './test-utils'; describe('temporal-util', () => { @@ -189,10 +190,12 @@ describe('temporal-util', () => { }); it('should get timezone offset in seconds from standard date', () => { - expect(util.timeZoneOffsetInSeconds(fakeStandardDateWithOffset(0))).toEqual(0); - expect(util.timeZoneOffsetInSeconds(fakeStandardDateWithOffset(2))).toEqual(120); - expect(util.timeZoneOffsetInSeconds(fakeStandardDateWithOffset(10))).toEqual(600); - expect(util.timeZoneOffsetInSeconds(fakeStandardDateWithOffset(101))).toEqual(6060); + expect(util.timeZoneOffsetInSeconds(testUtils.fakeStandardDateWithOffset(0))).toBe(0); + expect(util.timeZoneOffsetInSeconds(testUtils.fakeStandardDateWithOffset(2))).toBe(-120); + expect(util.timeZoneOffsetInSeconds(testUtils.fakeStandardDateWithOffset(10))).toBe(-600); + expect(util.timeZoneOffsetInSeconds(testUtils.fakeStandardDateWithOffset(101))).toBe(-6060); + expect(util.timeZoneOffsetInSeconds(testUtils.fakeStandardDateWithOffset(-180))).toBe(10800); + expect(util.timeZoneOffsetInSeconds(testUtils.fakeStandardDateWithOffset(-600))).toBe(36000); }); it('should verify year', () => { @@ -330,9 +333,3 @@ function localTime(hour, minute, second, nanosecond) { function localDateTime(year, month, day, hour, minute, second, nanosecond) { return new types.LocalDateTime(int(year), int(month), int(day), int(hour), int(minute), int(second), int(nanosecond)); } - -function fakeStandardDateWithOffset(offsetMinutes) { - const date = new Date(); - date.getTimezoneOffset = () => offsetMinutes; - return date; -} diff --git a/test/internal/test-utils.js b/test/internal/test-utils.js index 97398f3d4..a7a01d100 100644 --- a/test/internal/test-utils.js +++ b/test/internal/test-utils.js @@ -24,7 +24,14 @@ function isServer() { return !isClient(); } +function fakeStandardDateWithOffset(offsetMinutes) { + const date = new Date(); + date.getTimezoneOffset = () => offsetMinutes; + return date; +} + export default { isClient, isServer, + fakeStandardDateWithOffset }; diff --git a/test/v1/temporal-types.test.js b/test/v1/temporal-types.test.js index 9b1499c23..7afd55d22 100644 --- a/test/v1/temporal-types.test.js +++ b/test/v1/temporal-types.test.js @@ -19,10 +19,11 @@ import neo4j from '../../src'; import sharedNeo4j from '../internal/shared-neo4j'; -import {totalNanoseconds} from '../../src/v1/internal/temporal-util'; +import {timeZoneOffsetInSeconds, totalNanoseconds} from '../../src/v1/internal/temporal-util'; import {ServerVersion, VERSION_3_4_0} from '../../src/v1/internal/server-version'; import timesSeries from 'async/timesSeries'; import _ from 'lodash'; +import testUtils from '../internal/test-utils'; const RANDOM_VALUES_TO_TEST = 2000; const MIN_TEMPORAL_ARRAY_LENGTH = 20; @@ -921,6 +922,50 @@ describe('temporal-types', () => { expect(() => dateTimeWithZoneOffset(1, 1, 1, 1, 1, 1, 1000000000, 0)).toThrow(); }); + it('should convert standard Date with offset to neo4j Time', () => { + const standardDate1 = testUtils.fakeStandardDateWithOffset(0); + const neo4jTime1 = neo4j.types.Time.fromStandardDate(standardDate1); + verifyTimeZoneOffset(neo4jTime1, 0, 'Z'); + + const standardDate2 = testUtils.fakeStandardDateWithOffset(-600); + const neo4jTime2 = neo4j.types.Time.fromStandardDate(standardDate2); + verifyTimeZoneOffset(neo4jTime2, 600 * 60, '+10:00'); + + const standardDate3 = testUtils.fakeStandardDateWithOffset(480); + const neo4jTime3 = neo4j.types.Time.fromStandardDate(standardDate3); + verifyTimeZoneOffset(neo4jTime3, -1 * 480 * 60, '-08:00'); + + const standardDate4 = testUtils.fakeStandardDateWithOffset(-180); + const neo4jTime4 = neo4j.types.Time.fromStandardDate(standardDate4); + verifyTimeZoneOffset(neo4jTime4, 180 * 60, '+03:00'); + + const standardDate5 = testUtils.fakeStandardDateWithOffset(150); + const neo4jTime5 = neo4j.types.Time.fromStandardDate(standardDate5); + verifyTimeZoneOffset(neo4jTime5, -1 * 150 * 60, '-02:30'); + }); + + it('should convert standard Date with offset to neo4j DateTime', () => { + const standardDate1 = testUtils.fakeStandardDateWithOffset(0); + const neo4jDateTime1 = neo4j.types.DateTime.fromStandardDate(standardDate1); + verifyTimeZoneOffset(neo4jDateTime1, 0, 'Z'); + + const standardDate2 = testUtils.fakeStandardDateWithOffset(-600); + const neo4jDateTime2 = neo4j.types.DateTime.fromStandardDate(standardDate2); + verifyTimeZoneOffset(neo4jDateTime2, 600 * 60, '+10:00'); + + const standardDate3 = testUtils.fakeStandardDateWithOffset(480); + const neo4jDateTime3 = neo4j.types.DateTime.fromStandardDate(standardDate3); + verifyTimeZoneOffset(neo4jDateTime3, -1 * 480 * 60, '-08:00'); + + const standardDate4 = testUtils.fakeStandardDateWithOffset(-180); + const neo4jDateTime4 = neo4j.types.DateTime.fromStandardDate(standardDate4); + verifyTimeZoneOffset(neo4jDateTime4, 180 * 60, '+03:00'); + + const standardDate5 = testUtils.fakeStandardDateWithOffset(150); + const neo4jDateTime5 = neo4j.types.DateTime.fromStandardDate(standardDate5); + verifyTimeZoneOffset(neo4jDateTime5, -1 * 150 * 60, '-02:30'); + }); + function testSendAndReceiveRandomTemporalValues(valueGenerator, done) { const asyncFunction = (index, callback) => { const next = () => callback(); @@ -1117,7 +1162,7 @@ describe('temporal-types', () => { function testStandardDateToTimeConversion(date, nanosecond) { const converted = neo4j.types.Time.fromStandardDate(date, nanosecond); const expected = new neo4j.types.Time(date.getHours(), date.getMinutes(), date.getSeconds(), totalNanoseconds(date, nanosecond), - date.getTimezoneOffset() * 60); + timeZoneOffsetInSeconds(date)); expect(converted).toEqual(expected); } @@ -1137,7 +1182,14 @@ describe('temporal-types', () => { function testStandardDateToDateTimeConversion(date, nanosecond) { const converted = neo4j.types.DateTime.fromStandardDate(date, nanosecond); const expected = new neo4j.types.DateTime(date.getFullYear(), date.getMonth() + 1, date.getDate(), date.getHours(), date.getMinutes(), date.getSeconds(), - totalNanoseconds(date, nanosecond), date.getTimezoneOffset() * 60); + totalNanoseconds(date, nanosecond), timeZoneOffsetInSeconds(date)); expect(converted).toEqual(expected); } + + function verifyTimeZoneOffset(temporal, expectedValue, expectedStringValue) { + expect(temporal.timeZoneOffsetSeconds).toEqual(expectedValue); + const isoString = temporal.toString(); + // assert ISO string ends with the expected suffix + expect(isoString.indexOf(expectedStringValue, isoString.length - expectedStringValue.length)).toBeGreaterThan(0); + } });