Skip to content

Support unannotated single arg constructors as property creators #162

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

Closed

Conversation

WellingR
Copy link

@WellingR WellingR commented Feb 4, 2020

This is a (partial) fix for #50 and follows the existing documentation which can be found on:
https://github.com/FasterXML/jackson-modules-java8/tree/master/parameter-names#delegating-creator:
If the module is registered as follows:

objectMapper.registerModule(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES));

Then the given creator mode is used for constructors where no @JsonCreator annotation is used.

Includes the test from #31

I consider this a partial fix, because having this as default behaviour would be better, see FasterXML/jackson-databind#1498

@cowtowncoder
Copy link
Member

Thank you. I'll have to see if I can figure why behavior is/was the way...

I'd have to merge this in 2.11 since I expect change here to break someone's usage, just because this area is notoriously brittle. :)

@WellingR WellingR changed the base branch from 2.10 to 2.11 February 5, 2020 07:54
@WellingR WellingR force-pushed the bugfix/single-arg-constructor branch from 4cdd7ab to f0814a4 Compare February 5, 2020 07:57
@WellingR
Copy link
Author

WellingR commented Feb 5, 2020

Rebased against 2.11

@cowtowncoder
Copy link
Member

Um, I don't think this makes sense: override was designed to simply allow specifying PROPERTIES vs DELEGATING mode for @JsonCreator that omitted mode. Change would basically make every constructor (and static factory method) visible without annotation: this would not work well either functionally or from security perspective.

@cowtowncoder
Copy link
Member

@WellingR first of all: thank you for your contribution. I realized that in its current form this would unfortunately not work (as per comments), but that I can probably figure out an alternative that would.
Since this is a long-time request, I will go back and try to get the issue itself fixed for 2.11.

@WellingR
Copy link
Author

Thanks for figuring this out. I already was sort of doubting whether fixing this was really this simple.

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