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

Bug Fix: Failed to deserialize the date field from a number using the @JsonFormat annotation #927

Open
wants to merge 14 commits into
base: 2.11.x
Choose a base branch
from

Conversation

jjhz
Copy link

@jjhz jjhz commented Sep 9, 2024

Fixing the inconsistent behavior when deserializing date field annotated with Jackson @jsonformat annotation.

To reproduce the issue

Pojo:

import java.util.Date;

@Serdeable
class Test {
    @JsonFormat(pattern = "yyyy-MM-dd@HH:mm:ss.SSSZ")
    Date value;
}

Deserialize from Json string

{
    "value": 1640995200000
}

Error received:

Error decoding property [Date value] of type [class test.Test]: Text '1640995200000' could not be parsed at index 0
io.micronaut.serde.exceptions.SerdeException: Error decoding property [Date value] of type [class test.Test]: Text '1640995200000' could not be parsed at index 0
	at app//io.micronaut.serde.support.deserializers.DeserBean$DerProperty.deserializeAndSetPropertyValue(DeserBean.java:869)
	at app//io.micronaut.serde.support.deserializers.SimpleObjectDeserializer.deserializeInto(SimpleObjectDeserializer.java:100)
	at app//io.micronaut.serde.support.deserializers.SimpleObjectDeserializer.deserialize(SimpleObjectDeserializer.java:70)
	at app//io.micronaut.serde.support.deserializers.SimpleObjectDeserializer.deserializeNullable(SimpleObjectDeserializer.java:80)
	at app//io.micronaut.serde.support.deserializers.ErrorCatchingDeserializer.deserializeNullable(ErrorCatchingDeserializer.java:65)

Inconsistent Behavior

The same Json string input will be deserialized properly if the @JsonFormat(pattern = "yyyy-MM-dd@HH:mm:ss.SSSZ") removed since the FormattedTemporalSerde.deserialize() is missing the error handling when processing the annotated date field compared to DefaultFormattedTemporalSerde.deserialize() method.

@CLAassistant
Copy link

CLAassistant commented Sep 9, 2024

CLA assistant check
All committers have signed the CLA.

@dstepanov
Copy link
Contributor

dstepanov commented Sep 9, 2024

Can you please add the test to serde-jackson-tck to check if the behavior is the same as with Jackson Databind?

@jjhz
Copy link
Author

jjhz commented Sep 9, 2024

@dstepanov test has been added into serde-jackson-tck

@dstepanov
Copy link
Contributor

You can delete the other test

@jjhz
Copy link
Author

jjhz commented Sep 9, 2024

@dstepanov test case removed from other module. Only added toserde-jackson-tck

@dstepanov
Copy link
Contributor

Sorry, my suggestion was a bit incorrect. It looks like DatabindJsonFormatSpec is disabled.

I tried your test with Jackson, and it returned different results. Can you please align it as much as possible with Jackson Databind?

@dstepanov
Copy link
Contributor

Some tests looks like need to add startsWith because of extra zeros

@jjhz
Copy link
Author

jjhz commented Oct 19, 2024

Hi @dstepanov , sorry about the late reply.
I modified the test cases and changed the implementation slightly. Now the same test cases should work fine under the both serde-jackson-tck and serde-jackson. Let me know what you think. Thank you.

@jjhz
Copy link
Author

jjhz commented Oct 25, 2024

@dstepanov The last build failed and now I merged the latest main into my branch. Do you mind to trigger another build?

@dstepanov
Copy link
Contributor

I will review next week. Thanks!

@jjhz
Copy link
Author

jjhz commented Oct 25, 2024

I will review next week. Thanks!

Thanks. Found few test failures regarding the timezone. Will fix them on weekend.

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

Successfully merging this pull request may close these issues.

3 participants