Support for jsonformat in duration deserializer

[obarcelonap]

Implementation based on Duration::of(long,TemporalUnit). Closes #184

[obarcelonap]

Did the changes in form of --fixup I will rebase once is ok from your side

[obarcelonap]

Thanks for the changes, they look good to me. Approved - assuming you'll get the documentation in. If they look good to @cowtowncoder too, you'll need to send him the CLA if you haven't already.

I haven't sent the CLA, where can I find it?

Just rebased on top of 2.12 branch, added missing documentation and squashed all the commits 🚀 🚀

[cowtowncoder]

CLA can be found from jackson repo:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

[obarcelonap]

Just sent the CLA by email.

[cowtowncoder]

Oh. Hmm. I should have looked at this earlier. I'll have to think how I feel about specifying new patterns -- but it is good that at least it is based on ChronoUnit; and the limitation of Java annotations prevents doing much more than matching it explicitly. I'll go over code and hopefully will be able to merge this soon.

[cowtowncoder]

Ok I think code looks pretty good on its own; I can make small changes myself if need be.

So the first question I have is whether decimal/floating-point numbers (vs integers) should be handled -- and it looks like only "ofSeconds()" handles two parts, although in theory we could calculate "1.5 hours" into 90 minutes just fine... but it gets complicated and I wonder would it not be better to simply throw an exception to let user know that fractions are not allowed for anything but seconds?

Second part is wrt serialization: should format not also be considered when serializing?

[kupci]

Second part is wrt serialization: should format not also be considered when serializing?

Hmm, glossed over this, but now I think so. The DurationSerializer supports milliseconds, seconds/nanoseconds, or "string representation":

A string representation of this duration using ISO-8601 seconds based representation, such as PT8H6M12.345S.

https://github.com/FasterXML/jackson-modules-java8/blob/e0c62bbe62eb17add9431f3d75cb4d9fcb7c7a16/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/ser/DurationSerializer.java#L92

So I would think there should be a corresponding "withPattern" method in DurationSerializer. @obarcelonap

Now in reviewing, it looks like we can trim supported units down the actual permitted ChronoUnits supported by Duration methods, i.e. toDays(), toHours(), toMillis(), toMinutes(), toNanos()?

Probably also need to test (with the serializer) edge cases like ChronUnit.FOREVER ;)

[cowtowncoder]

I think I have a few different things/ideas to discuss: will add to issue itself. I hope we can get this merged soon, but I really want to Do It Right, including backwards compatibility.

[obarcelonap]

Solved your comments guys, thanks for your feedback!