Prohibit bean overriding by default and analyze override failures

Closes gh-13609

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/AbstractEndpointDocumentationTests.java

@@ -31,6 +31,7 @@ import org.springframework.boot.actuate.autoconfigure.endpoint.EndpointAutoConfi
 import org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointAutoConfiguration;
 import org.springframework.boot.actuate.autoconfigure.endpoint.web.reactive.WebFluxEndpointManagementContextConfiguration;
 import org.springframework.boot.actuate.autoconfigure.endpoint.web.servlet.WebMvcEndpointManagementContextConfiguration;
+import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
 import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
 import org.springframework.boot.autoconfigure.http.HttpMessageConvertersAutoConfiguration;
 import org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration;
@@ -39,7 +40,6 @@ import org.springframework.boot.autoconfigure.web.reactive.WebFluxAutoConfigurat
 import org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoConfiguration;
 import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.Import;
 import org.springframework.restdocs.operation.preprocess.ContentModifyingOperationPreprocessor;
 import org.springframework.restdocs.operation.preprocess.OperationPreprocessor;
 import org.springframework.restdocs.payload.FieldDescriptor;
@@ -128,7 +128,7 @@ public abstract class AbstractEndpointDocumentationTests {
 	}
 
 	@Configuration
-	@Import({ JacksonAutoConfiguration.class,
+	@ImportAutoConfiguration({ JacksonAutoConfiguration.class,
 			HttpMessageConvertersAutoConfiguration.class, WebMvcAutoConfiguration.class,
 			DispatcherServletAutoConfiguration.class, EndpointAutoConfiguration.class,
 			WebEndpointAutoConfiguration.class,

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/FlywayEndpointDocumentationTests.java

@@ -26,8 +26,8 @@ import org.flywaydb.core.api.MigrationType;
 import org.junit.Test;
 
 import org.springframework.boot.actuate.flyway.FlywayEndpoint;
+import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
 import org.springframework.boot.autoconfigure.flyway.FlywayAutoConfiguration;
-import org.springframework.boot.autoconfigure.jdbc.EmbeddedDataSourceConfiguration;
 import org.springframework.boot.jdbc.EmbeddedDatabaseConnection;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
@@ -98,8 +98,8 @@ public class FlywayEndpointDocumentationTests extends MockMvcEndpointDocumentati
 	}
 
 	@Configuration
-	@Import({ BaseDocumentationConfiguration.class, EmbeddedDataSourceConfiguration.class,
+	@Import(BaseDocumentationConfiguration.class)
-			FlywayAutoConfiguration.class })
+	@ImportAutoConfiguration(FlywayAutoConfiguration.class)
 	static class TestConfiguration {
 
 		@Bean

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorControllerIntegrationTests.java

@@ -36,6 +36,7 @@ import org.junit.After;
 import org.junit.Test;
 
 import org.springframework.boot.SpringApplication;
+import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
 import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
 import org.springframework.boot.autoconfigure.freemarker.FreeMarkerAutoConfiguration;
 import org.springframework.boot.autoconfigure.http.HttpMessageConvertersAutoConfiguration;
@@ -46,7 +47,6 @@ import org.springframework.boot.test.web.client.TestRestTemplate;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.Import;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.RequestEntity;
@@ -236,7 +236,7 @@ public class BasicErrorControllerIntegrationTests {
 	@Target(ElementType.TYPE)
 	@Retention(RetentionPolicy.RUNTIME)
 	@Documented
-	@Import({ ServletWebServerFactoryAutoConfiguration.class,
+	@ImportAutoConfiguration({ ServletWebServerFactoryAutoConfiguration.class,
 			DispatcherServletAutoConfiguration.class, WebMvcAutoConfiguration.class,
 			HttpMessageConvertersAutoConfiguration.class, ErrorMvcAutoConfiguration.class,
 			PropertyPlaceholderAutoConfiguration.class })
@@ -246,7 +246,7 @@ public class BasicErrorControllerIntegrationTests {
 
 	@Configuration
 	@MinimalWebConfiguration
-	@Import(FreeMarkerAutoConfiguration.class)
+	@ImportAutoConfiguration(FreeMarkerAutoConfiguration.class)
 	public static class TestConfiguration {
 
 		// For manual testing

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorControllerMockMvcTests.java

@@ -34,6 +34,7 @@ import org.junit.runner.RunWith;
 
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.SpringApplication;
+import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
 import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
 import org.springframework.boot.autoconfigure.http.HttpMessageConvertersAutoConfiguration;
 import org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoConfiguration;
@@ -42,7 +43,6 @@ import org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguratio
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.Import;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.mock.web.MockHttpServletRequest;
@@ -129,7 +129,7 @@ public class BasicErrorControllerMockMvcTests {
 	@Target(ElementType.TYPE)
 	@Retention(RetentionPolicy.RUNTIME)
 	@Documented
-	@Import({ ServletWebServerFactoryAutoConfiguration.class,
+	@ImportAutoConfiguration({ ServletWebServerFactoryAutoConfiguration.class,
 			DispatcherServletAutoConfiguration.class, WebMvcAutoConfiguration.class,
 			HttpMessageConvertersAutoConfiguration.class, ErrorMvcAutoConfiguration.class,
 			PropertyPlaceholderAutoConfiguration.class })

spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc

@@ -113,6 +113,7 @@ content into your application. Rather, pick only the properties that you need.
 	spring.mail.username= # Login user of the SMTP server.
 
 	# APPLICATION SETTINGS ({sc-spring-boot}/SpringApplication.{sc-ext}[SpringApplication])
+	spring.main.allow-bean-definition-overriding=false # Whether bean definition overriding, by registering a definition with the same name as an existing definition, is allowed.
 	spring.main.banner-mode=console # Mode used to display the banner when the application runs.
 	spring.main.sources= # Sources (class names, package names, or XML resource locations) to include in the ApplicationContext.
 	spring.main.web-application-type= # Flag to explicitly request a specific type of web application. If not set, auto-detected based on the classpath.

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java

@@ -35,9 +35,11 @@ import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.BeanUtils;
 import org.springframework.beans.CachedIntrospectionResults;
+import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.groovy.GroovyBeanDefinitionReader;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.beans.factory.support.BeanNameGenerator;
+import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
 import org.springframework.boot.Banner.Mode;
 import org.springframework.boot.context.properties.bind.Bindable;
@@ -235,6 +237,8 @@ public class SpringApplication {
 
 	private Set<String> additionalProfiles = new HashSet<>();
 
+	private boolean allowBeanDefinitionOverriding;
+
 	/**
 	 * Create a new {@link SpringApplication} instance. The application context will load
 	 * beans from the specified primary sources (see {@link SpringApplication class-level}
@@ -381,12 +385,15 @@ public class SpringApplication {
 		}
 
 		// Add boot specific singleton beans
-		context.getBeanFactory().registerSingleton("springApplicationArguments",
+		ConfigurableListableBeanFactory beanFactory = context.getBeanFactory();
-				applicationArguments);
+		beanFactory.registerSingleton("springApplicationArguments", applicationArguments);
 		if (printedBanner != null) {
-			context.getBeanFactory().registerSingleton("springBootBanner", printedBanner);
+			beanFactory.registerSingleton("springBootBanner", printedBanner);
+		}
+		if (beanFactory instanceof DefaultListableBeanFactory) {
+			((DefaultListableBeanFactory) beanFactory)
+					.setAllowBeanDefinitionOverriding(this.allowBeanDefinitionOverriding);
 		}
-
 		// Load the sources
 		Set<Object> sources = getAllSources();
 		Assert.notEmpty(sources, "Sources must not be empty");
@@ -955,6 +962,17 @@ public class SpringApplication {
 		this.webApplicationType = webApplicationType;
 	}
 
+	/**
+	 * Sets if bean definition overriding, by registering a definition with the same name
+	 * as an existing definition, should be allowed. Defaults to {@code false}.
+	 * @param allowBeanDefinitionOverriding if overriding is allowed
+	 * @since 2.1
+	 * @see DefaultListableBeanFactory#setAllowBeanDefinitionOverriding(boolean)
+	 */
+	public void setAllowBeanDefinitionOverriding(boolean allowBeanDefinitionOverriding) {
+		this.allowBeanDefinitionOverriding = allowBeanDefinitionOverriding;
+	}
+
 	/**
 	 * Sets if the application is headless and should not instantiate AWT. Defaults to
 	 * {@code true} to prevent java icons appearing.

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

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2012-2018 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
+ *
+ *      http://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.io.PrintWriter;
+import java.io.StringWriter;
+
+import org.springframework.beans.factory.support.BeanDefinitionOverrideException;
+import org.springframework.boot.diagnostics.AbstractFailureAnalyzer;
+import org.springframework.boot.diagnostics.FailureAnalysis;
+
+/**
+ * An {@link AbstractFailureAnalyzer} that performs analysis of failures caused by a
+ * {@link BeanDefinitionOverrideException}.
+ *
+ * @author Andy Wilkinson
+ */
+class BeanDefinitionOverrideFailureAnalyzer
+		extends AbstractFailureAnalyzer<BeanDefinitionOverrideException> {
+
+	private static final String ACTION = "Consider renaming one of the beans or enabling "
+			+ "overriding by setting spring.main.allow-bean-definition-overriding=true";
+
+	@Override
+	protected FailureAnalysis analyze(Throwable rootFailure,
+			BeanDefinitionOverrideException cause) {
+		return new FailureAnalysis(getDescription(cause), ACTION, cause);
+	}
+
+	private String getDescription(BeanDefinitionOverrideException ex) {
+		StringWriter description = new StringWriter();
+		PrintWriter printer = new PrintWriter(description);
+		printer.printf(
+				"The bean '%s', defined in %s, could not be registered. A bean with that "
+						+ "name has already been defined in %s and overriding is disabled.",
+				ex.getBeanName(), ex.getBeanDefinition().getResourceDescription(),
+				ex.getExistingDefinition().getResourceDescription());
+		return description.toString();
+	}
+
+}

spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json

@@ -172,6 +172,13 @@
         "com.arjuna.ats.internal.arjuna.recovery.ExpiredTransactionStatusManagerScanner"
       ]
     },
+    {
+      "name": "spring.main.allow-bean-definition-overriding",
+      "type": "java.lang.Boolean",
+      "sourceType": "org.springframework.boot.SpringApplication",
+      "description": "Whether bean definition overriding, by registering a definition with the same name as an existing definition, is allowed",
+      "defaultValue": false
+    },
     {
       "name": "spring.main.banner-mode",
       "type": "org.springframework.boot.Banner$Mode",

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

@@ -39,6 +39,7 @@ org.springframework.boot.env.SystemEnvironmentPropertySourceEnvironmentPostProce
 # Failure Analyzers
 org.springframework.boot.diagnostics.FailureAnalyzer=\
 org.springframework.boot.diagnostics.analyzer.BeanCurrentlyInCreationFailureAnalyzer,\
+org.springframework.boot.diagnostics.analyzer.BeanDefinitionOverrideFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.BeanNotOfRequiredTypeFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.BindFailureAnalyzer,\
 org.springframework.boot.diagnostics.analyzer.BindValidationFailureAnalyzer,\

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/OverrideSourcesTests.java

@@ -56,6 +56,7 @@ public class OverrideSourcesTests {
 		this.context = SpringApplication.run(
 				new Class<?>[] { MainConfiguration.class, TestConfiguration.class },
 				new String[] { "--spring.main.web-application-type=none",
+						"--spring.main.allow-bean-definition-overriding=true",
 						"--spring.main.sources=org.springframework.boot.OverrideSourcesTests.MainConfiguration" });
 		assertThat(this.context.getBean(Service.class).bean.name).isEqualTo("bar");
 	}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java

@@ -45,6 +45,7 @@ import org.springframework.beans.BeansException;
 import org.springframework.beans.CachedIntrospectionResults;
 import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
+import org.springframework.beans.factory.support.BeanDefinitionOverrideException;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.beans.factory.support.BeanNameGenerator;
 import org.springframework.beans.factory.support.DefaultBeanNameGenerator;
@@ -1118,6 +1119,21 @@ public class SpringApplicationTests {
 		assertThat(occurrences).as("Expected single stacktrace").isEqualTo(1);
 	}
 
+	@Test
+	public void beanDefinitionOverridingIsDisabledByDefault() {
+		this.thrown.expect(BeanDefinitionOverrideException.class);
+		new SpringApplication(ExampleConfig.class, OverrideConfig.class).run();
+	}
+
+	@Test
+	public void beanDefinitionOverridingCanBeEnabled() {
+		assertThat(
+				new SpringApplication(ExampleConfig.class, OverrideConfig.class)
+						.run("--spring.main.allow-bean-definition-overriding=true",
+								"--spring.main.web-application-type=none")
+						.getBean("someBean")).isEqualTo("override");
+	}
+
 	private Condition<ConfigurableEnvironment> matchingPropertySource(
 			final Class<?> propertySourceClass, final String name) {
 		return new Condition<ConfigurableEnvironment>("has property source") {
@@ -1228,6 +1244,21 @@ public class SpringApplicationTests {
 	@Configuration
 	static class ExampleConfig {
 
+		@Bean
+		public String someBean() {
+			return "test";
+		}
+
+	}
+
+	@Configuration
+	static class OverrideConfig {
+
+		@Bean
+		public String someBean() {
+			return "override";
+		}
+
 	}
 
 	@Configuration

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

@@ -0,0 +1,92 @@
+/*
+ * Copyright 2012-2018 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
+ *
+ *      http://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.beans.factory.support.BeanDefinitionOverrideException;
+import org.springframework.boot.diagnostics.FailureAnalysis;
+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 static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link BeanDefinitionOverrideFailureAnalyzer}.
+ *
+ * @author Andy Wilkinson
+ */
+public class BeanDefinitionOverrideFailureAnalyzerTests {
+
+	@Test
+	public void bindExceptionWithFieldErrorsDueToValidationFailure() {
+		FailureAnalysis analysis = performAnalysis(BeanOverrideConfiguration.class);
+		String description = analysis.getDescription();
+		assertThat(description).contains("The bean 'testBean', defined in "
+				+ SecondConfiguration.class.getName() + ", could not be registered.");
+		assertThat(description).contains(FirstConfiguration.class.getName());
+	}
+
+	private FailureAnalysis performAnalysis(Class<?> configuration) {
+		BeanDefinitionOverrideException failure = createFailure(configuration);
+		assertThat(failure).isNotNull();
+		return new BeanDefinitionOverrideFailureAnalyzer().analyze(failure);
+	}
+
+	private BeanDefinitionOverrideException createFailure(Class<?> configuration) {
+		try {
+			AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
+			context.setAllowBeanDefinitionOverriding(false);
+			context.register(configuration);
+			context.refresh();
+			context.close();
+			return null;
+		}
+		catch (BeanDefinitionOverrideException ex) {
+			return ex;
+		}
+	}
+
+	@Configuration
+	@Import({ FirstConfiguration.class, SecondConfiguration.class })
+	static class BeanOverrideConfiguration {
+
+	}
+
+	@Configuration
+	static class FirstConfiguration {
+
+		@Bean
+		public String testBean() {
+			return "test";
+		}
+
+	}
+
+	@Configuration
+	static class SecondConfiguration {
+
+		@Bean
+		public String testBean() {
+			return "test";
+		}
+
+	}
+
+}