FasterXML / jackson-modules-java8

Set of support modules for Java 8 datatypes (Optionals, date/time) and features (parameter names)
Apache License 2.0
399 stars 116 forks source link

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

Closed WolfEkk closed 3 years ago

WolfEkk commented 3 years ago

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:

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 commented 3 years ago

@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 commented 3 years ago

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"}
cowtowncoder commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

BluYous commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@cowtowncoder Thanks, I created a new issue at https://github.com/FasterXML/jackson-modules-java8/issues/228