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

Allow DeserializationProblemHandler to respond to primitive types #1767

Closed
wants to merge 1 commit into from

Conversation

nhtzr
Copy link

@nhtzr nhtzr commented Sep 15, 2017

When registering a DeserializationProblemHandler, one is allowed to override its methods to return a converted value.

DeserializationContext has a sanity check in case the problem handler returns an Object that is not assignable to the property that is currently being parsed.

However for properties that rely on primitive types, this sanity check will always fail due to autoboxing.

(As a side note: Some upper code swallows the generated exception by this check, so the user only sees the original problem that caused the Problem Handler to be invoked in the first place even though the sanity check builds and throws a proper exception)

This PR add an extra comparison for primitive type (against their wrapped versions) while moving the sanity check into its own method.

Please let me know if I'm missing anything!
Thank you

@nhtzr nhtzr changed the title Allow handlers to respond to primitive types Allow DeserializationProblemHandler to respond to primitive types Sep 15, 2017
@cowtowncoder
Copy link
Member

Ok I idea seems reasonable, thank you for suggesting it. I can probably clean it up a little bit (there are helper methods for finding wrapper types), but basic idea seems sound.

Thank you for contributing this!

@cowtowncoder
Copy link
Member

One request: do you happen to have a reproduction of the problem? Would be good to verify fix as well as have a regression test.

@nhtzr
Copy link
Author

nhtzr commented Sep 17, 2017

Sure, no problem.
I couldn't find a way to run it inside the repo, but I've made a little project that reproduces the issue:
https://github.com/nhtzr/jackson-databind/tree/test

@cowtowncoder cowtowncoder changed the title Allow DeserializationProblemHandler to respond to primitive types Allow DeserializationProblemHandler to respond to primitive types Sep 19, 2017
@cowtowncoder cowtowncoder added this to the 2.9.2 milestone Sep 19, 2017
@cowtowncoder
Copy link
Member

Thank you again for reporting this, showing how to fix it.
I ended up merging changes manually since these are needed for 2.9 (although I did actually merge them in 2.8 just in case there'll be one last patch), but the idea is same.

Fix will be in 2.9.2.

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.

2 participants