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

Consider deprecating @EnableConfigurationProperties value attribute #23324

Closed
philwebb opened this issue Sep 15, 2020 · 3 comments
Closed

Consider deprecating @EnableConfigurationProperties value attribute #23324

philwebb opened this issue Sep 15, 2020 · 3 comments

Comments

@philwebb
Copy link
Member

#23172 has introduced a new @ImportAsConfigurationPropertiesBean annotation which now means we have two very similar ways to import configuration properties classes. We might want to consider deprecating the value attribute from EnableConfigurationProperties.

There's one subtle difference between the two at the moment. If you import a class via @EnableConfigurationProperties, it will not use constructor binding unless the class itself has a @ConstructorBinding annotation. With @ImportAsConfigurationPropertiesBean, it will attempt to deduce if constructor binding should be used.

It's possible that a user may have an @EnableConfigurationProperties with a constructor that they expect to be autowired, and with getters/setters that they expect to be bound. The new annotation doesn't currently support this. We might be able to offer an additional attribute to deal with this, e.g.:

@ImportAsConfigurationPropertiesBean(type = MyBean.class, bindType = JAVA_BEAN)
@philwebb
Copy link
Member Author

Requiring @ConstructorBinding annotations on @ConfigurationProperties beans is quite annoying in general. Perhaps we can investigate also adding an option to @ConfigurationPropertiesScan when we investigate this one.

I'd very much like a user to be able to define a constructor bound properties class without needing the extra annotation. We've investigated this before in #23216.

@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Sep 15, 2020
@philwebb philwebb added this to the 2.4.x milestone Sep 15, 2020
@philwebb philwebb added type: task A general task and removed for: team-attention An issue we'd like other members of the team to review labels Sep 16, 2020
@Davio
Copy link

Davio commented Sep 17, 2020

Would it be possible to end up with a default implementation where:

  • A primary constructor is used if found (if it's the only constructor, otherwise it would be ambiguous which one we should use)
  • Any leftover properties which are not set through the constructor are set by using setters (if possible).

@philwebb
Copy link
Member Author

We'll consider this as part of #23607

@philwebb philwebb removed this from the 2.4.x milestone Oct 21, 2020
@philwebb philwebb removed the type: task A general task label Oct 21, 2020
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

No branches or pull requests

2 participants