Skip to content

Commit a316163

Browse files
committed
Backport "Use ExtendedBeanInfo on an as-needed basis only"
Prior to this change, CachedIntrospectionResults delegated to ExtendedBeanInfo by default in order to inspect JavaBean PropertyDescriptor information for bean classes. Originally introduced with SPR-8079, ExtendedBeanInfo was designed to go beyond the capabilities of the default JavaBeans Introspector in order to support non-void returning setter methods, principally to support use of builder-style APIs within Spring XML. This is a complex affair, and the non-trivial logic in ExtendedBeanInfo has led to various bugs including regressions for bean classes that do not declare non-void returning setters. Many of these issues were fixed when overhauling non-void JavaBean write method support in SPR-10029, however it appears that some extremely subtle issues may still remain. ExtendedBeanInfo was taken out of use completely in the 3.2.x line with SPR-9677 and the new BeanInfoFactory mechanism. This support was not backported to 3.1.x, however, in the interest of stability. This commit, then, inlines the conditional logic introduced by BeanInfoFactory directly into CachedIntrospectionResults. The net effect is that ExtendedBeanInfo is out of the code path entirely except for bean classes that actually contain 'candidate' methods, i.e. non-void and/or static write methods. This commit also includes the changes made in SPR-10115. Issue: SPR-9723, SPR-10029, SPR-9677, SPR-8079
1 parent e731746 commit a316163

File tree

4 files changed

+64
-12
lines changed

4 files changed

+64
-12
lines changed

org.springframework.beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.beans.PropertyDescriptor;
2323
import java.lang.ref.Reference;
2424
import java.lang.ref.WeakReference;
25+
import java.lang.reflect.Method;
2526
import java.util.Collections;
2627
import java.util.HashSet;
2728
import java.util.Iterator;
@@ -32,7 +33,6 @@
3233

3334
import org.apache.commons.logging.Log;
3435
import org.apache.commons.logging.LogFactory;
35-
3636
import org.springframework.util.ClassUtils;
3737
import org.springframework.util.StringUtils;
3838

