Analyze failure if configprop scanning results in two beans

Closes gh-16581

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesScanRegistrar.java

@@ -25,8 +25,11 @@ import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 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.type.AnnotationMetadata;
 import org.springframework.core.type.filter.AnnotationTypeFilter;
+import org.springframework.stereotype.Component;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.StringUtils;
 
@@ -85,6 +88,7 @@ class ConfigurationPropertiesScanRegistrar implements ImportBeanDefinitionRegist
 			String beanClassName = candidate.getBeanClassName();
 			try {
 				Class<?> type = ClassUtils.forName(beanClassName, null);
+				validateScanConfiguration(type);
 				ConfigurationPropertiesBeanDefinitionRegistrar.register(registry,
 						beanFactory, type);
 			}
@@ -94,4 +98,17 @@ class ConfigurationPropertiesScanRegistrar implements ImportBeanDefinitionRegist
 		}
 	}
 
+	private void validateScanConfiguration(Class<?> type) {
+		MergedAnnotation<Component> component = MergedAnnotations
+				.from(type, MergedAnnotations.SearchStrategy.EXHAUSTIVE)
+				.get(Component.class);
+		if (component.isPresent()) {
+			MergedAnnotation<?> parent = component;
+			while (parent.getParent() != null) {
+				parent = parent.getParent();
+			}
+			throw new InvalidConfigurationPropertiesException(type, parent.getType());
+		}
+	}
+
 }

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/InvalidConfigurationPropertiesException.java

@@ -0,0 +1,50 @@
+/*
+ * 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} 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;
+	}
+
+}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertiesFailureAnalyzer.java

@@ -0,0 +1,52 @@
+/*
+ * 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 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
+ */
+public class InvalidConfigurationPropertiesFailureAnalyzer
+		extends AbstractFailureAnalyzer<InvalidConfigurationPropertiesException> {
+
+	@Override
+	protected FailureAnalysis analyze(Throwable rootFailure,
+			InvalidConfigurationPropertiesException cause) {
+		Class<?> configurationProperties = cause.getConfigurationProperties();
+		String component = cause.getComponent().getSimpleName();
+		return new FailureAnalysis(getDescription(configurationProperties, component),
+				getAction(configurationProperties, component), cause);
+	}
+
+	private String getDescription(Class<?> configurationProperties, String component) {
+		return configurationProperties.getName()
+				+ " is annotated with @ConfigurationProperties and @" + component
+				+ ". This may cause the @ConfigurationProperties bean to be registered twice.";
+	}
+
+	private String getAction(Class<?> configurationProperties, String component) {
+		return "Remove either @ConfigurationProperties or @" + component + " from "
+				+ configurationProperties;
+	}
+
+}

spring-boot-project/spring-boot/src/main/resources/META-INF/spring.factories

@@ -49,6 +49,7 @@ org.springframework.boot.diagnostics.analyzer.NoSuchMethodFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.NoUniqueBeanDefinitionFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.PortInUseFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.ValidationExceptionFailureAnalyzer,\
+ org.springframework.boot.diagnostics.analyzer.InvalidConfigurationPropertiesFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.InvalidConfigurationPropertyNameFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.InvalidConfigurationPropertyValueFailureAnalyzer
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesScanRegistrarTests.java

