From d471266d44199112b6e030615a395209e5265ec0 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Tue, 15 Mar 2011 07:09:49 +0000 Subject: [PATCH] @Feature methods accept @Value-annotated params Previously errors were being raised when trying to inject @Value annotated paramaters such as: @Feature public FeatureSpec feature(@Value("#{environment['foo']}") String foo) { return new FeatureSpec(foo); } This is not so much because dependency resolution of @Value-annotated types was failing, but rather because the 'early bean reference' proxying mechanism was throwing an exception if any final type was detected as a parameter. This is of course because final types are non-subclassable by CGLIB. On review, however, it's obvious that certain final types must be allowed for injection. @Value injection is an obvious one, but the rarer case of a Spring bean of type String or int is another. The explicit guard against final types as parameters to @Feature methods has been removed. Final types are still checked for, however, and if found, no proxing is attempted. The dependency is immediately resolved against the current BeanFactory and injected into the @Feature method. This means that @Value injection, @Qualifier injection, etc all work as expected, but does mean that premature bean instantiation may occur if a user unwittingly injects non-String, non-primitive final bean types as @Feature method parameters. Issue: SPR-7974 --- .../ConfigurationClassPostProcessor.java | 4 +- .../EarlyBeanReferenceProxyCreator.java | 16 +-- .../EarlyBeanReferenceProxyCreatorTests.java | 44 +++--- .../FeatureMethodValueInjectionTests.java | 135 ++++++++++++++++++ 4 files changed, 164 insertions(+), 35 deletions(-) create mode 100644 org.springframework.context/src/test/java/org/springframework/context/annotation/FeatureMethodValueInjectionTests.java diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java index ec1fec1e65..1c821f5ff6 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java @@ -47,9 +47,9 @@ import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; import org.springframework.context.EnvironmentAware; import org.springframework.context.ResourceLoaderAware; -import org.springframework.context.config.SpecificationContext; import org.springframework.context.config.FeatureSpecification; import org.springframework.context.config.SourceAwareSpecification; +import org.springframework.context.config.SpecificationContext; import org.springframework.core.MethodParameter; import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationUtils; @@ -331,7 +331,7 @@ public class ConfigurationClassPostProcessor implements BeanDefinitionRegistryPo for (int i = 0; i < parameterTypes.length; i++) { MethodParameter mp = new MethodParameter(featureMethod, i); DependencyDescriptor dd = new DependencyDescriptor(mp, true, false); - Object proxiedBean = proxyCreator.createProxy(dd); + Object proxiedBean = proxyCreator.createProxyIfPossible(dd); beanArgs.add(proxiedBean); } diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/EarlyBeanReferenceProxyCreator.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/EarlyBeanReferenceProxyCreator.java index 121be542bd..e7480a2de3 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/EarlyBeanReferenceProxyCreator.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/EarlyBeanReferenceProxyCreator.java @@ -47,10 +47,6 @@ import org.springframework.util.ReflectionUtils; */ class EarlyBeanReferenceProxyCreator { - static final String FINAL_CLASS_ERROR_MESSAGE = - "Cannot create subclass proxy for bean type %s because it is a final class. " + - "Make the class non-final or inject the bean by interface rather than by concrete class."; - static final String MISSING_NO_ARG_CONSTRUCTOR_ERROR_MESSAGE = "Cannot create subclass proxy for bean type %s because it does not have a no-arg constructor. " + "Add a no-arg constructor or attempt to inject the bean by interface rather than by concrete class."; @@ -73,9 +69,14 @@ class EarlyBeanReferenceProxyCreator { /** * Create a proxy that will ultimately dereference its target object using - * the given dependency descriptor. + * the given dependency descriptor. No proxy is created if the dependency type + * is final, rather the dependency is resolved immediately. This is important + * especially with regard to supporting @Value injection. */ - public Object createProxy(DependencyDescriptor descriptor) { + public Object createProxyIfPossible(DependencyDescriptor descriptor) { + if (Modifier.isFinal(descriptor.getDependencyType().getModifiers())) { + return beanFactory.resolveDependency(descriptor, ""); + } return doCreateProxy(new ResolveDependencyTargetBeanDereferencingInterceptor(descriptor)); } @@ -123,9 +124,6 @@ class EarlyBeanReferenceProxyCreator { */ private static void assertClassIsProxyCapable(Class clazz) { Assert.isTrue(!clazz.isInterface(), "class parameter must be a concrete type"); - if ((clazz.getModifiers() & Modifier.FINAL) != 0) { - throw new ProxyCreationException(String.format(FINAL_CLASS_ERROR_MESSAGE, clazz.getName())); - } try { // attempt to retrieve the no-arg constructor for the class Constructor noArgCtor = clazz.getDeclaredConstructor(); diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/EarlyBeanReferenceProxyCreatorTests.java b/org.springframework.context/src/test/java/org/springframework/context/annotation/EarlyBeanReferenceProxyCreatorTests.java index ae481bdae0..d5813821e7 100644 --- a/org.springframework.context/src/test/java/org/springframework/context/annotation/EarlyBeanReferenceProxyCreatorTests.java +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/EarlyBeanReferenceProxyCreatorTests.java @@ -25,7 +25,6 @@ import static org.hamcrest.CoreMatchers.sameInstance; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; -import static org.springframework.context.annotation.EarlyBeanReferenceProxyCreator.FINAL_CLASS_ERROR_MESSAGE; import static org.springframework.context.annotation.EarlyBeanReferenceProxyCreator.MISSING_NO_ARG_CONSTRUCTOR_ERROR_MESSAGE; import static org.springframework.context.annotation.EarlyBeanReferenceProxyCreator.PRIVATE_NO_ARG_CONSTRUCTOR_ERROR_MESSAGE; @@ -64,7 +63,7 @@ public class EarlyBeanReferenceProxyCreatorTests { public void proxyToStringAvoidsEagerInstantiation() throws Exception { EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - TestBean proxy = (TestBean) pc.createProxy(descriptorFor(TestBean.class)); + TestBean proxy = (TestBean) pc.createProxyIfPossible(descriptorFor(TestBean.class)); assertThat(proxy.toString(), equalTo("EarlyBeanReferenceProxy for bean of type TestBean")); } @@ -72,7 +71,7 @@ public class EarlyBeanReferenceProxyCreatorTests { @Test(expected=NoSuchBeanDefinitionException.class) public void proxyThrowsNoSuchBeanDefinitionExceptionWhenDelegatingMethodCallToNonExistentBean() throws Exception { EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - TestBean proxy = (TestBean) pc.createProxy(descriptorFor(TestBean.class)); + TestBean proxy = (TestBean) pc.createProxyIfPossible(descriptorFor(TestBean.class)); proxy.getName(); } @@ -80,7 +79,7 @@ public class EarlyBeanReferenceProxyCreatorTests { @Test public void proxyHashCodeAvoidsEagerInstantiation() throws Exception { EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - TestBean proxy = (TestBean) pc.createProxy(descriptorFor(TestBean.class)); + TestBean proxy = (TestBean) pc.createProxyIfPossible(descriptorFor(TestBean.class)); assertThat(proxy.hashCode(), equalTo(System.identityHashCode(proxy))); } @@ -89,12 +88,12 @@ public class EarlyBeanReferenceProxyCreatorTests { public void proxyEqualsAvoidsEagerInstantiation() throws Exception { EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - TestBean proxy = (TestBean) pc.createProxy(descriptorFor(TestBean.class)); + TestBean proxy = (TestBean) pc.createProxyIfPossible(descriptorFor(TestBean.class)); assertThat(proxy.equals(new Object()), is(false)); assertThat(proxy.equals(proxy), is(true)); - TestBean proxy2 = (TestBean) pc.createProxy(descriptorFor(TestBean.class)); + TestBean proxy2 = (TestBean) pc.createProxyIfPossible(descriptorFor(TestBean.class)); assertThat(proxy, not(sameInstance(proxy2))); assertThat(proxy.equals(proxy2), is(false)); @@ -106,7 +105,7 @@ public class EarlyBeanReferenceProxyCreatorTests { public void proxyFinalizeAvoidsEagerInstantiation() throws Exception { EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - BeanWithFinalizer proxy = (BeanWithFinalizer) pc.createProxy(descriptorFor(BeanWithFinalizer.class)); + BeanWithFinalizer proxy = (BeanWithFinalizer) pc.createProxyIfPossible(descriptorFor(BeanWithFinalizer.class)); assertThat(BeanWithFinalizer.finalizerWasCalled, is(false)); BeanWithFinalizer.class.getDeclaredMethod("finalize").invoke(proxy); @@ -119,7 +118,7 @@ public class EarlyBeanReferenceProxyCreatorTests { BeanDefinitionBuilder.rootBeanDefinition(TestBean.class) .addPropertyValue("name", "testBeanName").getBeanDefinition()); EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - TestBean proxy = (TestBean) pc.createProxy(descriptorFor(TestBean.class)); + TestBean proxy = (TestBean) pc.createProxyIfPossible(descriptorFor(TestBean.class)); assertThat(bf.containsBeanDefinition("testBean"), is(true)); assertThat(bf.containsSingleton("testBean"), is(false)); @@ -131,7 +130,7 @@ public class EarlyBeanReferenceProxyCreatorTests { public void beanAnnotatedMethodsReturnEarlyProxyAsWell() throws Exception { bf.registerBeanDefinition("componentWithInterfaceBeanMethod", new RootBeanDefinition(ComponentWithInterfaceBeanMethod.class)); EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - ComponentWithInterfaceBeanMethod proxy = (ComponentWithInterfaceBeanMethod) pc.createProxy(descriptorFor(ComponentWithInterfaceBeanMethod.class)); + ComponentWithInterfaceBeanMethod proxy = (ComponentWithInterfaceBeanMethod) pc.createProxyIfPossible(descriptorFor(ComponentWithInterfaceBeanMethod.class)); ITestBean bean = proxy.aBeanMethod(); assertThat(bean, instanceOf(EarlyBeanReferenceProxy.class)); @@ -153,7 +152,7 @@ public class EarlyBeanReferenceProxyCreatorTests { beanMethodBeanDef.setFactoryMethodName("aBeanMethod"); bf.registerBeanDefinition("aBeanMethod", beanMethodBeanDef); EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - ComponentWithConcreteBeanMethod proxy = (ComponentWithConcreteBeanMethod) pc.createProxy(descriptorFor(ComponentWithConcreteBeanMethod.class)); + ComponentWithConcreteBeanMethod proxy = (ComponentWithConcreteBeanMethod) pc.createProxyIfPossible(descriptorFor(ComponentWithConcreteBeanMethod.class)); TestBean bean = proxy.aBeanMethod(); assertThat(bean.getName(), equalTo("concrete")); @@ -162,7 +161,7 @@ public class EarlyBeanReferenceProxyCreatorTests { @Test public void interfaceBeansAreProxied() throws Exception { EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - ITestBean proxy = (ITestBean) pc.createProxy(descriptorFor(ITestBean.class)); + ITestBean proxy = (ITestBean) pc.createProxyIfPossible(descriptorFor(ITestBean.class)); assertThat(proxy, instanceOf(EarlyBeanReferenceProxy.class)); assertThat(AopUtils.isCglibProxyClass(proxy.getClass()), is(true)); @@ -174,7 +173,7 @@ public class EarlyBeanReferenceProxyCreatorTests { @Test public void concreteBeansAreProxied() throws Exception { EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - TestBean proxy = (TestBean) pc.createProxy(descriptorFor(TestBean.class)); + TestBean proxy = (TestBean) pc.createProxyIfPossible(descriptorFor(TestBean.class)); assertThat(proxy, instanceOf(EarlyBeanReferenceProxy.class)); assertThat(AopUtils.isCglibProxyClass(proxy.getClass()), is(true)); @@ -187,7 +186,7 @@ public class EarlyBeanReferenceProxyCreatorTests { public void beanAnnotatedMethodsWithInterfaceReturnTypeAreProxied() throws Exception { bf.registerBeanDefinition("componentWithInterfaceBeanMethod", new RootBeanDefinition(ComponentWithInterfaceBeanMethod.class)); EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - ComponentWithInterfaceBeanMethod proxy = (ComponentWithInterfaceBeanMethod) pc.createProxy(descriptorFor(ComponentWithInterfaceBeanMethod.class)); + ComponentWithInterfaceBeanMethod proxy = (ComponentWithInterfaceBeanMethod) pc.createProxyIfPossible(descriptorFor(ComponentWithInterfaceBeanMethod.class)); ITestBean bean = proxy.aBeanMethod(); assertThat(bean, instanceOf(EarlyBeanReferenceProxy.class)); @@ -201,7 +200,7 @@ public class EarlyBeanReferenceProxyCreatorTests { public void beanAnnotatedMethodsWithConcreteReturnTypeAreProxied() throws Exception { bf.registerBeanDefinition("componentWithConcreteBeanMethod", new RootBeanDefinition(ComponentWithConcreteBeanMethod.class)); EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - ComponentWithConcreteBeanMethod proxy = (ComponentWithConcreteBeanMethod) pc.createProxy(descriptorFor(ComponentWithConcreteBeanMethod.class)); + ComponentWithConcreteBeanMethod proxy = (ComponentWithConcreteBeanMethod) pc.createProxyIfPossible(descriptorFor(ComponentWithConcreteBeanMethod.class)); TestBean bean = proxy.aBeanMethod(); assertThat(bean, instanceOf(EarlyBeanReferenceProxy.class)); @@ -215,7 +214,7 @@ public class EarlyBeanReferenceProxyCreatorTests { public void attemptToProxyClassMissingNoArgConstructorFailsGracefully() throws Exception { EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); try { - pc.createProxy(descriptorFor(BeanMissingNoArgConstructor.class)); + pc.createProxyIfPossible(descriptorFor(BeanMissingNoArgConstructor.class)); fail("expected ProxyCreationException"); } catch(ProxyCreationException ex) { assertThat(ex.getMessage(), @@ -227,7 +226,7 @@ public class EarlyBeanReferenceProxyCreatorTests { public void attemptToProxyClassWithPrivateNoArgConstructorFailsGracefully() throws Exception { EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); try { - pc.createProxy(descriptorFor(BeanWithPrivateNoArgConstructor.class)); + pc.createProxyIfPossible(descriptorFor(BeanWithPrivateNoArgConstructor.class)); fail("expected ProxyCreationException"); } catch(ProxyCreationException ex) { assertThat(ex.getMessage(), @@ -236,15 +235,12 @@ public class EarlyBeanReferenceProxyCreatorTests { } @Test - public void attemptToProxyFinalClassFailsGracefully() throws Exception { + public void attemptToProxyFinalClassReturnsNonProxiedInstance() throws Exception { + bf.registerBeanDefinition("finalBean", new RootBeanDefinition(FinalBean.class)); EarlyBeanReferenceProxyCreator pc = new EarlyBeanReferenceProxyCreator(bf); - try { - pc.createProxy(descriptorFor(FinalBean.class)); - fail("expected ProxyCreationException"); - } catch(ProxyCreationException ex) { - assertThat(ex.getMessage(), - equalTo(format(FINAL_CLASS_ERROR_MESSAGE, FinalBean.class.getName()))); - } + Object bean = pc.createProxyIfPossible(descriptorFor(FinalBean.class)); + assertThat(bean, instanceOf(FinalBean.class)); + assertThat(bean, not(instanceOf(EarlyBeanReferenceProxy.class))); } private DependencyDescriptor descriptorFor(Class paramType) throws Exception { diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/FeatureMethodValueInjectionTests.java b/org.springframework.context/src/test/java/org/springframework/context/annotation/FeatureMethodValueInjectionTests.java new file mode 100644 index 0000000000..71d0c0dd0b --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/FeatureMethodValueInjectionTests.java @@ -0,0 +1,135 @@ +/* + * Copyright 2002-2011 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.context.annotation; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +import org.junit.Test; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.annotation.configuration.StubSpecification; + +/** + * Tests ensuring that @Feature methods can accept @Value-annoted + * parameters, particularly String types. SPR-7974 revealed this + * was failing due to attempting to proxy objects of type String, + * which cannot be done. + * + * @author Chris Beams + */ +public class FeatureMethodValueInjectionTests { + + @Test + public void control() { + System.setProperty("foo", "bar"); + System.setProperty("num", "2"); + Config config = new AnnotationConfigApplicationContext(Config.class).getBean(Config.class); + System.clearProperty("foo"); + System.clearProperty("num"); + assertThat(config.foo, is("bar")); + assertThat(config.num, is(2)); + } + + @Test + public void spelValueInjection() { + System.setProperty("foo", "bar"); + new AnnotationConfigApplicationContext(SpelValueInjectionFeatureConfig.class); + System.clearProperty("foo"); + } + + @Test + public void spelIntValueInjection() { + System.setProperty("num", "5"); + new AnnotationConfigApplicationContext(SpelIntValueInjectionFeatureConfig.class); + System.clearProperty("num"); + } + + @Test + public void stringBeanInjection() { + new AnnotationConfigApplicationContext(StringBeanConfig.class, StringBeanInjectionByTypeFeatureConfig.class); + } + + @Test + public void qualifiedStringBeanInjection() { + new AnnotationConfigApplicationContext(StringBeanSubConfig.class, StringBeanInjectionByQualifierFeatureConfig.class); + } + + + @FeatureConfiguration + static class SpelValueInjectionFeatureConfig { + @Feature + public StubSpecification feature(@Value("#{environment['foo']}") String foo) { + return new StubSpecification(); + } + } + + + @FeatureConfiguration + static class SpelIntValueInjectionFeatureConfig { + @Feature + public StubSpecification feature(@Value("#{environment['num']}") int num) { + assertThat(num, is(5)); + return new StubSpecification(); + } + } + + + @Configuration + static class StringBeanConfig { + @Bean + public String stringBean() { + return "sb"; + } + } + + + @Configuration + static class StringBeanSubConfig extends StringBeanConfig { + @Bean + public String stringBean2() { + return "sb2"; + } + } + + + @FeatureConfiguration + static class StringBeanInjectionByTypeFeatureConfig { + @Feature + public StubSpecification feature(String string) { + assertThat(string, is("sb")); + return new StubSpecification(); + } + } + + + @FeatureConfiguration + static class StringBeanInjectionByQualifierFeatureConfig { + @Feature + public StubSpecification feature(@Qualifier("stringBean2") String string) { + assertThat(string, is("sb2")); + return new StubSpecification(); + } + } + + + @Configuration + static class Config { + @Value("#{environment['foo']}") String foo; + @Value("#{environment['num']}") int num; + } +}