FasterXML / jackson-modules-java8

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

Milliseconds timestamps not parsing correctly for `LocalDate` even with `READ_DATE_TIMESTAMPS_AS_NANOSECONDS` as FALSE #283

Closed ibrahimatcha closed 9 months ago

ibrahimatcha commented 10 months ago

An example is trying to parse a field with an epoch millisecond value of 1532908800000 to a LocalDate. I get the error:

com.fasterxml.jackson.databind.JsonMappingException: Invalid value for EpochDay (valid values -365243219162 - 365241780471): 1532908800000

I have READ_DATE_TIMESTAMPS_AS_NANOSECONDS turned off in my configuration and set my object mapper bean to primary so spring doesn't override it but the issue persists. For some reason it keeps going back to the LocalDateDeserializer method that uses ofEpochDay and I cannot figure out why.

cowtowncoder commented 10 months ago

Cannot help without stand-alone reproduction, without Spring dependency. For help with Spring configuration please reach out to Spring (Boot) communities instead.

Put another way, please try to reproduce with a stand-alone test, include here, first.

ibrahimatcha commented 9 months ago

Apologies @cowtowncoder, I've replicated it in this codebase for you, let me know if what i have provided is still not adequate.

Interestingly, ZonedDateTime's are read just fine, it's LocalDate's that are the issue. ZonedDateTime's are read fine even with READ_DATE_TIMESTAMPS_AS_NANOSECONDS. Has it got something to do with a lil extra data that provides timezone context that is tripping up LocalDate?

cowtowncoder commented 9 months ago

@ibrahimatcha Thank you for providing reproduction: that looks perfect!

It should help figure out the issue whenever someone has time to look into it (possibly me, eventually, if no one else has time).

cowtowncoder commented 9 months ago

For more convenient access, here's the code:


Time.java:

import java.time.LocalDate;
import java.time.ZonedDateTime;

public class Time {
    public ZonedDateTime zonedDateTime;
    public LocalDate localDate;
}

TimeTest.java

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.json.JsonMapper;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import org.junit.jupiter.api.Test;

import java.time.*;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class TimeTest {

    @Test
    void name() throws Exception {
        Time expectedTime = new Time();
        expectedTime.zonedDateTime = ZonedDateTime.ofInstant(Instant.ofEpochMilli(1548236954181L), ZoneOffset.UTC);
        expectedTime.localDate = LocalDate.ofInstant(Instant.ofEpochMilli(1532908800000L), ZoneOffset.UTC);
        ObjectMapper objectMapper = JsonMapper.builder().addModule(new JavaTimeModule())
                .disable(DeserializationFeature.READ_DATE_TIMESTAMPS_AS_NANOSECONDS)
                .disable(SerializationFeature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS)
                .build();

        String timeJson = "{\n" +
                "\"zonedDateTime\": 1548236954181,\n" +
                "\"localDate\": 1532908800000, \n " +
                "}";

        Time actualTime = objectMapper.readValue(timeJson, Time.class);
        assertEquals(expectedTime, actualTime);
    }
}
cowtowncoder commented 9 months ago

Ok, this is not a valid issue: LocalDates are not timestamp-based -- they are DATEs, without Time part, for one. They are also not physical in the sense that they have no TimeZone at all, being "local".

From JavaDocs: "This class does not store or represent a time or time-zone. "

As such, integer values may be used but only to store

date.toEpochDay()

This can be seen by using the usual diagnostics technique: try writing (serializing) value with settings, then reading that value. But, actually, README for datetime:

https://github.com/FasterXML/jackson-modules-java8/tree/2.16/datetime

points out that the default representation is actually a JSON Array.

Now: if you wanted to use timestamp-based serialization, you'd need to write custom (de)serializer -- or we'd need to provide configurability for JavaTimeModule to do that. Adding support might be tricky, too, given the lack of TimeZone for timestamps: translation from timestamp to LocalDate would probably require assumption of UTC as "time zone" (it's not technically one) for constructing ZonedDateTime and extracting/converting LocalDate from that.

cowtowncoder commented 9 months ago

Since this is not a valid flag, will close -- LocalDate does not (or intend to) support reading/writing from/as millisecond timestamp, due to reasons explained above.

ibrahimatcha commented 9 months ago

Okay that makes sense thanks @cowtowncoder. I wrote a custom deserialiser anyway as a stop-gap but I will stick with this approach then if it isn't supposed to be expected behaviour. Thanks!

cowtowncoder commented 9 months ago

@ibrahimatcha Correct: for non-standard handling here (to support legacy use case or such), custom deserializer is needed. I don't see this as being supported in future.