[break] use OffsetDatetime instead of ZonedDateTime

[qinfchen]

Before this PR

ZonedDateTime was used for conjure primitive datetime type and it is not always compliant with conjure spec which requires date time to be ISO-8601 compliant. It is becauseZonedDateTime's string representation is not compatible with ISO-8601 when ZoneId and ZoneOffset are different

ZonedDateTime's JavaDoc

• The format consists of the {@code LocalDateTime} followed by the {@code ZoneOffset}.
• If the {@code ZoneId} is not the same as the offset, then the ID is output.
• The output is compatible with ISO-8601 if the offset and ID are the same.

System.out.println(ZonedDateTime.now(ZoneId.of("Europe/Paris")));
// 2017-04-26T15:13:12.006+02:00[Europe/Paris]

After this PR

OffsetDateTime class is used as conjure datetime primitive to be conjure spec compliant because OffsetDateTime guarantees ISO-8601 compliance

According to OffsetDateTime javadoc

OffsetDateTime, ZonedDateTime and Instant all store an instant on the time-line to nanosecond precision. Instant is the simplest, simply representing the instant. OffsetDateTime adds to the instant the offset from UTC/Greenwich, which allows the local date-time to be obtained. ZonedDateTime adds full time-zone rules.

Even though Instant is the simplest date time representation, we favor OffsetDateTime for readability and the flexibility in obtaining local date date time.

Implications

• This will break apps that have stored date time in the format containing zone id. e.g. 2017-04-26T15:13:12.006+02:00[Europe/Paris]
• This means we lose the full timezone information that ZonedDateTime provides, including the rules to cover daylight saving time adjustments.

[dansanduleac]

Let's cut 2.0.0

[iamdanfox]

Really appreciate the PR description - possibly worth explicitly mention that this ZonedDateTime was not compliant with the conjure spec (which required IOS-8601) and that OffsetDateTime does now comply with ISO-8601?

[iamdanfox]

Is it worth tagging an RC version before merging this so that we can give people a helpful guide for how to migrate?

[markelliot]

I really think if we're going to break we ought to pay attention to the precision that ZonedDateTime originally offered: https://docs.oracle.com/javase/8/docs/api/java/time/ZonedDateTime.html

In an ideal form, IMO we shouldn't be carrying the offset, and there's an issue with offsets that needs to be dealt with around set equality.

[markelliot]

Also, since ZonedDateTime did in fact serialize compatibly, it was in fact spec-compatible (for pedantic readers).

[qinfchen]

@markelliot, i believe OffsetDateTime offers the same level of precision that ZonedDateTime offers which is to nanoseconds. Given that we will have a canonical presentation for equality that is compliant to ISO8601, e.g.yyyy-MM-ddTHH:mm:ss.SSSZ, wouldn't that alleviate the concern regarding Offsets?

I wrote a quick test to see how ZonedDateTime as keys being serialized using http-remoting mapper

   Map<ZonedDateTime, String> maps = ImmutableMap.of(
            ZonedDateTime.of(2017, 1, 2, 3, 4, 5, 0, ZoneId.of("Europe/Berlin")), "One",
            ZonedDateTime.of(2017, 1, 2, 3, 4, 5, 0, ZoneId.of("Europe/Paris")), "Two");
    String serialized = MAPPER.writeValueAsString(maps);
    TypeReference<Map<ZonedDateTime,String>> typeRef
            = new TypeReference<Map<ZonedDateTime,String>>() {};
    Map<ZonedDateTime, String> deserialized = MAPPER.readValue(serialized, typeRef);
    assertEquals(deserialized, maps);

Turns out, serialization plus deserialization removes the ZoneId and treats both ZonedDateTime objects with different ZoneIds as the same key, thus completely changing the original map content.

java.lang.AssertionError: 
Expected :{2017-01-02T03:04:05+01:00[Europe/Berlin]=One, 2017-01-02T03:04:05+01:00[Europe/Paris]=Two}
Actual   :{2017-01-02T03:04:05+01:00=Two}

And I did the same test using OffsetDatetime and it passed the map equality test

[markelliot]

Sorry, “precision” was probably an improper word choice. We’re leaving behind:

ZonedDateTime is an immutable representation of a date-time with a time-zone. This class stores all date and time fields, to a precision of nanoseconds, and a time-zone, with a zone offset used to handle ambiguous local date-times.
...
Any method that converts directly or implicitly from a local date-time to an instant by obtaining the offset has the potential to be complicated.
...
In terms of design, this class should be viewed primarily as the combination of a LocalDateTime and a ZoneId. The ZoneOffset is a vital, but secondary, piece of information, used to ensure that the class represents an instant, especially during a daylight savings overlap.

[qinfchen]

Got it. I added a note about the full timezone information of ZonedDateTime in the implication section.

[iamdanfox]

I am going to tag 2.0.0-rc1 on this branch now, so that we can try out this upgrade for people