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

Problem in serializing negative Duration values #165

Closed
jpsyri opened this issue Mar 5, 2020 · 4 comments
Closed

Problem in serializing negative Duration values #165

jpsyri opened this issue Mar 5, 2020 · 4 comments
Milestone

Comments

@jpsyri
Copy link

jpsyri commented Mar 5, 2020

When SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS is true, Durations with negative value get serialized wrong. For example Duration.ofMillis(-1) gets serialized as "-1.999000000"

Problem seems to be in DurationSerializer where following code is used for serializing

                generator.writeNumber(DecimalUtils.toBigDecimal(
                        duration.getSeconds(), duration.getNano()
                ));

However Duration internally stores duration in format where seconds part basically contains floor(duration) and nanoseconds part contains offset from that value to the actual duration. So n case of duration of -1 millisecond, the seconds part value will be -1 and nanoseconds part will be 999000000. getSeconds() and getNano() return values according to that internal representation, which leads to the problem.

@kupci
Copy link
Member

kupci commented Mar 5, 2020

Will take a look, but in the meantime, what release are you seeing this in, and what flags do you have set. (If you have a test case, that would be great.) Taking a quick look, the DurationSerializer should be using millis, unless otherwise specified.

However, we happened to just be discussing this the other day and I think the nanos flag WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS is (probably incorrectly) defaulting to true:

      if (useTimestamp(provider)) {
           if (useNanoseconds(provider)) {
               generator.writeNumber(DecimalUtils.toBigDecimal(
                       duration.getSeconds(), duration.getNano()
               ));
           } else {
               **generator.writeNumber(duration.toMillis());**
           }

@jpsyri
Copy link
Author

jpsyri commented Mar 5, 2020

I think we are using 2.10.2 (can't check at the moment). Only feature we have set is the "write datetimes as timestamps"- one. I however did run the serialization on debugger while investigating and both if-conditions on that snippet were evaluating to true, but I didn't really think about that further

I might be able to provide test case tomorrow

@kupci
Copy link
Member

kupci commented Mar 5, 2020

and both if-conditions on that snippet were evaluating to true,

Would you please try setting the SerializationFeature WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS to false and retry, and see if that resolves the issue? You can see some examples here:

https://github.com/FasterXML/jackson-modules-java8/blob/master/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/ser/DurationSerTest.java

@kupci kupci added the date-time-config Work related to possible larger DateTimeConfig feature label Mar 5, 2020
@jpsyri
Copy link
Author

jpsyri commented Mar 6, 2020

I created some test cases at https://github.com/jpsyri/jackson-duration-serialization

Disabling the WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS seems to result serialization as whole milliseconds as expected: Duration of -1 milliseconds is serialized as "-1". That feature enabled (including when state of that feature is not explicitly changed) results "-1.999000000"

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

3 participants