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

LocalDateTime serialization issue with custom-configured LocalDateTimeSerializer #288

Closed
mklinkj opened this issue Nov 19, 2023 · 12 comments
Labels
Milestone

Comments

@mklinkj
Copy link

mklinkj commented Nov 19, 2023

Hello.

There was no problem specifying the date format up to version 2.15.3, but it seems like it’s not applied in version 2.16.0

Example Project (v2.15.3)

Example Project Zip File (v2.15.3)

Where to change version

ObjectMapper Setting

    ObjectMapper mapper =
        new ObjectMapper()
            .registerModule(
                new JavaTimeModule()
                    .addSerializer(
                        LocalDateTime.class,
                        new LocalDateTimeSerializer(DateTimeFormatter.ofPattern("MM/dd/yyyy"))));

Run in Jackson 2.15.3

$ gradlew clean run

> Task :run
memberJson: {"name":"user00","registerDate":"12/25/2023"}

BUILD SUCCESSFUL in 2s
5 actionable tasks: 5 executed
$ 

Run in Jackson 2.16.0

$ gradlew clean run

> Task :run
memberJson: {"name":"user00","registerDate":[2023,12,25,1,2,3]}

BUILD SUCCESSFUL in 2s
5 actionable tasks: 5 executed
$ 
  • ✨ The date format set in LocalDateTimeSerializer is not applied in the 2.16.0

I think it might be due to my incorrect settings… Anyway, I’ve shared it

Thank you, Have a nice day 👍

@cowtowncoder
Copy link
Member

Ok, first of all: thank you for reporting this issue.

Second of all: I would highly recommend NOT constructing (de)serializers directly but letting JavaTimeModule do that; you can still apply pattern overrides with:

mapper.configOverride(LocalDateTime.class)
   .setFormat(JsonFormat.Value.forPattern(""MM/dd/yyyy""))

But be that as it may, the way you do things should also work.

@mklinkj
Copy link
Author

mklinkj commented Nov 21, 2023

@cowtowncoder

Hello.

I replaced the code as you replied above and confirmed that it works fine.

Thanks. Have a nice day. 👍

@cowtowncoder
Copy link
Member

@mklinkj Great!

I will still see if I can resolve the problem itself, but I am glad you were able to work around that.

@cowtowncoder
Copy link
Member

Ok, I can see why this happens: 2.16 changed registration of (de)serializers in a way that would ignore anything added via JavaTimeModule.addXxx() methods -- this was not meant to be a way to add (de)serializers.

I'll have to think of how to change this; it should be doable, just not trivially simple.

@cowtowncoder cowtowncoder changed the title LocalDateTime serialization issue in Jackson 2.16.0 LocalDateTime serialization issue with custom-configured LocalDateTimeSerializer Nov 21, 2023
@cowtowncoder cowtowncoder modified the milestones: 2.10.0, 2.16.1 Nov 21, 2023
@cowtowncoder
Copy link
Member

Was quite simple in the end, fixed for 2.16.1. I assume there will be other reports for other custom registrations.

@cowtowncoder
Copy link
Member

And there is at least #293.

@mklinkj
Copy link
Author

mklinkj commented Dec 5, 2023

Hello.

I have confirmed that the method I used before also works well in the 2.16.1-SNAPSHOT version.

Thanks. Have a nice day. 👍

@cowtowncoder
Copy link
Member

Thank you for confirming @mklinkj !

@mdindoffer
Copy link

Ok, I can see why this happens: 2.16 changed registration of (de)serializers in a way that would ignore anything added via JavaTimeModule.addXxx() methods -- this was not meant to be a way to add (de)serializers.

I'll have to think of how to change this; it should be doable, just not trivially simple.

Hey @cowtowncoder , would you mind sharing what is the intended way of registering custom de/serializers for example for java.time.Instant ? Or point me to a documentation? Jackson's docs are so fragmented that I could not find a proper unified reference on this topic. We've been hit by this bug as well, so I'd rather rewrite our code in compliance with the guidelines.
Thanks.

@cowtowncoder
Copy link
Member

@mdindoffer Ok, two pointers:

  1. 2.16.1 fixes the issue, so existing usage now works again: not recommended, but supported.
  2. For better way, create SimpleModule of your own (instead of, or in addition to JavaTimeModule) and add custom (de)serializers using that -- same ways you used to add them to JavaTimeModule (using methods inherited from SimpleModule, in that case)

So the only concern is that the fact that JavaTimeModule happens to be SimpleModule (in 2.x) might lead to usage that will break with 3.x (when JavaTimeModule direct implements Module and not SimpleModule).

I hope this clarifies matters.

@mdindoffer
Copy link

@cowtowncoder OK, so would you accept a PR that marks the methods as @Deprecated in JavaTimeModule? I see no point in leaving them as is, if they are not meant to be used on the JavaTimeModule.

@cowtowncoder
Copy link
Member

The problem is that methods come from SimpleModule and are not introduced by JavaTimeModule. I guess it would be possible to add overrides, for purpose of deprecating... but not sure how valuable that'd be.
Same would also (in theory) need to be done for all methods SimpleModule exposes (it is possible to call all other configuration methods).

From my perspective deprecation is no longer necessary as usage is supported (even if not recommended), including regression test verifying functioning (ability to register custom (de)serializers).
And in 3.0, due to change in base class, methods are no longer exposed at all.

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

3 participants