Skip to content

Smarter @ConfigurationProperties binding for nested namespaces #3445

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
snicoll opened this issue Jul 8, 2015 · 2 comments
Closed

Smarter @ConfigurationProperties binding for nested namespaces #3445

snicoll opened this issue Jul 8, 2015 · 2 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@snicoll
Copy link
Member

snicoll commented Jul 8, 2015

ignoreUnknownFields on @ConfigurationProperties is handled locally while it should probably be validated against a given application.

Consider the following case:

@ConfigurationProperties(prefix= "foo", ignoreUnknownFields=true)
public class FooProperties { ...}

Then somewhere else in the project we have the following

@ConfigurationProperties("foo.bar")
public class FooBarProperties { ...}

The binder might fail if we provide a valid property, let's say foo.bar.name because when it is processed against FooProperties that class does not have such field. In practice, this setup is perfectly OK, that property is valid, it's just that it's managed by a different class.

Maybe we should build that knowledge since we have the list @ConfigurationProperties annotated POJO for a given app.

snicoll added a commit that referenced this issue Jul 8, 2015
Unfortunately, we have no other choice to flip the ignoreUnknownFields
attribute of `SecurityProperties` has many different target are now set
for that namespace outside the class. See gh-3445 for a potential way
to improve that.

Closes gh-3327
@dsyer
Copy link
Member

dsyer commented Jul 16, 2015

I like the idea of a global validation. It would be a bit hairy to implement though (as anything global is wont to be).

@snicoll snicoll added this to the 1.4.0 milestone Oct 2, 2015
@snicoll snicoll added type: enhancement A general enhancement and removed discussion labels Oct 2, 2015
@snicoll snicoll assigned snicoll and unassigned snicoll Oct 2, 2015
@philwebb philwebb modified the milestones: 1.4.0.RC1, 1.4.0 Jan 7, 2016
@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label May 25, 2016
@philwebb philwebb removed this from the 1.4.0.RC1 milestone May 25, 2016
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label May 25, 2016
@snicoll snicoll added the theme: config-data Issues related to the configuration theme label Jan 24, 2017
@snicoll
Copy link
Member Author

snicoll commented Mar 22, 2018

This enhancement is there for quite a while and there isn't a obvious way to replace it so we're considering removing that feature altogether instead (#12601) rather than keeping it with false promise.

Looking at properties unbound on startup and generating a report would be a nice alternative, see #10030 for more details.

@snicoll snicoll closed this as completed Mar 22, 2018
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed theme: config-data Issues related to the configuration theme type: enhancement A general enhancement labels Mar 22, 2018
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
Projects
None yet
Development

No branches or pull requests

3 participants