diff --git a/org.springframework.aop/src/main/java/org/springframework/aop/support/AopUtils.java b/org.springframework.aop/src/main/java/org/springframework/aop/support/AopUtils.java index 2f441102cbd02dad48bfa5cf370e5a5a1ea33d4e..fc073b6fb4b387df622329922861600503a4d6c9 100644 --- a/org.springframework.aop/src/main/java/org/springframework/aop/support/AopUtils.java +++ b/org.springframework.aop/src/main/java/org/springframework/aop/support/AopUtils.java @@ -86,7 +86,15 @@ public abstract class AopUtils { * @param clazz the class to check */ public static boolean isCglibProxyClass(Class clazz) { - return (clazz != null && clazz.getName().contains(ClassUtils.CGLIB_CLASS_SEPARATOR)); + return (clazz != null && isCglibProxyClassName(clazz.getName())); + } + + /** + * Check whether the specified class name is a CGLIB-generated class. + * @param className the class name to check + */ + public static boolean isCglibProxyClassName(String className) { + return (className != null && className.contains(ClassUtils.CGLIB_CLASS_SEPARATOR)); } /** diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/AbstractConfigurationClassProcessor.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/AbstractConfigurationClassProcessor.java index 0d9c08c0bee729ab7c79704a5cc651890c35c36e..b6fadec63c4beaedd578a27c1ae17f3b9b288a11 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/AbstractConfigurationClassProcessor.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/AbstractConfigurationClassProcessor.java @@ -16,7 +16,6 @@ package org.springframework.config.java.support; import org.springframework.beans.factory.config.BeanDefinition; -import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; import org.springframework.beans.factory.parsing.FailFastProblemReporter; import org.springframework.beans.factory.parsing.ProblemReporter; import org.springframework.beans.factory.support.BeanDefinitionRegistry; @@ -40,6 +39,7 @@ import org.springframework.config.java.Configuration; * * @author Chris Beams * @since 3.0 + * @see Configuration * @see ConfigurationClassPostProcessor */ public abstract class AbstractConfigurationClassProcessor { @@ -71,14 +71,6 @@ public abstract class AbstractConfigurationClassProcessor { */ protected abstract ConfigurationParser createConfigurationParser(); - /** - * Validate the given model and handle any errors. Implementations may choose to throw - * {@link BeanDefinitionParsingException}, or in the case of tooling register problems - * with the UI. - * @param configModel {@link ConfigurationModel} to validate - */ - protected abstract void validateModel(ConfigurationModel configModel); - /** * Override the default {@link ProblemReporter}. * @param problemReporter custom problem reporter @@ -122,8 +114,7 @@ public abstract class AbstractConfigurationClassProcessor { ConfigurationModel configModel = parser.getConfigurationModel(); - // validate the ConfigurationModel - validateModel(configModel); + configModel.validate(problemReporter); // read the model and create bean definitions based on its content return new ConfigurationModelBeanDefinitionReader().loadBeanDefinitions(configModel); diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/BeanMethodInterceptor.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/BeanMethodInterceptor.java index 9d94a8118c659c66766709617c72b6e1b4f37f8f..33fd91b87a4b1e68ed726ab51cfbe5a9f2dccaa4 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/BeanMethodInterceptor.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/BeanMethodInterceptor.java @@ -38,6 +38,7 @@ import org.springframework.core.annotation.AnnotationUtils; * * @author Chris Beams * @see Bean + * @see ConfigurationEnhancer */ class BeanMethodInterceptor implements MethodInterceptor { diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationClassPostProcessor.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationClassPostProcessor.java index 246449176411ccca97999514362b591b7cbfcb4e..da2cfa173f78d4cd2447a4a14b3e823c6975dd8f 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationClassPostProcessor.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationClassPostProcessor.java @@ -31,6 +31,7 @@ import org.springframework.config.java.Bean; import org.springframework.config.java.Configuration; import org.springframework.core.Ordered; import org.springframework.core.type.AnnotationMetadata; +import org.springframework.core.type.ClassMetadata; import org.springframework.core.type.classreading.MetadataReader; import org.springframework.core.type.classreading.SimpleMetadataReaderFactory; import org.springframework.util.Assert; @@ -122,28 +123,18 @@ public class ConfigurationClassPostProcessor extends AbstractConfigurationClassP if (beanDef.isAbstract() && !includeAbstractBeanDefs) continue; - if (isConfigClass(beanDef)) + if (isConfigurationClassBeanDefinition(beanDef)) configBeanDefs.registerBeanDefinition(beanName, beanDef); } return configBeanDefs; } - /** - * Validates the given model. Any problems found are delegated - * to {@link #getProblemReporter()}. - */ - @Override - protected void validateModel(ConfigurationModel model) { - model.validate(this.getProblemReporter()); - } - /** * Post-processes a BeanFactory in search of Configuration class BeanDefinitions; any * candidates are then enhanced by a {@link ConfigurationEnhancer}. Candidate status is * determined by BeanDefinition attribute metadata. * - * @author Chris Beams * @see ConfigurationEnhancer * @see BeanFactoryPostProcessor */ @@ -171,12 +162,13 @@ public class ConfigurationClassPostProcessor extends AbstractConfigurationClassP /** * Tests for the presence of CGLIB on the classpath by trying to * classload {@link #CGLIB_TEST_CLASS}. + * @throws IllegalStateException if CGLIB is not present. */ private void assertCglibIsPresent(BeanDefinitionRegistry configBeanDefs) { try { Class.forName(CGLIB_TEST_CLASS); } catch (ClassNotFoundException e) { - throw new RuntimeException("CGLIB is required to process @Configuration classes. " + + throw new IllegalStateException("CGLIB is required to process @Configuration classes. " + "Either add CGLIB v2.2.3 to the classpath or remove the following " + "@Configuration bean definitions: [" + StringUtils.arrayToCommaDelimitedString(configBeanDefs.getBeanDefinitionNames()) + "]"); @@ -184,23 +176,30 @@ public class ConfigurationClassPostProcessor extends AbstractConfigurationClassP } /** - * @return whether the BeanDefinition's beanClass is Configuration-annotated, - * false if no beanClass is specified. + * @return whether the BeanDefinition's beanClass (or its ancestry) is + * {@link Configuration}-annotated, false if no beanClass is specified. */ - private static boolean isConfigClass(BeanDefinition beanDef) { + private static boolean isConfigurationClassBeanDefinition(BeanDefinition beanDef) { String className = beanDef.getBeanClassName(); - if(className == null) - return false; + while (className != null && !(className.equals(Object.class.getName()))) { + try { + MetadataReader metadataReader = + new SimpleMetadataReaderFactory().getMetadataReader(className); + AnnotationMetadata annotationMetadata = metadataReader.getAnnotationMetadata(); + ClassMetadata classMetadata = metadataReader.getClassMetadata(); - try { - MetadataReader metadataReader = new SimpleMetadataReaderFactory().getMetadataReader(className); - AnnotationMetadata annotationMetadata = metadataReader.getAnnotationMetadata(); - return annotationMetadata.hasAnnotation(Configuration.class.getName()); - } catch (IOException ex) { - throw new RuntimeException(ex); + if (annotationMetadata.hasAnnotation(Configuration.class.getName())) + return true; + + className = classMetadata.getSuperClassName(); + } catch (IOException ex) { + throw new RuntimeException(ex); + } } + + return false; } } diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationEnhancer.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationEnhancer.java index a916087b5523150cf16d555f3ef2a7972f78c28f..841a388202cdd09c573087182f0a5a05587f438e 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationEnhancer.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationEnhancer.java @@ -47,6 +47,7 @@ import org.springframework.util.Assert; * @see #enhance(String) * * @author Chris Beams + * @see ConfigurationClassPostProcessor */ class ConfigurationEnhancer { diff --git a/org.springframework.config.java/src/test/java/org/springframework/config/java/support/package-info.java b/org.springframework.config.java/src/test/java/org/springframework/config/java/support/package-info.java new file mode 100644 index 0000000000000000000000000000000000000000..cb7b7c78ed980df3f9f180f2089632fe5c1df498 --- /dev/null +++ b/org.springframework.config.java/src/test/java/org/springframework/config/java/support/package-info.java @@ -0,0 +1,5 @@ +/** + * Unit tests for classes in + * {@literal org.springframework.context.annotation.support} + */ +package org.springframework.config.java.support; \ No newline at end of file diff --git a/org.springframework.config.java/src/test/java/test/basic/AbstractBeanDefinitionConfigurationClassTests.java b/org.springframework.config.java/src/test/java/test/basic/AbstractBeanDefinitionConfigurationClassTests.java new file mode 100644 index 0000000000000000000000000000000000000000..589f3f3713a4faeb80ebbc91a800f15536f39a8e --- /dev/null +++ b/org.springframework.config.java/src/test/java/test/basic/AbstractBeanDefinitionConfigurationClassTests.java @@ -0,0 +1,62 @@ +/* + * Copyright 2002-2009 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 test.basic; + +import static org.junit.Assert.*; +import static org.springframework.beans.factory.support.BeanDefinitionBuilder.*; + +import org.junit.Test; +import org.springframework.aop.support.AopUtils; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.config.java.Bean; +import org.springframework.config.java.Configuration; +import org.springframework.config.java.support.ConfigurationClassPostProcessor; + + +/** + * Covers the somewhat unlilely case of a {@link Configuration} class being declared + * as an abstract {@link BeanDefinition}. + * + * @author Chris Beams + * @see BeanDefinition#isAbstract() + */ +public class AbstractBeanDefinitionConfigurationClassTests { + + @SuppressWarnings("unused") + @Test + public void abstractConfigurationClassBeanDefinitionsAreIgnored() { + @Configuration class Abstract { @Bean Object foo1() { return null; } } + @Configuration class Concrete { @Bean Object foo2() { return null; } } + + DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); + factory.registerBeanDefinition("abstract", + rootBeanDefinition(Abstract.class).setAbstract(true).getBeanDefinition()); + factory.registerBeanDefinition("concrete", + rootBeanDefinition(Concrete.class).setAbstract(false).getBeanDefinition()); + new ConfigurationClassPostProcessor().postProcessBeanFactory(factory); + + assertTrue("abstract configuration should be CGLIB-enhanced", + AopUtils.isCglibProxyClassName(factory.getBeanDefinition("abstract").getBeanClassName())); + assertTrue("concrete configuration should be CGLIB-enhanced", + AopUtils.isCglibProxyClassName(factory.getBeanDefinition("concrete").getBeanClassName())); + + assertFalse("abstract configuration's @Bean method should not be registered", + factory.containsBeanDefinition("foo1")); + assertTrue("concrete configuration's @Bean method should be registered", + factory.containsBeanDefinition("foo2")); + } +} diff --git a/org.springframework.config.java/src/test/java/test/basic/AspectTests.java b/org.springframework.config.java/src/test/java/test/basic/AspectTests.java new file mode 100644 index 0000000000000000000000000000000000000000..5b8f50da0336421e8833f832c20c403ec97959ba --- /dev/null +++ b/org.springframework.config.java/src/test/java/test/basic/AspectTests.java @@ -0,0 +1,79 @@ +package test.basic; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.junit.Test; +import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.beans.factory.xml.XmlBeanFactory; +import org.springframework.config.java.Bean; +import org.springframework.config.java.Configuration; +import org.springframework.config.java.support.ConfigurationClassPostProcessor; +import org.springframework.context.support.GenericApplicationContext; +import org.springframework.core.io.ClassPathResource; + +import test.beans.TestBean; + + +public class AspectTests { + private void assertAdviceWasApplied(Class configClass) { + GenericApplicationContext ctx = new GenericApplicationContext( + new XmlBeanFactory(new ClassPathResource("aspectj-autoproxy-config.xml", AspectTests.class))); + ctx.addBeanFactoryPostProcessor(new ConfigurationClassPostProcessor()); + ctx.registerBeanDefinition("config", new RootBeanDefinition(configClass)); + ctx.refresh(); + + TestBean testBean = ctx.getBean("testBean", TestBean.class); + assertThat(testBean.getName(), equalTo("name")); + testBean.absquatulate(); + assertThat(testBean.getName(), equalTo("advisedName")); + } + + @Test + public void aspectAnnotatedConfiguration() { + assertAdviceWasApplied(AspectConfig.class); + } + + @Test + public void configurationIncludesAspect() { + assertAdviceWasApplied(ConfigurationWithAspect.class); + } + + + @Aspect + @Configuration + static class AspectConfig { + @Bean + public TestBean testBean() { + return new TestBean("name"); + } + + @Before("execution(* test.beans.TestBean.absquatulate(..)) && target(testBean)") + public void touchBean(TestBean testBean) { + testBean.setName("advisedName"); + } + } + + @Configuration + static class ConfigurationWithAspect { + @Bean + public TestBean testBean() { + return new TestBean("name"); + } + + @Bean + public NameChangingAspect nameChangingAspect() { + return new NameChangingAspect(); + } + } + + @Aspect + static class NameChangingAspect { + @Before("execution(* test.beans.TestBean.absquatulate(..)) && target(testBean)") + public void touchBean(TestBean testBean) { + testBean.setName("advisedName"); + } + } +} diff --git a/org.springframework.config.java/src/test/java/test/basic/PolymorphicConfigurationTests.java b/org.springframework.config.java/src/test/java/test/basic/PolymorphicConfigurationTests.java new file mode 100644 index 0000000000000000000000000000000000000000..d340cc7088df982d48f1bf92f18a967c1f4775af --- /dev/null +++ b/org.springframework.config.java/src/test/java/test/basic/PolymorphicConfigurationTests.java @@ -0,0 +1,56 @@ +/* + * Copyright 2002-2009 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 test.basic; + +import java.lang.annotation.Inherited; + +import org.junit.Test; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.config.java.Bean; +import org.springframework.config.java.Configuration; +import org.springframework.config.java.support.ConfigurationClassPostProcessor; + +import test.beans.TestBean; + +/** + * 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. + * + * @author Chris Beams + */ +public class PolymorphicConfigurationTests { + + @Test + public void subclassNeedNotDeclareConfigurationAnnotation() { + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + beanFactory.registerBeanDefinition("config", new RootBeanDefinition(Config.class)); + new ConfigurationClassPostProcessor().postProcessBeanFactory(beanFactory); + + beanFactory.getBean("testBean", TestBean.class); + } + + @Configuration + static class SuperConfig { + @Bean + public TestBean testBean() { + return new TestBean(); + } + } + + static class Config extends SuperConfig { } +} diff --git a/org.springframework.config.java/src/test/java/test/basic/aspectj-autoproxy-config.xml b/org.springframework.config.java/src/test/java/test/basic/aspectj-autoproxy-config.xml new file mode 100644 index 0000000000000000000000000000000000000000..2c0f9adae0b74c1c7799aacdc160b51a1fdfd0a0 --- /dev/null +++ b/org.springframework.config.java/src/test/java/test/basic/aspectj-autoproxy-config.xml @@ -0,0 +1,11 @@ + + + + + diff --git a/org.springframework.config.java/src/test/java/test/common/scope/CustomScope.java b/org.springframework.config.java/src/test/java/test/beans/CustomScope.java similarity index 98% rename from org.springframework.config.java/src/test/java/test/common/scope/CustomScope.java rename to org.springframework.config.java/src/test/java/test/beans/CustomScope.java index fa3d0ad67ae819267d33ac5a49923e4a36ea0f57..88a13a84b60f4b194b57d4f3ddc0b8feb220cf12 100644 --- a/org.springframework.config.java/src/test/java/test/common/scope/CustomScope.java +++ b/org.springframework.config.java/src/test/java/test/beans/CustomScope.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package test.common.scope; +package test.beans; import java.util.HashMap; import java.util.Map; diff --git a/org.springframework.config.java/src/test/java/test/feature/lifecycle/scoping/ScopingTests.java b/org.springframework.config.java/src/test/java/test/feature/lifecycle/scoping/ScopingTests.java index fcdaba3123f24916301cd11a0967a4cb06878367..09bc7c9b85b1450bd82c4e8a618d17cac451d24b 100644 --- a/org.springframework.config.java/src/test/java/test/feature/lifecycle/scoping/ScopingTests.java +++ b/org.springframework.config.java/src/test/java/test/feature/lifecycle/scoping/ScopingTests.java @@ -33,9 +33,9 @@ import org.springframework.context.annotation.Scope; import org.springframework.context.annotation.ScopedProxyMode; import org.springframework.context.support.GenericApplicationContext; +import test.beans.CustomScope; import test.beans.ITestBean; import test.beans.TestBean; -import test.common.scope.CustomScope;