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

`LocalDateTime` serialization issue with custom-configured `LocalDateTimeSerializer` #288

Closed mklinkj closed 9 months ago

mklinkj commented 9 months ago

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
$ 

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

Thank you, Have a nice day 👍

cowtowncoder commented 9 months ago

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 commented 9 months ago

@cowtowncoder

Hello.

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

Thanks. Have a nice day. 👍

cowtowncoder commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

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

cowtowncoder commented 9 months ago

And there is at least #293.

mklinkj commented 9 months ago

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 commented 9 months ago

Thank you for confirming @mklinkj !

mdindoffer commented 8 months ago

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 commented 7 months ago

@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 commented 7 months ago

@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 commented 7 months ago

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.