From 063e8e4dc6d42fd574e54ea42f116a77764f9607 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 9 Jul 2018 17:58:49 +0100 Subject: [PATCH] Revert "Prohibit bean overriding by default and analyze override failures" This reverts commit 710cdbab9216256df8d40725a2612306663aebaa. --- .../appendix-application-properties.adoc | 1 - .../boot/SpringApplication.java | 26 +----- ...BeanDefinitionOverrideFailureAnalyzer.java | 55 ----------- ...itional-spring-configuration-metadata.json | 7 -- .../main/resources/META-INF/spring.factories | 1 - .../boot/OverrideSourcesTests.java | 1 - .../boot/SpringApplicationTests.java | 31 ------- ...efinitionOverrideFailureAnalyzerTests.java | 92 ------------------- 8 files changed, 4 insertions(+), 210 deletions(-) delete mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanDefinitionOverrideFailureAnalyzer.java delete mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanDefinitionOverrideFailureAnalyzerTests.java diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index a1b8e209bf3..4eab17e2288 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -113,7 +113,6 @@ 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. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java index 90dbc2ecac8..404b2207a55 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java @@ -35,11 +35,9 @@ 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; @@ -237,8 +235,6 @@ public class SpringApplication { private Set 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} @@ -385,15 +381,12 @@ public class SpringApplication { } // Add boot specific singleton beans - ConfigurableListableBeanFactory beanFactory = context.getBeanFactory(); - beanFactory.registerSingleton("springApplicationArguments", applicationArguments); + context.getBeanFactory().registerSingleton("springApplicationArguments", + applicationArguments); if (printedBanner != null) { - beanFactory.registerSingleton("springBootBanner", printedBanner); - } - if (beanFactory instanceof DefaultListableBeanFactory) { - ((DefaultListableBeanFactory) beanFactory) - .setAllowBeanDefinitionOverriding(this.allowBeanDefinitionOverriding); + context.getBeanFactory().registerSingleton("springBootBanner", printedBanner); } + // Load the sources Set sources = getAllSources(); Assert.notEmpty(sources, "Sources must not be empty"); @@ -962,17 +955,6 @@ 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. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanDefinitionOverrideFailureAnalyzer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanDefinitionOverrideFailureAnalyzer.java deleted file mode 100644 index 2bd543c932b..00000000000 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanDefinitionOverrideFailureAnalyzer.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * 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 { - - 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(); - } - -} diff --git a/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json index c8ef53d679f..f25fb511deb 100644 --- a/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -172,13 +172,6 @@ "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", 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 index 2250eba38a9..ff98c96f037 100644 --- 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 @@ -39,7 +39,6 @@ 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,\ diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/OverrideSourcesTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/OverrideSourcesTests.java index 52913484a6e..0e7dd9c30cf 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/OverrideSourcesTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/OverrideSourcesTests.java @@ -56,7 +56,6 @@ 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"); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java index 0d575bc7653..e31a5b6a670 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java @@ -45,7 +45,6 @@ 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; @@ -1119,21 +1118,6 @@ 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 matchingPropertySource( final Class propertySourceClass, final String name) { return new Condition("has property source") { @@ -1244,21 +1228,6 @@ public class SpringApplicationTests { @Configuration static class ExampleConfig { - @Bean - public String someBean() { - return "test"; - } - - } - - @Configuration - static class OverrideConfig { - - @Bean - public String someBean() { - return "override"; - } - } @Configuration diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanDefinitionOverrideFailureAnalyzerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanDefinitionOverrideFailureAnalyzerTests.java deleted file mode 100644 index 861e52159df..00000000000 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanDefinitionOverrideFailureAnalyzerTests.java +++ /dev/null @@ -1,92 +0,0 @@ -/* - * 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"; - } - - } - -}