fixing minute offset issue in certain time zones (#1238)

MaxAake · web-flow · commit 4090cb6ae18d · 2024-12-18T09:32:29.000+01:00
Timezones with minute offsets not divisible by 60 were causing issues for converting from the driver implementation of Date and Datetime to JS Date. A better implementation has been created and some documentation has been updated to reflect limitations that already existed.
diff --git a/packages/bolt-connection/test/test-utils.js b/packages/bolt-connection/test/test-utils.js
@@ -31,12 +31,6 @@ function isServer () {
   return !isClient()
 }
 
-function fakeStandardDateWithOffset (offsetMinutes) {
-  const date = new Date()
-  date.getTimezoneOffset = () => offsetMinutes
-  return date
-}
-
 const matchers = {
   toBeElementOf: function (actual, expected) {
     if (expected === undefined) {
@@ -161,7 +155,6 @@ function arbitraryTimeZoneId () {
 export default {
   isClient,
   isServer,
-  fakeStandardDateWithOffset,
   matchers,
   MessageRecordingConnection,
   spyProtocolWrite,
diff --git a/packages/core/src/internal/temporal-util.ts b/packages/core/src/internal/temporal-util.ts
@@ -339,24 +339,54 @@ export function totalNanoseconds (
 /**
  * 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: Date): number {
-  const secondsPortion = standardDate.getSeconds() >= standardDate.getUTCSeconds()
-    ? standardDate.getSeconds() - standardDate.getUTCSeconds()
-    : standardDate.getSeconds() - standardDate.getUTCSeconds() + 60
-  const offsetInMinutes = standardDate.getTimezoneOffset()
-  if (offsetInMinutes === 0) {
-    return 0 + secondsPortion
+  const secondsPortion = standardDate.getSeconds() - standardDate.getUTCSeconds()
+  const minutesPortion = standardDate.getMinutes() - standardDate.getUTCMinutes()
+  const hoursPortion = standardDate.getHours() - standardDate.getUTCHours()
+  const daysPortion = _getDayOffset(standardDate)
+  return hoursPortion * SECONDS_PER_HOUR + minutesPortion * SECONDS_PER_MINUTE + secondsPortion + daysPortion * SECONDS_PER_DAY
+}
+
+/**
+ * Get the difference in days from the given JavaScript date in local time and UTC.
+ *
+ * @private
+ * @param {global.Date} standardDate the date to evaluate
+ * @returns  {number} the difference in days between date local time and UTC
+ */
+function _getDayOffset (standardDate: Date): number {
+  if (standardDate.getMonth() === standardDate.getUTCMonth()) {
+    return standardDate.getDate() - standardDate.getUTCDate()
+  } else if ((standardDate.getFullYear() > standardDate.getUTCFullYear()) || (standardDate.getMonth() > standardDate.getUTCMonth() && standardDate.getFullYear() === standardDate.getUTCFullYear())) {
+    return standardDate.getDate() + _daysUntilNextMonth(standardDate.getUTCMonth(), standardDate.getUTCFullYear()) - standardDate.getUTCDate()
+  } else {
+    return standardDate.getDate() - (standardDate.getUTCDate() + _daysUntilNextMonth(standardDate.getMonth(), standardDate.getFullYear()))
+  }
+}
+
+/**
+ * Get the number of days in a month, including a check for leap years.
+ *
+ * @private
+ * @param {number} month the month of the date to evalutate
+ * @param {number} year the month of the date to evalutate
+ * @returns {number} the total number of days in the month evaluated
+ */
+function _daysUntilNextMonth (month: number, year: number): number {
+  if (month === 1) {
+    if (year % 400 === 0 || (year % 4 === 0 && year % 100 !== 0)) {
+      return 29
+    } else {
+      return 28
+    }
+  } else if ([0, 2, 4, 6, 7, 9, 11].includes(month)) {
+    return 31
+  } else {
+    return 30
   }
-  return -1 * offsetInMinutes * SECONDS_PER_MINUTE + secondsPortion
 }
 
 /**
diff --git a/packages/core/src/temporal-types.ts b/packages/core/src/temporal-types.ts
@@ -353,6 +353,10 @@ export class Date<T extends NumberOrInteger = Integer> {
   /**
    * Create a {@link Date} object from the given standard JavaScript `Date`.
    * Hour, minute, second, millisecond and time zone offset components of the given date are ignored.
+   *
+   * NOTE: the function {@link toStandardDate} and {@link fromStandardDate} are not inverses of one another. {@link fromStandardDate} takes the Day, Month and Year in local time from the supplies JavaScript Date object, while {@link toStandardDate} creates a new JavaScript Date object at midnight UTC. This incongruity will be rectified in 6.0
+   * If your timezone has a negative offset from UTC, creating a JavaScript Date at midnight UTC and converting it with {@link fromStandardDate} will result in a Date for the day before.
+   *
    * @param {global.Date} standardDate - The standard JavaScript date to convert.
    * @return {Date} New Date.
    */
@@ -372,6 +376,8 @@ export class Date<T extends NumberOrInteger = Integer> {
    * The time component of the returned `Date` is set to midnight
    * and the time zone is set to UTC.
    *
+   * NOTE: the function {@link toStandardDate} and {@link fromStandardDate} are not inverses of one another. {@link fromStandardDate} takes the Day, Month and Year in local time from the supplies JavaScript Date object, while {@link toStandardDate} creates a new JavaScript Date object at midnight UTC. This incongruity will be rectified in 6.0
+   *
    * @returns {StandardDate} Standard JavaScript `Date` at `00:00:00.000` UTC.
    */
   toStandardDate (): StandardDate {
diff --git a/packages/core/test/temporal-types.test.ts b/packages/core/test/temporal-types.test.ts
@@ -15,7 +15,6 @@
  * limitations under the License.
  */
 
-import { StandardDate } from '../src/graph-types'
 import { LocalDateTime, Date, DateTime, Duration, isDuration, LocalTime, isLocalTime, Time, isTime, isDate, isLocalDateTime, isDateTime } from '../src/temporal-types'
 import { temporalUtil } from '../src/internal'
 import fc from 'fast-check'
@@ -31,9 +30,9 @@ describe('Date', () => {
 
       const standardDate = localDatetime.toStandardDate()
 
-      expect(standardDate.getFullYear()).toEqual(localDatetime.year)
-      expect(standardDate.getMonth()).toEqual(localDatetime.month - 1)
-      expect(standardDate.getDate()).toEqual(localDatetime.day)
+      expect(standardDate.getUTCFullYear()).toEqual(localDatetime.year)
+      expect(standardDate.getUTCMonth()).toEqual(localDatetime.month - 1)
+      expect(standardDate.getUTCDate()).toEqual(localDatetime.day)
     })
 
     it('should be the reverse operation of fromStandardDate but losing time information', () => {
@@ -47,14 +46,11 @@ describe('Date', () => {
             const date = Date.fromStandardDate(standardDate)
             const receivedDate = date.toStandardDate()
 
-            const adjustedDateTime = temporalUtil.newDate(standardDate)
-            adjustedDateTime.setHours(0, offset(receivedDate))
-
-            expect(receivedDate.getFullYear()).toEqual(adjustedDateTime.getFullYear())
-            expect(receivedDate.getMonth()).toEqual(adjustedDateTime.getMonth())
-            expect(receivedDate.getDate()).toEqual(adjustedDateTime.getDate())
-            expect(receivedDate.getHours()).toEqual(adjustedDateTime.getHours())
-            expect(receivedDate.getMinutes()).toEqual(adjustedDateTime.getMinutes())
+            expect(receivedDate.getUTCFullYear()).toEqual(standardDate.getFullYear()) // Date converts from local time but to UTC
+            expect(receivedDate.getUTCMonth()).toEqual(standardDate.getMonth())
+            expect(receivedDate.getUTCDate()).toEqual(standardDate.getDate())
+            expect(receivedDate.getUTCHours()).toEqual(0)
+            expect(receivedDate.getUTCMinutes()).toEqual(0)
           })
       )
     })
@@ -113,35 +109,33 @@ describe('DateTime', () => {
 
       const standardDate = datetime.toStandardDate()
 
-      expect(standardDate.getFullYear()).toEqual(datetime.year)
-      expect(standardDate.getMonth()).toEqual(datetime.month - 1)
-      expect(standardDate.getDate()).toEqual(datetime.day)
-      const offsetInMinutes = offset(standardDate)
-      const offsetAdjust = offsetInMinutes - (datetime.timeZoneOffsetSeconds ?? 0) / 60
-      const hourDiff = Math.abs(offsetAdjust / 60)
+      expect(standardDate.getUTCFullYear()).toEqual(datetime.year)
+      expect(standardDate.getUTCMonth()).toEqual(datetime.month - 1)
+      expect(standardDate.getUTCDate()).toEqual(datetime.day) // The datetime in this test will never cross the date line in conversion, it is therefore safe to use UTC here to avoid machine timezone from altering the result of the test.
+      const offsetAdjust = (datetime.timeZoneOffsetSeconds ?? 0) / 60
+      const hourDiff = Math.abs((offsetAdjust - offsetAdjust % 60) / 60)
       const minuteDiff = Math.abs(offsetAdjust % 60)
-      expect(standardDate.getHours()).toBe(datetime.hour - hourDiff)
-      expect(standardDate.getMinutes()).toBe(datetime.minute - minuteDiff)
-      expect(standardDate.getSeconds()).toBe(datetime.second)
-      expect(standardDate.getMilliseconds()).toBe(Math.round(datetime.nanosecond / 1000000))
+      expect(standardDate.getUTCHours()).toBe(datetime.hour - hourDiff)
+      expect(standardDate.getUTCMinutes()).toBe(datetime.minute - minuteDiff)
+      expect(standardDate.getUTCSeconds()).toBe(datetime.second)
+      expect(standardDate.getUTCMilliseconds()).toBe(Math.round(datetime.nanosecond / 1000000))
     })
 
     it('should convert to a standard date (offset)', () => {
       const datetime = new DateTime(2020, 12, 15, 12, 2, 3, 4000000, 120 * 60)
 
       const standardDate = datetime.toStandardDate()
 
-      expect(standardDate.getFullYear()).toEqual(datetime.year)
-      expect(standardDate.getMonth()).toEqual(datetime.month - 1)
-      expect(standardDate.getDate()).toEqual(datetime.day)
-      const offsetInMinutes = offset(standardDate)
-      const offsetAdjust = offsetInMinutes - (datetime.timeZoneOffsetSeconds ?? 0) / 60
-      const hourDiff = Math.abs(offsetAdjust / 60)
+      expect(standardDate.getUTCFullYear()).toEqual(datetime.year)
+      expect(standardDate.getUTCMonth()).toEqual(datetime.month - 1)
+      expect(standardDate.getUTCDate()).toEqual(datetime.day)
+      const offsetAdjust = (datetime.timeZoneOffsetSeconds ?? 0) / 60
+      const hourDiff = Math.abs((offsetAdjust - offsetAdjust % 60) / 60)
       const minuteDiff = Math.abs(offsetAdjust % 60)
-      expect(standardDate.getHours()).toBe(datetime.hour - hourDiff)
-      expect(standardDate.getMinutes()).toBe(datetime.minute - minuteDiff)
-      expect(standardDate.getSeconds()).toBe(datetime.second)
-      expect(standardDate.getMilliseconds()).toBe(Math.round(datetime.nanosecond / 1000000))
+      expect(standardDate.getUTCHours()).toBe(datetime.hour - hourDiff)
+      expect(standardDate.getUTCMinutes()).toBe(datetime.minute - minuteDiff)
+      expect(standardDate.getUTCSeconds()).toBe(datetime.second)
+      expect(standardDate.getUTCMilliseconds()).toBe(Math.round(datetime.nanosecond / 1000000))
     })
 
     it('should not convert to a standard date (zoneid)', () => {
@@ -153,12 +147,16 @@ describe('DateTime', () => {
 
     it('should be the reverse operation of fromStandardDate', () => {
       fc.assert(
-        fc.property(fc.date(), (date) => {
-          const datetime = DateTime.fromStandardDate(date)
-          const receivedDate = datetime.toStandardDate()
+        fc.property(
+          fc.date({
+            max: temporalUtil.newDate(MAX_UTC_IN_MS - ONE_DAY_IN_MS),
+            min: temporalUtil.newDate(MIN_UTC_IN_MS + ONE_DAY_IN_MS)
+          }), (date) => {
+            const datetime = DateTime.fromStandardDate(date)
+            const receivedDate = datetime.toStandardDate()
 
-          expect(receivedDate).toEqual(date)
-        })
+            expect(receivedDate).toEqual(date)
+          })
       )
     })
   })
@@ -284,15 +282,3 @@ describe('isDateTime', () => {
     }
   })
 })
-
-/**
- * The offset in StandardDate is the number of minutes
- * to sum to the date and time to get the UTC time.
- *
- * This function change the sign of the offset,
- * this way using the most common meaning.
- * The time to add to UTC to get the local time.
- */
-function offset (date: StandardDate): number {
-  return date.getTimezoneOffset() * -1
-}
diff --git a/packages/neo4j-driver-deno/lib/core/internal/temporal-util.ts b/packages/neo4j-driver-deno/lib/core/internal/temporal-util.ts
diff --git a/packages/neo4j-driver-deno/lib/core/temporal-types.ts b/packages/neo4j-driver-deno/lib/core/temporal-types.ts
diff --git a/packages/neo4j-driver/test/internal/temporal-util.test.js b/packages/neo4j-driver/test/internal/temporal-util.test.js
@@ -16,7 +16,6 @@
  */
 
 import { int, internal } from 'neo4j-driver-core'
-import testUtils from './test-utils'
 
 const { temporalUtil: util } = internal
 
@@ -261,27 +260,6 @@ describe('#unit temporal-util', () => {
     ).toEqual(BigInt(999000111))
   })
 
-  it('should get timezone offset in seconds from standard date', () => {
-    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', () => {
     expect(util.assertValidYear(-1)).toEqual(-1)
     expect(util.assertValidYear(-2010)).toEqual(-2010)
diff --git a/packages/neo4j-driver/test/internal/test-utils.js b/packages/neo4j-driver/test/internal/test-utils.js
@@ -24,12 +24,6 @@ function isServer () {
   return !isClient()
 }
 
-function fakeStandardDateWithOffset (offsetMinutes) {
-  const date = new Date()
-  date.getTimezoneOffset = () => offsetMinutes
-  return date
-}
-
 const matchers = {
   toBeElementOf: function (util, customEqualityTesters) {
     return {
@@ -138,7 +132,6 @@ function spyProtocolWrite (protocol, callRealMethod = false) {
 export default {
   isClient,
   isServer,
-  fakeStandardDateWithOffset,
   matchers,
   MessageRecordingConnection,
   spyProtocolWrite
diff --git a/packages/neo4j-driver/test/temporal-types.test.js b/packages/neo4j-driver/test/temporal-types.test.js
