Handle non-void write methods deterministically

This change resolves a specific issue with processing
java.math.BigDecimal via ExtendedBeanInfo. BigDecimal has a particular
constellation of #setScale methods that, prior to this change, had the
potential to cause ExtendedBeanInfo to throw an IntrospectionException
depending on the order in which the methods were processed.

Because JDK 7 no longer returns deterministic results from
Class#getDeclaredMethods, it became a genuine possibility - indeed a
statistical certainty that the 'wrong' setScale method handling order
happens sooner or later. Typically one could observe this failure once
out of every four test runs.

This commit introduces deterministic method ordering of all discovered
non-void returning write methods in such a way that solves the problem
for BigDecimal as well as for any other class having a similar method
arrangement.

Also:

 - Remove unnecessary cast

 - Pass no method information to PropertyDescriptor superclasses when
   invoking super(...). This ensures that any 'type mismatch'
   IntrospectionExceptions are handled locally in ExtendedBeanInfo and
   its Simple* PropertyDescriptor variants where we have full control.

Issue: SPR-10111, SPR-9702
Backport-Commit: aa3e0be (forward-ported via cherry-pick from 3.1.x)
master
Chris Beams 12 years ago
parent 6888a6f286
commit 26d5ef93e6
  1. 13
      spring-beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java
  2. 26
      spring-beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java

@ -31,6 +31,7 @@ import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.Enumeration;
import java.util.List;
@ -116,6 +117,14 @@ class ExtendedBeanInfo implements BeanInfo {
matches.add(method);
}
}
// sort non-void returning write methods to guard against the ill effects of
// non-deterministic sorting of methods returned from Class#getDeclaredMethods
// under JDK 7. See http://bugs.sun.com/view_bug.do?bug_id=7023180
Collections.sort(matches, new Comparator<Method>() {
public int compare(Method m1, Method m2) {
return m2.toString().compareTo(m1.toString());
}
});
return matches;
}
@ -264,7 +273,7 @@ class SimpleNonIndexedPropertyDescriptor extends PropertyDescriptor {
public SimpleNonIndexedPropertyDescriptor(String propertyName,
Method readMethod, Method writeMethod) throws IntrospectionException {
super(propertyName, readMethod, writeMethod);
super(propertyName, null, null);
this.setReadMethod(readMethod);
this.setWriteMethod(writeMethod);
this.propertyType = findPropertyType(readMethod, writeMethod);
@ -353,7 +362,7 @@ class SimpleIndexedPropertyDescriptor extends IndexedPropertyDescriptor {
Method indexedReadMethod, Method indexedWriteMethod)
throws IntrospectionException {
super(propertyName, readMethod, writeMethod, indexedReadMethod, indexedWriteMethod);
super(propertyName, null, null, null, null);
this.setReadMethod(readMethod);
this.setWriteMethod(writeMethod);
this.propertyType = findPropertyType(readMethod, writeMethod);

@ -23,6 +23,7 @@ import java.beans.Introspector;
import java.beans.PropertyDescriptor;
import java.lang.reflect.Method;
import java.math.BigDecimal;
import org.junit.Test;
@ -192,7 +193,6 @@ public class ExtendedBeanInfoTests {
}
}
class Child extends Parent {
@Override
public Integer getProperty1() {
return 2;
}
@ -214,7 +214,6 @@ public class ExtendedBeanInfoTests {
@Test
public void cornerSpr9453() throws IntrospectionException {
final class Bean implements Spr9453<Class<?>> {
@Override
public Class<?> getProp() {
return null;
}
@ -583,6 +582,20 @@ public class ExtendedBeanInfoTests {
}
}
/**
* Prior to SPR-10111 (a follow-up fix for SPR-9702), this method would throw an
* IntrospectionException regarding a "type mismatch between indexed and non-indexed
* methods" intermittently (approximately one out of every four times) under JDK 7
* due to non-deterministic results from {@link Class#getDeclaredMethods()}.
* See http://bugs.sun.com/view_bug.do?bug_id=7023180
* @see #cornerSpr9702()
*/
@Test
public void cornerSpr10111() throws Exception {
new ExtendedBeanInfo(Introspector.getBeanInfo(BigDecimal.class));
}
@Test
public void subclassWriteMethodWithCovariantReturnType() throws IntrospectionException {
@SuppressWarnings("unused") class B {
@ -590,9 +603,7 @@ public class ExtendedBeanInfoTests {
public Number setFoo(String foo) { return null; }
}
class C extends B {
@Override
public String getFoo() { return null; }
@Override
public Integer setFoo(String foo) { return null; }
}
@ -695,7 +706,7 @@ public class ExtendedBeanInfoTests {
for (PropertyDescriptor pd : ebi.getPropertyDescriptors()) {
if (pd.getName().equals("foo")) {
assertThat(pd.getWriteMethod(), is(C.class.getMethod("setFoo", int.class)));
assertThat(pd.getWriteMethod(), is(C.class.getMethod("setFoo", String.class)));
return;
}
}
@ -733,7 +744,7 @@ public class ExtendedBeanInfoTests {
assertThat(hasReadMethodForProperty(ebi, "dateFormat"), is(false));
assertThat(hasWriteMethodForProperty(ebi, "dateFormat"), is(true));
assertThat(hasIndexedReadMethodForProperty(ebi, "dateFormat"), is(false));
assertThat(hasIndexedWriteMethodForProperty(ebi, "dateFormat"), is(true));
assertThat(hasIndexedWriteMethodForProperty(ebi, "dateFormat"), is(trueUntilJdk17()));
}
@Test
@ -864,7 +875,6 @@ public class ExtendedBeanInfoTests {
}
interface TextBookOperations extends BookOperations {
@Override
TextBook getBook();
}
@ -874,7 +884,6 @@ public class ExtendedBeanInfoTests {
}
class LawLibrary extends Library implements TextBookOperations {
@Override
public LawBook getBook() { return null; }
}
@ -889,7 +898,6 @@ public class ExtendedBeanInfoTests {
}
class B extends A {
@Override
public boolean isTargetMethod() {
return false;
}

Loading…
Cancel
Save