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.

Author: lutovich

src/v1/internal/temporal-util.js

@@ -311,11 +311,18 @@ export function totalNanoseconds(standardDate, nanoseconds) {
 
 /**
  * Get the time zone offset in seconds from the given standard JavaScript date.
+ *
+ * <b>Implementation note:</b>
+ * 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;
+  return -1 * standardDate.getTimezoneOffset() * SECONDS_PER_MINUTE;
 }
 
 /**

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) {

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;
-}

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
 };

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).toBe(expectedValue);
+    const isoString = temporal.toString();
+    // assert ISO string ends with the expected suffix
+    expect(isoString.indexOf(expectedStringValue, isoString.length - expectedStringValue.length)).toBeGreaterThan(0);
+  }
 });