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

Negative Duration does not round-trip properly with WRITE_DURATIONS_AS_TIMESTAMPS enabled #337

Closed
jmuia opened this issue Dec 19, 2024 · 3 comments · Fixed by #350
Closed
Labels
2.19 has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue

Comments

@jmuia
Copy link

jmuia commented Dec 19, 2024

I found this similar issue #165 but it seems different, as that user reported a bug on 2.10.2 and refers to a different config option.

WRITE_DURATIONS_AS_TIMESTAMPS defaults to true.

Before v2.12.0

WRITE_DURATIONS_AS_TIMESTAMPS set to true.

It seems that a negative value...

  • Serializes incorrectly ❌
  • Deserializes back to the original value ✅

v2.12.0 and later (including 2.18.2)

WRITE_DURATIONS_AS_TIMESTAMPS set to true.

It seems that a negative value...

  • Serializes correctly ✅
  • Deserializes incorrectly ❌

Here is an example repo demonstrating the problem https://github.com/jmuia/jackson-duration-serialization

@cowtowncoder
Copy link
Member

Here's inlined class from reproduction link from above:

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

import java.time.Duration;

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


public class DurationTest {
    // Default behavior doesn't seem to work
    @Test
    public void test_default() throws Exception {
        JsonMapper mapper = JsonMapper.builder()
                .addModule(new JavaTimeModule())
                .build();

        Duration duration = Duration.parse("PT-43.636S");

        String ser = mapper.writeValueAsString(duration);

        // 2.12.0+
        assertEquals("-43.636000000", ser);

        // 2.11.4
        // assertEquals("-44.364000000", ser);

        Duration deser = mapper.readValue(ser, Duration.class);

        // 2.12.0+
        assertNotEquals(duration, deser);
        assertEquals(deser.toString(), "PT-42.364S");

        // 2.11.4
        // assertEquals(duration, deser);
        // assertEquals(deser.toString(), "PT-43.636S");
    }

    // Handling with WRITE_DURATIONS_AS_TIMESTAMPS enabled can't round-trip a value
    @Test
    public void test_with() throws Exception {
        JsonMapper mapper = JsonMapper.builder()
                .addModule(new JavaTimeModule())
                .configure(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS, true)
                .build();

        Duration duration = Duration.parse("PT-43.636S");

        String ser = mapper.writeValueAsString(duration);

        // 2.12.0+
        assertEquals("-43.636000000", ser);

        // 2.11.4
        // assertEquals("-44.364000000", ser);

        Duration deser = mapper.readValue(ser, Duration.class);

        // 2.12.0+
        assertNotEquals(duration, deser);
        assertEquals(deser.toString(), "PT-42.364S");

        // 2.11.4
        // assertEquals(duration, deser);
        // assertEquals(deser.toString(), "PT-43.636S");
    }

    // Handling with WRITE_DURATIONS_AS_TIMESTAMPS disabled works
    @Test
    public void test_without() throws Exception {
        JsonMapper mapper = JsonMapper.builder()
                .addModule(new JavaTimeModule())
                .configure(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS, false)
                .build();

        Duration duration = Duration.parse("PT-43.636S");

        String ser = mapper.writeValueAsString(duration);
        assertEquals("\"PT-43.636S\"", ser);

        Duration deser = mapper.readValue(ser, Duration.class);
        assertEquals(duration, deser);
    }
}

@cowtowncoder
Copy link
Member

And just so it is clear, 2.11 and 2.12 are old unmaintained versions (minus possible security fixes): fix, if any should go in 2.18.x so let's repro with 2.18.2.

@cowtowncoder
Copy link
Member

Ok confusing part of the tests are commented out parts: going forward let's leave expected correct case uncommented.

But I think I figured it out.

cowtowncoder added a commit that referenced this issue Jan 27, 2025
cowtowncoder added a commit that referenced this issue Jan 27, 2025
@cowtowncoder cowtowncoder added has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue 2.18 2.19 and removed 2.18 labels Jan 28, 2025
@cowtowncoder cowtowncoder changed the title Negative Duration does not round-trip properly after v2.12.0 with WRITE_DURATIONS_AS_TIMESTAMPS enabled Negative Duration does not round-trip properly with WRITE_DURATIONS_AS_TIMESTAMPS enabled Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.19 has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants