diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/config/BeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/config/BeanDefinition.java index b0b4142ec6..80d543a4c0 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/config/BeanDefinition.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/config/BeanDefinition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2013 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. @@ -229,11 +229,11 @@ public interface BeanDefinition extends AttributeAccessor, BeanMetadataElement { /** * Get the role hint for this {@code BeanDefinition}. The role hint - * provides tools with an indication of the importance of a particular - * {@code BeanDefinition}. + * provides the frameworks as well as tools with an indication of + * the role and importance of a particular {@code BeanDefinition}. * @see #ROLE_APPLICATION - * @see #ROLE_INFRASTRUCTURE * @see #ROLE_SUPPORT + * @see #ROLE_INFRASTRUCTURE */ int getRole(); 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 3191bc4490..8761f8227c 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 @@ -691,13 +691,21 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } synchronized (this.beanDefinitionMap) { - Object oldBeanDefinition = this.beanDefinitionMap.get(beanName); + BeanDefinition oldBeanDefinition = this.beanDefinitionMap.get(beanName); if (oldBeanDefinition != null) { if (!this.allowBeanDefinitionOverriding) { throw new BeanDefinitionStoreException(beanDefinition.getResourceDescription(), beanName, "Cannot register bean definition [" + beanDefinition + "] for bean '" + beanName + "': There is already [" + oldBeanDefinition + "] bound."); } + else if (oldBeanDefinition.getRole() < beanDefinition.getRole()) { + // e.g. was ROLE_APPLICATION, now overriding with ROLE_SUPPORT or ROLE_INFRASTRUCTURE + if (this.logger.isWarnEnabled()) { + this.logger.warn("Overriding user-defined bean definition for bean '" + beanName + + " with a framework-generated bean definition ': replacing [" + + oldBeanDefinition + "] with [" + beanDefinition + "]"); + } + } else { if (this.logger.isInfoEnabled()) { this.logger.info("Overriding bean definition for bean '" + beanName + diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java index ee93b97905..c47d43f97c 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java @@ -27,7 +27,6 @@ import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.annotation.AnnotatedBeanDefinition; import org.springframework.beans.factory.annotation.AnnotatedGenericBeanDefinition; import org.springframework.beans.factory.annotation.Autowire; @@ -119,11 +118,12 @@ class ConfigurationClassBeanDefinitionReader { } /** - * Read a particular {@link ConfigurationClass}, registering bean definitions for the - * class itself, all its {@link Bean} methods + * Read a particular {@link ConfigurationClass}, registering bean definitions + * for the class itself and all of its {@link Bean} methods. */ private void loadBeanDefinitionsForConfigurationClass(ConfigurationClass configClass, TrackedConditionEvaluator trackedConditionEvaluator) { + if (trackedConditionEvaluator.shouldSkip(configClass)) { removeBeanDefinition(configClass); return; @@ -140,12 +140,8 @@ class ConfigurationClassBeanDefinitionReader { } private void removeBeanDefinition(ConfigurationClass configClass) { - if (StringUtils.hasLength(configClass.getBeanName())) { - try { - this.registry.removeBeanDefinition(configClass.getBeanName()); - } - catch (NoSuchBeanDefinitionException ex) { - } + if (StringUtils.hasLength(configClass.getBeanName()) && this.registry.containsBeanDefinition(configClass.getBeanName())) { + this.registry.removeBeanDefinition(configClass.getBeanName()); } } @@ -196,7 +192,7 @@ class ConfigurationClassBeanDefinitionReader { beanDef.setAutowireMode(RootBeanDefinition.AUTOWIRE_CONSTRUCTOR); beanDef.setAttribute(RequiredAnnotationBeanPostProcessor.SKIP_REQUIRED_CHECK_ATTRIBUTE, Boolean.TRUE); - // consider name and any aliases + // Consider name and any aliases AnnotationAttributes bean = AnnotationConfigUtils.attributesFor(metadata, Bean.class); List names = new ArrayList(Arrays.asList(bean.getStringArray("name"))); String beanName = (names.size() > 0 ? names.remove(0) : beanMethod.getMetadata().getMethodName()); @@ -204,19 +200,9 @@ class ConfigurationClassBeanDefinitionReader { this.registry.registerAlias(beanName, alias); } - // has this already been overridden (e.g. via XML)? - if (this.registry.containsBeanDefinition(beanName)) { - BeanDefinition existingBeanDef = registry.getBeanDefinition(beanName); - // is the existing bean definition one that was created from a configuration class? - if (!(existingBeanDef instanceof ConfigurationClassBeanDefinition)) { - // no -> then it's an external override, probably XML - // overriding is legal, return immediately - if (logger.isDebugEnabled()) { - logger.debug(String.format("Skipping loading bean definition for %s: a definition for bean " + - "'%s' already exists. This is likely due to an override in XML.", beanMethod, beanName)); - } - return; - } + // Has this effectively been overridden before (e.g. via XML)? + if (isOverriddenByExistingDefinition(beanMethod, beanName)) { + return; } AnnotationConfigUtils.processCommonDefinitionAnnotations(beanDef, metadata); @@ -236,7 +222,7 @@ class ConfigurationClassBeanDefinitionReader { beanDef.setDestroyMethodName(destroyMethodName); } - // consider scoping + // Consider scoping ScopedProxyMode proxyMode = ScopedProxyMode.NO; AnnotationAttributes scope = AnnotationConfigUtils.attributesFor(metadata, Scope.class); if (scope != null) { @@ -247,7 +233,7 @@ class ConfigurationClassBeanDefinitionReader { } } - // replace the original bean definition with the target one, if necessary + // Replace the original bean definition with the target one, if necessary BeanDefinition beanDefToRegister = beanDef; if (proxyMode != ScopedProxyMode.NO) { BeanDefinitionHolder proxyDef = ScopedProxyCreator.createScopedProxy( @@ -257,12 +243,40 @@ class ConfigurationClassBeanDefinitionReader { } if (logger.isDebugEnabled()) { - logger.debug(String.format("Registering bean definition for @Bean method %s.%s()", configClass.getMetadata().getClassName(), beanName)); + logger.debug(String.format("Registering bean definition for @Bean method %s.%s()", + configClass.getMetadata().getClassName(), beanName)); } - registry.registerBeanDefinition(beanName, beanDefToRegister); + this.registry.registerBeanDefinition(beanName, beanDefToRegister); } + protected boolean isOverriddenByExistingDefinition(BeanMethod beanMethod, String beanName) { + if (!this.registry.containsBeanDefinition(beanName)) { + return false; + } + BeanDefinition existingBeanDef = this.registry.getBeanDefinition(beanName); + + // Is the existing bean definition one that was created from a configuration class? + // -> allow the current bean method to override, since both are at second-pass level + if (existingBeanDef instanceof ConfigurationClassBeanDefinition) { + return false; + } + + // Has the existing bean definition bean marked as a framework-generated bean? + // -> allow the current bean method to override it, since it is application-level + if (existingBeanDef.getRole() > BeanDefinition.ROLE_APPLICATION) { + return false; + } + + // At this point, it's a top-level override (probably XML), just having been parsed + // before configuration class processing kicks in... + if (logger.isInfoEnabled()) { + logger.info(String.format("Skipping bean definition for %s: a definition for bean '%s' " + + "already exists. This top-level bean definition is considered as an override.", + beanMethod, beanName)); + } + return true; + } private void loadBeanDefinitionsFromImportedResources( Map> importedResources) { @@ -294,9 +308,9 @@ class ConfigurationClassBeanDefinitionReader { } } - private void loadBeanDefinitionsFromRegistrars( - AnnotationMetadata importingClassMetadata, + private void loadBeanDefinitionsFromRegistrars(AnnotationMetadata importingClassMetadata, Set importBeanDefinitionRegistrars) { + for (ImportBeanDefinitionRegistrar registrar : importBeanDefinitionRegistrars) { registrar.registerBeanDefinitions(importingClassMetadata, this.registry); } diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java index 815c487294..50b0accd8d 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java @@ -16,13 +16,15 @@ package org.springframework.context.annotation; -import static org.junit.Assert.*; import org.junit.Test; -import org.springframework.tests.sample.beans.TestBean; import org.springframework.beans.factory.support.ChildBeanDefinition; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.core.io.DescriptiveResource; +import org.springframework.tests.sample.beans.TestBean; + +import static org.junit.Assert.*; /** * @author Chris Beams @@ -82,6 +84,33 @@ public class ConfigurationClassPostProcessorTests { assertSame(foo, bar.foo); } + @Test + public void testPostProcessorOverridesNonApplicationBeanDefinitions() { + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + RootBeanDefinition rbd = new RootBeanDefinition(TestBean.class); + rbd.setRole(RootBeanDefinition.ROLE_SUPPORT); + beanFactory.registerBeanDefinition("bar", rbd); + beanFactory.registerBeanDefinition("config", new RootBeanDefinition(SingletonBeanConfig.class)); + ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor(); + pp.postProcessBeanFactory(beanFactory); + Foo foo = beanFactory.getBean("foo", Foo.class); + Bar bar = beanFactory.getBean("bar", Bar.class); + assertSame(foo, bar.foo); + } + + @Test + public void testPostProcessorDoesNotOverrideRegularBeanDefinitions() { + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + RootBeanDefinition rbd = new RootBeanDefinition(TestBean.class); + rbd.setResource(new DescriptiveResource("XML or something")); + beanFactory.registerBeanDefinition("bar", rbd); + beanFactory.registerBeanDefinition("config", new RootBeanDefinition(SingletonBeanConfig.class)); + ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor(); + pp.postProcessBeanFactory(beanFactory); + Foo foo = beanFactory.getBean("foo", Foo.class); + TestBean bar = beanFactory.getBean("bar", TestBean.class); + } + @Test public void testProcessingAllowedOnlyOncePerProcessorRegistryPair() { DefaultListableBeanFactory bf1 = new DefaultListableBeanFactory(); @@ -91,13 +120,15 @@ public class ConfigurationClassPostProcessorTests { try { pp.postProcessBeanFactory(bf1); // second invocation for bf1 -- should throw fail("expected exception"); - } catch (IllegalStateException ex) { + } + catch (IllegalStateException ex) { } pp.postProcessBeanFactory(bf2); // first invocation for bf2 -- should succeed try { pp.postProcessBeanFactory(bf2); // second invocation for bf2 -- should throw fail("expected exception"); - } catch (IllegalStateException ex) { + } + catch (IllegalStateException ex) { } }