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

[2.12.0-rc1] Can no longer serialize using different timezones on independent OffsetDateTime fields #188

Closed
WolfEkk opened this issue Oct 15, 2020 · 10 comments

Comments

@WolfEkk
Copy link

WolfEkk commented Oct 15, 2020

Hi it would seem like #185 (PR for #175) removes the possibility of producing different offset formats with the same ObjectMapper.

In the example below you will see 3 cases:

  • One using an ObjectMapper without setting it's time zone.
  • One using an ObjectMapper with it's time zone set to the value appropriate for the the offset field.
  • One using an ObjectMapper with it's time zone set to the value appropriate for the the offset2 field.

None of these options seem to work. I think it might just be necessary to honor the given object's timezone when the timezone of the ObjectMapper is not set, I would love to hear your opinion on this or if there's an alternate way to make this work I would also love to hear about it.

I've added a fourth test case which shows that deserialization is possible when using DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE. This makes me think that at a higher level the change introduces asymmetry in the behavior, and I think being able to go back and forth preserving the data is important.

Please let me know if I can improve this report or assist in any way.

import static org.junit.Assert.assertEquals;

import com.fasterxml.jackson.annotation.*;
import com.fasterxml.jackson.annotation.JsonCreator.Mode;
import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import java.time.*;
import java.util.TimeZone;
import org.junit.Test;

public class ObjectMapperTest {

    private static final LocalDateTime LOCAL = LocalDateTime.of(2018, 11, 26, 17, 53, 22);
    private static final OffsetDateTime OFFSET = OffsetDateTime.of(LOCAL.plusSeconds(1), ZoneOffset.of("-05:00"));
    private static final OffsetDateTime OFFSET_2 = OffsetDateTime.of(LOCAL.plusSeconds(3), ZoneOffset.of("Z"));
    private static final TestPojo TEST_POJO = new TestPojo(OFFSET, OFFSET_2);
    private static final String EXPECTED_JSON = "{"
            + "\"offset\":\"2018-11-26T17:53:23-05:00\","
            + "\"offset2\":\"11/26/2018 17:53:25Z\""
            + "}";

    @Test
    public void testSerialization() throws Exception {
        ObjectMapper subject = newObjectMapper();
        String json = subject.writeValueAsString(TEST_POJO);
        assertEquals("Json should include timezone", EXPECTED_JSON, json);
    }

    @Test
    public void testSerializationWithUTCTimezoneInMapper() throws Exception {
        ObjectMapper subject = newObjectMapper();
        subject.setTimeZone(TimeZone.getTimeZone(ZoneId.of("UTC")));
        String json = subject.writeValueAsString(TEST_POJO);
        assertEquals("Json should include timezone", EXPECTED_JSON, json);
    }

    @Test
    public void testSerializationWithMinus5TimezoneInMapper() throws Exception {
        ObjectMapper subject = newObjectMapper();
        subject.setTimeZone(TimeZone.getTimeZone(ZoneId.of("-05:00")));
        String json = subject.writeValueAsString(TEST_POJO);
        assertEquals("Json should include timezone", EXPECTED_JSON, json);
    }

    @Test
    public void testDeserialization() throws Exception {
        ObjectMapper subject = newObjectMapper();
        subject.configure(DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE, false);
        String json = ("{"
                + "  'offset': '2018-11-26T17:53:23-05:00',"
                + "  'offset2': '11/26/2018 17:53:25Z'"
                + "}").replaceAll("'", "\"");
        TestPojo pojo = subject.readValue(json, TestPojo.class);
        assertEquals("The time is equivalent for offset", OFFSET, pojo.getOffset());
        assertEquals("The time is equivalent for offset2", OFFSET_2, pojo.getOffset2());
    }

