Commit f0330163 authored by Andy Wilkinson's avatar Andy Wilkinson Committed by Phillip Webb

Allow @Component on mutable @ConfigurationProperties

Update configuration properties support to allow the `@Component`
annotation to be used on `@ConfigurationProperties` beans as long
as they are mutable.

This restores the behavior of Spring Boot 2.1 for mutable beans whilst
still allowing us to enforce the stricter rules for immutable value
object configuration properties.

Closes gh-18138
parent 4c0cf8f8
......@@ -903,6 +903,8 @@ In this setup one, and only one constructor must be defined with the list of pro
Default values can be specified using `@DefaultValue` and the same conversion service will be applied to coerce the `String` value to the target type of a missing property.
NOTE: To use constructor binding the class must not be annotated with `@Component` and must be enabled using `@EnableConfigurationProperties` or configuration property scanning instead.
[[boot-features-external-config-enabling]]
......@@ -933,20 +935,15 @@ In these cases, you can specify the list of types to process on any `@Configurat
[NOTE]
====
When the `@ConfigurationProperties` bean is registered using scanning or via `@EnableConfigurationProperties`, the bean has a conventional name: `<prefix>-<fqn>`, where `<prefix>` is the environment key prefix specified in the `@ConfigurationProperties` annotation and `<fqn>` is the fully qualified name of the bean.
When the `@ConfigurationProperties` bean is registered using configuration property scanning or via `@EnableConfigurationProperties`, the bean has a conventional name: `<prefix>-<fqn>`, where `<prefix>` is the environment key prefix specified in the `@ConfigurationProperties` annotation and `<fqn>` is the fully qualified name of the bean.
If the annotation does not provide any prefix, only the fully qualified name of the bean is used.
The bean name in the example above is `acme-com.example.AcmeProperties`.
====
We recommend that `@ConfigurationProperties` only deal with the environment and, in particular, does not inject other beans from the context.
In particular, it is not possible to inject other beans using the constructor as this would trigger the constructor binder that only deals with the environment.
For corner cases, setter injection can be used or any of the `*Aware` interfaces provided
by the framework (such as `EnvironmentAware` if you need access to the `Environment`).
NOTE: Annotating a `@ConfigurationProperties` type with `@Component` will result in two beans of the same type if the type is also scanned as part of classpath scanning.
If you want to register the bean yourself using `@Component`, consider disabling scanning of `@ConfigurationProperties`.
For corner cases, setter injection can be used or any of the `*Aware` interfaces provided by the framework (such as `EnvironmentAware` if you need access to the `Environment`).
If you still want to inject other beans using the constructor, the configuration properties bean must be annotated with `@Component` and use JavaBean-based property binding.
......
......@@ -28,8 +28,8 @@ import org.springframework.context.ResourceLoaderAware;
import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider;
import org.springframework.context.annotation.ImportBeanDefinitionRegistrar;
import org.springframework.core.annotation.AnnotationAttributes;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.core.annotation.MergedAnnotations.SearchStrategy;
import org.springframework.core.env.Environment;
import org.springframework.core.io.ResourceLoader;
import org.springframework.core.type.AnnotationMetadata;
......@@ -99,8 +99,9 @@ class ConfigurationPropertiesScanRegistrar
String beanClassName = candidate.getBeanClassName();
try {
Class<?> type = ClassUtils.forName(beanClassName, null);
validateScanConfiguration(type);
ConfigurationPropertiesBeanDefinitionRegistrar.register(registry, beanFactory, type);
if (!isComponent(type)) {
ConfigurationPropertiesBeanDefinitionRegistrar.register(registry, beanFactory, type);
}
}
catch (ClassNotFoundException ex) {
// Ignore
......@@ -108,12 +109,8 @@ class ConfigurationPropertiesScanRegistrar
}
}
private void validateScanConfiguration(Class<?> type) {
MergedAnnotation<Component> component = MergedAnnotations
.from(type, MergedAnnotations.SearchStrategy.TYPE_HIERARCHY).get(Component.class);
if (component.isPresent()) {
throw new InvalidConfigurationPropertiesException(type, component.getRoot().getType());
}
private boolean isComponent(Class<?> type) {
return MergedAnnotations.from(type, SearchStrategy.TYPE_HIERARCHY).isPresent(Component.class);
}
@Override
......
/*
* Copyright 2012-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.context.properties;
import org.springframework.util.Assert;
/**
* Exception thrown when a {@link ConfigurationProperties @ConfigurationProperties} has
* been misconfigured.
*
* @author Madhura Bhave
* @since 2.2.0
*/
public class InvalidConfigurationPropertiesException extends RuntimeException {
private final Class<?> configurationProperties;
private final Class<?> component;
public InvalidConfigurationPropertiesException(Class<?> configurationProperties, Class<?> component) {
super("Found @" + component.getSimpleName() + " and @ConfigurationProperties on "
+ configurationProperties.getName() + ".");
Assert.notNull(configurationProperties, "Class must not be null");
this.configurationProperties = configurationProperties;
this.component = component;
}
public Class<?> getConfigurationProperties() {
return this.configurationProperties;
}
public Class<?> getComponent() {
return this.component;
}
}
/*
* Copyright 2012-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.diagnostics.analyzer;
import java.lang.reflect.Constructor;
import org.springframework.boot.context.properties.InvalidConfigurationPropertiesException;
import org.springframework.boot.diagnostics.AbstractFailureAnalyzer;
import org.springframework.boot.diagnostics.FailureAnalysis;
/**
* An {@link AbstractFailureAnalyzer} that performs analysis of failures caused by
* {@link InvalidConfigurationPropertiesException}.
*
* @author Madhura Bhave
* @author Stephane Nicoll
* @since 2.2.0
*/
public class InvalidConfigurationPropertiesFailureAnalyzer
extends AbstractFailureAnalyzer<InvalidConfigurationPropertiesException> {
@Override
protected FailureAnalysis analyze(Throwable rootFailure, InvalidConfigurationPropertiesException cause) {
Class<?> target = cause.getConfigurationProperties();
Constructor<?> autowiringConstructor = getAutowiringConstructor(target);
String componentName = cause.getComponent().getSimpleName();
return new FailureAnalysis(getDescription(target, autowiringConstructor, componentName),
getAction(target, autowiringConstructor, componentName), cause);
}
private String getDescription(Class<?> target, Constructor<?> autowiringConstructor, String componentName) {
String targetName = target.getSimpleName();
StringBuilder sb = new StringBuilder(targetName);
sb.append(" is annotated with @ConfigurationProperties and @").append(componentName)
.append(". This may cause the @ConfigurationProperties bean to be registered twice.");
if (autowiringConstructor != null) {
sb.append(" Also, autowiring by constructor is enabled for ").append(targetName)
.append(" which conflicts with properties constructor binding.");
}
return sb.toString();
}
private String getAction(Class<?> target, Constructor<?> autowiringConstructor, String componentName) {
StringBuilder sb = new StringBuilder();
if (autowiringConstructor != null) {
sb.append("Consider refactoring ").append(target.getSimpleName()).append(
" so that it does not rely on other beans. Alternatively, a default constructor should be added and @Autowired should be defined on ")
.append(autowiringConstructor.toGenericString()).append(String.format(".%n%n"));
}
sb.append("Remove @").append(componentName).append(" from ").append(target.getName())
.append(" or consider disabling automatic @ConfigurationProperties scanning.");
return sb.toString();
}
private Constructor<?> getAutowiringConstructor(Class<?> target) {
Constructor<?>[] candidates = target.getDeclaredConstructors();
if (candidates.length == 1) {
Constructor<?> candidate = candidates[0];
if (candidate.getParameterCount() > 0) {
return candidate;
}
}
return null;
}
}
......@@ -23,15 +23,14 @@ import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
import org.springframework.beans.factory.support.GenericBeanDefinition;
import org.springframework.boot.context.properties.scan.invalid.c.InvalidConfiguration;
import org.springframework.boot.context.properties.scan.invalid.d.OtherInvalidConfiguration;
import org.springframework.boot.context.properties.scan.combined.c.CombinedConfiguration;
import org.springframework.boot.context.properties.scan.combined.d.OtherCombinedConfiguration;
import org.springframework.boot.context.properties.scan.valid.ConfigurationPropertiesScanConfiguration;
import org.springframework.core.type.AnnotationMetadata;
import org.springframework.core.type.classreading.SimpleMetadataReaderFactory;
import org.springframework.mock.env.MockEnvironment;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
/**
* Tests for {@link ConfigurationPropertiesScanRegistrar}.
......@@ -99,38 +98,33 @@ class ConfigurationPropertiesScanRegistrarTests {
}
@Test
void scanWhenComponentAnnotationPresentShouldThrowException() {
void scanWhenComponentAnnotationPresentShouldSkipType() throws IOException {
DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
beanFactory.setAllowBeanDefinitionOverriding(false);
assertThatExceptionOfType(InvalidConfigurationPropertiesException.class)
.isThrownBy(() -> this.registrar
.registerBeanDefinitions(getAnnotationMetadata(InvalidScanConfiguration.class), beanFactory))
.withMessageContaining(
"Found @Component and @ConfigurationProperties on org.springframework.boot.context.properties.scan.invalid.c.InvalidConfiguration$MyProperties.");
this.registrar.registerBeanDefinitions(getAnnotationMetadata(CombinedScanConfiguration.class), beanFactory);
assertThat(beanFactory.getBeanDefinitionCount()).isEqualTo(0);
}
@Test
void scanWhenOtherComponentAnnotationPresentShouldThrowException() {
void scanWhenOtherComponentAnnotationPresentShouldSkipType() throws IOException {
DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
beanFactory.setAllowBeanDefinitionOverriding(false);
assertThatExceptionOfType(InvalidConfigurationPropertiesException.class)
.isThrownBy(() -> this.registrar.registerBeanDefinitions(
getAnnotationMetadata(OtherInvalidScanConfiguration.class), beanFactory))
.withMessageContaining(
"Found @RestController and @ConfigurationProperties on org.springframework.boot.context.properties.scan.invalid.d.OtherInvalidConfiguration$MyControllerProperties.");
this.registrar.registerBeanDefinitions(getAnnotationMetadata(OtherCombinedScanConfiguration.class),
beanFactory);
assertThat(beanFactory.getBeanDefinitionCount()).isEqualTo(0);
}
private AnnotationMetadata getAnnotationMetadata(Class<?> source) throws IOException {
return new SimpleMetadataReaderFactory().getMetadataReader(source.getName()).getAnnotationMetadata();
}
@ConfigurationPropertiesScan(basePackageClasses = InvalidConfiguration.class)
static class InvalidScanConfiguration {
@ConfigurationPropertiesScan(basePackageClasses = CombinedConfiguration.class)
static class CombinedScanConfiguration {
}
@ConfigurationPropertiesScan(basePackageClasses = OtherInvalidConfiguration.class)
static class OtherInvalidScanConfiguration {
@ConfigurationPropertiesScan(basePackageClasses = OtherCombinedConfiguration.class)
static class OtherCombinedScanConfiguration {
}
......
......@@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.context.properties.scan.invalid.c;
package org.springframework.boot.context.properties.scan.combined.c;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.stereotype.Component;
......@@ -21,7 +21,7 @@ import org.springframework.stereotype.Component;
/**
* @author Madhura Bhave
*/
public class InvalidConfiguration {
public class CombinedConfiguration {
@Component
@ConfigurationProperties(prefix = "b")
......
......@@ -14,7 +14,7 @@
* limitations under the License.
*/
package org.springframework.boot.context.properties.scan.invalid.d;
package org.springframework.boot.context.properties.scan.combined.d;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.web.bind.annotation.RestController;
......@@ -22,7 +22,7 @@ import org.springframework.web.bind.annotation.RestController;
/**
* @author Madhura Bhave
*/
public class OtherInvalidConfiguration {
public class OtherCombinedConfiguration {
@RestController
@ConfigurationProperties(prefix = "c")
......
/*
* Copyright 2012-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.boot.diagnostics.analyzer;
import java.util.function.Function;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.InvalidConfigurationPropertiesException;
import org.springframework.boot.diagnostics.FailureAnalysis;
import org.springframework.stereotype.Component;
import static org.assertj.core.api.Assertions.assertThat;
/**
* Tests for {@link InvalidConfigurationPropertiesFailureAnalyzer}
*
* @author Madhura Bhave
* @author Stephane Nicoll
*/
class InvalidConfigurationPropertiesFailureAnalyzerTests {
private final InvalidConfigurationPropertiesFailureAnalyzer analyzer = new InvalidConfigurationPropertiesFailureAnalyzer();
@Test
void analysisForInvalidConfigurationOfConfigurationProperties() {
FailureAnalysis analysis = performAnalysis(TestProperties.class);
assertThat(analysis.getDescription()).isEqualTo(getBasicDescription(TestProperties.class));
assertThat(analysis.getAction()).isEqualTo(getBasicAction(TestProperties.class));
}
@Test
void analysisForInvalidConfigurationOfConfigurationPropertiesWithSingleConstructor() {
FailureAnalysis analysis = performAnalysis(TestPropertiesWithSingleConstructor.class);
assertThat(analysis.getDescription()).containsSequence(
getBasicDescription(TestPropertiesWithSingleConstructor.class),
" Also, autowiring by constructor is enabled for "
+ TestPropertiesWithSingleConstructor.class.getSimpleName()
+ " which conflicts with properties constructor binding.");
assertThat(analysis.getAction()).containsSubsequence("Consider refactoring TestPropertiesWithSingleConstructor "
+ "so that it does not rely on other beans. Alternatively, a default constructor should be added and "
+ "@Autowired should be defined on "
+ "org.springframework.boot.diagnostics.analyzer.InvalidConfigurationPropertiesFailureAnalyzerTests$TestPropertiesWithSingleConstructor(java.lang.Object,java.util.function.Function<java.lang.String, java.lang.Integer>).",
getBasicAction(TestPropertiesWithSingleConstructor.class));
}
@Test
void analysisForInvalidConfigurationOfConfigurationPropertiesWithSeveralConstructors() {
FailureAnalysis analysis = performAnalysis(TestPropertiesWithSeveralConstructors.class);
assertThat(analysis.getDescription())
.isEqualTo(getBasicDescription(TestPropertiesWithSeveralConstructors.class));
assertThat(analysis.getAction()).isEqualTo(getBasicAction(TestPropertiesWithSeveralConstructors.class));
}
private String getBasicDescription(Class<?> target) {
return target.getSimpleName() + " is annotated with @ConfigurationProperties and @Component"
+ ". This may cause the @ConfigurationProperties bean to be registered twice.";
}
private String getBasicAction(Class<?> target) {
return "Remove @Component from " + target.getName()
+ " or consider disabling automatic @ConfigurationProperties scanning.";
}
private FailureAnalysis performAnalysis(Class<?> target) {
FailureAnalysis analysis = this.analyzer
.analyze(new InvalidConfigurationPropertiesException(target, Component.class));
assertThat(analysis).isNotNull();
return analysis;
}
@ConfigurationProperties
@Component
static class TestProperties {
}
@ConfigurationProperties
@Component
static class TestPropertiesWithSingleConstructor {
TestPropertiesWithSingleConstructor(Object firstService, Function<String, Integer> factory) {
}
}
@ConfigurationProperties
@Component
static class TestPropertiesWithSeveralConstructors {
TestPropertiesWithSeveralConstructors() {
}
@Autowired
TestPropertiesWithSeveralConstructors(Object someService) {
}
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment