From 182243d20d8c4793d65ab6ea8d4a50d59e2e6545 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 4 Jul 2018 21:32:51 +0200 Subject: [PATCH] BeanDefinitionOverrideException in case of overriding not allowed Issue: SPR-16982 --- .../factory/BeanDefinitionStoreException.java | 16 ++-- .../BeanDefinitionOverrideException.java | 91 +++++++++++++++++++ .../support/BeanDefinitionRegistry.java | 7 +- .../support/DefaultListableBeanFactory.java | 22 ++--- .../DefaultListableBeanFactoryTests.java | 14 ++- 5 files changed, 122 insertions(+), 28 deletions(-) create mode 100644 spring-beans/src/main/java/org/springframework/beans/factory/support/BeanDefinitionOverrideException.java diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/BeanDefinitionStoreException.java b/spring-beans/src/main/java/org/springframework/beans/factory/BeanDefinitionStoreException.java index 2f8b5d3321..f89b59adb8 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/BeanDefinitionStoreException.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/BeanDefinitionStoreException.java @@ -84,7 +84,7 @@ public class BeanDefinitionStoreException extends FatalBeanException { /** * Create a new BeanDefinitionStoreException. * @param resourceDescription description of the resource that the bean definition came from - * @param beanName the name of the bean requested + * @param beanName the name of the bean * @param msg the detail message (appended to an introductory message that indicates * the resource and the name of the bean) */ @@ -95,21 +95,23 @@ public class BeanDefinitionStoreException extends FatalBeanException { /** * Create a new BeanDefinitionStoreException. * @param resourceDescription description of the resource that the bean definition came from - * @param beanName the name of the bean requested + * @param beanName the name of the bean * @param msg the detail message (appended to an introductory message that indicates * the resource and the name of the bean) * @param cause the root cause (may be {@code null}) */ - public BeanDefinitionStoreException(@Nullable String resourceDescription, String beanName, String msg, @Nullable Throwable cause) { - super("Invalid bean definition with name '" + beanName + "' defined in " + resourceDescription + ": " + msg, cause); + public BeanDefinitionStoreException( + @Nullable String resourceDescription, String beanName, String msg, @Nullable Throwable cause) { + + super("Invalid bean definition with name '" + beanName + "' defined in " + resourceDescription + ": " + msg, + cause); this.resourceDescription = resourceDescription; this.beanName = beanName; } /** - * Return the description of the resource that the bean - * definition came from, if any. + * Return the description of the resource that the bean definition came from, if available. */ @Nullable public String getResourceDescription() { @@ -117,7 +119,7 @@ public class BeanDefinitionStoreException extends FatalBeanException { } /** - * Return the name of the bean requested, if any. + * Return the name of the bean, if available. */ @Nullable public String getBeanName() { diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/BeanDefinitionOverrideException.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/BeanDefinitionOverrideException.java new file mode 100644 index 0000000000..40209882d9 --- /dev/null +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/BeanDefinitionOverrideException.java @@ -0,0 +1,91 @@ +/* + * Copyright 2002-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.beans.factory.support; + +import org.springframework.beans.factory.BeanDefinitionStoreException; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.lang.NonNull; + +/** + * Subclass of {@link BeanDefinitionStoreException} indicating an invalid override + * attempt: typically registering a new definition for the same bean name while + * {@link DefaultListableBeanFactory#isAllowBeanDefinitionOverriding()} is {@code false}. + * + * @author Juergen Hoeller + * @since 5.1 + * @see DefaultListableBeanFactory#setAllowBeanDefinitionOverriding + * @see DefaultListableBeanFactory#registerBeanDefinition + */ +public class BeanDefinitionOverrideException extends BeanDefinitionStoreException { + + private final BeanDefinition beanDefinition; + + private final BeanDefinition existingDefinition; + + + /** + * Create a new BeanDefinitionOverrideException for the given new and existing definition. + * @param beanName the name of the bean + * @param beanDefinition the newly registered bean definition + * @param existingDefinition the existing bean definition for the same name + */ + public BeanDefinitionOverrideException( + String beanName, BeanDefinition beanDefinition, BeanDefinition existingDefinition) { + + super(beanDefinition.getResourceDescription(), beanName, + "Cannot register bean definition [" + beanDefinition + "] for bean '" + beanName + + "': There is already [" + existingDefinition + "] bound."); + this.beanDefinition = beanDefinition; + this.existingDefinition = existingDefinition; + } + + + /** + * Return the description of the resource that the bean definition came from. + */ + @Override + @NonNull + public String getResourceDescription() { + return String.valueOf(super.getResourceDescription()); + } + + /** + * Return the name of the bean. + */ + @Override + @NonNull + public String getBeanName() { + return String.valueOf(super.getBeanName()); + } + + /** + * Return the newly registered bean definition. + * @see #getBeanName() + */ + public BeanDefinition getBeanDefinition() { + return this.beanDefinition; + } + + /** + * Return the existing bean definition for the same name. + * @see #getBeanName() + */ + public BeanDefinition getExistingDefinition() { + return this.existingDefinition; + } + +} diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/BeanDefinitionRegistry.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/BeanDefinitionRegistry.java index c4c670fba0..12e4d0556d 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/BeanDefinitionRegistry.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/BeanDefinitionRegistry.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-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. @@ -53,8 +53,9 @@ public interface BeanDefinitionRegistry extends AliasRegistry { * @param beanName the name of the bean instance to register * @param beanDefinition definition of the bean instance to register * @throws BeanDefinitionStoreException if the BeanDefinition is invalid - * or if there is already a BeanDefinition for the specified bean name - * (and we are not allowed to override it) + * @throws BeanDefinitionOverrideException if there is already a BeanDefinition + * for the specified bean name and we are not allowed to override it + * @see GenericBeanDefinition * @see RootBeanDefinition * @see ChildBeanDefinition */ diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java index 68dd686043..c6a0a560e6 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java @@ -802,34 +802,30 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } } - BeanDefinition oldBeanDefinition; - - oldBeanDefinition = this.beanDefinitionMap.get(beanName); - if (oldBeanDefinition != null) { + BeanDefinition existingDefinition = this.beanDefinitionMap.get(beanName); + if (existingDefinition != null) { if (!isAllowBeanDefinitionOverriding()) { - throw new BeanDefinitionStoreException(beanDefinition.getResourceDescription(), beanName, - "Cannot register bean definition [" + beanDefinition + "] for bean '" + beanName + - "': There is already [" + oldBeanDefinition + "] bound."); + throw new BeanDefinitionOverrideException(beanName, beanDefinition, existingDefinition); } - else if (oldBeanDefinition.getRole() < beanDefinition.getRole()) { + else if (existingDefinition.getRole() < beanDefinition.getRole()) { // e.g. was ROLE_APPLICATION, now overriding with ROLE_SUPPORT or ROLE_INFRASTRUCTURE if (logger.isWarnEnabled()) { logger.warn("Overriding user-defined bean definition for bean '" + beanName + "' with a framework-generated bean definition: replacing [" + - oldBeanDefinition + "] with [" + beanDefinition + "]"); + existingDefinition + "] with [" + beanDefinition + "]"); } } - else if (!beanDefinition.equals(oldBeanDefinition)) { + else if (!beanDefinition.equals(existingDefinition)) { if (logger.isInfoEnabled()) { logger.info("Overriding bean definition for bean '" + beanName + - "' with a different definition: replacing [" + oldBeanDefinition + + "' with a different definition: replacing [" + existingDefinition + "] with [" + beanDefinition + "]"); } } else { if (logger.isDebugEnabled()) { logger.debug("Overriding bean definition for bean '" + beanName + - "' with an equivalent definition: replacing [" + oldBeanDefinition + + "' with an equivalent definition: replacing [" + existingDefinition + "] with [" + beanDefinition + "]"); } } @@ -860,7 +856,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto this.frozenBeanDefinitionNames = null; } - if (oldBeanDefinition != null || containsSingleton(beanName)) { + if (existingDefinition != null || containsSingleton(beanName)) { resetBeanDefinition(beanName); } } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java index 4042868443..7d987e735c 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java @@ -68,6 +68,7 @@ import org.springframework.beans.factory.config.TypedStringValue; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.AbstractBeanFactory; import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.support.BeanDefinitionOverrideException; import org.springframework.beans.factory.support.ChildBeanDefinition; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.ManagedList; @@ -841,14 +842,17 @@ public class DefaultListableBeanFactoryTests { public void testBeanDefinitionOverridingNotAllowed() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); lbf.setAllowBeanDefinitionOverriding(false); - lbf.registerBeanDefinition("test", new RootBeanDefinition(TestBean.class)); + BeanDefinition oldDef = new RootBeanDefinition(TestBean.class); + BeanDefinition newDef = new RootBeanDefinition(NestedTestBean.class); + lbf.registerBeanDefinition("test", oldDef); try { - lbf.registerBeanDefinition("test", new RootBeanDefinition(NestedTestBean.class)); - fail("Should have thrown BeanDefinitionStoreException"); + lbf.registerBeanDefinition("test", newDef); + fail("Should have thrown BeanDefinitionOverrideException"); } - catch (BeanDefinitionStoreException ex) { + catch (BeanDefinitionOverrideException ex) { assertEquals("test", ex.getBeanName()); - // expected + assertSame(newDef, ex.getBeanDefinition()); + assertSame(oldDef, ex.getExistingDefinition()); } } -- GitLab