From ec3967a6c7e071d17cf1a65e7a9892214e81ed36 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 22 Dec 2014 13:44:01 +0100 Subject: [PATCH] Consistent bridge method handling in annotation post-processors Issue: SPR-12495 Issue: SPR-12187 (cherry picked from commit d97add0) --- .../org/springframework/beans/BeanUtils.java | 25 +++++++-- .../AutowiredAnnotationBeanPostProcessor.java | 44 +++++++-------- .../CommonAnnotationBeanPostProcessor.java | 8 +-- .../BridgeMethodAutowiringTests.java | 54 ++++++++++--------- ...ommonAnnotationBeanPostProcessorTests.java | 42 ++++++++++----- ...ersistenceAnnotationBeanPostProcessor.java | 18 +++---- .../support/PersistenceInjectionTests.java | 7 ++- 7 files changed, 120 insertions(+), 78 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java index 84c8abebf5..d18b1ed1ea 100644 --- a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.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. @@ -379,13 +379,28 @@ public abstract class BeanUtils { * Find a JavaBeans {@code PropertyDescriptor} for the given method, * with the method either being the read method or the write method for * that bean property. - * @param method the method to find a corresponding PropertyDescriptor for + * @param method the method to find a corresponding PropertyDescriptor for, + * introspecting its declaring class * @return the corresponding PropertyDescriptor, or {@code null} if none * @throws BeansException if PropertyDescriptor lookup fails */ public static PropertyDescriptor findPropertyForMethod(Method method) throws BeansException { + return findPropertyForMethod(method, method.getDeclaringClass()); + } + + /** + * Find a JavaBeans {@code PropertyDescriptor} for the given method, + * with the method either being the read method or the write method for + * that bean property. + * @param method the method to find a corresponding PropertyDescriptor for + * @param clazz the (most specific) class to introspect for descriptors + * @return the corresponding PropertyDescriptor, or {@code null} if none + * @throws BeansException if PropertyDescriptor lookup fails + * @since 3.2.13 + */ + public static PropertyDescriptor findPropertyForMethod(Method method, Class clazz) throws BeansException { Assert.notNull(method, "Method must not be null"); - PropertyDescriptor[] pds = getPropertyDescriptors(method.getDeclaringClass()); + PropertyDescriptor[] pds = getPropertyDescriptors(clazz); for (PropertyDescriptor pd : pds) { if (method.equals(pd.getReadMethod()) || method.equals(pd.getWriteMethod())) { return pd; @@ -591,11 +606,11 @@ public abstract class BeanUtils { actualEditable = editable; } PropertyDescriptor[] targetPds = getPropertyDescriptors(actualEditable); - List ignoreList = (ignoreProperties != null) ? Arrays.asList(ignoreProperties) : null; + List ignoreList = (ignoreProperties != null ? Arrays.asList(ignoreProperties) : null); for (PropertyDescriptor targetPd : targetPds) { Method writeMethod = targetPd.getWriteMethod(); - if (writeMethod != null && (ignoreProperties == null || (!ignoreList.contains(targetPd.getName())))) { + if (writeMethod != null && (ignoreList == null || !ignoreList.contains(targetPd.getName()))) { PropertyDescriptor sourcePd = getPropertyDescriptor(source.getClass(), targetPd.getName()); if (sourcePd != null) { Method readMethod = sourcePd.getReadMethod(); 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 bc7e56b60b..024470ae7f 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 @@ -233,8 +233,8 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean Constructor requiredConstructor = null; Constructor defaultConstructor = null; for (Constructor candidate : rawCandidates) { - Annotation annotation = findAutowiredAnnotation(candidate); - if (annotation != null) { + Annotation ann = findAutowiredAnnotation(candidate); + if (ann != null) { if (requiredConstructor != null) { throw new BeanCreationException(beanName, "Invalid autowire-marked constructor: " + candidate + @@ -245,7 +245,7 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean throw new IllegalStateException( "Autowired annotation requires at least one argument: " + candidate); } - boolean required = determineRequiredStatus(annotation); + boolean required = determineRequiredStatus(ann); if (required) { if (!candidates.isEmpty()) { throw new BeanCreationException(beanName, @@ -319,9 +319,9 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean private InjectionMetadata findAutowiringMetadata(String beanName, Class clazz) { - // Quick check on the concurrent map first, with minimal locking. // Fall back to class name as cache key, for backwards compatibility with custom callers. String cacheKey = (StringUtils.hasLength(beanName) ? beanName : clazz.getName()); + // Quick check on the concurrent map first, with minimal locking. InjectionMetadata metadata = this.injectionMetadataCache.get(cacheKey); if (InjectionMetadata.needsRefresh(metadata, clazz)) { synchronized (this.injectionMetadataCache) { @@ -342,23 +342,25 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean do { LinkedList currElements = new LinkedList(); for (Field field : targetClass.getDeclaredFields()) { - Annotation annotation = findAutowiredAnnotation(field); - if (annotation != null) { + Annotation ann = findAutowiredAnnotation(field); + if (ann != null) { if (Modifier.isStatic(field.getModifiers())) { if (logger.isWarnEnabled()) { logger.warn("Autowired annotation is not supported on static fields: " + field); } continue; } - boolean required = determineRequiredStatus(annotation); + boolean required = determineRequiredStatus(ann); currElements.add(new AutowiredFieldElement(field, required)); } } for (Method method : targetClass.getDeclaredMethods()) { + Annotation ann = null; Method bridgedMethod = BridgeMethodResolver.findBridgedMethod(method); - Annotation annotation = BridgeMethodResolver.isVisibilityBridgeMethodPair(method, bridgedMethod) ? - findAutowiredAnnotation(bridgedMethod) : findAutowiredAnnotation(method); - if (annotation != null && method.equals(ClassUtils.getMostSpecificMethod(method, clazz))) { + if (BridgeMethodResolver.isVisibilityBridgeMethodPair(method, bridgedMethod)) { + ann = findAutowiredAnnotation(bridgedMethod); + } + if (ann != null && method.equals(ClassUtils.getMostSpecificMethod(method, clazz))) { if (Modifier.isStatic(method.getModifiers())) { if (logger.isWarnEnabled()) { logger.warn("Autowired annotation is not supported on static methods: " + method); @@ -370,8 +372,8 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean logger.warn("Autowired annotation should be used on methods with actual parameters: " + method); } } - boolean required = determineRequiredStatus(annotation); - PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method); + boolean required = determineRequiredStatus(ann); + PropertyDescriptor pd = BeanUtils.findPropertyForMethod(bridgedMethod, clazz); currElements.add(new AutowiredMethodElement(method, required, pd)); } } @@ -385,9 +387,9 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean private Annotation findAutowiredAnnotation(AccessibleObject ao) { for (Class type : this.autowiredAnnotationTypes) { - Annotation annotation = AnnotationUtils.getAnnotation(ao, type); - if (annotation != null) { - return annotation; + Annotation ann = AnnotationUtils.getAnnotation(ao, type); + if (ann != null) { + return ann; } } return null; @@ -412,21 +414,21 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean *

A 'required' dependency means that autowiring should fail when no beans * are found. Otherwise, the autowiring process will simply bypass the field * or method when no beans are found. - * @param annotation the Autowired annotation + * @param ann the Autowired annotation * @return whether the annotation indicates that a dependency is required */ - protected boolean determineRequiredStatus(Annotation annotation) { + protected boolean determineRequiredStatus(Annotation ann) { try { - Method method = ReflectionUtils.findMethod(annotation.annotationType(), this.requiredParameterName); + Method method = ReflectionUtils.findMethod(ann.annotationType(), this.requiredParameterName); if (method == null) { - // annotations like @Inject and @Value don't have a method (attribute) named "required" + // Annotations like @Inject and @Value don't have a method (attribute) named "required" // -> default to required status return true; } - return (this.requiredParameterValue == (Boolean) ReflectionUtils.invokeMethod(method, annotation)); + return (this.requiredParameterValue == (Boolean) ReflectionUtils.invokeMethod(method, ann)); } catch (Exception ex) { - // an exception was thrown during reflective invocation of the required attribute + // An exception was thrown during reflective invocation of the required attribute // -> default to required status return true; } diff --git a/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java b/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java index e7aec9e286..95f72e0117 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java @@ -311,9 +311,9 @@ public class CommonAnnotationBeanPostProcessor extends InitDestroyAnnotationBean private InjectionMetadata findResourceMetadata(String beanName, final Class clazz) { - // Quick check on the concurrent map first, with minimal locking. // Fall back to class name as cache key, for backwards compatibility with custom callers. String cacheKey = (StringUtils.hasLength(beanName) ? beanName : clazz.getName()); + // Quick check on the concurrent map first, with minimal locking. InjectionMetadata metadata = this.injectionMetadataCache.get(cacheKey); if (InjectionMetadata.needsRefresh(metadata, clazz)) { synchronized (this.injectionMetadataCache) { @@ -357,7 +357,7 @@ public class CommonAnnotationBeanPostProcessor extends InitDestroyAnnotationBean if (method.getParameterTypes().length != 1) { throw new IllegalStateException("@WebServiceRef annotation requires a single-arg method: " + method); } - PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method); + PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method, clazz); currElements.add(new WebServiceRefElement(method, pd)); } else if (ejbRefClass != null && method.isAnnotationPresent(ejbRefClass)) { @@ -367,7 +367,7 @@ public class CommonAnnotationBeanPostProcessor extends InitDestroyAnnotationBean if (method.getParameterTypes().length != 1) { throw new IllegalStateException("@EJB annotation requires a single-arg method: " + method); } - PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method); + PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method, clazz); currElements.add(new EjbRefElement(method, pd)); } else if (method.isAnnotationPresent(Resource.class)) { @@ -379,7 +379,7 @@ public class CommonAnnotationBeanPostProcessor extends InitDestroyAnnotationBean throw new IllegalStateException("@Resource annotation requires a single-arg method: " + method); } if (!ignoredResourceTypes.contains(paramTypes[0].getName())) { - PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method); + PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method, clazz); currElements.add(new ResourceElement(method, pd)); } } diff --git a/spring-context/src/test/java/org/springframework/beans/factory/annotation/BridgeMethodAutowiringTests.java b/spring-context/src/test/java/org/springframework/beans/factory/annotation/BridgeMethodAutowiringTests.java index c69ab5658b..381c76d970 100644 --- a/spring-context/src/test/java/org/springframework/beans/factory/annotation/BridgeMethodAutowiringTests.java +++ b/spring-context/src/test/java/org/springframework/beans/factory/annotation/BridgeMethodAutowiringTests.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. @@ -16,47 +16,51 @@ package org.springframework.beans.factory.annotation; -import static org.junit.Assert.assertNotNull; - import javax.inject.Inject; import javax.inject.Named; import org.junit.Test; + import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.stereotype.Component; +import static org.junit.Assert.*; + public class BridgeMethodAutowiringTests { @Test - public void SPR_8434() { + public void SPR8434() { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); ctx.register(UserServiceImpl.class, Foo.class); ctx.refresh(); assertNotNull(ctx.getBean(UserServiceImpl.class).object); } -} + static abstract class GenericServiceImpl { -abstract class GenericServiceImpl { - - public abstract void setObject(D object); - -} - - -class UserServiceImpl extends GenericServiceImpl { - - protected Foo object; - - @Override - @Inject - @Named("userObject") - public void setObject(Foo object) { - this.object = object; + public abstract void setObject(D object); } + + + public static class UserServiceImpl extends GenericServiceImpl { + + protected Foo object; + + @Override + @Inject + @Named("userObject") + public void setObject(Foo object) { + if (this.object != null) { + throw new IllegalStateException("Already called"); + } + this.object = object; + } + } + + + @Component("userObject") + public static class Foo { + } + } - -@Component("userObject") -class Foo { } - diff --git a/spring-context/src/test/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessorTests.java b/spring-context/src/test/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessorTests.java index e2c1cc3b57..c44662b418 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessorTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessorTests.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. @@ -25,11 +25,6 @@ import javax.ejb.EJB; import org.junit.Test; import org.springframework.beans.BeansException; -import org.springframework.tests.mock.jndi.ExpectedLookupTemplate; -import org.springframework.tests.sample.beans.INestedTestBean; -import org.springframework.tests.sample.beans.ITestBean; -import org.springframework.tests.sample.beans.NestedTestBean; -import org.springframework.tests.sample.beans.TestBean; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.NoSuchBeanDefinitionException; @@ -42,6 +37,11 @@ import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.context.support.GenericApplicationContext; import org.springframework.jndi.support.SimpleJndiBeanFactory; +import org.springframework.tests.mock.jndi.ExpectedLookupTemplate; +import org.springframework.tests.sample.beans.INestedTestBean; +import org.springframework.tests.sample.beans.ITestBean; +import org.springframework.tests.sample.beans.NestedTestBean; +import org.springframework.tests.sample.beans.TestBean; import org.springframework.util.SerializationTestUtils; import static org.junit.Assert.*; @@ -566,20 +566,20 @@ public class CommonAnnotationBeanPostProcessorTests { } - public static class ExtendedResourceInjectionBean extends ResourceInjectionBean { + static class NonPublicResourceInjectionBean extends ResourceInjectionBean { @Resource(name="testBean4", type=TestBean.class) protected ITestBean testBean3; - private ITestBean testBean4; + private B testBean4; @Resource - private INestedTestBean testBean5; + INestedTestBean testBean5; - private INestedTestBean testBean6; + INestedTestBean testBean6; @Resource - private BeanFactory beanFactory; + BeanFactory beanFactory; @Override @Resource @@ -588,12 +588,18 @@ public class CommonAnnotationBeanPostProcessorTests { } @Resource(name="${tb}", type=ITestBean.class) - private void setTestBean4(ITestBean testBean4) { + private void setTestBean4(B testBean4) { + if (this.testBean4 != null) { + throw new IllegalStateException("Already called"); + } this.testBean4 = testBean4; } @Resource public void setTestBean6(INestedTestBean testBean6) { + if (this.testBean6 != null) { + throw new IllegalStateException("Already called"); + } this.testBean6 = testBean6; } @@ -601,7 +607,7 @@ public class CommonAnnotationBeanPostProcessorTests { return testBean3; } - public ITestBean getTestBean4() { + public B getTestBean4() { return testBean4; } @@ -630,6 +636,10 @@ public class CommonAnnotationBeanPostProcessorTests { } + public static class ExtendedResourceInjectionBean extends NonPublicResourceInjectionBean { + } + + public static class ExtendedEjbInjectionBean extends ResourceInjectionBean { @EJB(name="testBean4", beanInterface=TestBean.class) @@ -653,11 +663,17 @@ public class CommonAnnotationBeanPostProcessorTests { @EJB(beanName="testBean3", beanInterface=ITestBean.class) private void setTestBean4(ITestBean testBean4) { + if (this.testBean4 != null) { + throw new IllegalStateException("Already called"); + } this.testBean4 = testBean4; } @EJB public void setTestBean6(INestedTestBean testBean6) { + if (this.testBean6 != null) { + throw new IllegalStateException("Already called"); + } this.testBean6 = testBean6; } diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java b/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java index ce5fb245bd..fc5cfbc746 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java @@ -50,6 +50,7 @@ import org.springframework.beans.factory.config.DestructionAwareBeanPostProcesso import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessor; import org.springframework.beans.factory.support.MergedBeanDefinitionPostProcessor; import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.core.BridgeMethodResolver; import org.springframework.core.Ordered; import org.springframework.core.PriorityOrdered; import org.springframework.jndi.JndiLocatorDelegate; @@ -361,9 +362,9 @@ public class PersistenceAnnotationBeanPostProcessor private InjectionMetadata findPersistenceMetadata(String beanName, final Class clazz) { - // Quick check on the concurrent map first, with minimal locking. // Fall back to class name as cache key, for backwards compatibility with custom callers. String cacheKey = (StringUtils.hasLength(beanName) ? beanName : clazz.getName()); + // Quick check on the concurrent map first, with minimal locking. InjectionMetadata metadata = this.injectionMetadataCache.get(cacheKey); if (InjectionMetadata.needsRefresh(metadata, clazz)) { synchronized (this.injectionMetadataCache) { @@ -375,9 +376,8 @@ public class PersistenceAnnotationBeanPostProcessor do { LinkedList currElements = new LinkedList(); for (Field field : targetClass.getDeclaredFields()) { - PersistenceContext pc = field.getAnnotation(PersistenceContext.class); - PersistenceUnit pu = field.getAnnotation(PersistenceUnit.class); - if (pc != null || pu != null) { + if (field.isAnnotationPresent(PersistenceContext.class) || + field.isAnnotationPresent(PersistenceUnit.class)) { if (Modifier.isStatic(field.getModifiers())) { throw new IllegalStateException("Persistence annotations are not supported on static fields"); } @@ -385,17 +385,17 @@ public class PersistenceAnnotationBeanPostProcessor } } for (Method method : targetClass.getDeclaredMethods()) { - PersistenceContext pc = method.getAnnotation(PersistenceContext.class); - PersistenceUnit pu = method.getAnnotation(PersistenceUnit.class); - if ((pc != null || pu != null) && - method.equals(ClassUtils.getMostSpecificMethod(method, clazz))) { + method = BridgeMethodResolver.findBridgedMethod(method); + if ((method.isAnnotationPresent(PersistenceContext.class) || + method.isAnnotationPresent(PersistenceUnit.class)) && + method.equals(BridgeMethodResolver.findBridgedMethod(ClassUtils.getMostSpecificMethod(method, clazz)))) { if (Modifier.isStatic(method.getModifiers())) { throw new IllegalStateException("Persistence annotations are not supported on static methods"); } if (method.getParameterTypes().length != 1) { throw new IllegalStateException("Persistence annotation requires a single-arg method: " + method); } - PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method); + PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method, clazz); currElements.add(new PersistenceElement(method, pd)); } } diff --git a/spring-orm/src/test/java/org/springframework/orm/jpa/support/PersistenceInjectionTests.java b/spring-orm/src/test/java/org/springframework/orm/jpa/support/PersistenceInjectionTests.java index 04c4ce0ac3..0bbc6e6eb5 100644 --- a/spring-orm/src/test/java/org/springframework/orm/jpa/support/PersistenceInjectionTests.java +++ b/spring-orm/src/test/java/org/springframework/orm/jpa/support/PersistenceInjectionTests.java @@ -773,7 +773,7 @@ public class PersistenceInjectionTests extends AbstractEntityManagerFactoryBeanT @SuppressWarnings("serial") - public static class SpecificPublicPersistenceContextSetter extends DefaultPublicPersistenceContextSetter { + static class PublicPersistenceContextSetterOnNonPublicClass extends DefaultPublicPersistenceContextSetter { @Override @PersistenceContext(unitName="unit2", type = PersistenceContextType.EXTENDED) @@ -783,6 +783,11 @@ public class PersistenceInjectionTests extends AbstractEntityManagerFactoryBeanT } + @SuppressWarnings("serial") + public static class SpecificPublicPersistenceContextSetter extends PublicPersistenceContextSetterOnNonPublicClass { + } + + public static class DefaultPrivatePersistenceUnitField { @PersistenceUnit