From 044d68b33628008b7a43f82ac98154ea15027c60 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 5 Nov 2013 00:52:28 +0100 Subject: [PATCH] @Bean method metadata is always being picked from the most concrete subclass As a side effect, @Bean overrides and overloads work with 'allowBeanDefinitionOverriding'=false as well now. Issue: SPR-10992 --- ...onfigurationClassBeanDefinitionReader.java | 24 ++++-- .../BeanMethodPolymorphismTests.java | 82 +++++++++++-------- 2 files changed, 66 insertions(+), 40 deletions(-) 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 c5b07237cf..5250f5e34c 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 @@ -156,7 +156,7 @@ class ConfigurationClassBeanDefinitionReader { ConfigurationClass configClass = beanMethod.getConfigurationClass(); MethodMetadata metadata = beanMethod.getMetadata(); - RootBeanDefinition beanDef = new ConfigurationClassBeanDefinition(configClass); + ConfigurationClassBeanDefinition beanDef = new ConfigurationClassBeanDefinition(configClass); beanDef.setResource(configClass.getResource()); beanDef.setSource(this.sourceExtractor.extractSource(metadata, configClass.getResource())); if (metadata.isStatic()) { @@ -188,9 +188,18 @@ class ConfigurationClassBeanDefinitionReader { // 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)) { + 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. + // However, if the bean method is an overloaded case on the same configuration class, + // preserve the existing bean definition. + if (existingBeanDef instanceof ConfigurationClassBeanDefinition) { + ConfigurationClassBeanDefinition ccbd = (ConfigurationClassBeanDefinition) existingBeanDef; + if (ccbd.getMetadata().getClassName().equals(beanMethod.getConfigurationClass().getMetadata().getClassName())) { + return; + } + } + else { // no -> then it's an external override, probably XML // overriding is legal, return immediately if (logger.isDebugEnabled()) { @@ -238,7 +247,7 @@ class ConfigurationClassBeanDefinitionReader { beanDef.setDestroyMethodName(destroyMethodName); } - // consider scoping + // Consider scoping ScopedProxyMode proxyMode = ScopedProxyMode.NO; AnnotationAttributes scope = attributesFor(metadata, Scope.class); if (scope != null) { @@ -249,7 +258,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( @@ -259,7 +268,8 @@ 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); diff --git a/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java b/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java index b666f8be92..ebdc073b9b 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java @@ -16,7 +16,6 @@ package org.springframework.context.annotation; -import java.lang.annotation.Inherited; import java.util.List; import org.junit.Rule; @@ -24,8 +23,6 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; -import org.springframework.beans.factory.support.DefaultListableBeanFactory; -import org.springframework.beans.factory.support.RootBeanDefinition; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; @@ -34,25 +31,33 @@ import static org.junit.Assert.*; * Tests regarding overloading and overriding of bean methods. * Related to SPR-6618. * - * Bean-annotated methods should be able to be overridden, just as any regular - * method. This is straightforward. - * - * Bean-annotated methods should be able to be overloaded, though supporting this - * is more subtle. Essentially, it must be unambiguous to the container which bean - * method to call. A simple way to think about this is that no one Configuration - * class may declare two bean methods with the same name. In the case of inheritance, - * the most specific subclass bean method will always be the one that is invoked. - * * @author Chris Beams * @author Phillip Webb + * @author Juergen Hoeller */ -@SuppressWarnings("resource") public class BeanMethodPolymorphismTests { @Rule public ExpectedException thrown = ExpectedException.none(); + @Test + public void beanMethodDetectedOnSuperClass() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Config.class); + ctx.getBean("testBean", TestBean.class); + } + + @Test + public void beanMethodOverriding() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(OverridingConfig.class); + ctx.setAllowBeanDefinitionOverriding(false); + ctx.refresh(); + assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")); + assertEquals("overridden", ctx.getBean("testBean", TestBean.class).toString()); + assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")); + } + @Test public void beanMethodOverloadingWithoutInheritance() { @@ -69,15 +74,25 @@ public class BeanMethodPolymorphismTests { @Test public void beanMethodOverloadingWithInheritance() { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubConfig.class); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(SubConfig.class); + ctx.setAllowBeanDefinitionOverriding(false); + ctx.refresh(); + assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("aString")); assertThat(ctx.getBean(String.class), equalTo("overloaded5")); + assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("aString")); } + // SPR-11025 @Test public void beanMethodOverloadingWithInheritanceAndList() { - // SPR-11025 - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(SubConfigWithList.class); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(SubConfigWithList.class); + ctx.setAllowBeanDefinitionOverriding(false); + ctx.refresh(); + assertFalse(ctx.getDefaultListableBeanFactory().containsSingleton("aString")); assertThat(ctx.getBean(String.class), equalTo("overloaded5")); + assertTrue(ctx.getDefaultListableBeanFactory().containsSingleton("aString")); } /** @@ -91,20 +106,6 @@ public class BeanMethodPolymorphismTests { assertThat(ctx.getBean(String.class), equalTo("shadow")); } - /** - * Tests that polymorphic Configuration classes need not explicitly redeclare the - * {@link Configuration} annotation. This respects the {@link Inherited} nature - * of the Configuration annotation, even though it's being detected via ASM. - */ - @Test - public void beanMethodsDetectedOnSuperClass() { - DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); - beanFactory.registerBeanDefinition("config", new RootBeanDefinition(Config.class)); - ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor(); - pp.postProcessBeanFactory(beanFactory); - beanFactory.getBean("testBean", TestBean.class); - } - @Configuration static class BaseConfig { @@ -113,7 +114,6 @@ public class BeanMethodPolymorphismTests { public TestBean testBean() { return new TestBean(); } - } @@ -122,6 +122,22 @@ public class BeanMethodPolymorphismTests { } + @Configuration + static class OverridingConfig extends BaseConfig { + + @Bean @Lazy + @Override + public TestBean testBean() { + return new TestBean() { + @Override + public String toString() { + return "overridden"; + } + }; + } + } + + @Configuration static class SuperConfig { @@ -140,7 +156,7 @@ public class BeanMethodPolymorphismTests { return 5; } - @Bean + @Bean @Lazy String aString(Integer dependency) { return "overloaded" + dependency; } @@ -155,7 +171,7 @@ public class BeanMethodPolymorphismTests { return 5; } - @Bean + @Bean @Lazy String aString(List dependency) { return "overloaded" + dependency.get(0); } -- GitLab