+ Added isCglibClassName() to AopUtils for checking bean class names before classloading

+ Added tests for @Aspect support in @Configuration classes
+ Added tests for @Inherited @Configuration behavior


git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@781 50f2f4bb-b051-0410-bef5-90022cba6387

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));
 	}
 
 	/**

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);

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 {
 

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 <var>model</var>. 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;
 	}
 
 }

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 {
 

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;

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"));
+	}
+}

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");
+		}
+	}
+}

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 { }
+}

org.springframework.config.java/src/test/java/test/basic/aspectj-autoproxy-config.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans"
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xmlns:aop="http://www.springframework.org/schema/aop"
+	xsi:schemaLocation="http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop.xsd
+		http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
+		http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context.xsd"
+	xmlns:context="http://www.springframework.org/schema/context">
+	
+	<aop:aspectj-autoproxy proxy-target-class="true"/>
+</beans>

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;

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;