@ConfigurationProperties should be mandatory

[snicoll]

Let's consider this sample use case: https://gist.github.com/snicoll/b2b08f783014ed2c6f02

Note that it only has @EnableConfigurationProperties(FooProperties.class) and no @ConfigurationProperties on FooProperties. This is the output of that application with the following configuration

name=foobar
description=my-desc

2015-07-10 21:37:21.137  INFO 2418 --- [           main] demo.DemoApplication                     : Starting DemoApplication on cancer-2.local with PID 2418 (/Users/snicoll/Downloads/demo-cp/target/classes started by snicoll in /Users/snicoll/Downloads/demo-cp)
2015-07-10 21:37:21.170  INFO 2418 --- [           main] s.c.a.AnnotationConfigApplicationContext : Refreshing org.springframework.context.annotation.AnnotationConfigApplicationContext@52f759d7: startup date [Fri Jul 10 21:37:21 CEST 2015]; root of context hierarchy
Name --> null
Description --> null
2015-07-10 21:37:21.726  INFO 2418 --- [           main] o.s.j.e.a.AnnotationMBeanExporter        : Registering beans for JMX exposure on startup
2015-07-10 21:37:21.735  INFO 2418 --- [           main] demo.DemoApplication                     : Started DemoApplication in 0.785 seconds (JVM running for 1.087)
2015-07-10 21:37:21.736  INFO 2418 --- [       Thread-1] s.c.a.AnnotationConfigApplicationContext : Closing org.springframework.context.annotation.AnnotationConfigApplicationContext@52f759d7: startup date [Fri Jul 10 21:37:21 CEST 2015]; root of context hierarchy
2015-07-10 21:37:21.736  INFO 2418 --- [       Thread-1] o.s.j.e.a.AnnotationMBeanExporter        : Unregistering JMX-exposed beans on shutdown

The binding actually does happen but after the @PostConstruct method is invoked. If I uncomment @ConfigurationProperties the output shows the foobar and my-desc values respectively.

I think that @ConfigurationProperties should be mandatory and the processor should fail if a class is referenced and is not (meta) annotated with @ConfigurationProperties. Note also that no meta-data will be generated for such file since the processor uses the (legitimate) presence of @ConfigurationProperties.

This also makes the behaviour more consistent: when target classes are processed as beans, the presence of @ConfigurationProperties triggers the processing.

I'd also argue that the prefix should be mandatory and non empty but that's maybe going a tad too far.

[lucassaldanha]

I'd like to put my 2 cent in this discussion from a developer point of view:

I think that @ConfigurationProperties should be mandatory and the processor should fail if a class is referenced and is not (meta) annotated with @ConfigurationProperties.

I agree with you and I prefer using @ConfigurationProperties. When navigating the code you instantly realizes that this is a class with properties from the configuration files, and the generation of meta-data files to be used by IDEs are cool.
Actually in my team we avoid using the @EnableConfigurationProperties(FooProperties.class) style.

I'd also argue that the prefix should be mandatory and non empty but that's maybe going a tad too far.

Yes, I think you are going too far... :) Actually I think that is more interesting to have a naming pattern to infer the prefix. Take for example a class named ConnectionSettings. We could remove the "Settings" part and infer the prefix "connection". What do you think? I believe that is better that making the prefix mandatory.

[snicoll]

Thanks for the reply! I'd rather not go down the road of inferring the prefix. My point is that not having any prefix isn't explicit (that's why I'd argue to make it mandatory). Guessing the prefix from the class is worse from that perspective.

Even though, I am not buying the case where one wants to bind to the root, I understand this is something folks would like to do for very simple apps.

[lucassaldanha]

Maybe I didn't make myself clear. The property class will continue to have the @ConfigurationProperties, but in the absence of the prefix, then it will infer from the name of the class.

Guessing the prefix from the class is worse from that perspective.

You think so? For me it's a convention over configuration approach. I agree that it isn't explicit but I think that makes sense. But I can live without it. :)

[snicoll]

I understood that. This seems a bit unrelated to what we're discussing here anyway and I really think it makes things more confusing than the current or proposed approaches.

I'd like to enforce a proper namespacing of keys and the only way (IMO) to achieve that is by asking users to be explicit. In particular, prefixing your keys by the ID (short name) of your app is a good idea; prefixing your configuration classes with that same id isn't.

[dsyer]

I don't think there's a way to put the genie back in the bottle here, unless we make the prefix mandatory (which it never has been) as well as the annotation.

[snicoll]

How about blowing up if you refer to FooProperties and it's not annotated. It will break stuff I am sure but at least we're back in track with "our" (my?) expectations.

(i.e. people complaining that no meta-data is generated)