From b985011b24cb40671ad6d2b8cc34fbea763ed7a4 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Fri, 6 Mar 2009 03:12:53 +0000 Subject: [PATCH] + Fleshed out, documented, tested and polished the ConfigurationPostProcessor implementation + Removed @FactoryMethod indirection and extension point in favor of direct processing of @Bean annotations --- .../config/java/BeanDefinitionRegistrar.java | 1 - .../config/java/BeanMethod.java | 84 +++++++---- .../config/java/ConfigurationModel.java | 68 +++++++-- .../config/java/FactoryMethod.java | 67 --------- .../org/springframework/config/java/Util.java | 2 +- .../springframework/config/java/ext/Bean.java | 73 ---------- .../enhancement/ConfigurationEnhancer.java | 4 +- .../ConfigurationClassMethodVisitor.java | 6 +- .../AbstractConfigurationClassProcessor.java | 11 -- ...onfigurationModelBeanDefinitionReader.java | 10 +- .../support/ConfigurationPostProcessor.java | 103 ++++++++++---- .../ConfigurationPostProcessorTests.java | 130 ++++++++++++++++++ 12 files changed, 321 insertions(+), 238 deletions(-) delete mode 100644 org.springframework.config.java/src/main/java/org/springframework/config/java/FactoryMethod.java create mode 100644 org.springframework.config.java/src/test/java/org/springframework/config/java/support/ConfigurationPostProcessorTests.java diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/BeanDefinitionRegistrar.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/BeanDefinitionRegistrar.java index 262672a75ab..4de15092f42 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/BeanDefinitionRegistrar.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/BeanDefinitionRegistrar.java @@ -12,7 +12,6 @@ import org.springframework.beans.factory.support.BeanDefinitionRegistry; *

Constraints

