Wrong semantic for immutable @ConfigurationProperties contributed via @Import

[mpalourdio]

Hello,

As of SB 2.2.0.M5, the changelog states that @ConfigurationProperties-annotated types are no longer scanned in slice tests unless imported explicitly. This restores the behaviour that slice tests should only scan what is described in the documentation.

When an immutable @ConfigurationProperties is imported in a test slice, and the constructor parameters are not beans (ex : strings), the test fail.

As it's not that easy to describe, I have done a repository that reproduces the issue : https://github.com/mpalourdio/demorepro . Please run the single test to see it fail.

In order to make it work with SB 2.0.0.M4, remove the @Import in the test class, and change the SB version in pom.xml to target SB 2.0.0.M4. The test will succeed.

Thanks by advance, and let me know if you need more information.

[mpalourdio]

Replacing @Import by @EnableConfigurationProperties fixes the issue.

@EnableConfigurationProperties makes more sense of course... I was mislead by the wording of the changelog I think.

[snicoll]

@mpalourdio Thanks for reporting it. It would be nice if @Import would work as well or that we at least make this crystal clear in the documentation.

What bugs me is that if it wasn't immutable it would just work and I'd like users don't have to care about that.

[mpalourdio]

Thanks for feedback. My bad, just deleted the repository as I thought the intended behaviour was EnableConfigurationProperties.

Do you want me to reproduce another sample ?

[snicoll]

No, I have one. Thank you.

[mpalourdio]

The repository has been restored!

[philwebb]

Thanks for the report @mpalourdio. We've decided that we need a clearer signal for constructor property binding so we've introduced some new annotations (see #18469). Whilst this doesn't solve your problem, it does allow us to give a much better error message.

If you update your sample and change MyPropertyConfigHolder from a @ConfigurationProperties to a @ImmutableConfigurationProperties the latest build should give you the following error:

java.lang.IllegalStateException: Failed to load ApplicationContext
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:132)
	at ...
Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'com.example.demo.MyPropertyConfigHolder': @EnableConfigurationProperties or @ConfigurationPropertiesScan must be used to add @ConstructorBinding type com.example.demo.MyPropertyConfigHolder
	at org.springframework.boot.context.properties.ConfigurationPropertiesBeanDefinitionValidator.validate(ConfigurationPropertiesBeanDefinitionValidator.java:59)
	at org.springframework.boot.context.properties.ConfigurationPropertiesBeanDefinitionValidator.postProcessBeanFactory(ConfigurationPropertiesBeanDefinitionValidator.java:45)
	at org.springframework.context.support.PostProcessorRegistrationDelegate.invokeBeanFactoryPostProcessors(PostProcessorRegistrationDelegate.java:286)
	at org.springframework.context.support.PostProcessorRegistrationDelegate.invokeBeanFactoryPostProcessors(PostProcessorRegistrationDelegate.java:174)
	at org.springframework.context.support.AbstractApplicationContext.invokeBeanFactoryPostProcessors(AbstractApplicationContext.java:706)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:532)
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:747)
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:397)
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:315)
	at org.springframework.boot.test.context.SpringBootContextLoader.loadContext(SpringBootContextLoader.java:125)
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContextInternal(DefaultCacheAwareContextLoaderDelegate.java:99)
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:124)
	... 66 more

[mpalourdio]

@philwebb Thanks for feedback and clarification !

[mpalourdio]

@philwebb @snicoll So I have updated my sample with the very fresh RC1. Here is what I observe at the moment :

• With 2.2.0.M6, replacing @Import by @EnableConfigurationProperties in a test slice works
• With 2.2.0.RC1, replacing @Import by @EnableConfigurationProperties in a test slice produces now the same error as the original problem : No qualifying bean of type 'java.lang.String [...]'
• With 2.2.0.RC1, replacing @ConfigurationProperties by ImmutableConfigurationProperties and keeping @Import fails as expected
• With 2.2.0.RC1, replacing @ConfigurationProperties by ImmutableConfigurationProperties and replacing @Import by @EnableConfigurationProperties works as expected

What bugs me is that something that used to work before (point 2 above) does not work anymore. Is this expected ? Should we really have to use the good annotation on a property class regardless it is immutable or not ?

Thanks for feedback !

[wilkinsona]

@mpalourdio Thank you for trying out 2.2.0.RC1.

Yes, the behaviour you are now seeing is expected. We realised that we were trying to be too clever in attempting to determine whether a @ConfigurationProperties class should use constructor-based binding or setter-based binding. You now have to explicitly opt in to constructor-based binding either by using @ImmutableConfigurationProperties (or by using @ConfigurationProperties and @ConstructorBinding).

[mpalourdio]

@wilkinsona Thanks for clarification. Looks like the way to go :)