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 failing on null values for creator (DeserializationFeature.FAIL_ON_NULL_CREATOR_PROPERTIES) #990

Closed
wants to merge 1 commit into from

Conversation

NTCoding
Copy link

In relation to this discussion: #988 and this issue in Scala: FasterXML/jackson-module-scala#203 I would like to provide a way for deserialization to fail when a value is missing for a property except when the property is a type of optional.

In Scala the default value should be a None, but that is handled by the custom deserializers in the Jackson Scala Module and isn't a concern here. What I've done here is added a hook that will fail on null default values - ensuring that no values can be null. That's what we are really trying to achieve with optionals - to enforce there are no nulls in the codebase.

I belive this pull request is useful to anyone - using Java or Scala - that wants to avoid nulls in their codebase. It's still possible to return a different value from getNullValue so it won't just work for optionals - any type who's getNullValue doesn't return a null.

This is just an example pull request. I'm happy to put more effort into it if you are happy to go ahead with the idea.

@cowtowncoder
Copy link
Member

Ok, I have think of this a bit more, but it seems reasonable enough on surface.

I am not big believer of "no nulls" approach myself, but since many users like such approaches (and inevitable overuse of Optionals in all kinds of wrong places :) ) it seems reasonable to try to support as conveniently as possible.

@NTCoding
Copy link
Author

Great :) I'm happy to work with you and find the best solution if I can. I'm more than happy to do the work if you point me in the right direction.

@potmo
Copy link

potmo commented Dec 3, 2015

👍
I have been looking for this for quite some time. Right now I have to do validation manually after deserialising and that is a pain. I think that this is a really great PR.

@sommestad
Copy link

👍

3 similar comments
@petrenkotino
Copy link

👍

@kotlinski
Copy link

👍

@petruswrango
Copy link

👍

@cowtowncoder
Copy link
Member

@NTCoding One wish: it would be great to get a unit test for this. Partly this because I think patch does not actually cover the case of explicit null, which would have to be done in method "assignParameter(...)" I think.
If this could be added, I would be able to get patch checked in for inclusion in 2.7.0.

@NTCoding
Copy link
Author

@cowtowncoder Hi, I'll be more than happy to add some tests for this. The reason that I did not initially was because this is just an example of what I was trying to achieve.

If you're happy for me to proceed then I'll definitely spend some time and make this a robust implementation.

Could you tell me when you need it by please?

@cowtowncoder
Copy link
Member

@NTCoding a simple unit tests that cover both missing creator property and explicit null would be good. I think I can handle the missing case (if it's missing like I think). But tests are also good to verify that I didn't misunderstand expected functionality (even if it is quite straightforward here).

@cowtowncoder cowtowncoder changed the title example of fail on null creators for discussion Allow failing on null values for creator (DeserializationFeature.FAIL_ON_NULL_CREATOR_PROPERTIES) Apr 15, 2016
cowtowncoder added a commit that referenced this pull request Apr 15, 2016
@cowtowncoder cowtowncoder added this to the 2.8.0 milestone Apr 15, 2016
@cowtowncoder
Copy link
Member

Implemented, will be in 2.8.0.

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.

7 participants