Merge branch '2.2.x' into 2.3.x

Closes gh-22325
spring-projects · Jul 14, 2020 · 196d205 · 196d205
2 parents f52cdf1 + 9558779
commit 196d205
Show file tree

Hide file tree

Showing 6 changed files with 209 additions and 107 deletions.
diff --git a/...ringframework/boot/context/properties/ConfigurationPropertiesBeanDefinitionValidator.java b/...ringframework/boot/context/properties/ConfigurationPropertiesBeanDefinitionValidator.java
diff --git a/...a/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrar.java b/...a/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrar.java
@@ -50,7 +50,6 @@ private Set<Class<?>> getTypes(AnnotationMetadata metadata) {
 	static void registerInfrastructureBeans(BeanDefinitionRegistry registry) {
 		ConfigurationPropertiesBindingPostProcessor.register(registry);
 		BoundConfigurationProperties.register(registry);
-		ConfigurationPropertiesBeanDefinitionValidator.register(registry);
 		ConfigurationBeanFactoryMetadata.register(registry);
 	}
 

diff --git a/.../springframework/boot/context/properties/NotConstructorBoundInjectionFailureAnalyzer.java b/.../springframework/boot/context/properties/NotConstructorBoundInjectionFailureAnalyzer.java
@@ -0,0 +1,83 @@
+/*
+ * Copyright 2012-2020 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 java.lang.reflect.Constructor;
+
+import org.springframework.beans.factory.InjectionPoint;
+import org.springframework.beans.factory.NoSuchBeanDefinitionException;
+import org.springframework.beans.factory.UnsatisfiedDependencyException;
+import org.springframework.boot.context.properties.ConfigurationPropertiesBean.BindMethod;
+import org.springframework.boot.diagnostics.FailureAnalysis;
+import org.springframework.boot.diagnostics.analyzer.AbstractInjectionFailureAnalyzer;
+import org.springframework.core.Ordered;
+import org.springframework.core.annotation.MergedAnnotation;
+import org.springframework.core.annotation.MergedAnnotations;
+
+/**
+ * An {@AbstractInjectionFailureAnalyzer} for
+ * {@link ConfigurationProperties @ConfigurationProperties} that are intended to use
+ * {@link ConstructorBinding constructor binding} but did not.
+ *
+ * @author Andy Wilkinson
+ */
+class NotConstructorBoundInjectionFailureAnalyzer
+		extends AbstractInjectionFailureAnalyzer<NoSuchBeanDefinitionException> implements Ordered {
+
+	@Override
+	public int getOrder() {
+		return 0;
+	}
+
+	@Override
+	protected FailureAnalysis analyze(Throwable rootFailure, NoSuchBeanDefinitionException cause, String description) {
+		InjectionPoint injectionPoint = findInjectionPoint(rootFailure);
+		if (isConstructorBindingConfigurationProperties(injectionPoint)) {
+			String simpleName = injectionPoint.getMember().getDeclaringClass().getSimpleName();
+			String action = String.format("Update your configuration so that " + simpleName + " is defined via @"
+					+ ConfigurationPropertiesScan.class.getSimpleName() + " or @"
+					+ EnableConfigurationProperties.class.getSimpleName() + ".", simpleName);
+			return new FailureAnalysis(
+					simpleName + " is annotated with @" + ConstructorBinding.class.getSimpleName()
+							+ " but it is defined as a regular bean which caused dependency injection to fail.",
+					action, cause);
+		}
+		return null;
+	}
+
+	private boolean isConstructorBindingConfigurationProperties(InjectionPoint injectionPoint) {
+		if (injectionPoint != null && injectionPoint.getMember() instanceof Constructor) {
+			Constructor<?> constructor = (Constructor<?>) injectionPoint.getMember();
+			Class<?> declaringClass = constructor.getDeclaringClass();
+			MergedAnnotation<ConfigurationProperties> configurationProperties = MergedAnnotations.from(declaringClass)
+					.get(ConfigurationProperties.class);
+			return configurationProperties.isPresent()
+					&& BindMethod.forType(constructor.getDeclaringClass()) == BindMethod.VALUE_OBJECT;
+		}
+		return false;
+	}
+
+	private InjectionPoint findInjectionPoint(Throwable failure) {
+		UnsatisfiedDependencyException unsatisfiedDependencyException = findCause(failure,
+				UnsatisfiedDependencyException.class);
+		if (unsatisfiedDependencyException == null) {
+			return null;
+		}
+		return unsatisfiedDependencyException.getInjectionPoint();
+	}
+
+}
diff --git a/spring-boot-project/spring-boot/src/main/resources/META-INF/spring.factories b/spring-boot-project/spring-boot/src/main/resources/META-INF/spring.factories
@@ -41,6 +41,7 @@ org.springframework.boot.reactor.DebugAgentEnvironmentPostProcessor
 
 # Failure Analyzers
 org.springframework.boot.diagnostics.FailureAnalyzer=\
+org.springframework.boot.context.properties.NotConstructorBoundInjectionFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.BeanCurrentlyInCreationFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.BeanDefinitionOverrideFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.BeanNotOfRequiredTypeFailureAnalyzer,\

diff --git a/...c/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java b/...c/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java
@@ -59,7 +59,6 @@
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.Import;
 import org.springframework.context.annotation.ImportResource;
 import org.springframework.context.annotation.Scope;
 import org.springframework.context.support.PropertySourcesPlaceholderConfigurer;
@@ -762,13 +761,6 @@ void loadWhenBindingToConstructorParametersShouldBind() {
 		assertThat(bean.getBar()).isEqualTo(5);
 	}
 
-	@Test // gh-17831
-	void loadWhenBindingConstructorParametersViaImportShouldThrowException() {
-		assertThatExceptionOfType(BeanCreationException.class)
-				.isThrownBy(() -> load(ImportConstructorParameterPropertiesConfiguration.class))
-				.withMessageContaining("@EnableConfigurationProperties or @ConfigurationPropertiesScan must be used");
-	}
-
 	@Test
 	void loadWhenBindingToConstructorParametersWithDefaultValuesShouldBind() {
 		load(ConstructorParameterConfiguration.class);
@@ -1933,13 +1925,6 @@ int getBar() {
 
 	}
 
-	@Configuration(proxyBeanMethods = false)
-	@EnableConfigurationProperties
-	@Import(ConstructorParameterProperties.class)
-	static class ImportConstructorParameterPropertiesConfiguration {
-
-	}
-
 	@ConstructorBinding
 	@ConfigurationProperties(prefix = "test")
 	@Validated

diff --git a/...ngframework/boot/context/properties/NotConstructorBoundInjectionFailureAnalyzerTests.java b/...ngframework/boot/context/properties/NotConstructorBoundInjectionFailureAnalyzerTests.java
@@ -0,0 +1,125 @@
+/*
+ * Copyright 2012-2020 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.junit.jupiter.api.Test;
+
+import org.springframework.beans.FatalBeanException;
+import org.springframework.boot.diagnostics.FailureAnalysis;
+import org.springframework.boot.diagnostics.LoggingFailureAnalysisReporter;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link NotConstructorBoundInjectionFailureAnalyzer}.
+ *
+ * @author Andy Wilkinson
+ */
+class NotConstructorBoundInjectionFailureAnalyzerTests {
+
+	private final NotConstructorBoundInjectionFailureAnalyzer analyzer = new NotConstructorBoundInjectionFailureAnalyzer();
+
+	@Test
+	void failureAnalysisForConfigurationPropertiesThatShouldHaveBeenConstructorBound() {
+		FailureAnalysis analysis = analyzeFailure(
+				createFailure(ShouldHaveUsedConstructorBindingPropertiesConfiguration.class));
+		assertThat(analysis.getDescription()).isEqualTo(ConstructorBoundProperties.class.getSimpleName()
+				+ " is annotated with @" + ConstructorBinding.class.getSimpleName()
+				+ " but it is defined as a regular bean which caused dependency injection to fail.");
+		assertThat(analysis.getAction())
+				.isEqualTo("Update your configuration so that " + ConstructorBoundProperties.class.getSimpleName()
+						+ " is defined via @" + ConfigurationPropertiesScan.class.getSimpleName() + " or @"
+						+ EnableConfigurationProperties.class.getSimpleName() + ".");
+	}
+
+	@Test
+	void failureAnaylsisForNonConstructorBoundProperties() {
+		FailureAnalysis analysis = analyzeFailure(createFailure(JavaBeanBoundPropertiesConfiguration.class));
+		assertThat(analysis).isNull();
+	}
+
+	private FatalBeanException createFailure(Class<?> config) {
+		try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext()) {
+			context.register(config);
+			context.refresh();
+			return null;
+		}
+		catch (FatalBeanException ex) {
+			return ex;
+		}
+	}
+
+	private FailureAnalysis analyzeFailure(Exception failure) {
+		assertThat(failure).isNotNull();
+		FailureAnalysis analysis = this.analyzer.analyze(failure);
+		if (analysis != null) {
+			new LoggingFailureAnalysisReporter().report(analysis);
+		}
+		return analysis;
+	}
+
+	@ConstructorBinding
+	@ConfigurationProperties("test")
+	static class ConstructorBoundProperties {
+
+		private final String name;
+
+		ConstructorBoundProperties(String name) {
+			this.name = name;
+		}
+
+		String getName() {
+			return this.name;
+		}
+
+	}
+
+	@Configuration(proxyBeanMethods = false)
+	@Import(ConstructorBoundProperties.class)
+	static class ShouldHaveUsedConstructorBindingPropertiesConfiguration {
+
+	}
+
+	@ConfigurationProperties("test")
+	static class JavaBeanBoundProperties {
+
+		private String name;
+
+		JavaBeanBoundProperties(String dependency) {
+
+		}
+
+		String getName() {
+			return this.name;
+		}
+
+		void setName(String name) {
+			this.name = name;
+		}
+
+	}
+
+	@Configuration(proxyBeanMethods = false)
+	@Import(JavaBeanBoundProperties.class)
+	static class JavaBeanBoundPropertiesConfiguration {
+
+	}
+
+}