@@ -22,11 +22,14 @@ import org.junit.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.ConfigurationPropertiesScanConfiguration;
+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.valid.ConfigurationPropertiesScanConfiguration;
 import org.springframework.core.type.AnnotationMetadata;
 import org.springframework.core.type.classreading.SimpleMetadataReaderFactory;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 /**
  * Tests for {@link ConfigurationPropertiesScanRegistrar}.
@@ -46,11 +49,11 @@ public class ConfigurationPropertiesScanRegistrarTests {
 				getAnnotationMetadata(ConfigurationPropertiesScanConfiguration.class),
 				this.beanFactory);
 		BeanDefinition bingDefinition = this.beanFactory.getBeanDefinition(
-				"bing-org.springframework.boot.context.properties.scan.ConfigurationPropertiesScanConfiguration$BingProperties");
+				"bing-org.springframework.boot.context.properties.scan.valid.ConfigurationPropertiesScanConfiguration$BingProperties");
 		BeanDefinition fooDefinition = this.beanFactory.getBeanDefinition(
-				"foo-org.springframework.boot.context.properties.scan.ConfigurationPropertiesScanConfiguration$FooProperties");
+				"foo-org.springframework.boot.context.properties.scan.valid.ConfigurationPropertiesScanConfiguration$FooProperties");
 		BeanDefinition barDefinition = this.beanFactory.getBeanDefinition(
-				"bar-org.springframework.boot.context.properties.scan.ConfigurationPropertiesScanConfiguration$BarProperties");
+				"bar-org.springframework.boot.context.properties.scan.valid.ConfigurationPropertiesScanConfiguration$BarProperties");
 		assertThat(bingDefinition).isExactlyInstanceOf(GenericBeanDefinition.class);
 		assertThat(fooDefinition).isExactlyInstanceOf(GenericBeanDefinition.class);
 		assertThat(barDefinition)
@@ -66,7 +69,7 @@ public class ConfigurationPropertiesScanRegistrarTests {
 						ConfigurationPropertiesScanConfiguration.TestConfiguration.class),
 				beanFactory);
 		BeanDefinition fooDefinition = beanFactory.getBeanDefinition(
-				"foo-org.springframework.boot.context.properties.scan.ConfigurationPropertiesScanConfiguration$FooProperties");
+				"foo-org.springframework.boot.context.properties.scan.valid.ConfigurationPropertiesScanConfiguration$FooProperties");
 		assertThat(fooDefinition).isExactlyInstanceOf(GenericBeanDefinition.class);
 	}
 
@@ -79,19 +82,53 @@ public class ConfigurationPropertiesScanRegistrarTests {
 				ConfigurationPropertiesScanConfiguration.DifferentPackageConfiguration.class),
 				beanFactory);
 		assertThat(beanFactory.containsBeanDefinition(
-				"foo-org.springframework.boot.context.properties.scan.ConfigurationPropertiesScanConfiguration$FooProperties"))
+				"foo-org.springframework.boot.context.properties.scan.valid.ConfigurationPropertiesScanConfiguration$FooProperties"))
 						.isFalse();
 		BeanDefinition aDefinition = beanFactory.getBeanDefinition(
-				"a-org.springframework.boot.context.properties.scan.a.AScanConfiguration$AProperties");
+				"a-org.springframework.boot.context.properties.scan.valid.a.AScanConfiguration$AProperties");
 		BeanDefinition bDefinition = beanFactory.getBeanDefinition(
-				"b-org.springframework.boot.context.properties.scan.b.BScanConfiguration$BProperties");
+				"b-org.springframework.boot.context.properties.scan.valid.b.BScanConfiguration$BProperties");
 		assertThat(aDefinition).isExactlyInstanceOf(GenericBeanDefinition.class);
 		assertThat(bDefinition).isExactlyInstanceOf(GenericBeanDefinition.class);
 	}
 
+	@Test
+	public void scanWhenComponentAnnotationPresentShouldThrowException() {
+		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.");
+	}
+
+	@Test
+	public void scanWhenOtherComponentAnnotationPresentShouldThrowException() {
+		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.");
+	}
+
 	private AnnotationMetadata getAnnotationMetadata(Class<?> source) throws IOException {
 		return new SimpleMetadataReaderFactory().getMetadataReader(source.getName())
 				.getAnnotationMetadata();
 	}
 
+	@ConfigurationPropertiesScan(basePackageClasses = InvalidConfiguration.class)
+	static class InvalidScanConfiguration {
+
+	}
+
+	@ConfigurationPropertiesScan(basePackageClasses = OtherInvalidConfiguration.class)
+	static class OtherInvalidScanConfiguration {
+
+	}
+
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/invalid/c/InvalidConfiguration.java

@@ -0,0 +1,32 @@
+/*
+ * 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.scan.invalid.c;
+
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.stereotype.Component;
+
+/**
+ * @author Madhura Bhave
+ */
+public class InvalidConfiguration {
+
+	@Component
+	@ConfigurationProperties(prefix = "b")
+	static class MyProperties {
+
+	}
+
+}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/invalid/d/OtherInvalidConfiguration.java

@@ -0,0 +1,33 @@
+/*
+ * 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.scan.invalid.d;
+
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.web.bind.annotation.RestController;
+
+/**
+ * @author Madhura Bhave
+ */
+public class OtherInvalidConfiguration {
+
+	@RestController
+	@ConfigurationProperties(prefix = "c")
+	static class MyControllerProperties {
+
+	}
+
+}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/ConfigurationPropertiesScanConfiguration.java

@@ -13,12 +13,12 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.springframework.boot.context.properties.scan;
+package org.springframework.boot.context.properties.scan.valid;
 
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.boot.context.properties.ConfigurationPropertiesScan;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
-import org.springframework.boot.context.properties.scan.b.BScanConfiguration;
+import org.springframework.boot.context.properties.scan.valid.b.BScanConfiguration;
 
 /**
  * Used for testing {@link ConfigurationProperties} scanning.
@@ -36,7 +36,7 @@ public class ConfigurationPropertiesScanConfiguration {
 	}
 
 	@ConfigurationPropertiesScan(
-			basePackages = "org.springframework.boot.context.properties.scan.a",
+			basePackages = "org.springframework.boot.context.properties.scan.valid.a",
 			basePackageClasses = BScanConfiguration.class)
 	public static class DifferentPackageConfiguration {
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/a/AScanConfiguration.java

@@ -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.a;
+package org.springframework.boot.context.properties.scan.valid.a;
 
 import org.springframework.boot.context.properties.ConfigurationProperties;
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/scan/valid/b/BScanConfiguration.java

@@ -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.b;
+package org.springframework.boot.context.properties.scan.valid.b;
 
 import org.springframework.boot.context.properties.ConfigurationProperties;
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/InvalidConfigurationPropertiesFailureAnalyzerTests.java

@@ -0,0 +1,66 @@
+/*
+ * 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 org.junit.Test;
+
+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
+ */
+public class InvalidConfigurationPropertiesFailureAnalyzerTests {
+
+	private final InvalidConfigurationPropertiesFailureAnalyzer analyzer = new InvalidConfigurationPropertiesFailureAnalyzer();
+
+	@Test
+	public void analysisForInvalidConfigurationOfConfigurationProperties() {
+		FailureAnalysis analysis = performAnalysis();
+		assertThat(analysis.getDescription()).isEqualTo(getDescription());
+		assertThat(analysis.getAction())
+				.isEqualTo("Remove either @ConfigurationProperties or @Component from "
+						+ TestProperties.class);
+	}
+
+	private String getDescription() {
+		return TestProperties.class.getName()
+				+ " is annotated with @ConfigurationProperties and @Component"
+				+ ". This may cause the @ConfigurationProperties bean to be registered twice.";
+	}
+
+	private FailureAnalysis performAnalysis() {
+		FailureAnalysis analysis = this.analyzer
+				.analyze(new InvalidConfigurationPropertiesException(TestProperties.class,
+						Component.class));
+		assertThat(analysis).isNotNull();
+		return analysis;
+	}
+
+	@ConfigurationProperties
+	@Component
+	private static class TestProperties {
+
+	}
+
+}