From 26d5ef93e6ddb7d3ba5774b5e427b0dc6b61516f Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Thu, 20 Dec 2012 00:26:08 +0100 Subject: [PATCH] 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) --- .../beans/ExtendedBeanInfo.java | 13 ++++++++-- .../beans/ExtendedBeanInfoTests.java | 26 ++++++++++++------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java b/spring-beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java index 54467cd3ea..c3609c10c2 100644 --- a/spring-beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java +++ b/spring-beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.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() { + 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); diff --git a/spring-beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java b/spring-beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java index aaf6e56092..ec2ca31e2b 100644 --- a/spring-beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java @@ -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> { - @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; }