jackson-modules-java8 icon indicating copy to clipboard operation
jackson-modules-java8 copied to clipboard

Discrepancy in deserialization of ZonedDateTime

Open dscalzi opened this issue 3 years ago • 3 comments

Given input string: 2021-02-01T19:49:04.0513486Z

Jackson will deserialize a ZonedDateTime with the following properties:

  • offset: "Z" (type ZonedOffset)
  • zone: "UTC" (type ZoneRegion)

Using the standard call

DateTimeFormatter.ISO_ZONED_DATE_TIME.parse(inputString, ZonedDateTime::from);

produces:

  • offset: "Z" (type ZonedOffset)
  • zone: "Z" (type ZonedOffset)

This causes an issue in comparisons because id value "UTC" != id value "Z". This also produces a redundancy in the toString of the ZonedDateTime because it detects that the offset != zone.

toString for Jackson result: 2021-02-01T19:49:04.051348600Z[UTC] toString for standard result: 2021-02-01T19:49:04.051348600Z

The latter can be consumed without extra processing and is the desired result.

Would it be possible to update the deserializer to behave like the standard call?

dscalzi avatar Feb 01 '21 20:02 dscalzi

I'd be open to contributions that would unify handling -- I don't know if and how to this myself. @kupci or @beamerblvd might know.

If anyone wants to tackle this, I suspect it could go in a minor release (but should NOT be introduced in a patch) at least: if so, branch to base it against would be 2.13 at this point.

cowtowncoder avatar Feb 01 '21 23:02 cowtowncoder

@cowtowncoder I think it might just come down to this value being false https://github.com/FasterXML/jackson-modules-java8/blob/2.14/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/InstantDeserializer.java#L95

I haven't run any tests myself.

dscalzi avatar Oct 05 '22 16:10 dscalzi

Thank you for extra information @dscalzi!

At this point I am open to PR if anyone wants to submit it, but will not pro-actively make changes without someone actually needing it, providing test.

cowtowncoder avatar Oct 05 '22 20:10 cowtowncoder

Doing some investigating, it seems like this stems from DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE. When this feature is enabled, the timezone on the resolved ZonedDateTime overrided to be the value of ObjectMapper's set TimeZone. The function responsable for this (ZonedDateTime::withZoneSameInstant) https://github.com/FasterXML/jackson-modules-java8/blob/2.14/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/InstantDeserializer.java#L94

When calling this method with UTC timezone, it changes the zone to UTC (instead of Z), resulting in the discrepancy as noted in the original post.

dscalzi avatar Feb 19 '23 21:02 dscalzi

Further, the output of that depends on the input you pass to withZoneSameInstant

ZonedDateTime manipulatedUTC = standard.withZoneSameInstant(ZoneId.of("UTC"));
System.out.println("UTC Manipulated " + manipulatedUTC.getZone());
// Prints UTC
// Resultant ZonedDateTIme would be 2021-02-01T19:49:04.051348600Z[UTC]

ZonedDateTime manipulatedUTCStd = standard.withZoneSameInstant(ZoneOffset.UTC);
System.out.println("UTC Std Manipulated " + manipulatedUTCStd.getZone());
// Prints Z
//  Resultant ZonedDateTIme would be 2021-02-01T19:49:04.051348600Z

https://stackoverflow.com/a/39507023/5295111

dscalzi avatar Feb 19 '23 21:02 dscalzi

Just raised a PR that fixes this issue. I have only investigated this for ZonedDateTimes, as I havent noticed any issues with other java date types.

dscalzi avatar Feb 19 '23 21:02 dscalzi

Ok unfortunately this gets deep into Date/Time area where my context is bit incomplete. What you say does make sense. And yes, backwards compatibility is usually the big challenge.

About the only contribution I have here is that we are trying to avoid adding new datatype-specific features into general DeserializationFeature / SerializationFeature, and have added concept of DatatypeFeature, with 2 implementations:

  1. EnumFeature (for new settings added that affect reading/writing of Enum types)
  2. JsonNodeFeature (similarly for JsonNode types, aka Tree Model)

I have planned to add DateTimeFeature, fwtw, when it is needed. It coudl be easily added in 2.15; adding new DatatypeFeatures is itself easy (as is adding entries). I wrote JSTEPs:

  • https://github.com/FasterXML/jackson-future-ideas/wiki/JSTEP-7 explains DatatypeFeature idea (added in 2.14, extended in 2.15)
  • https://github.com/FasterXML/jackson-future-ideas/wiki/JSTEP-5 discusses ideas of unifying Date/Time handling.

cowtowncoder avatar Feb 20 '23 00:02 cowtowncoder

Hmmh. Ok, after re-reading full explanation, perhaps we do NOT need backwards-compatibility feature unless someone actually does request it during 2.15 release candidate phase. So DateTimeFeature is not needed, yet; but one can be added if and as necessary.

cowtowncoder avatar Feb 20 '23 00:02 cowtowncoder