Cannot deserialize `OffsetDateTime.MIN` or `OffsetDateTime.MAX` with `ADJUST_DATES_TO_CONTEXT_TIME_ZONE` enabled

[morth]

This probably relates to #124.

I cannot deserialize OffsetDateTime.MIN and OffsetDateTime.MAX.

With jackson-databind and jackson-datatype-jsr310 (both 2.10.3) on my class path the following works:

ObjectMapper o = new ObjectMapper();
o.findAndRegisterModules();
o.readValue("\"" + OffsetDateTime.now().toString() + "\"", OffsetDateTime.class);

The following does not work:

// (OffsetDateTime.MIN is -999999999-01-01T00:00+18:00)
o.readValue("\"" + OffsetDateTime.MIN.toString() + "\"", OffsetDateTime.class);
// Or:
// (OffsetDateTime.MAX is +999999999-12-31T23:59:59.999999999-18:00)
o.readValue("\"" + OffsetDateTime.MAX.toString() + "\"", OffsetDateTime.class);

Removing the offset or shifting the time by the respective amount of hours seems to work again:

o.readValue("\"-999999999-01-01T00:00+18:00\"", OffsetDateTime.class); // doesn't work
o.readValue("\"-999999999-01-01T18:00+18:00\"", OffsetDateTime.class); // works
o.readValue("\"-999999999-01-01T00:00+00\"", OffsetDateTime.class); // works

[kupci]

Ok, thanks, this does seem to be an error.

[cowtowncoder]

I wonder if code has assumption that values, but in UTC, would be limits? And thereby actual min/max would be considered to be outside, since normalized date (once value normalized to UTC) would then fall outside numeric values, in both cases.

[morth]

I digged a little bit deeper into the issue and the problem seems to be the ADJUST_DATES_TO_CONTEXT_TIME_ZONE deserialization feature.

When enabled the deserializer will try to convert the OffsetDateTime to another OffsetDateTime representing the same instant but storing another zone offset. With OffsetDateTime.MIN this will hit Java's limits for representing the instant for any offset less than +18:00. You just cannot represent OffsetDateTime.MIN without an offset in Java, so ADJUST_DATES_TO_CONTEXT_TIME_ZONE cannot work with OffsetDateTime.MIN (and OffsetDateTime.MAX, respectively).

Deserializing OffsetDateTime.MIN and OffsetDateTime.MAX without the feature enabled is working properly.

Any ideas on how to fix this @cowtowncoder @kupci ? As this is more or less a conceptual problem with the ADJUST_DATES_TO_CONTEXT_TIME_ZONE deserialization feature, I currently cannot think of another solution than just mentioning this in the docs.

[kupci]

Nice find! A workaround is always good to have. I agree, first thing is to update the documentation.

As for a solution, I've been wondering how to solve this and one thought is that, with the MAX and MIN, I picture these as infinity in mathematics. So applying that, if I have ∞ and then apply some zone offset, I still have ∞, correct? So, at a small performance cost, there could be a check somewhere before the ADJUST_DATES_TO_CONTEXT_TIME_ZONE that says, if we are already at OffsetDateTime.MIN or OffsetDateTime.MAX, then consider it ∞ and we don't change the offset? Should discuss with @cowtowncoder on the e-mail list perhaps, or maybe I'm overthinking things and there's a simpler way?

[morth]

Your approach seems reasonable to me! I think the required check would need to be a little more complicated though, since we need to look for OffsetDateTime.MIN but also for any other OffsetDateTime that cannot be normalized (which is up to 18 hours after OffsetDateTime.MIN). I think the easiest "check" would be to handle the DateTimeException thrown by OffsetDateTime.withOffsetSameInstant, as this would cover exactly the required cases (when the result would exceed the supported date range, as stated in the docs).

The current behaviour is that the code catches the DateTimeException and then lets the deserialization fail with an error message. The solution could be to just catch the exception and then return the unadjusted Temporal (see InstantDeserializer.java around line 226).

[kupci]

also for any other OffsetDateTime that cannot be normalized (which is up to 18 hours after OffsetDateTime.MIN)

Good point, that would be a cleaner solution. Though instead of the catch block, I would put this as part of the ADJUST_DATES_TO_CONTEXT_TIME_ZONE check, instead of in the catch, keeping with Joshua Bloch, i.e. exception handling for exceptions. Small performance hit, but there's already a bit of time required to do the context time zone adjustment anyway.

If you have tests & implementation, I can review a PR, or vice versa, and then see what @cowtowncoder thinks..

[cowtowncoder]

What @kupci just said. :)