Serialization of ZonedDateTime with ZoneId bug since 2.12

[vmeunier]

Hello,

I have a simple test case, where I expect my ObjectMapper to keep the ZoneId I've put in my code, for example UTC, Europe/Paris, ... during Serialization.

If I read the documentation

For TimeZone handling, ADJUST_DATES_TO_CONTEXT_TIME_ZONE (default: true) specifies whether the context provided by java.time.TimeZone 'SerializedProvider#getTimeZone()' should be used to adjust Date/Time values on deserialization, even if the value itself contains timezone information. If the value is OffsetDateTime.MIN or OffsetDateTime.MAX, the Date/Time value will not be adjusted. If disabled, it will only be used if the value itself does not contain any TimeZone information.

Here is my ObjectMapper configuration :

        final JavaTimeModule module = new JavaTimeModule();
        mapper = new ObjectMapper() //
            .setSerializationInclusion(Include.NON_NULL) //
            .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) //
            .disable(DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE) //
            .setTimeZone(TimeZone.getTimeZone("Europe/Paris")) //
            .registerModule(module);

If I test :

        final BeanForJson bean = new BeanForJson();
        bean.setZonedDateTime(ZonedDateTime.of(2016, 10, 15, 17, 51, 50, 0, ZoneId.of("UTC")));

        final String json = mapper.writeValueAsString(bean);
        assertThat(json).isEqualTo("{\"zonedDateTime\":\"2016-10-15T17:51:50Z\"}");

I get :

org.opentest4j.AssertionFailedError: 
expected: "{"zonedDateTime":"2016-10-15T17:51:50Z"}"
but was : "{"zonedDateTime":"2016-10-15T19:51:50+02:00"}"

It is working in 2.11.4 (same use-case).

Thanks

[kupci]

I notice the test cases use SerializationFeature.WRITE_DATES_WITH_ZONE_ID, I wonder if that is required. Though I wouldn't have expected any recent changes to suddenly require a new setting, where it wasn't required before. Perhaps a test case was missed somewhere?

[vmeunier]

Ok sorry, thanks to your reply I noticed I mixed things up when I tested. First I talk about Serialization and not deserialization... So the parameter I mention is only taken into account during Serialization and not Deserialization. Same goes for the documentation I quoted.

That means this issue is not valid, it is directly linked to that issue though : https://github.com/FasterXML/jackson-modules-java8/issues/175

[vmeunier]

So I have a question.

If we want to be able to serialize a ZonedDateTime with the TimeZone that was used to create the object, how can we achieve that ?

We need to specify the default TZ of our ObjectMapper because for dates that don't contain this info such as java.util.Date, we still want to add it at serialization.

From my understanding, it isn't normal that if I create a ZonedDateTime in a certain TimeZone, my objectMapper cannot Serialize with the Zone I created it with, just because I gave a default TZ to my objectMapper. Otherwise you can never send any other TZ info than the "default" one.

If i go back to https://github.com/FasterXML/jackson-modules-java8/issues/175 that made that change in behaviour. I disagree with the expected output. If in your code you specify the TZ to create your date, why would you expect to have a different one used for Serialization ? Otherwise to me, the best solution to 175 was to provide a public Constructor of OffsetDateTimeSerializer, so that we can configure the Formatter used for serializating an OffsetDateTime.

[vmeunier]

Seems to be linked to https://github.com/FasterXML/jackson-modules-java8/issues/188 as well.

[cowtowncoder]

Ok, yes. It does look like we have a problem here (I wouldn't classify it as a bug per se, but clearly there are multiple desired behaviors).

It seems to me that this cannot be resolved without introducing a way to control between 2 choices:

1. Overriding whatever timezone/timeoffset Java 8 (and Joda?) zoned date/time has with explicitly defined contextual timezone
2. Using timezone/offset of Java 8 (and Joda) zoned date/time value has

Ideally at this point we would have some sort of "DateConfig" to contain more settings, but with my not having enough time work on Jackson right now there isn't. But adding one more SerializationFeature and then changing relevant serializers would be doable still for 2.13.

Now: DeserializationFeature has setting ADJUST_DATES_TO_CONTEXT_TIME_ZONE which is sort of similar, and I guess we could similar naming. But SerializationFeature has a few WRITE_DATE_xxx settings which suggest different naming; maybe WRITE_DATES_WITH_CONTEXT_TIME_ZONE? (note: we could separate Value/Key aspects, keys relating to date/time values as Map keys, but I assume there is no specific reason for distinction here).

So... if anyone wanted to work on something like that, I'd be happy to help. It'd start with a small PR against 2.13 of jackson-databind, to add the new SerializationFeature; and then most of the work would be for date/time part of this module, along with tests -- and ideally also for jackson-datatype-joda.

ps. I am not too keen on adding configurability for serializers as direct instantiation seems bit fragile, including but not limited to having to know a set of all serializers. It is also likely not less work than a general-purpose feature.

[vmeunier]

SerializationFeature.WRITE_DATES_WITH_CONTEXT_TIME_ZONE is a good option, I guess true would mean you force the use of the default TZ you configured the ObjectMapper with (which means you must specify a default TZ), and false wouldn't ?

I may have time to send a PR, if that would work for you :)

[cowtowncoder]

@vmeunier Right, true would force override as currently, and should be the default to keep 2.12 -> 2.13 to minimum (since 2.11->2.12 change already went in).

One minor (?) thing to consider is that the existing logic, as of 2.12, checks for explicit setting of the context timezone, and only overrides offset/zone if setting was explicit. But technically there is also the default of UTC.

So the question would be whether to ignore implicit/explicit distinction (which would change behavior in one small part) or keep it? To me it seems that for 2.x we should keep it.

@kupci WDYT?

[kupci]

Yes, figuring it would be better to keep the implicit/explicit distinction for 2.x.

[cowtowncoder]

@vmeunier @kupci I went ahead and added SerializationFeature.WRITE_DATES_WITH_CONTEXT_TIME_ZONE into 2.13.0-rc1-SNAPSHOT, so no need for PR for that. Will check how Joda deals with this.

[cowtowncoder]

Ok Joda does not seem to apply context time zone for zoned values... may need to change description Feature if it only affects Java 8 date/time.

[vmeunier]

I modified InstantSerializerBase, and added 2 testcases in ZonedDateTimeSerTest.

I could add same tests in other classes like OffsetDateTimeSerTest, i'm not sure if it is necessary, tell me :)

Btw, i couldn't find the formatter you use, I'm using Eclipse and the imports are not in the same order as the default ones in IntelliJ (I guess you use IntelliJ). Do you have an Eclipse formatter that I can use to be sure not to mess anything ?

[kupci]

I think the extra tests would be helpful given that OffsetDateTimeSerializer is a pretty thin extension of InstantSerializerBase, if only to give a little extra confidence in the change, and then also for future changes.

Formatting looks ok to me, when looking via GitHub. Maybe GitHub handles that? Otherwise, I couldn't find a way to import IntelliJ code style settings into eclipse. Lots of examples / support going from eclipse to IntelliJ...

[cowtowncoder]

Fixed via #222 by supporting SerializationFeature.WRITE_DATES_WITH_CONTEXT_TIME_ZONE