    private ObjectMapper newObjectMapper() {
      ObjectMapper mapper = new ObjectMapper();
      mapper.registerModule(new JavaTimeModule());
      mapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false);
      return mapper;
    }

    @JsonPropertyOrder({"offset", "offset2"})
    static class TestPojo {
        private static final String UTC_DATETIME_FORMAT = "yyyy-MM-dd'T'HH:mm:ssXXX";
        private static final String ANOTHER_DATETIME_FORMAT = "MM/dd/yyyy' 'HH:mm:ssXXX";

        private final OffsetDateTime offset;
        private final OffsetDateTime offset2;

        @JsonCreator(mode = Mode.PROPERTIES)
        public TestPojo(
                @JsonProperty("offset") @JsonFormat(shape = JsonFormat.Shape.STRING,
                        pattern = UTC_DATETIME_FORMAT) OffsetDateTime offset,
                @JsonProperty("offset2") @JsonFormat(shape = JsonFormat.Shape.STRING,
                        pattern = ANOTHER_DATETIME_FORMAT) OffsetDateTime offset2) {
            this.offset = offset;
            this.offset2 = offset2;
        }

        @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = UTC_DATETIME_FORMAT)
        public OffsetDateTime getOffset() { return offset; }
        @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = ANOTHER_DATETIME_FORMAT)
        public OffsetDateTime getOffset2() { return offset2; }
    }
}
@cowtowncoder
Copy link
Member

@WolfEkk quick question: by "producing different offset formats " do you mean that pattern specified with @JsonFormat was not applied, or that offset information of value being serialized is not used (instead using mapper.setTimeZone()))?
I would guess latter, but just want to make sure.

@WolfEkk
Copy link
Author

WolfEkk commented Oct 15, 2020

That's a good question, maybe I didn't stop to understand it entirely. I think it is as you suspect, it's just using the timezone configured in the mapper.

I see the tests were a bit lousy in keeping them easy to follow, sorry I based them of off other tests that had broke (those tests had a bit of a different aim).

Perhaps it's easier to read if I break it down a bit (I'm attaching the full working tests instead of pasting them fully here just the test files zipped, working maven project zipped).

Given three values:

OffsetDateTime OFFSET = OffsetDateTime.of(2018, 11, 26, 17, 53, 22, 0, ZoneOffset.of("-05:00"));
OffsetDateTime OFFSET_2 = OffsetDateTime.of(2018, 11, 26, 17, 53, 23, 0, ZoneOffset.of("Z"));
OffsetDateTime OFFSET_3 = OffsetDateTime.of(2018, 11, 26, 17, 53, 24, 0, ZoneOffset.of("Z"));

The following POJO:

    @JsonPropertyOrder({"offset", "offset2", "offset3"})
    static class TestPojo {
        final OffsetDateTime offset, offset2, offset3;
        public TestPojo(OffsetDateTime offset, OffsetDateTime offset2, OffsetDateTime offset3) {
            this.offset = offset;
            this.offset2 = offset2;
            this.offset3 = offset3;
        }

        @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ssXXX")
        public OffsetDateTime getOffset() { return offset; }
        @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ssXXX")
        public OffsetDateTime getOffset2() { return offset2; }
        @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm:ssxx")
        public OffsetDateTime getOffset3() { return offset3; }
    }

