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.

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

Concerto assumes UTC for unqualified "Local Time" DateTime values #552

Closed
mttrbrts opened this issue Nov 29, 2022 · 2 comments
Closed

Concerto assumes UTC for unqualified "Local Time" DateTime values #552

mttrbrts opened this issue Nov 29, 2022 · 2 comments
Labels

Comments

@mttrbrts
Copy link
Sponsor Member

mttrbrts commented Nov 29, 2022

Bug Report 🐛

When validating data against a model with the Concerto CLI, dates with the format YYYY-MM-DD are assumed to be in UTC. In general, this same issues is present for all DateTime values that are unqualified, i.e. they don't specify a zone designation.

Expected Behavior

When an unqualified date, or date-time value is provided (i.e. one without an explicit Z or offset component), users expect the date to be interpreted as in their local timezone.

This aligns with the ISO 8601 specification
https://en.wikipedia.org/wiki/ISO_8601#Local_time_(unqualified)

Local time (unqualified)
If no UTC relation information is given with a time representation, the time is assumed to be in local time.

Current Behavior

For a user with a UTC-8 offset, a DateTime value of 2020-01-01 will be normalized to 2019-12-31T16:00:00.000-08:00. Note how the year, month, day and time have all changed. This is surprising to users.

This is caused by an explicit assumption that the default offset for values should be 0.
https://github.com/accordproject/concerto/blob/main/packages/concerto-core/lib/serializer/jsonpopulator.js#L94

Possible Solution

  1. Explicitly detect usage of date-times without a zone designation, and to assume an offset that matches the local offset of the user's machine/server.
  2. Use of the --utcOffset flag in the CLI should always allow the offset to be set explicitly.

Steps to Reproduce

  1. Set your time zone to PST (GMT-8).
  2. Create two files:
    test.cto:
namespace test@1.0.0

concept C {
  o DateTime d
}

test.json:

{ "$class": "[email protected]", "d":"2020-01-01" }
  1. Run
$ concerto validate --model test.cto --input obj.json
8:47:23 AM - INFO: Input is valid
{"$class":"[email protected]","d":"2019-12-31T16:00:00.000-08:00"}

Discussion

Fixing this bug could be sensitive. Existing users may rely on this behaviour, even though I believe that it should be considered a bug.

@mttrbrts mttrbrts changed the title CLI adds offsets for DateTime values with YYYY-MM-DD format Concerto assumes UTC for unqualified "Local Time" DateTime values Nov 29, 2022
@DS-AdamMilazzo
Copy link

DS-AdamMilazzo commented Nov 29, 2022

When an unqualified date, or date-time value is provided (i.e. one without an explicit Z or offset component), users expect the date to be interpreted as in their local timezone. This aligns with the ISO 8601 specification
https://en.wikipedia.org/wiki/ISO_8601#Local_time_(unqualified)

Local time (unqualified)
If no UTC relation information is given with a time representation, the time is assumed to be in local time.

First, In the example you gave, there is no time representation, only a date representation. The value has no time zone at all and the snippet from ISO 8601 does not apply. (But as you mentioned, the bug report applies to date-times as well.)

Perhaps some users expect it to assume local time, but it's not what I, as a user, would expect. I would expect a date-time with no time-zone information to be assumed to be in UTC (if an assumption must be made at all). This is a data interchange format, not an interactive user interface. The interpretation of a value shouldn't change as the data crosses time zones. So I would expect it to assume UTC.

Furthermore, "assume local time" makes no sense when dealing with a service, as in the case of the model registry, because the "local time" would be the local time of some remote server. The server would not know the user's local time. I know that the model registry apparently runs with a "local time" of UTC, and so doesn't exhibit the buggy behavior, but in general the server should not be reinterpreting user timestamps according to its own local time zone. Whether this means Concerto should behave differently in client-side and server-side scenarios, I can't say, but my vote would be "no".

  • Explicitly detect usage of date-times without a zone designation, and to assume an offset that matches the local offset of the user's machine/server.
  • Use of the --utcOffset flag in the CLI should always allow the offset to be set explicitly.

I think a better solution is: 1) assume times are in UTC unless otherwise specified, 2) if you must normalize date-times, normalize them to UTC (or else leave their time zones alone), and 3) if you can, stop treating dates as date-times.

@mttrbrts
Copy link
Sponsor Member Author

We now have an interim solution in 3.3 that allows clients to specify more strict behaviour. This is expected to become default behaviour in future.

#553

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

2 participants