From 8be158bd0a0e0a1a38ac8a0c114f06a6f2bc8f03 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Mon, 18 Jan 2010 08:54:45 +0000 Subject: [PATCH] Resolved SPR-6602, relating to FactoryBean behavior in @Configuration classes. See issue and code comments for full details. git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@2824 50f2f4bb-b051-0410-bef5-90022cba6387 --- org.springframework.context/.springBeans | 3 +- .../ConfigurationClassEnhancer.java | 73 ++++++++++++++++- .../annotation/Spr6602Tests-context.xml | 12 +++ .../context/annotation/Spr6602Tests.java | 80 +++++++++++++++++++ 4 files changed, 165 insertions(+), 3 deletions(-) create mode 100644 org.springframework.context/src/test/java/org/springframework/context/annotation/Spr6602Tests-context.xml create mode 100644 org.springframework.context/src/test/java/org/springframework/context/annotation/Spr6602Tests.java diff --git a/org.springframework.context/.springBeans b/org.springframework.context/.springBeans index 23e83d35b2b..264f050b81d 100644 --- a/org.springframework.context/.springBeans +++ b/org.springframework.context/.springBeans @@ -1,7 +1,7 @@ 1 - + @@ -10,6 +10,7 @@ src/test/java/org/springframework/context/annotation/configuration/ImportXmlConfig-context.xml src/test/java/org/springframework/context/annotation/configuration/SecondLevelSubConfig-context.xml src/test/java/org/springframework/context/annotation/configuration/ImportXmlWithAopNamespace-context.xml + src/test/java/org/springframework/context/annotation/Spr6602Tests-context.xml diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java index 26e510a4c75..5e2c985f654 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java @@ -26,9 +26,10 @@ import net.sf.cglib.proxy.Enhancer; import net.sf.cglib.proxy.MethodInterceptor; import net.sf.cglib.proxy.MethodProxy; import net.sf.cglib.proxy.NoOp; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - +import org.springframework.aop.scope.ScopedProxyFactoryBean; import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.util.Assert; @@ -120,6 +121,24 @@ class ConfigurationClassEnhancer { Enhancer.registerStaticCallbacks(subclass, this.callbackInstances.toArray(new Callback[this.callbackInstances.size()])); return subclass; } + + + private static class GetObjectMethodInterceptor implements MethodInterceptor { + + private final ConfigurableBeanFactory beanFactory; + private final String beanName; + + public GetObjectMethodInterceptor(ConfigurableBeanFactory beanFactory, String beanName) { + this.beanFactory = beanFactory; + this.beanName = beanName; + } + + public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable { + System.out.println("intercepting request for getObject()"); + return beanFactory.getBean(beanName); + } + + } /** @@ -161,12 +180,29 @@ class ConfigurationClassEnhancer { // to handle the case of an inter-bean method reference, we must explicitly check the // container for already cached instances + + // first, check to see if the requested bean is a FactoryBean. If so, create a subclass + // proxy that intercepts calls to getObject() and returns any cached bean instance. + // this ensures that the semantics of calling a FactoryBean from within @Bean methods + // is the same as that of referring to a FactoryBean within XML. See SPR-6602. + if (factoryContainsBean('&'+beanName) && factoryContainsBean(beanName)) { + Object factoryBean = this.beanFactory.getBean('&'+beanName); + if (factoryBean instanceof ScopedProxyFactoryBean) { + // pass through - scoped proxy factory beans are a special case and should not + // be further proxied + } else { + // it is a candidate FactoryBean - go ahead with enhancement + return enhanceFactoryBean(factoryBean.getClass(), beanName); + } + } + + // the bean is not a FactoryBean - check to see if it has been cached if (factoryContainsBean(beanName)) { // we have an already existing cached instance of this bean -> retrieve it return this.beanFactory.getBean(beanName); } - // actually create and return the bean + // no cached instance of the bean exists - actually create and return the bean return proxy.invokeSuper(obj, args); } @@ -187,5 +223,38 @@ class ConfigurationClassEnhancer { return (this.beanFactory.containsBean(beanName) && !this.beanFactory.isCurrentlyInCreation(beanName)); } + /** + * Create a subclass proxy that intercepts calls to getObject(), delegating to the current BeanFactory + * instead of creating a new instance. These proxies are created only when calling a FactoryBean from + * within a Bean method, allowing for proper scoping semantics even when working against the FactoryBean + * instance directly. If a FactoryBean instance is fetched through the container via &-dereferencing, + * it will not be proxied. This too is aligned with the way XML configuration works. + */ + private Object enhanceFactoryBean(Class fbClass, String beanName) throws InstantiationException, IllegalAccessException { + Enhancer enhancer = new Enhancer(); + enhancer.setUseCache(false); + enhancer.setSuperclass(fbClass); + enhancer.setUseFactory(false); + enhancer.setCallbackFilter(new CallbackFilter() { + public int accept(Method method) { + return method.getName().equals("getObject") ? 0 : 1; + } + }); + List callbackInstances = new ArrayList(); + callbackInstances.add(new GetObjectMethodInterceptor(this.beanFactory, beanName)); + callbackInstances.add(NoOp.INSTANCE); + + List> callbackTypes = new ArrayList>(); + + for (Callback callback : callbackInstances) { + callbackTypes.add(callback.getClass()); + } + + enhancer.setCallbackTypes(callbackTypes.toArray(new Class[callbackTypes.size()])); + Class fbSubclass = enhancer.createClass(); + Enhancer.registerCallbacks(fbSubclass, callbackInstances.toArray(new Callback[callbackInstances.size()])); + return fbSubclass.newInstance(); + } + } } diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/Spr6602Tests-context.xml b/org.springframework.context/src/test/java/org/springframework/context/annotation/Spr6602Tests-context.xml new file mode 100644 index 00000000000..451fe7f36a1 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/Spr6602Tests-context.xml @@ -0,0 +1,12 @@ + + + + + + + + + + diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/Spr6602Tests.java b/org.springframework.context/src/test/java/org/springframework/context/annotation/Spr6602Tests.java new file mode 100644 index 00000000000..a817b5e4348 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/Spr6602Tests.java @@ -0,0 +1,80 @@ +package org.springframework.context.annotation; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; +import org.springframework.aop.support.AopUtils; +import org.springframework.beans.factory.FactoryBean; +import org.springframework.context.ApplicationContext; +import org.springframework.context.support.ClassPathXmlApplicationContext; + +/** + * Tests to verify that FactoryBean semantics are the same in Configuration + * classes as in XML. + * + * @author Chris Beams + */ +public class Spr6602Tests { + @Test + public void testXmlBehavior() throws Exception { + doAssertions(new ClassPathXmlApplicationContext("Spr6602Tests-context.xml", Spr6602Tests.class)); + } + + @Test + public void testConfigurationClassBehavior() throws Exception { + doAssertions(new AnnotationConfigApplicationContext(FooConfig.class)); + } + + private void doAssertions(ApplicationContext ctx) throws Exception { + Foo foo = ctx.getBean(Foo.class); + + Bar bar1 = ctx.getBean(Bar.class); + Bar bar2 = ctx.getBean(Bar.class); + assertThat(bar1, is(bar2)); + assertThat(bar1, is(foo.bar)); + + BarFactory barFactory1 = ctx.getBean(BarFactory.class); + BarFactory barFactory2 = ctx.getBean(BarFactory.class); + assertThat(barFactory1, is(barFactory2)); + + Bar bar3 = barFactory1.getObject(); + Bar bar4 = barFactory1.getObject(); + assertThat(bar3, is(not(bar4))); + } + +} + + +@Configuration +class FooConfig { + public @Bean Foo foo() throws Exception { + return new Foo(barFactory().getObject()); + } + + public @Bean BarFactory barFactory() { + return new BarFactory(); + } +} + +class Foo { final Bar bar; public Foo(Bar bar) { this.bar = bar; } } +class Bar { } + +class BarFactory implements FactoryBean { + + public Bar getObject() throws Exception { + return new Bar(); + } + + public Class getObjectType() { + return Bar.class; + } + + public boolean isSingleton() { + return true; + } + +}