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

Automatic registration of Jackson parameter names module [SPR-13296] #17886

Closed
spring-projects-issues opened this issue Jul 30, 2015 · 9 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jul 30, 2015

Lovro Pandžić opened SPR-13296 and commented

After upgrade to jackson 2.6.0 jackson parameter names module can be used in a java 8 environment to construct classes using non default constructors for classes that have parameter names stored. For such classes default constructors can be removed and no annotations are required!
I suggest this module be added for JDK 8+ projects.


Issue Links:

1 votes, 8 watchers

@spring-projects-issues
Copy link
Collaborator Author

Lovro Pandžić commented

Note that, due to backwards compatibility reasons, single argument constructors are recognized as delegating creators by jackson. This behavior can be overridden in this module.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

Lovro Pandžić I have 2 questions for you:

  • What happens when jackson-module-parameter-names is on the classpath and registered but -parameters compiler flag is not used?
  • Based on our last comment, do you think most users will expect new ParameterNamesModule(JsonCreator.Mode.PROPERTIES) to be registered automatically instead of new ParameterNamesModule()?

@spring-projects-issues
Copy link
Collaborator Author

Lovro Pandžić commented

  1. Reflection API returns null when resolving parameter names and the module returns those nulls to jackson core. When null is returned to core, it ignores it or better said, nulls mean that the introspector couldn't find parameter name. For instance, none of the jdk 8 classes (atleast last time I tested) have parameter names stored, when you use them for deserialization null will be returned. Parameter names flag only stores parameter names to classes your compiler has compiled. I suggest you try it out.
  2. I don't know because I don't know if any of the spring users use this default jackson behavior. If they do, this will be a breaking change for them.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

Thanks for your detailed feedback.

The first point just need more tests in order to check that it introduces no regression, no blocking issue here.

I am more concerned by the second point. We support registration of well known modules for JDK or widely used (Joda time for example) types, but these modules do not require any configuration, and since we do some classpath check we can register them, no problem.

As soon as the module requires configuration, this is another story. We could obviously register ParameterNamesModule with its default constructor, and let people customize it by disabling well known module registration and/or registering explicitly new ParameterNamesModule(JsonCreator.Mode.PROPERTIES). But I suspect most users are not aware of the delegating creators Jackson behavior, and they may be confused about the fact that only classes with single argument constructor does not work as expected.

That's why I am not 100% sure this is a good idea to automatically register jackson-module-parameter-names, even if I fully agree that this module is really useful as soon as you use Java 8.

Any thoughts Lovro Pandžić ?

@spring-projects-issues
Copy link
Collaborator Author

Lovro Pandžić commented

I agree that users will be confused once they encounter this situation and I understand your concern.
Is there any way to get feedback from the community about this issue? Would it be possible to introduce the module later on in a version where a breaking change is acceptable, with backwards compatible behavior overridden?

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

I guess it is more reasonable to put this issue in the 4.3 backlog for now, in order to see if we have users feedback, but I am not sure we will be able to introduce it later yet, since the default delegating creators behavior is default on Jackson side, and I think this is too much opinionated to change this in Spring Framework.

But keep in mind that automatic registration is not a must have for all modules, and adding a module is quite easy, especially with Spring Boot, you just have to register such bean in your configuration:

@Bean
Jackson2ObjectMapperBuilder jacksonBuilder() {
    Jackson2ObjectMapperBuilder builder = new Jackson2ObjectMapperBuilder();
    builder.modulesToInstall(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES));
    return builder;
}

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Aug 31, 2015

Sébastien Deleuze commented

Lovro Pandžić I have updated my Latest Jackson integration improvements in Spring blog post with more information about jackson parameter names module and how to register even more easily a Jackson Module with Spring Boot by just declaring:

@Bean
public Module parameterNamesModule() {
    return new ParameterNamesModule(JsonCreator.Mode.PROPERTIES);
}

I also plan to update the reference documentation as well, with a mention and a link to this module (see #17990).
Since I don't think Jackson default behavior about delegating creators will change in the future, would you consider that enough to consider this issue as resolved?

@spring-projects-issues
Copy link
Collaborator Author

Lovro Pandžić commented

Thanks for feedback, blog post and documentation mentions are nice, but I do hope this issue will be resolved sometime in the future. I'll try to pursue it's solution from within jackson so this issue is resolved from my point of view.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

Ok, so I resolve this issue as "Won't fix" for now, feel free to reopen it if things evolve on Jackson side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants