Detect ConfigurationProperties using autowiring constructor

This commit improves InvalidConfigurationPropertiesException failure
analysis by adding a dedicated description when a single autowiring
constructor is found. When @Component is removed, such constructor is
used for properties binding and will not apply the same semantics as
before.

Closes gh-17750
pull/17913/head
Stephane Nicoll 5 years ago
parent f58b3ea872
commit 7f06504006

@ -16,6 +16,8 @@
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;
@ -25,6 +27,7 @@ import org.springframework.boot.diagnostics.FailureAnalysis;
* {@link InvalidConfigurationPropertiesException}.
*
* @author Madhura Bhave
* @author Stephane Nicoll
* @since 2.2.0
*/
public class InvalidConfigurationPropertiesFailureAnalyzer
@ -32,20 +35,46 @@ public class InvalidConfigurationPropertiesFailureAnalyzer
@Override
protected FailureAnalysis analyze(Throwable rootFailure, InvalidConfigurationPropertiesException cause) {
String configurationProperties = cause.getConfigurationProperties().getName();
String component = cause.getComponent().getSimpleName();
return new FailureAnalysis(getDescription(configurationProperties, component),
getAction(configurationProperties, component), 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 getDescription(String configurationProperties, String component) {
return configurationProperties + " is annotated with @ConfigurationProperties and @" + component
+ ". This may cause the @ConfigurationProperties bean to be registered twice.";
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 String getAction(String configurationProperties, String component) {
return "Remove @" + component + " from " + configurationProperties
+ " or consider disabling automatic @ConfigurationProperties scanning.";
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;
}
}

@ -16,8 +16,11 @@
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;
@ -29,6 +32,7 @@ import static org.assertj.core.api.Assertions.assertThat;
* Tests for {@link InvalidConfigurationPropertiesFailureAnalyzer}
*
* @author Madhura Bhave
* @author Stephane Nicoll
*/
class InvalidConfigurationPropertiesFailureAnalyzerTests {
@ -36,20 +40,47 @@ class InvalidConfigurationPropertiesFailureAnalyzerTests {
@Test
void analysisForInvalidConfigurationOfConfigurationProperties() {
FailureAnalysis analysis = performAnalysis();
assertThat(analysis.getDescription()).isEqualTo(getDescription());
assertThat(analysis.getAction()).isEqualTo("Remove @Component from " + TestProperties.class.getName()
+ " or consider disabling automatic @ConfigurationProperties scanning.");
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 getDescription() {
return TestProperties.class.getName() + " is annotated with @ConfigurationProperties and @Component"
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 FailureAnalysis performAnalysis() {
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(TestProperties.class, Component.class));
.analyze(new InvalidConfigurationPropertiesException(target, Component.class));
assertThat(analysis).isNotNull();
return analysis;
}
@ -60,4 +91,28 @@ class InvalidConfigurationPropertiesFailureAnalyzerTests {
}
@ConfigurationProperties
@Component
static class TestPropertiesWithSingleConstructor {
TestPropertiesWithSingleConstructor(Object firstService, Function<String, Integer> factory) {
}
}
@ConfigurationProperties
@Component
static class TestPropertiesWithSeveralConstructors {
TestPropertiesWithSeveralConstructors() {
}
@Autowired
TestPropertiesWithSeveralConstructors(Object someService) {
}
}
}

Loading…
Cancel
Save