From a31ebb6c1e83ad277897d9918459e1a0b27b91b0 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 14 Aug 2014 23:49:29 +0200 Subject: [PATCH] Polishing --- .../beans/factory/BeanFactory.java | 5 +- .../AutowiredAnnotationBeanPostProcessor.java | 7 +-- .../factory/support/AbstractBeanFactory.java | 9 ++-- ...CglibSubclassingInstantiationStrategy.java | 36 ++++++-------- .../beans/factory/support/LookupOverride.java | 13 ++--- .../beans/factory/support/MethodOverride.java | 2 + .../factory/support/ReplaceOverride.java | 14 +++--- .../factory/xml/XmlBeanFactoryTestTypes.java | 48 ++++--------------- .../factory/xml/XmlBeanFactoryTests.java | 2 +- .../springframework/util/ReflectionUtils.java | 10 ++-- 10 files changed, 56 insertions(+), 90 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/BeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/BeanFactory.java index 8c73a16fd7a..7d65a3aaf41 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/BeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/BeanFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -170,8 +170,7 @@ public interface BeanFactory { *

Allows for specifying explicit constructor arguments / factory method arguments, * overriding the specified default arguments (if any) in the bean definition. * @param name the name of the bean to retrieve - * @param args arguments to use if creating a prototype using explicit arguments to a - * static factory method. It is invalid to use a non-null args value in any other case. + * @param args arguments to use if creating a prototype using explicit arguments * @return an instance of the bean * @throws NoSuchBeanDefinitionException if there is no such bean definition * @throws BeanDefinitionStoreException if arguments have been given but diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java index 189a76c81e2..7f67f5f1d06 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java @@ -239,7 +239,8 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean AnnotationAttributes annotation = findAutowiredAnnotation(candidate); if (annotation != null) { if (requiredConstructor != null) { - throw new BeanCreationException("Invalid autowire-marked constructor: " + candidate + + throw new BeanCreationException(beanName, + "Invalid autowire-marked constructor: " + candidate + ". Found another constructor with 'required' Autowired annotation: " + requiredConstructor); } @@ -250,10 +251,10 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean boolean required = determineRequiredStatus(annotation); if (required) { if (!candidates.isEmpty()) { - throw new BeanCreationException( + throw new BeanCreationException(beanName, "Invalid autowire-marked constructors: " + candidates + ". Found another constructor with 'required' Autowired annotation: " + - requiredConstructor); + candidate); } requiredConstructor = candidate; } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java index 8fa8bc1b0a0..526ebd73c55 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java @@ -285,8 +285,8 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp if (dependsOn != null) { for (String dependsOnBean : dependsOn) { if (isDependent(beanName, dependsOnBean)) { - throw new BeanCreationException("Circular depends-on relationship between '" + - beanName + "' and '" + dependsOnBean + "'"); + throw new BeanCreationException(mbd.getResourceDescription(), beanName, + "Circular depends-on relationship between '" + beanName + "' and '" + dependsOnBean + "'"); } registerDependentBean(dependsOnBean, beanName); getBean(dependsOnBean); @@ -1274,7 +1274,7 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp // Check validity of the usage of the args parameter. This can // only be used for prototypes constructed via a factory method. if (args != null && !mbd.isPrototype()) { - throw new BeanDefinitionStoreException( + throw new BeanDefinitionStoreException(mbd.getResourceDescription(), beanName, "Can only specify arguments for the getBean method when referring to a prototype bean definition"); } } @@ -1625,8 +1625,7 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp * instantiation within this class is performed by this method. * @param beanName the name of the bean * @param mbd the merged bean definition for the bean - * @param args arguments to use if creating a prototype using explicit arguments to a - * static factory method. This parameter must be {@code null} except in this case. + * @param args arguments to use if creating a prototype using explicit arguments * @return a new instance of the bean * @throws BeanCreationException if the bean could not be created */ diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/CglibSubclassingInstantiationStrategy.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/CglibSubclassingInstantiationStrategy.java index 740be138d72..82265bfd25a 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/CglibSubclassingInstantiationStrategy.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/CglibSubclassingInstantiationStrategy.java @@ -25,7 +25,6 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.BeanInstantiationException; import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.BeanFactory; - import org.springframework.cglib.core.SpringNamingPolicy; import org.springframework.cglib.proxy.Callback; import org.springframework.cglib.proxy.CallbackFilter; @@ -89,14 +88,13 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt */ private static class CglibSubclassCreator { - private static final Class[] CALLBACK_TYPES = new Class[] { NoOp.class, - LookupOverrideMethodInterceptor.class, ReplaceOverrideMethodInterceptor.class }; + private static final Class[] CALLBACK_TYPES = new Class[] + {NoOp.class, LookupOverrideMethodInterceptor.class, ReplaceOverrideMethodInterceptor.class}; private final RootBeanDefinition beanDefinition; private final BeanFactory owner; - CglibSubclassCreator(RootBeanDefinition beanDefinition, BeanFactory owner) { this.beanDefinition = beanDefinition; this.owner = owner; @@ -113,7 +111,6 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt */ Object instantiate(Constructor ctor, Object[] args) { Class subclass = createEnhancedSubclass(this.beanDefinition); - Object instance; if (ctor == null) { instance = BeanUtils.instantiate(subclass); @@ -123,19 +120,17 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt Constructor enhancedSubclassConstructor = subclass.getConstructor(ctor.getParameterTypes()); instance = enhancedSubclassConstructor.newInstance(args); } - catch (Exception e) { + catch (Exception ex) { throw new BeanInstantiationException(this.beanDefinition.getBeanClass(), String.format( - "Failed to invoke construcor for CGLIB enhanced subclass [%s]", subclass.getName()), e); + "Failed to invoke constructor for CGLIB enhanced subclass [%s]", subclass.getName()), ex); } } - // SPR-10785: set callbacks directly on the instance instead of in the // enhanced class (via the Enhancer) in order to avoid memory leaks. Factory factory = (Factory) instance; - factory.setCallbacks(new Callback[] { NoOp.INSTANCE,// - new LookupOverrideMethodInterceptor(beanDefinition, owner),// - new ReplaceOverrideMethodInterceptor(beanDefinition, owner) }); - + factory.setCallbacks(new Callback[] {NoOp.INSTANCE, + new LookupOverrideMethodInterceptor(this.beanDefinition, this.owner), + new ReplaceOverrideMethodInterceptor(this.beanDefinition, this.owner)}); return instance; } @@ -153,6 +148,7 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt } } + /** * Class providing hashCode and equals methods required by CGLIB to * ensure that CGLIB doesn't generate a distinct class per bean. @@ -162,7 +158,6 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt private final RootBeanDefinition beanDefinition; - CglibIdentitySupport(RootBeanDefinition beanDefinition) { this.beanDefinition = beanDefinition; } @@ -173,8 +168,8 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt @Override public boolean equals(Object other) { - return other.getClass().equals(this.getClass()) - && ((CglibIdentitySupport) other).getBeanDefinition().equals(this.getBeanDefinition()); + return (getClass().equals(other.getClass()) && + this.beanDefinition.equals(((CglibIdentitySupport) other).beanDefinition)); } @Override @@ -183,6 +178,7 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt } } + /** * CGLIB callback for filtering method interception behavior. */ @@ -190,7 +186,6 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt private static final Log logger = LogFactory.getLog(MethodOverrideCallbackFilter.class); - MethodOverrideCallbackFilter(RootBeanDefinition beanDefinition) { super(beanDefinition); } @@ -210,11 +205,12 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt else if (methodOverride instanceof ReplaceOverride) { return METHOD_REPLACER; } - throw new UnsupportedOperationException("Unexpected MethodOverride subclass: " - + methodOverride.getClass().getName()); + throw new UnsupportedOperationException("Unexpected MethodOverride subclass: " + + methodOverride.getClass().getName()); } } + /** * CGLIB MethodInterceptor to override methods, replacing them with an * implementation that returns a bean looked up in the container. @@ -223,7 +219,6 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt private final BeanFactory owner; - LookupOverrideMethodInterceptor(RootBeanDefinition beanDefinition, BeanFactory owner) { super(beanDefinition); this.owner = owner; @@ -245,7 +240,6 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt private final BeanFactory owner; - ReplaceOverrideMethodInterceptor(RootBeanDefinition beanDefinition, BeanFactory owner) { super(beanDefinition); this.owner = owner; @@ -255,7 +249,7 @@ public class CglibSubclassingInstantiationStrategy extends SimpleInstantiationSt public Object intercept(Object obj, Method method, Object[] args, MethodProxy mp) throws Throwable { ReplaceOverride ro = (ReplaceOverride) getBeanDefinition().getMethodOverrides().getOverride(method); // TODO could cache if a singleton for minor performance optimization - MethodReplacer mr = owner.getBean(ro.getMethodReplacerBeanName(), MethodReplacer.class); + MethodReplacer mr = this.owner.getBean(ro.getMethodReplacerBeanName(), MethodReplacer.class); return mr.reimplement(obj, method, args); } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/LookupOverride.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/LookupOverride.java index 75077729d55..4d4f62f782a 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/LookupOverride.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/LookupOverride.java @@ -37,8 +37,7 @@ public class LookupOverride extends MethodOverride { /** * Construct a new LookupOverride. - * @param methodName the name of the method to override. - * This method must have no arguments. + * @param methodName the name of the method to override * @param beanName the name of the bean in the current BeanFactory * that the overridden method should return */ @@ -48,6 +47,7 @@ public class LookupOverride extends MethodOverride { this.beanName = beanName; } + /** * Return the name of the bean that should be returned by this method. */ @@ -63,10 +63,6 @@ public class LookupOverride extends MethodOverride { return (method.getName().equals(getMethodName()) && method.getParameterTypes().length == 0); } - @Override - public String toString() { - return "LookupOverride for method '" + getMethodName() + "'; will return bean '" + this.beanName + "'"; - } @Override public boolean equals(Object other) { @@ -79,4 +75,9 @@ public class LookupOverride extends MethodOverride { return (29 * super.hashCode() + ObjectUtils.nullSafeHashCode(this.beanName)); } + @Override + public String toString() { + return "LookupOverride for method '" + getMethodName() + "'"; + } + } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/MethodOverride.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/MethodOverride.java index 75bb2e2194f..bf11af7eb51 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/MethodOverride.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/MethodOverride.java @@ -52,6 +52,7 @@ public abstract class MethodOverride implements BeanMetadataElement { this.methodName = methodName; } + /** * Return the name of the method to be overridden. */ @@ -99,6 +100,7 @@ public abstract class MethodOverride implements BeanMetadataElement { */ public abstract boolean matches(Method method); + @Override public boolean equals(Object other) { if (this == other) { diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/ReplaceOverride.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/ReplaceOverride.java index e570a24c573..957d3d17795 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/ReplaceOverride.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/ReplaceOverride.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2014 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. @@ -52,6 +52,7 @@ public class ReplaceOverride extends MethodOverride { this.methodReplacerBeanName = methodReplacerBeanName; } + /** * Return the name of the bean implementing MethodReplacer. */ @@ -97,12 +98,6 @@ public class ReplaceOverride extends MethodOverride { } - @Override - public String toString() { - return "Replace override for method '" + getMethodName() + "; will call bean '" + - this.methodReplacerBeanName + "'"; - } - @Override public boolean equals(Object other) { if (!(other instanceof ReplaceOverride) || !super.equals(other)) { @@ -121,4 +116,9 @@ public class ReplaceOverride extends MethodOverride { return hashCode; } + @Override + public String toString() { + return "Replace override for method '" + getMethodName() + "'"; + } + } diff --git a/spring-context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTestTypes.java b/spring-context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTestTypes.java index a5bc212cedb..242cb21f7bc 100644 --- a/spring-context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTestTypes.java +++ b/spring-context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTestTypes.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -27,24 +27,26 @@ import java.util.Set; import javax.sql.DataSource; import org.springframework.beans.BeansException; -import org.springframework.tests.sample.beans.ITestBean; -import org.springframework.tests.sample.beans.IndexedTestBean; -import org.springframework.tests.sample.beans.TestBean; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.BeanNameAware; import org.springframework.beans.factory.DisposableBean; -import org.springframework.tests.sample.beans.factory.DummyFactory; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.beans.factory.support.MethodReplacer; +import org.springframework.tests.sample.beans.ITestBean; +import org.springframework.tests.sample.beans.IndexedTestBean; +import org.springframework.tests.sample.beans.TestBean; +import org.springframework.tests.sample.beans.factory.DummyFactory; /** * Types used by {@link XmlBeanFactoryTests} and its attendant XML config files. * * @author Chris Beams */ -final class XmlBeanFactoryTestTypes { } +final class XmlBeanFactoryTestTypes { +} + /** * Simple bean used to check constructor dependency checking. @@ -174,18 +176,12 @@ abstract class ConstructorInjectedOverrides { return this.tb; } - protected abstract FactoryMethods createFactoryMethods(); - /** - * @return Returns the setterString. - */ public String getSetterString() { return setterString; } - /** - * @param setterString The setterString to set. - */ + public void setSetterString(String setterString) { this.setterString = setterString; } @@ -229,7 +225,6 @@ class DerivedConstructorDependenciesBean extends ConstructorDependenciesBean { private void destroy() { this.destroyed = true; } - } @@ -240,7 +235,6 @@ class DerivedConstructorDependenciesBean extends ConstructorDependenciesBean { interface DummyBo { void something(); - } @@ -258,9 +252,7 @@ class DummyBoImpl implements DummyBo { @Override public void something() { - } - } @@ -274,7 +266,6 @@ class DummyDao { public DummyDao(DataSource ds) { this.ds = ds; } - } @@ -290,7 +281,6 @@ class DummyReferencer { private DummyFactory dummyFactory; - public DummyReferencer() { } @@ -321,7 +311,6 @@ class DummyReferencer { public TestBean getTestBean2() { return testBean2; } - } @@ -370,13 +359,11 @@ class FactoryMethods { return Collections.EMPTY_LIST; } - private int num = 0; private String name = "default"; private TestBean tb; private String stringValue; - /** * Constructor is private: not for use outside this class, * even by IoC container. @@ -421,7 +408,6 @@ class FactoryMethods { public void setName(String name) { this.name = name; } - } /** @@ -436,7 +422,6 @@ class FixedMethodReplacer implements MethodReplacer { public Object reimplement(Object obj, Method method, Object[] args) throws Throwable { return VALUE; } - } @@ -469,22 +454,17 @@ class MethodReplaceCandidate { public String replaceMe(String echo) { return echo; } - } /** * Bean that exposes a simple property that can be set * to a mix of references and individual values. - * - * @author Rod Johnson - * @since 27.05.2003 */ class MixedCollectionBean { private Collection jumble; - public void setJumble(Collection jumble) { this.jumble = jumble; } @@ -492,7 +472,6 @@ class MixedCollectionBean { public Collection getJumble() { return jumble; } - } @@ -504,7 +483,6 @@ interface OverrideInterface { TestBean getPrototypeDependency(); TestBean getPrototypeDependency(Object someParam); - } @@ -521,7 +499,7 @@ abstract class OverrideOneMethod extends MethodReplaceCandidate implements Overr return new TestBean(); } - public TestBean invokesOverridenMethodOnSelf() { + public TestBean invokesOverriddenMethodOnSelf() { return getPrototypeDependency(); } @@ -563,7 +541,6 @@ abstract class OverrideOneMethodSubclass extends OverrideOneMethod { // This implementation does nothing! // It's not overloaded } - } @@ -591,7 +568,6 @@ class ProtectedLifecycleBean implements BeanNameAware, BeanFactoryAware, Initial protected boolean destroyed; - public void setInitMethodDeclared(boolean initMethodDeclared) { this.initMethodDeclared = initMethodDeclared; } @@ -707,7 +683,6 @@ class ProtectedLifecycleBean implements BeanNameAware, BeanFactoryAware, Initial return bean; } } - } @@ -722,7 +697,6 @@ class ReverseMethodReplacer implements MethodReplacer, Serializable { String s = (String) args[0]; return new StringBuffer(s).reverse().toString(); } - } @@ -733,7 +707,6 @@ class ReverseMethodReplacer implements MethodReplacer, Serializable { abstract class SerializableMethodReplacerCandidate extends MethodReplaceCandidate implements Serializable { //public abstract Point getPoint(); - } @@ -769,5 +742,4 @@ class SingleSimpleTypeConstructorBean { public String getTestString() { return testString; } - } diff --git a/spring-context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests.java b/spring-context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests.java index a095869e3db..94ec9db880c 100644 --- a/spring-context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests.java +++ b/spring-context/src/test/java/org/springframework/beans/factory/xml/XmlBeanFactoryTests.java @@ -1332,7 +1332,7 @@ public final class XmlBeanFactoryTests { // This differs from Spring's AOP support, which has a distinct notion // of a "target" object, meaning that the target needs explicit knowledge // of AOP proxying to invoke an advised method on itself. - TestBean jenny3 = oom.invokesOverridenMethodOnSelf(); + TestBean jenny3 = oom.invokesOverriddenMethodOnSelf(); assertEquals("Jenny", jenny3.getName()); assertNotSame(jenny1, jenny3); diff --git a/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java b/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java index 1bf528e9d27..87b7c6641af 100644 --- a/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java @@ -495,8 +495,7 @@ public abstract class ReflectionUtils { mc.doWith(method); } catch (IllegalAccessException ex) { - throw new IllegalStateException("Shouldn't be illegal to access method '" + method.getName() - + "': " + ex); + throw new IllegalStateException("Shouldn't be illegal to access method '" + method.getName() + "': " + ex); } } if (clazz.getSuperclass() != null) { @@ -607,8 +606,7 @@ public abstract class ReflectionUtils { fc.doWith(field); } catch (IllegalAccessException ex) { - throw new IllegalStateException( - "Shouldn't be illegal to access field '" + field.getName() + "': " + ex); + throw new IllegalStateException("Shouldn't be illegal to access field '" + field.getName() + "': " + ex); } } targetClass = targetClass.getSuperclass(); @@ -630,8 +628,8 @@ public abstract class ReflectionUtils { throw new IllegalArgumentException("Destination for field copy cannot be null"); } if (!src.getClass().isAssignableFrom(dest.getClass())) { - throw new IllegalArgumentException("Destination class [" + dest.getClass().getName() - + "] must be same or subclass as source class [" + src.getClass().getName() + "]"); + throw new IllegalArgumentException("Destination class [" + dest.getClass().getName() + + "] must be same or subclass as source class [" + src.getClass().getName() + "]"); } doWithFields(src.getClass(), new FieldCallback() { @Override