From b50bb5071ac353e77e9d38c72f888de3add923b2 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Fri, 7 Sep 2012 13:42:02 +0200 Subject: [PATCH] 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 --- .../beans/ExtendedBeanInfo.java | 72 +++++++---- .../beans/ExtendedBeanInfoTests.java | 112 ++++++++++++++++-- 2 files changed, 149 insertions(+), 35 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 211d1803ec..8d0fcf99c0 100644 --- a/spring-beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java +++ b/spring-beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java @@ -34,6 +34,7 @@ import java.util.TreeSet; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.BridgeMethodResolver; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; @@ -77,7 +78,7 @@ class ExtendedBeanInfo implements BeanInfo { ALL_METHODS: for (MethodDescriptor md : delegate.getMethodDescriptors()) { - Method method = md.getMethod(); + Method method = resolveMethod(md.getMethod()); // bypass non-getter java.lang.Class methods for efficiency if (ReflectionUtils.isObjectMethod(method) && !method.getName().startsWith("get")) { @@ -91,8 +92,8 @@ class ExtendedBeanInfo implements BeanInfo { continue ALL_METHODS; } for (PropertyDescriptor pd : delegate.getPropertyDescriptors()) { - Method readMethod = pd.getReadMethod(); - Method writeMethod = pd.getWriteMethod(); + Method readMethod = readMethodFor(pd); + Method writeMethod = writeMethodFor(pd); // has the setter already been found by the wrapped BeanInfo? if (writeMethod != null && writeMethod.getName().equals(method.getName())) { @@ -126,10 +127,10 @@ class ExtendedBeanInfo implements BeanInfo { continue DELEGATE_PD; } IndexedPropertyDescriptor ipd = (IndexedPropertyDescriptor) pd; - Method readMethod = ipd.getReadMethod(); - Method writeMethod = ipd.getWriteMethod(); - Method indexedReadMethod = ipd.getIndexedReadMethod(); - Method indexedWriteMethod = ipd.getIndexedWriteMethod(); + Method readMethod = readMethodFor(ipd); + Method writeMethod = writeMethodFor(ipd); + Method indexedReadMethod = indexedReadMethodFor(ipd); + Method indexedWriteMethod = indexedWriteMethodFor(ipd); // has the setter already been found by the wrapped BeanInfo? if (!(indexedWriteMethod != null && indexedWriteMethod.getName().equals(method.getName()))) { @@ -149,26 +150,26 @@ class ExtendedBeanInfo implements BeanInfo { for (PropertyDescriptor pd : delegate.getPropertyDescriptors()) { // have we already copied this read method to a property descriptor locally? String propertyName = pd.getName(); - Method readMethod = pd.getReadMethod(); + Method readMethod = readMethodFor(pd); Method mostSpecificReadMethod = ClassUtils.getMostSpecificMethod(readMethod, method.getDeclaringClass()); for (PropertyDescriptor existingPD : this.propertyDescriptors) { if (method.equals(mostSpecificReadMethod) && existingPD.getName().equals(propertyName)) { - if (existingPD.getReadMethod() == null) { + if (readMethodFor(existingPD) == null) { // no -> add it now - this.addOrUpdatePropertyDescriptor(pd, propertyName, method, pd.getWriteMethod()); + this.addOrUpdatePropertyDescriptor(pd, propertyName, method, writeMethodFor(pd)); } // yes -> do not add a duplicate continue ALL_METHODS; } } if (method.equals(mostSpecificReadMethod) - || (pd instanceof IndexedPropertyDescriptor && method.equals(((IndexedPropertyDescriptor) pd).getIndexedReadMethod()))) { + || (pd instanceof IndexedPropertyDescriptor && method.equals(indexedReadMethodFor((IndexedPropertyDescriptor) pd)))) { // yes -> copy it, including corresponding setter method (if any -- may be null) if (pd instanceof IndexedPropertyDescriptor) { - this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, pd.getWriteMethod(), ((IndexedPropertyDescriptor)pd).getIndexedReadMethod(), ((IndexedPropertyDescriptor)pd).getIndexedWriteMethod()); + this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, writeMethodFor(pd), indexedReadMethodFor((IndexedPropertyDescriptor)pd), indexedWriteMethodFor((IndexedPropertyDescriptor)pd)); } else { - this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, pd.getWriteMethod()); + this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, writeMethodFor(pd)); } continue ALL_METHODS; } @@ -176,6 +177,27 @@ class ExtendedBeanInfo implements BeanInfo { } } + + private static Method resolveMethod(Method method) { + return BridgeMethodResolver.findBridgedMethod(method); + } + + private static Method readMethodFor(PropertyDescriptor pd) { + return resolveMethod(pd.getReadMethod()); + } + + private static Method writeMethodFor(PropertyDescriptor pd) { + return resolveMethod(pd.getWriteMethod()); + } + + private static Method indexedReadMethodFor(IndexedPropertyDescriptor ipd) { + return resolveMethod(ipd.getIndexedReadMethod()); + } + + private static Method indexedWriteMethodFor(IndexedPropertyDescriptor ipd) { + return resolveMethod(ipd.getIndexedWriteMethod()); + } + private void addOrUpdatePropertyDescriptor(PropertyDescriptor pd, String propertyName, Method readMethod, Method writeMethod) throws IntrospectionException { addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, writeMethod, null, null); } @@ -186,9 +208,9 @@ class ExtendedBeanInfo implements BeanInfo { for (PropertyDescriptor existingPD : this.propertyDescriptors) { if (existingPD.getName().equals(propertyName)) { // is there already a descriptor that captures this read method or its corresponding write method? - if (existingPD.getReadMethod() != null) { - if (readMethod != null && existingPD.getReadMethod().getReturnType() != readMethod.getReturnType() - || writeMethod != null && existingPD.getReadMethod().getReturnType() != writeMethod.getParameterTypes()[0]) { + if (readMethodFor(existingPD) != null) { + if (readMethod != null && readMethodFor(existingPD).getReturnType() != readMethod.getReturnType() + || writeMethod != null && readMethodFor(existingPD).getReturnType() != writeMethod.getParameterTypes()[0]) { // no -> add a new descriptor for it below break; } @@ -205,9 +227,9 @@ class ExtendedBeanInfo implements BeanInfo { } // is there already a descriptor that captures this write method or its corresponding read method? - if (existingPD.getWriteMethod() != null) { - if (readMethod != null && existingPD.getWriteMethod().getParameterTypes()[0] != readMethod.getReturnType() - || writeMethod != null && existingPD.getWriteMethod().getParameterTypes()[0] != writeMethod.getParameterTypes()[0]) { + if (writeMethodFor(existingPD) != null) { + if (readMethod != null && writeMethodFor(existingPD).getParameterTypes()[0] != readMethod.getReturnType() + || writeMethod != null && writeMethodFor(existingPD).getParameterTypes()[0] != writeMethod.getParameterTypes()[0]) { // no -> add a new descriptor for it below break; } @@ -224,9 +246,9 @@ class ExtendedBeanInfo implements BeanInfo { IndexedPropertyDescriptor existingIPD = (IndexedPropertyDescriptor) existingPD; // is there already a descriptor that captures this indexed read method or its corresponding indexed write method? - if (existingIPD.getIndexedReadMethod() != null) { - if (indexedReadMethod != null && existingIPD.getIndexedReadMethod().getReturnType() != indexedReadMethod.getReturnType() - || indexedWriteMethod != null && existingIPD.getIndexedReadMethod().getReturnType() != indexedWriteMethod.getParameterTypes()[1]) { + if (indexedReadMethodFor(existingIPD) != null) { + if (indexedReadMethod != null && indexedReadMethodFor(existingIPD).getReturnType() != indexedReadMethod.getReturnType() + || indexedWriteMethod != null && indexedReadMethodFor(existingIPD).getReturnType() != indexedWriteMethod.getParameterTypes()[1]) { // no -> add a new descriptor for it below break; } @@ -243,9 +265,9 @@ class ExtendedBeanInfo implements BeanInfo { } // is there already a descriptor that captures this indexed write method or its corresponding indexed read method? - if (existingIPD.getIndexedWriteMethod() != null) { - if (indexedReadMethod != null && existingIPD.getIndexedWriteMethod().getParameterTypes()[1] != indexedReadMethod.getReturnType() - || indexedWriteMethod != null && existingIPD.getIndexedWriteMethod().getParameterTypes()[1] != indexedWriteMethod.getParameterTypes()[1]) { + if (indexedWriteMethodFor(existingIPD) != null) { + if (indexedReadMethod != null && indexedWriteMethodFor(existingIPD).getParameterTypes()[1] != indexedReadMethod.getReturnType() + || indexedWriteMethod != null && indexedWriteMethodFor(existingIPD).getParameterTypes()[1] != indexedWriteMethod.getParameterTypes()[1]) { // no -> add a new descriptor for it below break; } 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 2049039a03..5187ae2d9e 100644 --- a/spring-beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java @@ -16,29 +16,30 @@ package org.springframework.beans; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.lessThan; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.beans.BeanInfo; import java.beans.IndexedPropertyDescriptor; import java.beans.IntrospectionException; import java.beans.Introspector; import java.beans.PropertyDescriptor; + import java.lang.reflect.Method; +import org.junit.Ignore; import org.junit.Test; + import org.springframework.beans.ExtendedBeanInfo.PropertyDescriptorComparator; import org.springframework.core.JdkVersion; import org.springframework.util.ClassUtils; import test.beans.TestBean; +import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.lessThan; + /** * Unit tests for {@link ExtendedBeanInfo}. * @@ -149,7 +150,7 @@ public class ExtendedBeanInfoTests { ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); - assertThat(hasWriteMethodForProperty(bi, "foo"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "foo"), is(trueUntilJdk17())); assertThat(hasReadMethodForProperty(ebi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true)); @@ -163,6 +164,50 @@ public class ExtendedBeanInfoTests { fail("never matched write method"); } + @Test + public void cornerSpr9414() throws IntrospectionException { + @SuppressWarnings("unused") class Parent { + public Number getProperty1() { + return 1; + } + } + class Child extends Parent { + @Override + public Integer getProperty1() { + return 2; + } + } + { // always passes + ExtendedBeanInfo bi = new ExtendedBeanInfo(Introspector.getBeanInfo(Parent.class)); + assertThat(hasReadMethodForProperty(bi, "property1"), is(true)); + } + { // failed prior to fix for SPR-9414 + ExtendedBeanInfo bi = new ExtendedBeanInfo(Introspector.getBeanInfo(Child.class)); + assertThat(hasReadMethodForProperty(bi, "property1"), is(true)); + } + } + + interface Spr9453 { + T getProp(); + } + + @Test + public void cornerSpr9453() throws IntrospectionException { + final class Bean implements Spr9453> { + public Class getProp() { + return null; + } + } + { // always passes + BeanInfo info = Introspector.getBeanInfo(Bean.class); + assertThat(info.getPropertyDescriptors().length, equalTo(2)); + } + { // failed prior to fix for SPR-9453 + BeanInfo info = new ExtendedBeanInfo(Introspector.getBeanInfo(Bean.class)); + assertThat(info.getPropertyDescriptors().length, equalTo(2)); + } + } + @Test public void standardReadMethodInSuperclassAndNonStandardWriteMethodInSubclass() throws Exception { @SuppressWarnings("unused") class B { @@ -471,6 +516,53 @@ public class ExtendedBeanInfoTests { assertThat(hasIndexedWriteMethodForProperty(ebi, "foos"), is(true)); } + @Ignore // see comments at SPR-9702 + @Test + public void cornerSpr9702() throws IntrospectionException { + { // baseline with standard write method + @SuppressWarnings("unused") + class C { + // VOID-RETURNING, NON-INDEXED write method + public void setFoos(String[] foos) { } + // indexed read method + public String getFoos(int i) { return null; } + } + + BeanInfo bi = Introspector.getBeanInfo(C.class); + assertThat(hasReadMethodForProperty(bi, "foos"), is(false)); + assertThat(hasIndexedReadMethodForProperty(bi, "foos"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "foos"), is(true)); + assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(false)); + + BeanInfo ebi = Introspector.getBeanInfo(C.class); + assertThat(hasReadMethodForProperty(ebi, "foos"), is(false)); + assertThat(hasIndexedReadMethodForProperty(ebi, "foos"), is(true)); + assertThat(hasWriteMethodForProperty(ebi, "foos"), is(true)); + assertThat(hasIndexedWriteMethodForProperty(ebi, "foos"), is(false)); + } + { // variant with non-standard write method + @SuppressWarnings("unused") + class C { + // NON-VOID-RETURNING, NON-INDEXED write method + public C setFoos(String[] foos) { return this; } + // indexed read method + public String getFoos(int i) { return null; } + } + + BeanInfo bi = Introspector.getBeanInfo(C.class); + assertThat(hasReadMethodForProperty(bi, "foos"), is(false)); + assertThat(hasIndexedReadMethodForProperty(bi, "foos"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "foos"), is(false)); + assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(false)); + + BeanInfo ebi = new ExtendedBeanInfo(Introspector.getBeanInfo(C.class)); + assertThat(hasReadMethodForProperty(ebi, "foos"), is(false)); + assertThat(hasIndexedReadMethodForProperty(ebi, "foos"), is(true)); + assertThat(hasWriteMethodForProperty(ebi, "foos"), is(true)); + assertThat(hasIndexedWriteMethodForProperty(ebi, "foos"), is(false)); + } + } + @Test public void subclassWriteMethodWithCovariantReturnType() throws IntrospectionException { @SuppressWarnings("unused") class B {