Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(datastore): store time zone info in Temporal.DateTime #3393

Merged
merged 5 commits into from
Dec 11, 2023
Merged

fix(datastore): store time zone info in Temporal.DateTime #3393

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec693bf
fix(datastore): store time zone info in Temporal.DateTime
5d Nov 30, 2023
b37416c
change to use private enum
5d Nov 30, 2023
67c5baf
add support for hh:mm:ss timezone format
5d Dec 7, 2023
2dec042
resolve comments
5d Dec 8, 2023
c792c45
refactor implementation with enums
5d Dec 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Amplify/Categories/DataStore/Model/Internal/Model+DateFormatting.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public struct ModelDateFormatting {
public static let encodingStrategy: JSONEncoder.DateEncodingStrategy = {
let strategy = JSONEncoder.DateEncodingStrategy.custom { date, encoder in
var container = encoder.singleValueContainer()
try container.encode(Temporal.DateTime(date).iso8601String)
try container.encode(Temporal.DateTime(date, timeZone: .utc).iso8601String)
}
return strategy
}()
Expand Down
8 changes: 6 additions & 2 deletions Amplify/Categories/DataStore/Model/Temporal/Date.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,20 @@ extension Temporal {
///
/// - Note: `.medium`, `.long`, and `.full` are the same date format.
public struct Date: TemporalSpec {

// Inherits documentation from `TemporalSpec`
public let foundationDate: Foundation.Date

// Inherits documentation from `TemporalSpec`
public let timeZone: TimeZone? = .utc

// Inherits documentation from `TemporalSpec`
public static func now() -> Self {
Temporal.Date(Foundation.Date())
Temporal.Date(Foundation.Date(), timeZone: .utc)
}

// Inherits documentation from `TemporalSpec`
public init(_ date: Foundation.Date) {
public init(_ date: Foundation.Date, timeZone: TimeZone?) {
self.foundationDate = Temporal
.iso8601Calendar
.startOfDay(for: date)
Expand Down
14 changes: 11 additions & 3 deletions Amplify/Categories/DataStore/Model/Temporal/DateTime.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,33 @@ extension Temporal {
/// * `.long` => `yyyy-MM-dd'T'HH:mm:ssZZZZZ`
/// * `.full` => `yyyy-MM-dd'T'HH:mm:ss.SSSZZZZZ`
public struct DateTime: TemporalSpec {

// Inherits documentation from `TemporalSpec`
public let foundationDate: Foundation.Date

// Inherits documentation from `TemporalSpec`
public let timeZone: TimeZone?

// Inherits documentation from `TemporalSpec`
public static func now() -> Self {
Temporal.DateTime(Foundation.Date())
Temporal.DateTime(Foundation.Date(), timeZone: .utc)
}

/// `Temporal.Time` of this `Temporal.DateTime`.
public var time: Time {
Time(foundationDate)
Time(foundationDate, timeZone: timeZone)
}

// Inherits documentation from `TemporalSpec`
public init(_ date: Foundation.Date) {
public init(_ date: Foundation.Date, timeZone: TimeZone? = .utc) {
let calendar = Temporal.iso8601Calendar
let components = calendar.dateComponents(
DateTime.iso8601DateComponents,
from: date
)

self.timeZone = timeZone

foundationDate = calendar
.date(from: components) ?? date
}
Expand All @@ -57,3 +63,5 @@ extension Temporal {

// Allow date unit and time unit operations on `Temporal.DateTime`
extension Temporal.DateTime: DateUnitOperable, TimeUnitOperable {}

extension Temporal.DateTime: Sendable { }
8 changes: 5 additions & 3 deletions Amplify/Categories/DataStore/Model/Temporal/SpecBasedDateConverting.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import Foundation
@usableFromInline
internal struct SpecBasedDateConverting<Spec: TemporalSpec> {
@usableFromInline
internal typealias DateConverter = (_ string: String, _ format: TemporalFormat?) throws -> Date
internal typealias DateConverter = (_ string: String, _ format: TemporalFormat?) throws -> (Date, TimeZone)

@usableFromInline
internal let convert: DateConverter
Expand All @@ -28,19 +28,21 @@ internal struct SpecBasedDateConverting<Spec: TemporalSpec> {
internal static func `default`(
iso8601String: String,
format: TemporalFormat? = nil
) throws -> Date {
) throws -> (Date, TimeZone) {
let date: Foundation.Date
let tz: TimeZone = TimeZone(iso8601DateString: iso8601String) ?? .utc
if let format = format {
date = try Temporal.date(
from: iso8601String,
with: [format(for: Spec.self)]
)

} else {
date = try Temporal.date(
from: iso8601String,
with: TemporalFormat.sortedFormats(for: Spec.self)
)
}
return date
return (date, tz)
}
}
6 changes: 4 additions & 2 deletions Amplify/Categories/DataStore/Model/Temporal/Temporal+Comparable.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ import Foundation
extension TemporalSpec where Self: Comparable {

public static func == (lhs: Self, rhs: Self) -> Bool {
return lhs.iso8601String == rhs.iso8601String
return lhs.iso8601FormattedString(format: .full, timeZone: .utc)
== rhs.iso8601FormattedString(format: .full, timeZone: .utc)
}

public static func < (lhs: Self, rhs: Self) -> Bool {
return lhs.iso8601String < rhs.iso8601String
return lhs.iso8601FormattedString(format: .full, timeZone: .utc)
< rhs.iso8601FormattedString(format: .full, timeZone: .utc)
}
}

Expand Down
16 changes: 10 additions & 6 deletions Amplify/Categories/DataStore/Model/Temporal/Temporal.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ public protocol TemporalSpec {
/// by a Foundation `Date` instance.
var foundationDate: Foundation.Date { get }

/// The timezone field is an optional field used to specify the timezone associated
/// with a particular date.
var timeZone: TimeZone? { get }

/// The ISO-8601 formatted string in the UTC `TimeZone`.
/// - SeeAlso: `iso8601FormattedString(TemporalFormat, TimeZone) -> String`
var iso8601String: String { get }
Expand Down Expand Up @@ -57,7 +61,7 @@ public protocol TemporalSpec {
/// Constructs a `TemporalSpec` from a `Date` object.
/// - Parameter date: The `Date` instance that will be used as the reference of the
/// `TemporalSpec` instance.
init(_ date: Foundation.Date)
init(_ date: Foundation.Date, timeZone: TimeZone?)

/// A string representation of the underlying date formatted using ISO8601 rules.
///
Expand Down Expand Up @@ -90,25 +94,25 @@ extension TemporalSpec {
/// The ISO8601 representation of the scalar using `.full` as the format and `.utc` as `TimeZone`.
/// - SeeAlso: `iso8601FormattedString(format:timeZone:)`
public var iso8601String: String {
iso8601FormattedString(format: .full)
iso8601FormattedString(format: .full, timeZone: timeZone ?? .utc)
}

@inlinable
public init(iso8601String: String, format: TemporalFormat) throws {
let date = try SpecBasedDateConverting<Self>()
let (date, tz) = try SpecBasedDateConverting<Self>()
.convert(iso8601String, format)

self.init(date)
self.init(date, timeZone: tz)
}

@inlinable
public init(
iso8601String: String
) throws {
let date = try SpecBasedDateConverting<Self>()
let (date, tz) = try SpecBasedDateConverting<Self>()
.convert(iso8601String, nil)

self.init(date)
self.init(date, timeZone: tz)
}
}

Expand Down
2 changes: 1 addition & 1 deletion Amplify/Categories/DataStore/Model/Temporal/TemporalOperation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ extension TemporalSpec {
"""
)
}
return Self.init(date)
return Self.init(date, timeZone: timeZone)
}
}
8 changes: 5 additions & 3 deletions Amplify/Categories/DataStore/Model/Temporal/Time.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@ extension Temporal {
// Inherits documentation from `TemporalSpec`
public let foundationDate: Foundation.Date

// Inherits documentation from `TemporalSpec`
public let timeZone: TimeZone? = .utc

// Inherits documentation from `TemporalSpec`
public static func now() -> Self {
Temporal.Time(Foundation.Date())
Temporal.Time(Foundation.Date(), timeZone: .utc)
}

// Inherits documentation from `TemporalSpec`
public init(_ date: Foundation.Date) {
public init(_ date: Foundation.Date, timeZone: TimeZone?) {
// Sets the date to a fixed instant so time-only operations are safe
let calendar = Temporal.iso8601Calendar
var components = calendar.dateComponents(
Expand All @@ -45,7 +48,6 @@ extension Temporal {
components.year = 2_000
components.month = 1
components.day = 1

self.foundationDate = calendar
.date(from: components) ?? date
}
Expand Down
136 changes: 136 additions & 0 deletions Amplify/Categories/DataStore/Model/Temporal/TimeZone+Extension.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
//
// Copyright Amazon.com Inc. or its affiliates.
// All Rights Reserved.
//
// SPDX-License-Identifier: Apache-2.0
//


import Foundation

extension TimeZone {
private static let iso8601TimeZoneHHColonMMColonSSRegex = try? NSRegularExpression(pattern: "^[+-]\\d{2}:\\d{2}:\\d{2}$")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to HH:MM:SS, does AWSDateTime also support HHMMSS? HH:MMSS? HHMM:SS ? I think we should verify this and try to support it fully if it does not impact the performance of the app during parsing the input string

I suspect that HHMMSS is supported since the docs say "can optionally include a time zone offset" linking to the ISO 8601 wiki, while only providing "hh:mm:ss" as an example

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From current amplify-js implementation, it only supports HH:MM:SS format.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's surprising, I ran a few live tests

HH:MM:SS works as expected

mutation MyMutationHHColonMMColonSS {
  createTest(input: {userID: "123", dt: "2023-11-30T11:04:03+08:00:30"}) {
    dt
  }
}
{
  "data": {
    "createTest": {
      "dt": "2023-11-30T11:04:03+08:00:30"
    }
  }
}

HHMM does not work

mutation MyMutationHHMM {
  createTest(input: {userID: "123", dt: "2023-11-30T11:04:03+0800"}) {
    dt
  }
}

{
  "data": null,
  "errors": [
    {
      "path": null,
      "locations": [
        {
          "line": 2,
          "column": 14,
          "sourceName": null
        }
      ],
      "message": "Validation error of type WrongType: argument 'input.dt' with value 'StringValue{value='2023-11-30T11:04:03+0800'}' is not a valid 'AWSDateTime' @ 'createTest'"
    }
  ]
}

So expectedly, HHMMSS also does not work..

mutation MyMutationHHMMSS {
  createTest(input: {userID: "123", dt: "2023-11-30T11:04:03+080030"}) {
    dt
  }
}
{
  "data": null,
  "errors": [
    {
      "path": null,
      "locations": [
        {
          "line": 2,
          "column": 14,
          "sourceName": null
        }
      ],
      "message": "Validation error of type WrongType: argument 'input.dt' with value 'StringValue{value='2023-11-30T11:04:03+080030'}' is not a valid 'AWSDateTime' @ 'createTest'"
    }
  ]
}

HH:MM:SS works as expected

mutation MyMutationHHColonMMColonSS {
  createTest(input: {userID: "123", dt: "2023-11-30T11:04:03+08:00:30"}) {
    dt
  }
}

{
  "data": {
    "createTest": {
      "dt": "2023-11-30T11:04:03+08:00:30"
    }
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what we have now looks good to me.

  • Data that is AWSDateTime with HH:MM:SS will be decoded properly.

  • Data would never be stored in the format HHMM since AppSync doesn't allow it, so even though we support decoding HHMM, that code path won't ever be used. This means it may pose the inverse problem: AppSync type is more restrictive than the Amplify Swift type. We allow storing date times with offset containing HHMM but the mutation with AppSync will fail. I believe this is still the correct thing to do since it conforms to the iSO 8601 spec and we can have varying types of data sources, ie. SQLite / local-only use cases.

  • HHMMSS is not supported by AppSync AWSDateTime and not part of the ISO8601 spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for verifying this with AppSync.
We won't send date with string format HHMM or HH:MM:SS. We always send date to AppSync with string formatHH:MM code.

The format HHMM or HH:MM:SS are only the acceptable inputs. The code here is ensure we are able to decode these ISO8601 date formats to correct Tempral.DateTime instance. When encoding them back to string and submit to AppSync. They will always be in HH:MM format.

private static let iso8601TimeZoneHHColonMMRegex = try? NSRegularExpression(pattern: "^[+-]\\d{2}:\\d{2}$")
private static let iso8601TimeZoneHHMMRegex = try? NSRegularExpression(pattern: "^[+-]\\d{2}\\d{2}$")
private static let iso8601TimeZoneHHRegex = try? NSRegularExpression(pattern: "^[+-]\\d{2}$")
lawmicha marked this conversation as resolved.
Show resolved Hide resolved

/// ±hh:mm:ss is not a standard of ISO8601 date format, but it's supported by AWSDateTime.
/// https://docs.aws.amazon.com/appsync/latest/devguide/scalars.html
private enum ISO8601TimeZonePart {
case utc
case hhmmss(hours: Int, minutes: Int, seconds: Int)
case hhmm(hours: Int, minutes: Int)
case hh(hours: Int)

init?(iso8601DateString: String) {
func hasMatch(regex: NSRegularExpression?, str: String) -> Bool {
return regex.flatMap {
$0.firstMatch(in: str, range: NSRange(location: 0, length: str.count))
} != nil
}

// <time>±hh:mm:ss
func suffixHHColonMMColonSS() -> String? {
if iso8601DateString.count > 9 {
let tz = String(iso8601DateString.dropFirst(iso8601DateString.count - 9))
if hasMatch(regex: TimeZone.iso8601TimeZoneHHColonMMColonSSRegex, str: tz) {
return tz
}
}
return nil
}

// <time>±hh:mm
func suffixHHColonMM() -> String? {
if iso8601DateString.count > 6 {
let tz = String(iso8601DateString.dropFirst(iso8601DateString.count - 6))
if hasMatch(regex: TimeZone.iso8601TimeZoneHHColonMMRegex, str: tz) {
return tz
}
}
return nil
}

// <time>±hhmm
func suffixHHMM() -> String? {
if iso8601DateString.count > 5 {
let tz = String(iso8601DateString.dropFirst(iso8601DateString.count - 5))
if hasMatch(regex: TimeZone.iso8601TimeZoneHHMMRegex, str: tz) {
return tz
}
}
return nil
}

// <time>±hh
func suffixHH() -> String? {
if iso8601DateString.count > 3 {
let tz = String(iso8601DateString.dropFirst(iso8601DateString.count - 3))
if hasMatch(regex: TimeZone.iso8601TimeZoneHHRegex, str: tz) {
return tz
}
}
return nil
}

if iso8601DateString.hasPrefix("Z") { // <time>Z
self = .utc
return
}

if let tz = suffixHHColonMM(),
let hours = Int(tz.dropLast(3)),
let minutes = Int(tz.dropFirst(4))
{
self = .hhmm(hours: hours, minutes: minutes)
return
}

if let tz = suffixHHMM(),
let hours = Int(tz.dropLast(2)),
let minutes = Int(tz.dropFirst(3))
{
self = .hhmm(hours: hours, minutes: minutes)
return
}

if let tz = suffixHH(),
let hours = Int(tz)
{
self = .hh(hours: hours)
return
}

if let tz = suffixHHColonMMColonSS(),
let hours = Int(tz.dropLast(6)),
let minutes = Int(tz.dropFirst(4).dropLast(3)),
let seconds = Int(tz.dropFirst(7))
{
self = .hhmmss(hours: hours, minutes: minutes, seconds: seconds)
return
}

return nil
}
}

/// https://en.wikipedia.org/wiki/ISO_8601#Time_zone_designators
@usableFromInline
internal init?(iso8601DateString: String) {
switch ISO8601TimeZonePart(iso8601DateString: iso8601DateString) {
case .some(.utc):
self.init(abbreviation: "UTC")
case let .some(.hh(hours: hours)):
self.init(secondsFromGMT: hours * 60 * 60)
case let .some(.hhmm(hours: hours, minutes: minutes)):
self.init(secondsFromGMT: hours * 60 * 60 +
(hours > 0 ? 1 : -1) * minutes * 60)
lawmicha marked this conversation as resolved.
Show resolved Hide resolved
case let .some(.hhmmss(hours: hours, minutes: minutes, seconds: seconds)):
self.init(secondsFromGMT: hours * 60 * 60 +
(hours > 0 ? 1 : -1) * minutes * 60 +
(hours > 0 ? 1 : -1) * seconds)
case .none:
return nil
}
}
}
Loading
Loading