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

Add JavaTimeFeature.NORMALIZE_DESERIALIZED_ZONE_ID to allow disabling ZoneId normalization on deserialization #281

Closed
indyana opened this issue Oct 14, 2023 · 5 comments
Labels
Milestone

Comments

@indyana
Copy link

indyana commented Oct 14, 2023

Change in 2.15 to normalize zone when deserializing to ZonedDateTime causes breakage in some applications because normalized time zone does not match configured Jackson time zone. Would be nice to have ObjectMapper configuration that allows for turning normalization on and off.

Details
There are many places where we're comparing ZonedDateTime fields deserialized from stored JSON strings (format written out: "2023-02-02T20:23:11.057Z"), to ZonedDateTime objects created in code with ZoneId.of("UTC") .
Comparing a ZonedDateTime object created by application code to one deserialized from JSON now no longer works because the zones are not equal. (Zone of the deserialized JSON got normalized to "Z" starting in 2.15 release).

This happens whether relying on Jackson default time zone OR specifically configuring the ObjectMapper / Jackson to use time zone "UTC".

Fixing seems to mean replacing anywhere in code using ZoneId "UTC" with the offset zone Z, but that's a lot of application code to go through.

Notes from comments on issue #267

@indyana If you'd like to see configurability, please file a new issue as RFE, asking for configurability, referencing back to this issue. Then if anyone has time and interest they could work on adding this feature -- it does sound useful.

One practical complication here is just that module does not yet have configurability, so would probably need to add a JavaTimeModule.Feature enum and a bit of scaffolding. Aside from that should be quite straight-forward.

Originally posted by @cowtowncoder in #267 (comment)

@cowtowncoder cowtowncoder added the good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project label Oct 16, 2023
@cowtowncoder cowtowncoder changed the title Feature request: configuration for JavaTimeModule to disable zone normalization Add JavaTimeFeature.NORMALIZE_DESERIALIZED_ZONE_ID to allow disabling ZoneId normalization on deserialization Nov 12, 2023
@cowtowncoder cowtowncoder added 2.16 and removed good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project labels Nov 12, 2023
@cowtowncoder
Copy link
Member

@indyana I was able to implement this: configuration is via JavaTimeModule, like so:

ObjectMapper mapper = JsonMapper.builder()
            .addModule(new JavaTimeModule().disable(JavaTimeFeature.NORMALIZE_DESERIALIZED_ZONE_ID))
            .build();

I added just one test to verify and am not super happy with it, so I hope someone else can verify its functioning.
But implementation of actual normalization disabling in straight-forward enough so hoping it works as expected.

@indyana
Copy link
Author

indyana commented Nov 12, 2023

Great, thanks for considering that additional configuration feature! I'm sure I'm not the only one that will run into this time zone vs offset confusion, so will be good for users to be able to decide what behavior suits their application.

@cowtowncoder
Copy link
Member

@indyana Yeah, thank you for suggesting it! And like I said, if there's any way you could try it out (need to build/use 2.16.0-SNAPSHOT until 2.16.0 release is out; this was not part of 2.16.0-rc1) to verify it works as it should, that'd be great.

@indyana
Copy link
Author

indyana commented Nov 15, 2023

Confirmed that pulling 2.16.0-SNAPSHOT from Sonatype and adding disable(JavaTimeFeature.NORMALIZE_DESERIALIZED_ZONE_ID) correctly reverts ZonedDateTime objects to UTC again (vs Z), and our unit tests pass.

@cowtowncoder
Copy link
Member

Thank you @indyana , much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants