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

Different result of Instant deserialization between readValue and readTree/treeToValue #326

Open
valepakh opened this issue Nov 18, 2024 · 3 comments

Comments

@valepakh
Copy link

Deserializing Instant from the string of "millis.nanos", which is the default serialization method in Jacskon yields different result when doing so through the ObjectMapper.readValue method or first reading it into a JsonNode through ObjectMapper.readTree and then reading the value from the tree via ObjectMapper.treeToValue

The root cause seems to be that readTree stores the information in the node in the double format, which then gets truncated when deserializing it to the Instant, while when deserializing directly, this step is skipped.

I wrote the test, which doesn't really uses a JSON but if you create a proper class with annotations and the field of type Instant the result will be the same:

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import java.time.Instant;
import org.junit.jupiter.api.Test;

class InstantDeserializationTest {
    @Test
    void readValue() throws JsonProcessingException {
        ObjectMapper mapper = new ObjectMapper().registerModule(new JavaTimeModule());
        Instant instant = Instant.ofEpochSecond(1730977056L, 232784600);
        String instantString = mapper.writeValueAsString(instant);
        assertThat(instantString, is("1730977056.232784600"));

        Instant deserInstant = mapper.readValue(instantString, Instant.class);

        assertThat(deserInstant.getEpochSecond(), is(instant.getEpochSecond()));
        assertThat(deserInstant.getNano(), is(instant.getNano()));
    }

    @Test
    void readTree() throws JsonProcessingException {
        ObjectMapper mapper = new ObjectMapper().registerModule(new JavaTimeModule());
        Instant instant = Instant.ofEpochSecond(1730977056L, 232784600);
        String instantString = mapper.writeValueAsString(instant);
        assertThat(instantString, is("1730977056.232784600"));

        JsonNode jsonNode = mapper.readTree(instantString);
        Instant deserInstant = mapper.treeToValue(jsonNode, Instant.class);

        assertThat(deserInstant.getEpochSecond(), is(instant.getEpochSecond()));
        // fails here, the actual value is 232784500 due to the double truncation
        assertThat(deserInstant.getNano(), is(instant.getNano()));
    }
}
@cowtowncoder
Copy link
Member

Which Jackson version(s) was this tested with?

@valepakh
Copy link
Author

Which Jackson version(s) was this tested with?

2.18.1

@cowtowncoder
Copy link
Member

cowtowncoder commented Nov 19, 2024

Yes, I think this is an unfortunate side-effect of readTree() having to decide what to do with fractional numbers: whether to read them as 64-bit doubles (lossy, range), or as BigDecimals (unlimited precision, range, non-lossy; but slower to handle).

By default, doubles are used: this can be changed with config setting(s) but cannot be solved by Date/Time module itself.
Setting to enable is DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS and should retain accuracy.

@cowtowncoder cowtowncoder changed the title Different result of Instant deserialization between readValue and readTree/treeToValue Different result of Instant deserialization between readValue and readTree/treeToValue Nov 19, 2024
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

2 participants