@@ -214,7 +214,15 @@ private CachedIntrospectionResults(Class beanClass) throws BeansException {
214214
if (logger.isTraceEnabled()) {
215215
logger.trace("Getting BeanInfo for class [" + beanClass.getName() + "]");
216216
}
217-
this.beanInfo = new ExtendedBeanInfo(Introspector.getBeanInfo(beanClass));
217+
BeanInfo originalBeanInfo = Introspector.getBeanInfo(beanClass);
218+
BeanInfo extendedBeanInfo = null;
219+
for (Method method : beanClass.getMethods()) {
220+
if (ExtendedBeanInfo.isCandidateWriteMethod(method)) {
221+
extendedBeanInfo = new ExtendedBeanInfo(originalBeanInfo);
222+
break;
223+
}
224+
}
225+
this.beanInfo = extendedBeanInfo != null ? extendedBeanInfo : originalBeanInfo;
218226

219227
// Immediately remove class from Introspector cache, to allow for proper
220228
// garbage collection on class loader shutdown - we cache it here anyway,

org.springframework.beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java

Lines changed: 13 additions & 10 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-2013 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.
@@ -42,8 +42,8 @@
4242

4343
/**
4444
* Decorator for a standard {@link BeanInfo} object, e.g. as created by
45-
* {@link Introspector#getBeanInfo(Class)}, designed to discover and register non-void
46-
* returning setter methods. For example:
45+
* {@link Introspector#getBeanInfo(Class)}, designed to discover and register static
46+
* and/or non-void returning setter methods. For example:
4747
* <pre>{@code
4848
* public class Bean {
4949
* private Foo foo;
@@ -102,17 +102,17 @@ public ExtendedBeanInfo(BeanInfo delegate) throws IntrospectionException {
102102
new SimpleNonIndexedPropertyDescriptor(pd));
103103
}
104104

105-
for (Method method : findNonVoidWriteMethods(delegate.getMethodDescriptors())) {
106-
handleNonVoidWriteMethod(method);
105+
for (Method method : findCandidateWriteMethods(delegate.getMethodDescriptors())) {
106+
handleCandidateWriteMethod(method);
107107
}
108108
}
109109

110110

111-
private List<Method> findNonVoidWriteMethods(MethodDescriptor[] methodDescriptors) {
111+
private List<Method> findCandidateWriteMethods(MethodDescriptor[] methodDescriptors) {
112112
List<Method> matches = new ArrayList<Method>();
113113
for (MethodDescriptor methodDescriptor : methodDescriptors) {
114114
Method method = methodDescriptor.getMethod();
115-
if (isNonVoidWriteMethod(method)) {
115+
if (isCandidateWriteMethod(method)) {
116116
matches.add(method);
117117
}
118118
}
@@ -127,20 +127,23 @@ public int compare(Method m1, Method m2) {
127127
return matches;
128128
}
129129

130-
public static boolean isNonVoidWriteMethod(Method method) {
130+
public static boolean isCandidateWriteMethod(Method method) {
131131
String methodName = method.getName();
132132
Class<?>[] parameterTypes = method.getParameterTypes();
133133
int nParams = parameterTypes.length;
134134
if (methodName.length() > 3 && methodName.startsWith("set") &&
135135
Modifier.isPublic(method.getModifiers()) &&
136-
!void.class.isAssignableFrom(method.getReturnType()) &&
136+
(
137+
!void.class.isAssignableFrom(method.getReturnType()) ||
138+
Modifier.isStatic(method.getModifiers())
139+
) &&
137140
(nParams == 1 || (nParams == 2 && parameterTypes[0].equals(int.class)))) {
138141
return true;
139142
}
140143
return false;
141144
}
142145

143-
private void handleNonVoidWriteMethod(Method method) throws IntrospectionException {
146+
private void handleCandidateWriteMethod(Method method) throws IntrospectionException {
144147
int nParams = method.getParameterTypes().length;
145148
String propertyName = propertyNameFor(method);
146149
Class<?> propertyType = method.getParameterTypes()[nParams-1];

org.springframework.beans/src/test/java/org/springframework/beans/BeanWrapperTests.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1541,6 +1541,24 @@ public void testWildcardedGenericEnum() {
15411541
assertEquals(TestEnum.TEST_VALUE, consumer.getEnumValue());
15421542
}
15431543

1544+
@Test
1545+
public void cornerSpr10115() {
1546+
Spr10115Bean foo = new Spr10115Bean();
1547+
BeanWrapperImpl bwi = new BeanWrapperImpl();
1548+
bwi.setWrappedInstance(foo);
1549+
bwi.setPropertyValue("prop1", "val1");
1550+
assertEquals("val1", Spr10115Bean.prop1);
1551+
}
1552+
1553+
1554+
static class Spr10115Bean {
1555+
private static String prop1;
1556+
1557+
public static void setProp1(String prop1) {
1558+
Spr10115Bean.prop1 = prop1;
1559+
}
1560+
}
1561+
15441562

15451563
private static class Foo {
15461564

org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -959,4 +959,27 @@ public void setAddress(int index, String addr) { }
959959
}
960960
}
961961

962+
@Test
963+
public void shouldSupportStaticWriteMethod() throws IntrospectionException {
964+
{
965+
BeanInfo bi = Introspector.getBeanInfo(WithStaticWriteMethod.class);
966+
assertThat(hasReadMethodForProperty(bi, "prop1"), is(false));
967+
assertThat(hasWriteMethodForProperty(bi, "prop1"), is(false));
968+
assertThat(hasIndexedReadMethodForProperty(bi, "prop1"), is(false));
969+
assertThat(hasIndexedWriteMethodForProperty(bi, "prop1"), is(false));
970+
}
971+
{
972+
BeanInfo bi = new ExtendedBeanInfo(Introspector.getBeanInfo(WithStaticWriteMethod.class));
973+
assertThat(hasReadMethodForProperty(bi, "prop1"), is(false));
974+
assertThat(hasWriteMethodForProperty(bi, "prop1"), is(true));
975+
assertThat(hasIndexedReadMethodForProperty(bi, "prop1"), is(false));
976+
assertThat(hasIndexedWriteMethodForProperty(bi, "prop1"), is(false));
977+
}
978+
}
979+
980+
static class WithStaticWriteMethod {
981+
@SuppressWarnings("unused")
982+
public static void setProp1(String prop1) {
983+
}
984+
}
962985
}

0 commit comments

Comments
 (0)