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

DateTime can't be serialized with its own zone (WRITE_DATES_WITH_CONTEXT_TIME_ZONE not respected) #146

Open
pvgoran opened this issue Oct 30, 2024 · 9 comments
Labels
2.18 has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue pr-welcome Issue for which progress most likely if someone submits a Pull Request

Comments

@pvgoran
Copy link

pvgoran commented Oct 30, 2024

(This issue is about the same problem as #92. However, I decided to create it because the old issue describes the problem in an indirect way, and because WRITE_DATES_WITH_CONTEXT_TIME_ZONE was added to https://github.com/FasterXML/jackson-databind since then.)

In version 2.18.1, DateTime instances are always serialized using ObjectMapper's time zone (even if it was not explicitly specified - UTC is used in this case), rather than with DateTime's own zone. Disabling the WRITE_DATES_WITH_CONTEXT_TIME_ZONE serialization feature (which was added for FasterXML/jackson-modules-java8#222 to address the same problem but with JSR-310 date-time types) does not help.

Test code:

package local;

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.datatype.joda.JodaModule;
import org.joda.time.*;

public class App
{
    public static void main(String[] args) throws Exception
    {
        DateTime dateTime = DateTime.parse("2024-12-01T00:00:00+02:00");
        System.out.println("DateTime: " + dateTime);

        ObjectMapper mapper = _objectMapper();
        String json = mapper.writeValueAsString(dateTime);
        System.out.println("Serialized: " + json);
    }

    private static ObjectMapper _objectMapper()
    {
      ObjectMapper mapper = new ObjectMapper();
      mapper.registerModule(new JodaModule());
      mapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false);
      mapper.configure(SerializationFeature.WRITE_DATES_WITH_CONTEXT_TIME_ZONE, false);
      return mapper;
    }
}

Output:

DateTime: 2024-12-01T00:00:00.000+02:00
Serialized: "2024-11-30T22:00:00.000Z"

Expected:

DateTime: 2024-12-01T00:00:00.000+02:00
Serialized: "2024-12-01T00:00:00.000+02:00"
@pvgoran
Copy link
Author

pvgoran commented Oct 30, 2024

Additional information:

My use case is implementing a JSON API that needs to return date-times with (potentially) different time zones. Since I can't have a fixed time zone, the existing behaviour prevents me from using DateTime in my POJOs as is: I have to either store date-times as Strings, or use custom serializers, @JsonGetter annotations or similar solutions.

Also, I'm aware that the documentation of WRITE_DATES_WITH_CONTEXT_TIME_ZONE states that it doesn't work for Joda date/time values. This looks like an unfortunate omission. :)

@cowtowncoder
Copy link
Member

Wrt WRITE_DATES_WITH_CONTEXT_TIME_ZONE not being supported -- PRs welcome if you (or anyone else) has time to look into adding that; I don't recall it being something left out on purpose, but rather that Feature's are generally opt-in: modules need to add support. And so support was just added for Java 8 date/time module.

Whoever has time & itch to look into this may find other blockers, but that will be found by trying to make it work.

I don't have time to work on this any time soon, but I make time to help others who do. Fwtw.

@pjfanning
Copy link
Member

Is java.time a better bet these days? That is, does https://github.com/FasterXML/jackson-modules-java8 Java 8 Date/time support time zones?

@cowtowncoder cowtowncoder added pr-welcome Issue for which progress most likely if someone submits a Pull Request 2.18 labels Oct 31, 2024
@cowtowncoder
Copy link
Member

Java 8 date/time module has supported both time offsets and time zones from the beginning so it should indeed be better bet.

@cowtowncoder
Copy link
Member

@pvgoran Reading through Javadoc WRITE_DATES_WITH_CONTEXT_TIME_ZONE and this issue, it seems like adding support with Joda would NOT cause compatibility issues, right? Default being "do overwrite timezone/offset information for serialization"?

And if so, this could be done without introducing new Joda-specific features or changing compatibility (well, theoretically there would be change of course, making some feature's alternate setting have an effect -- if someone was using both Joda and Java 8 date/time, setting this feature to affect latter -- but seems bit remote)

cowtowncoder added a commit that referenced this issue Oct 31, 2024
@cowtowncoder cowtowncoder added the has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue label Oct 31, 2024
@cowtowncoder
Copy link
Member

Added reproduction (failing test) as com.fasterxml.jackson.datatype.joda.failing.DateTimeSerializationWithOffsets146Test.

Also tried a quick fix to JacksonJodaDateFormat method

public DateTimeFormatter createFormatter(SerializerProvider ctxt)

but that didn't work (I think DateTimeFormatter _formatter must be constructed differently).

But perhaps someone else could try to figure it out?

@pvgoran
Copy link
Author

pvgoran commented Oct 31, 2024

Is java.time a better bet these days?

Well, java.time is generally considered to be better. However, it does lack some features and convenience of Joda Time. Besides, not everyone has a freedom to choose. :)

@pvgoran
Copy link
Author

pvgoran commented Oct 31, 2024

And if so, this could be done without introducing new Joda-specific features or changing compatibility (well, theoretically there would be change of course, making some feature's alternate setting have an effect -- if someone was using both Joda and Java 8 date/time, setting this feature to affect latter -- but seems bit remote)

Yes, I think so.

Added reproduction (failing test) as com.fasterxml.jackson.datatype.joda.failing.DateTimeSerializationWithOffsets146Test.

Great! Adding proper tests would be a major obstacle for me in trying to implement this. :)

Also tried a quick fix to JacksonJodaDateFormat method

public DateTimeFormatter createFormatter(SerializerProvider ctxt)

but that didn't work (I think DateTimeFormatter _formatter must be constructed differently).

Could you share what exactly you tried that did not work?

@cowtowncoder
Copy link
Member

cowtowncoder commented Oct 31, 2024

I'd have to go back to code later on, but as per my note, method JacksonJodaDateFormat.createFormatter(SerializerProvider ctxt):

    public DateTimeFormatter createFormatter(SerializerProvider ctxt)
    {
        DateTimeFormatter formatter = createFormatterWithLocale(ctxt);
        if (!_explicitTimezone) {
            TimeZone tz = ctxt.getTimeZone();
            if ((tz != null) && !tz.equals(_jdkTimezone)) {
                formatter = formatter.withZone(DateTimeZone.forTimeZone(tz));
            }
        }
        return formatter;
    }

change line if (!_explicitTimezone) { to:

        if (!_explicitTimezone && !ctxt.isEnabled(SerializationFeature.WRITE_DATES_WITH_CONTEXT_TIME_ZONE)) {

to avoid setting timezone. But I think formatter already has configured default or so.
I did not dig any deeper.

Test I added is DateTimeSerializationWithOffsets146Test under package com.fasterxml.jackson.datatype.joda.failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.18 has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue pr-welcome Issue for which progress most likely if someone submits a Pull Request
Projects
None yet
Development

No branches or pull requests

3 participants