Skip to content

Commit b50bb50

Browse files
cbeamscbeams
committed
Address various ExtendedBeanInfo bugs
 - Ensure that ExtendedBeanInfoTests succeeds when building under JDK 7 - Improve handling of read and write method registration where generic interfaces are involved, per SPR-9453 - Add repro test for SPR-9702, in which EBI fails to register an indexed read method under certain circumstances Issue: SPR-9702, SPR-9414, SPR-9453
1 parent c795c1b commit b50bb50

File tree

2 files changed

+149
-35
lines changed

2 files changed

+149
-35
lines changed

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

Lines changed: 47 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
import org.apache.commons.logging.Log;
3636
import org.apache.commons.logging.LogFactory;
37+
import org.springframework.core.BridgeMethodResolver;
3738
import org.springframework.util.Assert;
3839
import org.springframework.util.ClassUtils;
3940
import org.springframework.util.ReflectionUtils;
@@ -77,7 +78,7 @@ public ExtendedBeanInfo(BeanInfo delegate) throws IntrospectionException {
7778

7879
ALL_METHODS:
7980
for (MethodDescriptor md : delegate.getMethodDescriptors()) {
80-
Method method = md.getMethod();
81+
Method method = resolveMethod(md.getMethod());
8182

8283
// bypass non-getter java.lang.Class methods for efficiency
8384
if (ReflectionUtils.isObjectMethod(method) && !method.getName().startsWith("get")) {
@@ -91,8 +92,8 @@ public ExtendedBeanInfo(BeanInfo delegate) throws IntrospectionException {
9192
continue ALL_METHODS;
9293
}
9394
for (PropertyDescriptor pd : delegate.getPropertyDescriptors()) {
94-
Method readMethod = pd.getReadMethod();
95-
Method writeMethod = pd.getWriteMethod();
95+
Method readMethod = readMethodFor(pd);
96+
Method writeMethod = writeMethodFor(pd);
9697
// has the setter already been found by the wrapped BeanInfo?
9798
if (writeMethod != null
9899
&& writeMethod.getName().equals(method.getName())) {
@@ -126,10 +127,10 @@ public ExtendedBeanInfo(BeanInfo delegate) throws IntrospectionException {
126127
continue DELEGATE_PD;
127128
}
128129
IndexedPropertyDescriptor ipd = (IndexedPropertyDescriptor) pd;
129-
Method readMethod = ipd.getReadMethod();
130-
Method writeMethod = ipd.getWriteMethod();
131-
Method indexedReadMethod = ipd.getIndexedReadMethod();
132-
Method indexedWriteMethod = ipd.getIndexedWriteMethod();
130+
Method readMethod = readMethodFor(ipd);
131+
Method writeMethod = writeMethodFor(ipd);
132+
Method indexedReadMethod = indexedReadMethodFor(ipd);
133+
Method indexedWriteMethod = indexedWriteMethodFor(ipd);
133134
// has the setter already been found by the wrapped BeanInfo?
134135
if (!(indexedWriteMethod != null
135136
&& indexedWriteMethod.getName().equals(method.getName()))) {
@@ -149,33 +150,54 @@ public ExtendedBeanInfo(BeanInfo delegate) throws IntrospectionException {
149150
for (PropertyDescriptor pd : delegate.getPropertyDescriptors()) {
150151
// have we already copied this read method to a property descriptor locally?
151152
String propertyName = pd.getName();
152-
Method readMethod = pd.getReadMethod();
153+
Method readMethod = readMethodFor(pd);
153154
Method mostSpecificReadMethod = ClassUtils.getMostSpecificMethod(readMethod, method.getDeclaringClass());
154155
for (PropertyDescriptor existingPD : this.propertyDescriptors) {
155156
if (method.equals(mostSpecificReadMethod)
156157
&& existingPD.getName().equals(propertyName)) {
157-
if (existingPD.getReadMethod() == null) {
158+
if (readMethodFor(existingPD) == null) {
158159
// no -> add it now
159-
this.addOrUpdatePropertyDescriptor(pd, propertyName, method, pd.getWriteMethod());
160+
this.addOrUpdatePropertyDescriptor(pd, propertyName, method, writeMethodFor(pd));
160161
}
161162
// yes -> do not add a duplicate
162163
continue ALL_METHODS;
163164
}
164165
}
165166
if (method.equals(mostSpecificReadMethod)
166-
|| (pd instanceof IndexedPropertyDescriptor && method.equals(((IndexedPropertyDescriptor) pd).getIndexedReadMethod()))) {
167+
|| (pd instanceof IndexedPropertyDescriptor && method.equals(indexedReadMethodFor((IndexedPropertyDescriptor) pd)))) {
167168
// yes -> copy it, including corresponding setter method (if any -- may be null)
168169
if (pd instanceof IndexedPropertyDescriptor) {
169-
this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, pd.getWriteMethod(), ((IndexedPropertyDescriptor)pd).getIndexedReadMethod(), ((IndexedPropertyDescriptor)pd).getIndexedWriteMethod());
170+
this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, writeMethodFor(pd), indexedReadMethodFor((IndexedPropertyDescriptor)pd), indexedWriteMethodFor((IndexedPropertyDescriptor)pd));
170171
} else {
171-
this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, pd.getWriteMethod());
172+
this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, writeMethodFor(pd));
172173
}
173174
continue ALL_METHODS;
174175
}
175176
}
176177
}
177178
}
178179

180+
181+
private static Method resolveMethod(Method method) {
182+
return BridgeMethodResolver.findBridgedMethod(method);
183+
}
184+
185+
private static Method readMethodFor(PropertyDescriptor pd) {
186+
return resolveMethod(pd.getReadMethod());
187+
}
188+
189+
private static Method writeMethodFor(PropertyDescriptor pd) {
190+
return resolveMethod(pd.getWriteMethod());
191+
}
192+
193+
private static Method indexedReadMethodFor(IndexedPropertyDescriptor ipd) {
194+
return resolveMethod(ipd.getIndexedReadMethod());
195+
}
196+
197+
private static Method indexedWriteMethodFor(IndexedPropertyDescriptor ipd) {
198+
return resolveMethod(ipd.getIndexedWriteMethod());
199+
}
200+
179201
private void addOrUpdatePropertyDescriptor(PropertyDescriptor pd, String propertyName, Method readMethod, Method writeMethod) throws IntrospectionException {
180202
addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, writeMethod, null, null);
181203
}
@@ -186,9 +208,9 @@ private void addOrUpdatePropertyDescriptor(PropertyDescriptor pd, String propert
186208
for (PropertyDescriptor existingPD : this.propertyDescriptors) {
187209
if (existingPD.getName().equals(propertyName)) {
188210
// is there already a descriptor that captures this read method or its corresponding write method?
189-
if (existingPD.getReadMethod() != null) {
190-
if (readMethod != null && existingPD.getReadMethod().getReturnType() != readMethod.getReturnType()
191-
|| writeMethod != null && existingPD.getReadMethod().getReturnType() != writeMethod.getParameterTypes()[0]) {
211+
if (readMethodFor(existingPD) != null) {
212+
if (readMethod != null && readMethodFor(existingPD).getReturnType() != readMethod.getReturnType()
213+
|| writeMethod != null && readMethodFor(existingPD).getReturnType() != writeMethod.getParameterTypes()[0]) {
192214
// no -> add a new descriptor for it below
193215
break;
194216
}
@@ -205,9 +227,9 @@ private void addOrUpdatePropertyDescriptor(PropertyDescriptor pd, String propert
205227
}
206228

207229
// is there already a descriptor that captures this write method or its corresponding read method?
208-
if (existingPD.getWriteMethod() != null) {
209-
if (readMethod != null && existingPD.getWriteMethod().getParameterTypes()[0] != readMethod.getReturnType()
210-
|| writeMethod != null && existingPD.getWriteMethod().getParameterTypes()[0] != writeMethod.getParameterTypes()[0]) {
230+
if (writeMethodFor(existingPD) != null) {
231+
if (readMethod != null && writeMethodFor(existingPD).getParameterTypes()[0] != readMethod.getReturnType()
232+
|| writeMethod != null && writeMethodFor(existingPD).getParameterTypes()[0] != writeMethod.getParameterTypes()[0]) {
211233
// no -> add a new descriptor for it below
212234
break;
213235
}
@@ -224,9 +246,9 @@ private void addOrUpdatePropertyDescriptor(PropertyDescriptor pd, String propert
224246
IndexedPropertyDescriptor existingIPD = (IndexedPropertyDescriptor) existingPD;
225247

226248
// is there already a descriptor that captures this indexed read method or its corresponding indexed write method?
227-
if (existingIPD.getIndexedReadMethod() != null) {
228-
if (indexedReadMethod != null && existingIPD.getIndexedReadMethod().getReturnType() != indexedReadMethod.getReturnType()
229-
|| indexedWriteMethod != null && existingIPD.getIndexedReadMethod().getReturnType() != indexedWriteMethod.getParameterTypes()[1]) {
249+
if (indexedReadMethodFor(existingIPD) != null) {
250+
if (indexedReadMethod != null && indexedReadMethodFor(existingIPD).getReturnType() != indexedReadMethod.getReturnType()
251+
|| indexedWriteMethod != null && indexedReadMethodFor(existingIPD).getReturnType() != indexedWriteMethod.getParameterTypes()[1]) {
230252
// no -> add a new descriptor for it below
231253
break;
232254
}
@@ -243,9 +265,9 @@ private void addOrUpdatePropertyDescriptor(PropertyDescriptor pd, String propert
243265
}
244266

245267
// is there already a descriptor that captures this indexed write method or its corresponding indexed read method?
246-
if (existingIPD.getIndexedWriteMethod() != null) {
247-
if (indexedReadMethod != null && existingIPD.getIndexedWriteMethod().getParameterTypes()[1] != indexedReadMethod.getReturnType()
248-
|| indexedWriteMethod != null && existingIPD.getIndexedWriteMethod().getParameterTypes()[1] != indexedWriteMethod.getParameterTypes()[1]) {
268+
if (indexedWriteMethodFor(existingIPD) != null) {
269+
if (indexedReadMethod != null && indexedWriteMethodFor(existingIPD).getParameterTypes()[1] != indexedReadMethod.getReturnType()
270+
|| indexedWriteMethod != null && indexedWriteMethodFor(existingIPD).getParameterTypes()[1] != indexedWriteMethod.getParameterTypes()[1]) {
249271
// no -> add a new descriptor for it below
250272
break;
251273
}

‎spring-beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java

Lines changed: 102 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,29 +16,30 @@
1616

1717
package org.springframework.beans;
1818

19-
import static org.hamcrest.CoreMatchers.equalTo;
20-
import static org.hamcrest.CoreMatchers.instanceOf;
21-
import static org.hamcrest.CoreMatchers.is;
22-
import static org.hamcrest.Matchers.greaterThan;
23-
import static org.hamcrest.Matchers.lessThan;
24-
import static org.junit.Assert.assertThat;
25-
import static org.junit.Assert.assertTrue;
26-
import static org.junit.Assert.fail;
27-
2819
import java.beans.BeanInfo;
2920
import java.beans.IndexedPropertyDescriptor;
3021
import java.beans.IntrospectionException;
3122
import java.beans.Introspector;
3223
import java.beans.PropertyDescriptor;
24+
3325
import java.lang.reflect.Method;
3426

27+
import org.junit.Ignore;
3528
import org.junit.Test;
29+
3630
import org.springframework.beans.ExtendedBeanInfo.PropertyDescriptorComparator;
3731
import org.springframework.core.JdkVersion;
3832
import org.springframework.util.ClassUtils;
3933

4034
import test.beans.TestBean;
4135

36+
import static org.junit.Assert.*;
37+
import static org.hamcrest.CoreMatchers.equalTo;
38+
import static org.hamcrest.CoreMatchers.instanceOf;
39+
import static org.hamcrest.CoreMatchers.is;
40+
import static org.hamcrest.Matchers.greaterThan;
41+
import static org.hamcrest.Matchers.lessThan;
42+
4243
/**
4344
* Unit tests for {@link ExtendedBeanInfo}.
4445
*
@@ -149,7 +150,7 @@ public void standardReadMethodsAndOverloadedNonStandardWriteMethods() throws Exc
149150
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
150151

151152
assertThat(hasReadMethodForProperty(bi, "foo"), is(true));
152-
assertThat(hasWriteMethodForProperty(bi, "foo"), is(true));
153+
assertThat(hasWriteMethodForProperty(bi, "foo"), is(trueUntilJdk17()));
153154

154155
assertThat(hasReadMethodForProperty(ebi, "foo"), is(true));
155156
assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true));
@@ -163,6 +164,50 @@ public void standardReadMethodsAndOverloadedNonStandardWriteMethods() throws Exc
163164
fail("never matched write method");
164165
}
165166

167+
@Test
168+
public void cornerSpr9414() throws IntrospectionException {
169+
@SuppressWarnings("unused") class Parent {
170+
public Number getProperty1() {
171+
return 1;
172+
}
173+
}
174+
class Child extends Parent {
175+
@Override
176+
public Integer getProperty1() {
177+
return 2;
178+
}
179+
}
180+
{ // always passes
181+
ExtendedBeanInfo bi = new ExtendedBeanInfo(Introspector.getBeanInfo(Parent.class));
182+
assertThat(hasReadMethodForProperty(bi, "property1"), is(true));
183+
}
184+
{ // failed prior to fix for SPR-9414
185+
ExtendedBeanInfo bi = new ExtendedBeanInfo(Introspector.getBeanInfo(Child.class));
186+
assertThat(hasReadMethodForProperty(bi, "property1"), is(true));
187+
}
188+
}
189+
190+
interface Spr9453<T> {
191+
T getProp();
192+
}
193+
194+
@Test
195+
public void cornerSpr9453() throws IntrospectionException {
196+
final class Bean implements Spr9453<Class<?>> {
197+
public Class<?> getProp() {
198+
return null;
199+
}
200+
}
201+
{ // always passes
202+
BeanInfo info = Introspector.getBeanInfo(Bean.class);
203+
assertThat(info.getPropertyDescriptors().length, equalTo(2));
204+
}
205+
{ // failed prior to fix for SPR-9453
206+
BeanInfo info = new ExtendedBeanInfo(Introspector.getBeanInfo(Bean.class));
207+
assertThat(info.getPropertyDescriptors().length, equalTo(2));
208+
}
209+
}
210+
166211
@Test
167212
public void standardReadMethodInSuperclassAndNonStandardWriteMethodInSubclass() throws Exception {
168213
@SuppressWarnings("unused") class B {
@@ -471,6 +516,53 @@ class C {
471516
assertThat(hasIndexedWriteMethodForProperty(ebi, "foos"), is(true));
472517
}
473518

519+
@Ignore // see comments at SPR-9702
520+
@Test
521+
public void cornerSpr9702() throws IntrospectionException {
522+
{ // baseline with standard write method
523+
@SuppressWarnings("unused")
524+
class C {
525+
// VOID-RETURNING, NON-INDEXED write method
526+
public void setFoos(String[] foos) { }
527+
// indexed read method
528+
public String getFoos(int i) { return null; }
529+
}
530+
531+
BeanInfo bi = Introspector.getBeanInfo(C.class);
532+
assertThat(hasReadMethodForProperty(bi, "foos"), is(false));
533+
assertThat(hasIndexedReadMethodForProperty(bi, "foos"), is(true));
534+
assertThat(hasWriteMethodForProperty(bi, "foos"), is(true));
535+
assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(false));
536+
537+
BeanInfo ebi = Introspector.getBeanInfo(C.class);
538+
assertThat(hasReadMethodForProperty(ebi, "foos"), is(false));
539+
assertThat(hasIndexedReadMethodForProperty(ebi, "foos"), is(true));
540+
assertThat(hasWriteMethodForProperty(ebi, "foos"), is(true));
541+
assertThat(hasIndexedWriteMethodForProperty(ebi, "foos"), is(false));
542+
}
543+
{ // variant with non-standard write method
544+
@SuppressWarnings("unused")
545+
class C {
546+
// NON-VOID-RETURNING, NON-INDEXED write method
547+
public C setFoos(String[] foos) { return this; }
548+
// indexed read method
549+
public String getFoos(int i) { return null; }
550+
}
551+
552+
BeanInfo bi = Introspector.getBeanInfo(C.class);
553+
assertThat(hasReadMethodForProperty(bi, "foos"), is(false));
554+
assertThat(hasIndexedReadMethodForProperty(bi, "foos"), is(true));
555+
assertThat(hasWriteMethodForProperty(bi, "foos"), is(false));
556+
assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(false));
557+
558+
BeanInfo ebi = new ExtendedBeanInfo(Introspector.getBeanInfo(C.class));
559+
assertThat(hasReadMethodForProperty(ebi, "foos"), is(false));
560+
assertThat(hasIndexedReadMethodForProperty(ebi, "foos"), is(true));
561+
assertThat(hasWriteMethodForProperty(ebi, "foos"), is(true));
562+
assertThat(hasIndexedWriteMethodForProperty(ebi, "foos"), is(false));
563+
}
564+
}
565+
474566
@Test
475567
public void subclassWriteMethodWithCovariantReturnType() throws IntrospectionException {
476568
@SuppressWarnings("unused") class B {

0 commit comments

Comments
 (0)