Prohibit bean overriding by default and analyze override failures
Closes gh-13609
This commit is contained in:
		
							parent
							
								
									918191664a
								
							
						
					
					
						commit
						710cdbab92
					
				|  | @ -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. | ||||
|  |  | |||
|  | @ -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", | ||||
| 				applicationArguments); | ||||
| 		ConfigurableListableBeanFactory beanFactory = context.getBeanFactory(); | ||||
| 		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. | ||||
|  |  | |||
|  | @ -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(); | ||||
| 	} | ||||
| 
 | ||||
| } | ||||
|  | @ -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", | ||||
|  |  | |||
|  | @ -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,\ | ||||
|  |  | |||
|  | @ -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"); | ||||
| 	} | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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"; | ||||
| 		} | ||||
| 
 | ||||
| 	} | ||||
| 
 | ||||
| } | ||||
		Loading…
	
		Reference in New Issue