Implementations must have only a default constructor, or explicitly * declare a no-arg constructor. * - * @see FactoryMethod * @see BeanMethod * * @author Chris Beams diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/BeanMethod.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/BeanMethod.java index 9799e433041..c2c24bbdf3d 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/BeanMethod.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/BeanMethod.java @@ -16,17 +16,13 @@ package org.springframework.config.java; import static java.lang.String.*; -import static org.springframework.config.java.Util.*; import java.lang.annotation.Annotation; import java.lang.reflect.Modifier; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; -import java.util.Set; - -import net.sf.cglib.proxy.Callback; +import org.springframework.config.java.ext.Bean; import org.springframework.util.Assert; @@ -39,7 +35,6 @@ public final class BeanMethod implements Validatable { private final List annotations = new ArrayList(); private transient ConfigurationClass declaringClass; private transient int lineNumber; - private transient FactoryMethod factoryAnno; private transient final List validators = new ArrayList(); public BeanMethod(String name, int modifiers, ModelClass returnType, Annotation... annotations) { @@ -47,11 +42,8 @@ public final class BeanMethod implements Validatable { this.name = name; Assert.notNull(annotations); - for (Annotation annotation : annotations) { + for (Annotation annotation : annotations) this.annotations.add(annotation); - if (factoryAnno == null) - factoryAnno = annotation.annotationType().getAnnotation(FactoryMethod.class); - } Assert.isTrue(modifiers >= 0, "modifiers must be non-negative: " + modifiers); this.modifiers = modifiers; @@ -140,30 +132,34 @@ public final class BeanMethod implements Validatable { if (Modifier.isFinal(getModifiers())) errors.add(new FinalMethodError()); + + new BeanValidator().validate(this, errors); } - public BeanDefinitionRegistrar getRegistrar() { - return getInstance(factoryAnno.registrar()); - } +// public BeanDefinitionRegistrar getRegistrar() { +// return getInstance(factoryAnno.registrar()); +// } - public Set getValidators() { - HashSet validator = new HashSet(); - - for (Class validatorType : factoryAnno.validators()) - validator.add(getInstance(validatorType)); - - return validator; - } - - public Callback getCallback() { - Class callbackType = factoryAnno.interceptor(); - - if (callbackType.equals(NoOpInterceptor.class)) - return NoOpInterceptor.INSTANCE; - - return getInstance(callbackType); - } +// public Set getValidators() { +// HashSet validators = new HashSet(); +// +//// for (Class validatorType : factoryAnno.validators()) +//// validator.add(getInstance(validatorType)); +// +// validators.add(IllegalB) +// +// return validators; +// } +// public Callback getCallback() { +// Class callbackType = factoryAnno.interceptor(); +// +// if (callbackType.equals(NoOpInterceptor.class)) +// return NoOpInterceptor.INSTANCE; +// +// return getInstance(callbackType); +// } +// @Override public String toString() { String returnTypeName = returnType == null ? "" : returnType.getSimpleName(); @@ -236,3 +232,31 @@ public final class BeanMethod implements Validatable { } } + +/** + * Detects any user errors when declaring {@link Bean}-annotated methods. + * + * @author Chris Beams + */ +class BeanValidator implements Validator { + + public boolean supports(Object object) { + return object instanceof BeanMethod; + } + + public void validate(Object object, List errors) { + BeanMethod method = (BeanMethod) object; + + // TODO: re-enable for @ScopedProxy support + // if (method.getAnnotation(ScopedProxy.class) == null) + // return; + // + // Bean bean = method.getRequiredAnnotation(Bean.class); + // + // if (bean.scope().equals(DefaultScopes.SINGLETON) + // || bean.scope().equals(DefaultScopes.PROTOTYPE)) + // errors.add(new InvalidScopedProxyDeclarationError(method)); + } + +} + diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/ConfigurationModel.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/ConfigurationModel.java index b0580bdd2f8..ef93341eb19 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/ConfigurationModel.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/ConfigurationModel.java @@ -20,6 +20,8 @@ import static java.lang.String.*; import java.util.ArrayList; import java.util.List; +import org.springframework.config.java.ext.Bean; + /** * An abstract representation of a set of user-provided "Configuration classes", usually but @@ -106,23 +108,24 @@ public final class ConfigurationModel implements Validatable { // cascade through model and allow handlers to register validators // depending on where they are registered (with the model, the class, or the method) // they will be called directly or indirectly below - for (ConfigurationClass configClass : getAllConfigurationClasses()) { - for (BeanMethod method : configClass.getMethods()) { - for (Validator validator : method.getValidators()) { - if (validator.supports(method)) - method.registerValidator(validator); - // TODO: support class-level validation - // if(validator.supports(configClass)) - // configClass.registerValidator(validator); - if (validator.supports(this)) - this.registerValidator(validator); - } - } - } +// for (ConfigurationClass configClass : getAllConfigurationClasses()) { +// for (BeanMethod method : configClass.getMethods()) { +// for (Validator validator : method.getValidators()) { +// if (validator.supports(method)) +// method.registerValidator(validator); +// // TODO: support class-level validation +// // if(validator.supports(configClass)) +// // configClass.registerValidator(validator); +// if (validator.supports(this)) +// this.registerValidator(validator); +// } +// } +// } // process any validators registered directly with this model object - for (Validator validator : validators) - validator.validate(this, errors); +// for (Validator validator : validators) +// validator.validate(this, errors); + new IllegalBeanOverrideValidator().validate(this, errors); // each individual configuration class must be well-formed // note that each configClass detects usage errors on its imports recursively @@ -175,3 +178,38 @@ public final class ConfigurationModel implements Validatable { } } + +/** + * Detects any illegally-overridden {@link Bean} definitions within a particular + * {@link ConfigurationModel} + * + * @see Bean#allowOverriding() + * + * @author Chris Beams + */ +class IllegalBeanOverrideValidator implements Validator { + + public boolean supports(Object object) { + return object instanceof ConfigurationModel; + } + + public void validate(Object object, List errors) { + ConfigurationModel model = (ConfigurationModel) object; + + ConfigurationClass[] allClasses = model.getAllConfigurationClasses(); + + for (int i = 0; i < allClasses.length; i++) { + for (BeanMethod method : allClasses[i].getMethods()) { + Bean bean = method.getAnnotation(Bean.class); + + if (bean == null || bean.allowOverriding()) + continue; + + for (int j = i + 1; j < allClasses.length; j++) + if (allClasses[j].hasMethod(method.getName())) + errors.add(allClasses[i].new IllegalBeanOverrideError(allClasses[j], method)); + } + } + } + +} diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/FactoryMethod.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/FactoryMethod.java deleted file mode 100644 index cd57b3207cc..00000000000 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/FactoryMethod.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright 2002-2008 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.config.java; - -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -import net.sf.cglib.proxy.MethodInterceptor; - -import org.springframework.config.java.ext.AbstractMethodInterceptor; -import org.springframework.config.java.ext.Bean; - - -/** - * Meta-annotation used to identify method annotations as producers of beans and/or values. - * Provides a model that's open for extension. i.e.: The {@link Bean} annotation is - * annotated as a {@link FactoryMethod}. In this same fashion, any custom annotation can be - * devised with its own semantics. It need only provide a custom registrar, interceptor and - * optionally, validators. - * - * @see Bean - * @see BeanDefinitionRegistrar - * @see AbstractMethodInterceptor - * @see Validator - * - * @author Chris Beams - */ -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.ANNOTATION_TYPE) -@Documented -public @interface FactoryMethod { - - /** - * Specifies which registrar should be used to register bean definitions when processing - * this {@link FactoryMethod}. - */ - Class registrar(); - - /** - * Specifies what interceptor should be used when processing this {@link FactoryMethod}. - * Defaults to {@link NoOpInterceptor} which does nothing. - */ - Class interceptor(); - - /** - * Optionally specifies any {@link Validator} types capable of validating the syntax of - * this {@link FactoryMethod}. Usually used when a factory method may have multiple - * annotations such as {@link Bean} and {@link ScopedProxy}. - */ - Class[] validators() default {}; -} diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/Util.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/Util.java index 3a9bdfd0d4d..3e7823b2024 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/Util.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/Util.java @@ -106,7 +106,7 @@ public class Util { *

* ASM class reading is used throughout JavaConfig, but there are certain cases where * classloading cannot be avoided - specifically in cases where users define their own - * {@link Extension} or {@link FactoryMethod} annotations. This method should therefore be + * {@link Extension} annotations. This method should therefore be * used sparingly but consistently where required. *

* Because {@link ClassNotFoundException} is compensated for by returning null, callers diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/ext/Bean.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/ext/Bean.java index 7a028ad49d8..4f90824c28e 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/ext/Bean.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/ext/Bean.java @@ -21,19 +21,12 @@ import java.lang.annotation.Inherited; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -import java.util.List; import org.springframework.beans.factory.annotation.Autowire; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.AbstractBeanDefinition; -import org.springframework.config.java.BeanMethod; import org.springframework.config.java.Configuration; -import org.springframework.config.java.ConfigurationClass; -import org.springframework.config.java.ConfigurationModel; -import org.springframework.config.java.FactoryMethod; import org.springframework.config.java.Scopes; -import org.springframework.config.java.UsageError; -import org.springframework.config.java.Validator; /** @@ -71,9 +64,6 @@ import org.springframework.config.java.Validator; @Retention(RetentionPolicy.RUNTIME) @Inherited @Documented -@FactoryMethod(registrar = BeanRegistrar.class, - interceptor = BeanMethodInterceptor.class, - validators = { BeanValidator.class, IllegalBeanOverrideValidator.class }) public @interface Bean { /** @@ -155,66 +145,3 @@ public @interface Bean { } - -/** - * Detects any user errors when declaring {@link Bean}-annotated methods. - * - * @author Chris Beams - */ -class BeanValidator implements Validator { - - public boolean supports(Object object) { - return object instanceof BeanMethod; - } - - public void validate(Object object, List errors) { - BeanMethod method = (BeanMethod) object; - - // TODO: re-enable for @ScopedProxy support - // if (method.getAnnotation(ScopedProxy.class) == null) - // return; - // - // Bean bean = method.getRequiredAnnotation(Bean.class); - // - // if (bean.scope().equals(DefaultScopes.SINGLETON) - // || bean.scope().equals(DefaultScopes.PROTOTYPE)) - // errors.add(new InvalidScopedProxyDeclarationError(method)); - } - -} - - -/** - * Detects any illegally-overridden {@link Bean} definitions within a particular - * {@link ConfigurationModel} - * - * @see Bean#allowOverriding() - * - * @author Chris Beams - */ -class IllegalBeanOverrideValidator implements Validator { - - public boolean supports(Object object) { - return object instanceof ConfigurationModel; - } - - public void validate(Object object, List errors) { - ConfigurationModel model = (ConfigurationModel) object; - - ConfigurationClass[] allClasses = model.getAllConfigurationClasses(); - - for (int i = 0; i < allClasses.length; i++) { - for (BeanMethod method : allClasses[i].getMethods()) { - Bean bean = method.getAnnotation(Bean.class); - - if (bean == null || bean.allowOverriding()) - continue; - - for (int j = i + 1; j < allClasses.length; j++) - if (allClasses[j].hasMethod(method.getName())) - errors.add(allClasses[i].new IllegalBeanOverrideError(allClasses[j], method)); - } - } - } - -} diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/internal/enhancement/ConfigurationEnhancer.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/internal/enhancement/ConfigurationEnhancer.java index f1320f14a99..7441f566cd7 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/internal/enhancement/ConfigurationEnhancer.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/internal/enhancement/ConfigurationEnhancer.java @@ -122,9 +122,9 @@ public class ConfigurationEnhancer { for (ConfigurationClass configClass : model.getAllConfigurationClasses()) { for (BeanMethod method : configClass.getMethods()) { - registrars.add(method.getRegistrar()); + registrars.add(new BeanRegistrar()); - Callback callback = method.getCallback(); + Callback callback = new BeanMethodInterceptor(); if (callback instanceof BeanFactoryAware) ((BeanFactoryAware) callback).setBeanFactory(beanFactory); diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/internal/parsing/ConfigurationClassMethodVisitor.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/internal/parsing/ConfigurationClassMethodVisitor.java index 6c32a39a98e..93e9c52778e 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/internal/parsing/ConfigurationClassMethodVisitor.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/internal/parsing/ConfigurationClassMethodVisitor.java @@ -31,14 +31,13 @@ import org.objectweb.asm.Opcodes; import org.springframework.config.java.BeanMethod; import org.springframework.config.java.Configuration; import org.springframework.config.java.ConfigurationClass; -import org.springframework.config.java.FactoryMethod; import org.springframework.config.java.ModelClass; import org.springframework.config.java.ext.Bean; /** * Visits a single method declared in a given {@link Configuration} class. Determines - * whether the method is a {@link FactoryMethod} method and if so, adds it to the + * whether the method is a {@link Bean} method and if so, adds it to the * {@link ConfigurationClass}. * * @author Chris Beams @@ -108,8 +107,7 @@ class ConfigurationClassMethodVisitor extends MethodAdapter { /** * Parses through all {@link #annotations} on this method in order to determine whether - * it is a {@link FactoryMethod} method or not and if so adds it to the enclosing - * {@link #configClass}. + * it is a {@link Bean} method or not and if so adds it to the enclosing {@link #configClass}. */ @Override public void visitEnd() { 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 2991397adf4..be58c0fb34c 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 @@ -1,7 +1,5 @@ package org.springframework.config.java.support; -import static org.springframework.util.StringUtils.*; - import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.config.java.ConfigurationModel; @@ -9,7 +7,6 @@ import org.springframework.config.java.internal.parsing.ConfigurationParser; public abstract class AbstractConfigurationClassProcessor { - static String CGLIB_PACKAGE = "net.sf.cglib.proxy"; protected abstract BeanDefinitionRegistry getConfigurationBeanDefinitions(boolean includeAbstractBeanDefs); @@ -23,14 +20,6 @@ public abstract class AbstractConfigurationClassProcessor { if(configBeanDefs.getBeanDefinitionCount() == 0) return configBeanDefs; // nothing to do - don't waste any more cycles - // TODO: the location of this cglib check is temporary, pending removal of the - // @FactoryMethod meta-annotation indirection - if(Package.getPackage(CGLIB_PACKAGE) == null) - throw new RuntimeException("CGLIB is required to process @Configuration classes. " + - "Either add CGLIB v2.2.3 to the classpath or remove the following " + - "@Configuration bean definitions: [" - + arrayToCommaDelimitedString(configBeanDefs.getBeanDefinitionNames()) + "]"); - ConfigurationModel configModel = createConfigurationModelFor(configBeanDefs); validateModel(configModel); diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationModelBeanDefinitionReader.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationModelBeanDefinitionReader.java index 9a86d3aef7b..0fa13559513 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationModelBeanDefinitionReader.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationModelBeanDefinitionReader.java @@ -30,7 +30,8 @@ import org.springframework.config.java.BeanMethod; import org.springframework.config.java.Configuration; import org.springframework.config.java.ConfigurationClass; import org.springframework.config.java.ConfigurationModel; -import org.springframework.config.java.FactoryMethod; +import org.springframework.config.java.ext.Bean; +import org.springframework.config.java.ext.BeanRegistrar; import org.springframework.config.java.plugin.Extension; import org.springframework.core.io.Resource; @@ -75,7 +76,7 @@ class ConfigurationModelBeanDefinitionReader { /** * Reads a particular {@link ConfigurationClass}, registering bean definitions for the - * class itself, all its {@link FactoryMethod} methods and all its {@link Extension} + * class itself, all its {@link Bean} methods and all its {@link Extension} * annotations. */ private void loadBeanDefinitionsForConfigurationClass(ConfigurationClass configClass) { @@ -137,11 +138,10 @@ class ConfigurationModelBeanDefinitionReader { /** * Reads a particular {@link BeanMethod}, registering bean definitions with * {@link #registry} based on its contents. - * - * @see FactoryMethod */ private void loadBeanDefinitionsForModelMethod(BeanMethod method) { - method.getRegistrar().register(method, registry); + new BeanRegistrar().register(method, registry); + //method.getRegistrar().register(method, registry); } // @SuppressWarnings("unchecked") diff --git a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationPostProcessor.java b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationPostProcessor.java index c6a350c4c08..a5707c1f8c3 100644 --- a/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationPostProcessor.java +++ b/org.springframework.config.java/src/main/java/org/springframework/config/java/support/ConfigurationPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * 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. @@ -40,65 +40,93 @@ import org.springframework.core.type.AnnotationMetadata; import org.springframework.core.type.classreading.MetadataReader; import org.springframework.core.type.classreading.SimpleMetadataReaderFactory; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** - * {@link BeanFactoryPostProcessor} used for bootstrapping {@link Configuration - * @Configuration} beans. Usually used in conjunction with Spring XML files. + * {@link BeanFactoryPostProcessor} used for bootstrapping processing of + * {@link Configuration @Configuration} classes. */ public class ConfigurationPostProcessor extends AbstractConfigurationClassProcessor implements Ordered, BeanFactoryPostProcessor { private static final Log logger = LogFactory.getLog(ConfigurationPostProcessor.class); + + /** + * A well-known class in the CGLIB API used when testing to see if CGLIB + * is present on the classpath. Package-private visibility allows for + * manipulation by tests. + * @see #assertCglibIsPresent(BeanDefinitionRegistry) + */ + static String CGLIB_TEST_CLASS = "net.sf.cglib.proxy.Callback"; + + /** + * Holder for the calling BeanFactory + * @see #postProcessBeanFactory(ConfigurableListableBeanFactory) + */ private DefaultListableBeanFactory beanFactory; /** - * Returns the order in which this {@link BeanPostProcessor} will be executed. Returns + * @return the order in which this {@link BeanPostProcessor} will be executed. Returns * {@link Ordered#HIGHEST_PRECEDENCE}. */ public int getOrder() { return Ordered.HIGHEST_PRECEDENCE; } - + + /** + * Finds {@link Configuration} bean definitions within clBeanFactory + * and processes them in order to register bean definitions for each Bean method + * found within; also prepares the the Configuration classes for servicing + * bean requests at runtime by replacing them with CGLIB-enhanced subclasses. + */ public void postProcessBeanFactory(ConfigurableListableBeanFactory clBeanFactory) throws BeansException { Assert.isInstanceOf(DefaultListableBeanFactory.class, clBeanFactory); beanFactory = (DefaultListableBeanFactory) clBeanFactory; - + BeanDefinitionRegistry factoryBeanDefs = processConfigBeanDefinitions(); - + for(String beanName : factoryBeanDefs.getBeanDefinitionNames()) beanFactory.registerBeanDefinition(beanName, factoryBeanDefs.getBeanDefinition(beanName)); - + enhanceConfigurationClasses(); } - + + /** + * @return a ConfigurationParser that uses the enclosing BeanFactory's + * classLoader to load all Configuration class artifacts. + */ @Override - protected ConfigurationParser createConfigurationParser() { - return new ConfigurationParser(beanFactory.getBeanClassLoader()); - } + protected ConfigurationParser createConfigurationParser() { + return new ConfigurationParser(beanFactory.getBeanClassLoader()); + } /** * @return map of all non-abstract {@link BeanDefinition}s in the enclosing {@link #beanFactory} */ @Override - protected BeanDefinitionRegistry getConfigurationBeanDefinitions(boolean includeAbstractBeanDefs) { - + protected BeanDefinitionRegistry getConfigurationBeanDefinitions(boolean includeAbstractBeanDefs) { + BeanDefinitionRegistry configBeanDefs = new DefaultListableBeanFactory(); - + for (String beanName : beanFactory.getBeanDefinitionNames()) { BeanDefinition beanDef = beanFactory.getBeanDefinition(beanName); - + if (beanDef.isAbstract() && !includeAbstractBeanDefs) continue; if (isConfigClass(beanDef)) configBeanDefs.registerBeanDefinition(beanName, beanDef); } - - return configBeanDefs; - } - + + return configBeanDefs; + } + + /** + * Validates the given model. + * @throws MalformedConfigurationException if any errors are detected + */ @Override protected void validateModel(ConfigurationModel model) { ArrayList errors = new ArrayList(); @@ -118,10 +146,12 @@ public class ConfigurationPostProcessor extends AbstractConfigurationClassProces */ private void enhanceConfigurationClasses() { - ConfigurationEnhancer enhancer = new ConfigurationEnhancer(beanFactory); - BeanDefinitionRegistry configBeanDefs = getConfigurationBeanDefinitions(true); - + + assertCglibIsPresent(configBeanDefs); + + ConfigurationEnhancer enhancer = new ConfigurationEnhancer(beanFactory); + for(String beanName : configBeanDefs.getBeanDefinitionNames()) { BeanDefinition beanDef = beanFactory.getBeanDefinition(beanName); String configClassName = beanDef.getBeanClassName(); @@ -129,23 +159,38 @@ public class ConfigurationPostProcessor extends AbstractConfigurationClassProces if (logger.isDebugEnabled()) logger.debug(format("Replacing bean definition '%s' existing class name '%s' with enhanced class name '%s'", - beanName, configClassName, enhancedClassName)); + beanName, configClassName, enhancedClassName)); beanDef.setBeanClassName(enhancedClassName); } } /** - * Determines whether the class for beanDef is a {@link Configuration} - * -annotated class. Returns false if beanDef has no class specified. + * Tests for the presence of CGLIB on the classpath by trying to + * classload {@link #CGLIB_TEST_CLASS}. + */ + private void assertCglibIsPresent(BeanDefinitionRegistry configBeanDefs) { + try { + Class.forName(CGLIB_TEST_CLASS); + } catch (ClassNotFoundException e) { + throw new RuntimeException("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()) + "]"); + } + } + + /** + * @return whether the BeanDefinition's beanClass is Configuration-annotated, + * false if no beanClass is specified. */ private static boolean isConfigClass(BeanDefinition beanDef) { - + String className = beanDef.getBeanClassName(); - + if(className == null) return false; - + try { MetadataReader metadataReader = new SimpleMetadataReaderFactory().getMetadataReader(className); AnnotationMetadata annotationMetadata = metadataReader.getAnnotationMetadata(); diff --git a/org.springframework.config.java/src/test/java/org/springframework/config/java/support/ConfigurationPostProcessorTests.java b/org.springframework.config.java/src/test/java/org/springframework/config/java/support/ConfigurationPostProcessorTests.java new file mode 100644 index 00000000000..2158e9a3834 --- /dev/null +++ b/org.springframework.config.java/src/test/java/org/springframework/config/java/support/ConfigurationPostProcessorTests.java @@ -0,0 +1,130 @@ +package org.springframework.config.java.support; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; +import static org.springframework.beans.factory.support.BeanDefinitionBuilder.*; + +import java.lang.reflect.Field; +import java.util.Vector; + +import org.junit.Test; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.config.java.Configuration; +import org.springframework.config.java.ext.Bean; +import org.springframework.util.ClassUtils; + +/** + * Unit tests for {@link ConfigurationPostProcessor} + * + * @author Chris Beams + */ +public class ConfigurationPostProcessorTests { + + private static final String ORIG_CGLIB_TEST_CLASS = ConfigurationPostProcessor.CGLIB_TEST_CLASS; + private static final String BOGUS_CGLIB_TEST_CLASS = "a.bogus.class"; + + /** + * CGLIB is an optional dependency for Core Spring. If users attempt + * to use {@link Configuration} classes, they'll need it on the classpath; + * if Configuration classes are present in the bean factory and CGLIB + * is not present, an instructive exception should be thrown. + */ + @Test + public void testFailFastIfCglibNotPresent() { + @Configuration class Config { + public @Bean String name() { return "foo"; } + } + + DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); + + factory.registerBeanDefinition("config1", rootBeanDefinition(Config.class).getBeanDefinition()); + + ConfigurationPostProcessor cpp = new ConfigurationPostProcessor(); + + // temporarily set the cglib test class to something bogus + ConfigurationPostProcessor.CGLIB_TEST_CLASS = BOGUS_CGLIB_TEST_CLASS; + + try { + cpp.postProcessBeanFactory(factory); + } catch (RuntimeException ex) { + assertTrue(ex.getMessage().contains("CGLIB is required to process @Configuration classes")); + } finally { + ConfigurationPostProcessor.CGLIB_TEST_CLASS = ORIG_CGLIB_TEST_CLASS; + } + } + + /** + * In order to keep Spring's footprint as small as possible, CGLIB must + * not be required on the classpath unless the user is taking advantage + * of {@link Configuration} classes. + * + * This test will fail if any CGLIB classes are classloaded before the call + * to {@link ConfigurationPostProcessor#enhanceConfigurationClasses} + */ + @Test + public void testCglibClassesAreLoadedJustInTimeForEnhancement() throws Exception { + ClassLoader classLoader = ClassUtils.getDefaultClassLoader(); + Field classesField = ClassLoader.class.getDeclaredField("classes"); + classesField.setAccessible(true); + + // first, remove any CGLIB classes that may have been loaded by other tests + @SuppressWarnings("unchecked") + Vector> classes = (Vector>) classesField.get(classLoader); + + Vector> cglibClassesAlreadyLoaded = new Vector>(); + for(Class loadedClass : classes) + if(loadedClass.getName().startsWith("net.sf.cglib")) + cglibClassesAlreadyLoaded.add(loadedClass); + + for(Class cglibClass : cglibClassesAlreadyLoaded) + classes.remove(cglibClass); + + // now, execute a scenario where everything except enhancement occurs + // -- no CGLIB classes should get loaded! + testFailFastIfCglibNotPresent(); + + // test to ensure that indeed no CGLIB classes have been loaded + for(Class loadedClass : classes) + if(loadedClass.getName().startsWith("net.sf.cglib")) + fail("CGLIB class should not have been eagerly loaded: " + loadedClass.getName()); + } + + /** + * Enhanced {@link Configuration} classes are only necessary for respecting + * certain bean semantics, like singleton-scoping, scoped proxies, etc. + * + * Technically, {@link ConfigurationPostProcessor} could fail to enhance the + * registered Configuration classes, and many use cases would still work. + * Certain cases, however, like inter-bean singleton references would not. + * We test for such a case below, and in doing so prove that enhancement is + * working. + */ + @Test + public void testEnhancementIsPresentBecauseSingletonSemanticsAreRespected() { + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + beanFactory.registerBeanDefinition("config", + rootBeanDefinition(SingletonBeanConfig.class).getBeanDefinition()); + new ConfigurationPostProcessor().postProcessBeanFactory(beanFactory); + Foo foo = (Foo) beanFactory.getBean("foo"); + Bar bar = (Bar) beanFactory.getBean("bar"); + assertThat(foo, sameInstance(bar.foo)); + } + + @Configuration + static class SingletonBeanConfig { + public @Bean Foo foo() { + return new Foo(); + } + + public @Bean Bar bar() { + return new Bar(foo()); + } + } + + static class Foo { } + static class Bar { + final Foo foo; + public Bar(Foo foo) { this.foo = foo; } + } + +}