Would be expected to output (it's what we get in 2.11.3):

{"offset":"2018-11-26T17:53:22-05:00","offset2":"2018-11-26T17:53:23Z","offset3":"2018-11-26T17:53:24+0000"}
  • When no timezone is configured and when timezone is set it outputs:
{"offset":"2018-11-26T22:53:22Z","offset2":"2018-11-26T17:53:23Z","offset3":"2018-11-26T17:53:24+0000"}
  • When timezone is set to TimeZone.getTimeZone(ZoneId.of("-05:00")), it outputs:
{"offset":"2018-11-26T17:53:22-05:00","offset2":"2018-11-26T12:53:23-05:00","offset3":"2018-11-26T12:53:24-0500"}

@WolfEkk WolfEkk changed the title [2.12.0-rc1] Can no longer serialize using different offset formats for OffsetDateTime fields [2.12.0-rc1] Can no longer serialize using different timezones base of off independent OffsetDateTime fields Oct 15, 2020
@WolfEkk WolfEkk changed the title [2.12.0-rc1] Can no longer serialize using different timezones base of off independent OffsetDateTime fields [2.12.0-rc1] Can no longer serialize using different timezones on independent OffsetDateTime fields Oct 15, 2020
@cowtowncoder
Copy link
Member

Ok, so it sounds like what I thought it did -- that offset/timezone included in value itself would not be used (by default), and that mapper-default offset is used instead; or, if specified, @JsonFormat.timezone as override.

This is bit of a challenge as I can see why either method could make sense. Adding a SerializationFeature could be an option (if we could get that done in time) -- or, module-specific setting -- to determine whether to use offset value already specifies or global default (JsonFormat.timezone should override both, however).
The main challenge here is just timing of changes; we are hoping to get 2.12.0 finalized soon.

@kupci WDYT?

@kupci
Copy link
Member

kupci commented Oct 17, 2020

Need to think about this a bit, first thought is to follow some precedence, to use offset/timezone included in the value, unless overridden, perhaps hasExplicitTimeZone could be used? Trying to avoid adding another SerializationFeature if possible.

@cowtowncoder
Copy link
Member

cowtowncoder commented Oct 18, 2020

@kupci I am not big fan of just adding SerializationFeatures, although this would be sort of cross-cutting feature. Then again there probably should be DateTimeFeature/DateTimeConfig, to carve out of SerializationFeature/DeserializationFeature.

As to hasExplicitTimeZone, I think you are referring to method in BaseSettings, which could be exposed via Ser/Deser config. I actually like this, while not perfect it would improve things I think.
And since getTimeZone() exists in MapperConfig, it could go right in there.

@cowtowncoder
Copy link
Member

I added MapperConfig.hasExplicitTimeZone() (base class of SerializationConfig and DeserializationConfig), so this is now accessible for 2.12.

@BluYous
Copy link

BluYous commented Oct 7, 2021

Hi @cowtowncoder
Can jackson ignore the time zone that set in objectMapper when serialize OffsetDateTime?
See below example, when I set time zone to Asia/Shanghai, then objectMapper.writeValueAsString is returning an OffsetDateTime with +08:00, that's not my expected. In my opinion, it should be -07:00.

System.out.println("default time zone = " + TimeZone.getDefault().getID());
ObjectMapper objectMapper = new ObjectMapper()
        .registerModule(new JavaTimeModule())
        .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
        .disable(DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE)
        .enable(SerializationFeature.WRITE_DATES_WITH_ZONE_ID);
final OffsetDateTime expected = OffsetDateTime.now(ZoneId.of("US/Pacific"));
String actualStringWithoutSetTimeZone = objectMapper.writeValueAsString(expected);
System.out.println("actualString without setting time zone: " + actualStringWithoutSetTimeZone);
OffsetDateTime actualWithoutSetTimeZone = objectMapper.readValue(actualStringWithoutSetTimeZone, OffsetDateTime.class);
Assertions.assertEquals(expected, actualWithoutSetTimeZone);

// set time zone
objectMapper.setTimeZone(TimeZone.getTimeZone("Asia/Shanghai"));
String actualString = objectMapper.writeValueAsString(expected);
System.out.println("actualString after setting time zone to Asia/Shanghai: " + actualString);
OffsetDateTime actual = objectMapper.readValue(actualString, OffsetDateTime.class);
Assertions.assertEquals(expected, actual);

Output:

default time zone = Asia/Shanghai
actualString without setting time zone: "2021-10-07T02:08:29.1089765-07:00"
actualString after setting time zone to Asia/Shanghai: "2021-10-07T17:08:29.1089765+08:00"

org.opentest4j.AssertionFailedError: 
Expected :2021-10-07T02:08:29.108976500-07:00
Actual   :2021-10-07T17:08:29.108976500+08:00

@kupci
Copy link
Member

kupci commented Oct 7, 2021

A few questions:

  1. What Jackson version are you using?
  2. Was this something that was working before, in an earlier version, or is this a new requirement?
  3. Is the requirement: "use object's timezone even if different one is set in ObjectMapper", as below?
// set time zone
objectMapper.setTimeZone(TimeZone.getTimeZone("Asia/Shanghai"));

@cowtowncoder
Copy link
Member

@BluYous please do not add new requests on closed issues; even if they may seem slightly related.
I don't typically follow these and always request filing a new issue even if I notice them.
Feel free to instead file a new request/issue report, with information that @kupci suggested.

@BluYous
Copy link

BluYous commented Oct 8, 2021

@cowtowncoder Thanks, I created a new issue at #228

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants