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

Deserializing `java.time.Month` from an int causes an off-by-one error (`0`->`Jan`,`11`->`Dec`), because it's an enum #274

Closed kreiger closed 7 months ago

kreiger commented 1 year ago

I found an issue with deserializing java.time.Month from an integer, or from a string containing an integer.

Since it's an enum, 0 deserializes as Month.JANUARY, 1/"01" as Month.FEBRUARY, 11/"11" as Month.DECEMBER, and so on.

I have a PR with a failing test that proves this: https://github.com/kreiger/jackson-modules-java8/pull/1

cowtowncoder commented 1 year ago

Looks like there is no explicit serializer/deserializer for type Month so it is dealt as a regular Enum. I can see how this would lead to unexpected results, although at the same time it is technically compliant with Jackson behavior for Enums.

A challenge is that change in this logic is backwards incompatible change: so we'd probably need to add a configuration option -- f.ex add new config enum JavaTimeFeature, with setting like... ONE_BASED_MONTHS (default: false) and then custom serializer, deserializer (to override default Enum serializer/deserializer).

This sounds like a good idea, but quite a bit of work. Since Jackson 2.16.0 is being released any day now this probably has to wait until 2.17.

cowtowncoder commented 9 months ago

Note: when implemented, should be enabled by a feature added to JavaTimeFeature (included in 2.16 to help configurability).

etrandafir93 commented 9 months ago

hello @cowtowncoder, I started looking into this issue. I have created a MR where you can see my progress: https://github.com/FasterXML/jackson-modules-java8/pull/292

I'll need a bit of guidance for the test related to the strictMapper - configOverride. Except for that, I fixed the other tests you provided and I added additional ones to check the solution is backwards compatible.

Which code formatter should I use for this project?

cowtowncoder commented 9 months ago

@etrandafir93 Ok sounds good: I added a few comments on PR. ConfigOverrides can be handled by something similar to what YearDeserializer does (it's pretty good source as basis, actually).

As to formatter, I don't think we have one so it's more of "When in Rome" style to follow -- most Jackson projects are similar, although there are some differences (esp. for Kotlin, Scala modules). But some basic rules:

  1. Indentation by spaces (4 per level)
  2. Imports from most generic to most specific (JDK, 3rd party lib, Jackson core, specific module)
  3. Variable naming, follow project (usually underscore-leading member variables, otherwise usual camel case)

there is a jackson-databind issue filed that asks writing down Jackson Coding Style, something I need to get done.

cowtowncoder commented 7 months ago

New feature merged in 2.17 for inclusion in 2.17.0.