Skip to content

Commit ec3967a

Browse files
jhoellerjhoeller
committed
Consistent bridge method handling in annotation post-processors
Issue: SPR-12495 Issue: SPR-12187 (cherry picked from commit d97add0)
1 parent 4c5e17e commit ec3967a

File tree

7 files changed

+114
-72
lines changed

7 files changed

+114
-72
lines changed

‎spring-beans/src/main/java/org/springframework/beans/BeanUtils.java‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -379,13 +379,28 @@ public static PropertyDescriptor getPropertyDescriptor(Class<?> clazz, String pr
379379
* Find a JavaBeans {@code PropertyDescriptor} for the given method,
380380
* with the method either being the read method or the write method for
381381
* that bean property.
382-
* @param method the method to find a corresponding PropertyDescriptor for
382+
* @param method the method to find a corresponding PropertyDescriptor for,
383+
* introspecting its declaring class
383384
* @return the corresponding PropertyDescriptor, or {@code null} if none
384385
* @throws BeansException if PropertyDescriptor lookup fails
385386
*/
386387
public static PropertyDescriptor findPropertyForMethod(Method method) throws BeansException {
388+
return findPropertyForMethod(method, method.getDeclaringClass());
389+
}
390+
391+
/**
392+
* Find a JavaBeans {@code PropertyDescriptor} for the given method,
393+
* with the method either being the read method or the write method for
394+
* that bean property.
395+
* @param method the method to find a corresponding PropertyDescriptor for
396+
* @param clazz the (most specific) class to introspect for descriptors
397+
* @return the corresponding PropertyDescriptor, or {@code null} if none
398+
* @throws BeansException if PropertyDescriptor lookup fails
399+
* @since 3.2.13
400+
*/
401+
public static PropertyDescriptor findPropertyForMethod(Method method, Class<?> clazz) throws BeansException {
387402
Assert.notNull(method, "Method must not be null");
388-
PropertyDescriptor[] pds = getPropertyDescriptors(method.getDeclaringClass());
403+
PropertyDescriptor[] pds = getPropertyDescriptors(clazz);
389404
for (PropertyDescriptor pd : pds) {
390405
if (method.equals(pd.getReadMethod()) || method.equals(pd.getWriteMethod())) {
391406
return pd;
@@ -591,11 +606,11 @@ private static void copyProperties(Object source, Object target, Class<?> editab
591606
actualEditable = editable;
592607
}
593608
PropertyDescriptor[] targetPds = getPropertyDescriptors(actualEditable);
594-
List<String> ignoreList = (ignoreProperties != null) ? Arrays.asList(ignoreProperties) : null;
609+
List<String> ignoreList = (ignoreProperties != null ? Arrays.asList(ignoreProperties) : null);
595610

596611
for (PropertyDescriptor targetPd : targetPds) {
597612
Method writeMethod = targetPd.getWriteMethod();
598-
if (writeMethod != null && (ignoreProperties == null || (!ignoreList.contains(targetPd.getName())))) {
613+
if (writeMethod != null && (ignoreList == null || !ignoreList.contains(targetPd.getName()))) {
599614
PropertyDescriptor sourcePd = getPropertyDescriptor(source.getClass(), targetPd.getName());
600615
if (sourcePd != null) {
601616
Method readMethod = sourcePd.getReadMethod();

‎spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,8 @@ public Constructor<?>[] determineCandidateConstructors(Class<?> beanClass, Strin
233233
Constructor<?> requiredConstructor = null;
234234
Constructor<?> defaultConstructor = null;
235235
for (Constructor<?> candidate : rawCandidates) {
236-
Annotation annotation = findAutowiredAnnotation(candidate);
237-
if (annotation != null) {
236+
Annotation ann = findAutowiredAnnotation(candidate);
237+
if (ann != null) {
238238
if (requiredConstructor != null) {
239239
throw new BeanCreationException(beanName,
240240
"Invalid autowire-marked constructor: " + candidate +
@@ -245,7 +245,7 @@ public Constructor<?>[] determineCandidateConstructors(Class<?> beanClass, Strin
245245
throw new IllegalStateException(
246246
"Autowired annotation requires at least one argument: " + candidate);
247247
}
248-
boolean required = determineRequiredStatus(annotation);
248+
boolean required = determineRequiredStatus(ann);
249249
if (required) {
250250
if (!candidates.isEmpty()) {
251251
throw new BeanCreationException(beanName,
@@ -319,9 +319,9 @@ public void processInjection(Object bean) throws BeansException {
319319

320320

321321
private InjectionMetadata findAutowiringMetadata(String beanName, Class<?> clazz) {
322-
// Quick check on the concurrent map first, with minimal locking.
323322
// Fall back to class name as cache key, for backwards compatibility with custom callers.
324323
String cacheKey = (StringUtils.hasLength(beanName) ? beanName : clazz.getName());
324+
// Quick check on the concurrent map first, with minimal locking.
325325
InjectionMetadata metadata = this.injectionMetadataCache.get(cacheKey);
326326
if (InjectionMetadata.needsRefresh(metadata, clazz)) {
327327
synchronized (this.injectionMetadataCache) {
@@ -342,23 +342,25 @@ private InjectionMetadata buildAutowiringMetadata(Class<?> clazz) {
342342
do {
343343
LinkedList<InjectionMetadata.InjectedElement> currElements = new LinkedList<InjectionMetadata.InjectedElement>();
344344
for (Field field : targetClass.getDeclaredFields()) {
345-
Annotation annotation = findAutowiredAnnotation(field);
346-
if (annotation != null) {
345+
Annotation ann = findAutowiredAnnotation(field);
346+
if (ann != null) {
347347
if (Modifier.isStatic(field.getModifiers())) {
348348
if (logger.isWarnEnabled()) {
349349
logger.warn("Autowired annotation is not supported on static fields: " + field);
350350
}
351351
continue;
352352
}
353-
boolean required = determineRequiredStatus(annotation);
353+
boolean required = determineRequiredStatus(ann);
354354
currElements.add(new AutowiredFieldElement(field, required));
355355
}
356356
}
357357
for (Method method : targetClass.getDeclaredMethods()) {
358+
Annotation ann = null;
358359
Method bridgedMethod = BridgeMethodResolver.findBridgedMethod(method);
359-
Annotation annotation = BridgeMethodResolver.isVisibilityBridgeMethodPair(method, bridgedMethod) ?
360-
findAutowiredAnnotation(bridgedMethod) : findAutowiredAnnotation(method);
361-
if (annotation != null && method.equals(ClassUtils.getMostSpecificMethod(method, clazz))) {
360+
if (BridgeMethodResolver.isVisibilityBridgeMethodPair(method, bridgedMethod)) {
361+
ann = findAutowiredAnnotation(bridgedMethod);
362+
}
363+
if (ann != null && method.equals(ClassUtils.getMostSpecificMethod(method, clazz))) {
362364
if (Modifier.isStatic(method.getModifiers())) {
363365
if (logger.isWarnEnabled()) {
364366
logger.warn("Autowired annotation is not supported on static methods: " + method);
@@ -370,8 +372,8 @@ private InjectionMetadata buildAutowiringMetadata(Class<?> clazz) {
370372
logger.warn("Autowired annotation should be used on methods with actual parameters: " + method);
371373
}
372374
}
373-
boolean required = determineRequiredStatus(annotation);
374-
PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method);
375+
boolean required = determineRequiredStatus(ann);
376+
PropertyDescriptor pd = BeanUtils.findPropertyForMethod(bridgedMethod, clazz);
375377
currElements.add(new AutowiredMethodElement(method, required, pd));
376378
}
377379
}
@@ -385,9 +387,9 @@ private InjectionMetadata buildAutowiringMetadata(Class<?> clazz) {
385387

386388
private Annotation findAutowiredAnnotation(AccessibleObject ao) {
387389
for (Class<? extends Annotation> type : this.autowiredAnnotationTypes) {
388-
Annotation annotation = AnnotationUtils.getAnnotation(ao, type);
389-
if (annotation != null) {
390-
return annotation;
390+
Annotation ann = AnnotationUtils.getAnnotation(ao, type);
391+
if (ann != null) {
392+
return ann;
391393
}
392394
}
393395
return null;
@@ -412,21 +414,21 @@ protected <T> Map<String, T> findAutowireCandidates(Class<T> type) throws BeansE
412414
* <p>A 'required' dependency means that autowiring should fail when no beans
413415
* are found. Otherwise, the autowiring process will simply bypass the field
414416
* or method when no beans are found.
415-
* @param annotation the Autowired annotation
417+
* @param ann the Autowired annotation
416418
* @return whether the annotation indicates that a dependency is required
417419
*/
418-
protected boolean determineRequiredStatus(Annotation annotation) {
420+
protected boolean determineRequiredStatus(Annotation ann) {
419421
try {
420-
Method method = ReflectionUtils.findMethod(annotation.annotationType(), this.requiredParameterName);
422+
Method method = ReflectionUtils.findMethod(ann.annotationType(), this.requiredParameterName);
421423
if (method == null) {
422-
// annotations like @Inject and @Value don't have a method (attribute) named "required"
424+
// Annotations like @Inject and @Value don't have a method (attribute) named "required"
423425
// -> default to required status
424426
return true;
425427
}
426-
return (this.requiredParameterValue == (Boolean) ReflectionUtils.invokeMethod(method, annotation));
428+
return (this.requiredParameterValue == (Boolean) ReflectionUtils.invokeMethod(method, ann));
427429
}
428430
catch (Exception ex) {
429-
// an exception was thrown during reflective invocation of the required attribute
431+
// An exception was thrown during reflective invocation of the required attribute
430432
// -> default to required status
431433
return true;
432434
}

‎spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,9 @@ public PropertyValues postProcessPropertyValues(
311311

312312

313313
private InjectionMetadata findResourceMetadata(String beanName, final Class<?> clazz) {
314-
// Quick check on the concurrent map first, with minimal locking.
315314
// Fall back to class name as cache key, for backwards compatibility with custom callers.
316315
String cacheKey = (StringUtils.hasLength(beanName) ? beanName : clazz.getName());
316+
// Quick check on the concurrent map first, with minimal locking.
317317
InjectionMetadata metadata = this.injectionMetadataCache.get(cacheKey);
318318
if (InjectionMetadata.needsRefresh(metadata, clazz)) {
319319
synchronized (this.injectionMetadataCache) {
@@ -357,7 +357,7 @@ else if (field.isAnnotationPresent(Resource.class)) {
357357
if (method.getParameterTypes().length != 1) {
358358
throw new IllegalStateException("@WebServiceRef annotation requires a single-arg method: " + method);
359359
}
360-
PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method);
360+
PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method, clazz);
361361
currElements.add(new WebServiceRefElement(method, pd));
362362
}
363363
else if (ejbRefClass != null && method.isAnnotationPresent(ejbRefClass)) {
@@ -367,7 +367,7 @@ else if (ejbRefClass != null && method.isAnnotationPresent(ejbRefClass)) {
367367
if (method.getParameterTypes().length != 1) {
368368
throw new IllegalStateException("@EJB annotation requires a single-arg method: " + method);
369369
}
370-
PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method);
370+
PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method, clazz);
371371
currElements.add(new EjbRefElement(method, pd));
372372
}
373373
else if (method.isAnnotationPresent(Resource.class)) {
@@ -379,7 +379,7 @@ else if (method.isAnnotationPresent(Resource.class)) {
379379
throw new IllegalStateException("@Resource annotation requires a single-arg method: " + method);
380380
}
381381
if (!ignoredResourceTypes.contains(paramTypes[0].getName())) {
382-
PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method);
382+
PropertyDescriptor pd = BeanUtils.findPropertyForMethod(method, clazz);
383383
currElements.add(new ResourceElement(method, pd));
384384
}
385385
}

‎spring-context/src/test/java/org/springframework/beans/factory/annotation/BridgeMethodAutowiringTests.java‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,47 +16,51 @@
1616

1717
package org.springframework.beans.factory.annotation;
1818

19-
import static org.junit.Assert.assertNotNull;
20-
2119
import javax.inject.Inject;
2220
import javax.inject.Named;
2321

2422
import org.junit.Test;
23+
2524
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
2625
import org.springframework.stereotype.Component;
2726

27+
import static org.junit.Assert.*;
28+
2829
public class BridgeMethodAutowiringTests {
2930

3031
@Test
31-
public void SPR_8434() {
32+
public void SPR8434() {
3233
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
3334
ctx.register(UserServiceImpl.class, Foo.class);
3435
ctx.refresh();
3536
assertNotNull(ctx.getBean(UserServiceImpl.class).object);
3637
}
3738

38-
}
3939

40+
static abstract class GenericServiceImpl<D> {
4041

41-
abstract class GenericServiceImpl<D extends Object> {
42+
public abstract void setObject(D object);
43+
}
4244

43-
public abstract void setObject(D object);
4445

45-
}
46+
public static class UserServiceImpl extends GenericServiceImpl<Foo> {
4647

48+
protected Foo object;
4749

48-
class UserServiceImpl extends GenericServiceImpl<Foo> {
50+
@Override
51+
@Inject
52+
@Named("userObject")
53+
public void setObject(Foo object) {
54+
if (this.object != null) {
55+
throw new IllegalStateException("Already called");
56+
}
57+
this.object = object;
58+
}
59+
}
4960

50-
protected Foo object;
5161

52-
@Override
53-
@Inject
54-
@Named("userObject")
55-
public void setObject(Foo object) {
56-
this.object = object;
62+
@Component("userObject")
63+
public static class Foo {
5764
}
58-
}
59-
60-
@Component("userObject")
61-
class Foo { }
6265

66+
}

‎spring-context/src/test/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessorTests.java‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -25,11 +25,6 @@
2525
import org.junit.Test;
2626

2727
import org.springframework.beans.BeansException;
28-
import org.springframework.tests.mock.jndi.ExpectedLookupTemplate;
29-
import org.springframework.tests.sample.beans.INestedTestBean;
30-
import org.springframework.tests.sample.beans.ITestBean;
31-
import org.springframework.tests.sample.beans.NestedTestBean;
32-
import org.springframework.tests.sample.beans.TestBean;
3328
import org.springframework.beans.factory.BeanCreationException;
3429
import org.springframework.beans.factory.BeanFactory;
3530
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
@@ -42,6 +37,11 @@
4237
import org.springframework.beans.factory.support.RootBeanDefinition;
4338
import org.springframework.context.support.GenericApplicationContext;
4439
import org.springframework.jndi.support.SimpleJndiBeanFactory;
40+
import org.springframework.tests.mock.jndi.ExpectedLookupTemplate;
41+
import org.springframework.tests.sample.beans.INestedTestBean;
42+
import org.springframework.tests.sample.beans.ITestBean;
43+
import org.springframework.tests.sample.beans.NestedTestBean;
44+
import org.springframework.tests.sample.beans.TestBean;
4545
import org.springframework.util.SerializationTestUtils;
4646

4747
import static org.junit.Assert.*;
@@ -566,20 +566,20 @@ public TestBean getTestBean2() {
566566
}
567567

568568

569-
public static class ExtendedResourceInjectionBean extends ResourceInjectionBean {
569+
static class NonPublicResourceInjectionBean<B> extends ResourceInjectionBean {
570570

571571
@Resource(name="testBean4", type=TestBean.class)
572572
protected ITestBean testBean3;
573573

574-
private ITestBean testBean4;
574+
private B testBean4;
575575

576576
@Resource
577-
private INestedTestBean testBean5;
577+
INestedTestBean testBean5;
578578

579-
private INestedTestBean testBean6;
579+
INestedTestBean testBean6;
580580

581581
@Resource
582-
private BeanFactory beanFactory;
582+
BeanFactory beanFactory;
583583

584584
@Override
585585
@Resource
@@ -588,20 +588,26 @@ public void setTestBean2(TestBean testBean2) {
588588
}
589589

590590
@Resource(name="${tb}", type=ITestBean.class)
591-
private void setTestBean4(ITestBean testBean4) {
591+
private void setTestBean4(B testBean4) {
592+
if (this.testBean4 != null) {
593+
throw new IllegalStateException("Already called");
594+
}
592595
this.testBean4 = testBean4;
593596
}
594597

595598
@Resource
596599
public void setTestBean6(INestedTestBean testBean6) {
600+
if (this.testBean6 != null) {
601+
throw new IllegalStateException("Already called");
602+
}
597603
this.testBean6 = testBean6;
598604
}
599605

600606
public ITestBean getTestBean3() {
601607
return testBean3;
602608
}
603609

604-
public ITestBean getTestBean4() {
610+
public B getTestBean4() {
605611
return testBean4;
606612
}
607613

@@ -630,6 +636,10 @@ protected void destroy2() {
630636
}
631637

632638

639+
public static class ExtendedResourceInjectionBean extends NonPublicResourceInjectionBean<ITestBean> {
640+
}
641+
642+
633643
public static class ExtendedEjbInjectionBean extends ResourceInjectionBean {
634644

635645
@EJB(name="testBean4", beanInterface=TestBean.class)
@@ -653,11 +663,17 @@ public void setTestBean2(TestBean testBean2) {
653663

654664
@EJB(beanName="testBean3", beanInterface=ITestBean.class)
655665
private void setTestBean4(ITestBean testBean4) {
666+
if (this.testBean4 != null) {
667+
throw new IllegalStateException("Already called");
668+
}
656669
this.testBean4 = testBean4;
657670
}
658671

659672
@EJB
660673
public void setTestBean6(INestedTestBean testBean6) {
674+
if (this.testBean6 != null) {
675+
throw new IllegalStateException("Already called");
676+
}
661677
this.testBean6 = testBean6;
662678
}
663679

0 commit comments

Comments
 (0)