diff --git a/org.springframework.beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java b/org.springframework.beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java index 6404685cd5..1b63f37a3d 100644 --- a/org.springframework.beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java +++ b/org.springframework.beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java @@ -16,6 +16,8 @@ package org.springframework.beans; +import static java.lang.String.format; + import java.awt.Image; import java.beans.BeanDescriptor; import java.beans.BeanInfo; @@ -30,6 +32,9 @@ import java.util.Comparator; import java.util.SortedSet; import java.util.TreeSet; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; @@ -48,15 +53,20 @@ import org.springframework.util.StringUtils; * @see CachedIntrospectionResults */ class ExtendedBeanInfo implements BeanInfo { + + private final Log logger = LogFactory.getLog(getClass()); + private final BeanInfo delegate; + private final SortedSet propertyDescriptors = new TreeSet(new PropertyDescriptorComparator()); + /** * Wrap the given delegate {@link BeanInfo} instance and find any non-void returning * setter methods, creating and adding a {@link PropertyDescriptor} for each. * - *

The wrapped {@code BeanInfo} is not modified in any way by this process. + *

Note that the wrapped {@code BeanInfo} is modified by this process. * * @see #getPropertyDescriptors() * @throws IntrospectionException if any problems occur creating and adding new {@code PropertyDescriptors} @@ -92,20 +102,20 @@ class ExtendedBeanInfo implements BeanInfo { if (writeMethod != null && writeMethod.getName().equals(method.getName())) { // yes -> copy it, including corresponding getter method (if any -- may be null) - this.addOrUpdatePropertyDescriptor(propertyName, readMethod, writeMethod); + this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, writeMethod); continue ALL_METHODS; } // has a getter corresponding to this setter already been found by the wrapped BeanInfo? if (readMethod != null && readMethod.getName().equals(getterMethodNameFor(propertyName)) && readMethod.getReturnType().equals(method.getParameterTypes()[0])) { - this.addOrUpdatePropertyDescriptor(propertyName, readMethod, method); + this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, method); continue ALL_METHODS; } } // the setter method was not found by the wrapped BeanInfo -> add a new PropertyDescriptor for it // no corresponding getter was detected, so the 'read method' parameter is null. - this.addOrUpdatePropertyDescriptor(propertyName, null, method); + this.addOrUpdatePropertyDescriptor(null, propertyName, null, method); continue ALL_METHODS; } @@ -129,20 +139,20 @@ class ExtendedBeanInfo implements BeanInfo { if (indexedWriteMethod != null && indexedWriteMethod.getName().equals(method.getName())) { // yes -> copy it, including corresponding getter method (if any -- may be null) - this.addOrUpdatePropertyDescriptor(propertyName, readMethod, writeMethod, indexedReadMethod, indexedWriteMethod); + this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, writeMethod, indexedReadMethod, indexedWriteMethod); continue ALL_METHODS; } // has a getter corresponding to this setter already been found by the wrapped BeanInfo? if (indexedReadMethod != null && indexedReadMethod.getName().equals(getterMethodNameFor(propertyName)) && indexedReadMethod.getReturnType().equals(method.getParameterTypes()[1])) { - this.addOrUpdatePropertyDescriptor(propertyName, readMethod, writeMethod, indexedReadMethod, method); + this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, writeMethod, indexedReadMethod, method); continue ALL_METHODS; } } // the INDEXED setter method was not found by the wrapped BeanInfo -> add a new PropertyDescriptor // for it. no corresponding INDEXED getter was detected, so the 'indexed read method' parameter is null. - this.addOrUpdatePropertyDescriptor(propertyName, null, null, null, method); + this.addOrUpdatePropertyDescriptor(null, propertyName, null, null, null, method); continue ALL_METHODS; } @@ -154,7 +164,7 @@ class ExtendedBeanInfo implements BeanInfo { && existingPD.getName().equals(pd.getName())) { if (existingPD.getReadMethod() == null) { // no -> add it now - this.addOrUpdatePropertyDescriptor(pd.getName(), method, pd.getWriteMethod()); + this.addOrUpdatePropertyDescriptor(pd, pd.getName(), method, pd.getWriteMethod()); } // yes -> do not add a duplicate continue ALL_METHODS; @@ -164,9 +174,9 @@ class ExtendedBeanInfo implements BeanInfo { || (pd instanceof IndexedPropertyDescriptor && method == ((IndexedPropertyDescriptor) pd).getIndexedReadMethod())) { // yes -> copy it, including corresponding setter method (if any -- may be null) if (pd instanceof IndexedPropertyDescriptor) { - this.addOrUpdatePropertyDescriptor(pd.getName(), pd.getReadMethod(), pd.getWriteMethod(), ((IndexedPropertyDescriptor)pd).getIndexedReadMethod(), ((IndexedPropertyDescriptor)pd).getIndexedWriteMethod()); + this.addOrUpdatePropertyDescriptor(pd, pd.getName(), pd.getReadMethod(), pd.getWriteMethod(), ((IndexedPropertyDescriptor)pd).getIndexedReadMethod(), ((IndexedPropertyDescriptor)pd).getIndexedWriteMethod()); } else { - this.addOrUpdatePropertyDescriptor(pd.getName(), pd.getReadMethod(), pd.getWriteMethod()); + this.addOrUpdatePropertyDescriptor(pd, pd.getName(), pd.getReadMethod(), pd.getWriteMethod()); } continue ALL_METHODS; } @@ -174,11 +184,13 @@ class ExtendedBeanInfo implements BeanInfo { } } - private void addOrUpdatePropertyDescriptor(String propertyName, Method readMethod, Method writeMethod) throws IntrospectionException { - addOrUpdatePropertyDescriptor(propertyName, readMethod, writeMethod, null, null); + private void addOrUpdatePropertyDescriptor(PropertyDescriptor pd, String propertyName, Method readMethod, Method writeMethod) throws IntrospectionException { + addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, writeMethod, null, null); } - private void addOrUpdatePropertyDescriptor(String propertyName, Method readMethod, Method writeMethod, Method indexedReadMethod, Method indexedWriteMethod) throws IntrospectionException { + private void addOrUpdatePropertyDescriptor(PropertyDescriptor pd, String propertyName, Method readMethod, Method writeMethod, Method indexedReadMethod, Method indexedWriteMethod) throws IntrospectionException { + Assert.notNull(propertyName, "propertyName may not be null"); + propertyName = pd == null ? propertyName : pd.getName(); 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? @@ -256,10 +268,32 @@ class ExtendedBeanInfo implements BeanInfo { } // we haven't yet seen read or write methods for this property -> add a new descriptor - if (indexedReadMethod == null && indexedWriteMethod == null) { - this.propertyDescriptors.add(new PropertyDescriptor(propertyName, readMethod, writeMethod)); - } else { - this.propertyDescriptors.add(new IndexedPropertyDescriptor(propertyName, readMethod, writeMethod, indexedReadMethod, indexedWriteMethod)); + if (pd == null) { + try { + if (indexedReadMethod == null && indexedWriteMethod == null) { + pd = new PropertyDescriptor(propertyName, readMethod, writeMethod); + } + else { + pd = new IndexedPropertyDescriptor(propertyName, readMethod, writeMethod, indexedReadMethod, indexedWriteMethod); + } + this.propertyDescriptors.add(pd); + } catch (IntrospectionException ex) { + logger.warn(format("Could not create new PropertyDescriptor for readMethod [%s] writeMethod [%s] " + + "indexedReadMethod [%s] indexedWriteMethod [%s] for property [%s]. Reason: %s", + readMethod, writeMethod, indexedReadMethod, indexedWriteMethod, propertyName, ex.getMessage())); + // suppress exception and attempt to continue + } + } + else { + pd.setReadMethod(readMethod); + try { + pd.setWriteMethod(writeMethod); + } catch (IntrospectionException ex) { + logger.warn(format("Could not add write method [%s] for property [%s]. Reason: %s", + writeMethod, propertyName, ex.getMessage())); + // fall through -> add property descriptor as best we can + } + this.propertyDescriptors.add(pd); } } diff --git a/org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java b/org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java index 9d18e7a0ec..144ce6f13a 100644 --- a/org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java +++ b/org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java @@ -16,6 +16,7 @@ package org.springframework.beans; +import static java.lang.String.format; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; @@ -29,7 +30,9 @@ 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; @@ -116,11 +119,15 @@ public class ExtendedBeanInfoTests { } BeanInfo bi = Introspector.getBeanInfo(C.class); - ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(bi, "foo"), is(false)); + ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); + + assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "foo"), is(true)); + assertThat(hasReadMethodForProperty(ebi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true)); } @@ -134,11 +141,15 @@ public class ExtendedBeanInfoTests { } BeanInfo bi = Introspector.getBeanInfo(C.class); - ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(bi, "foo"), is(false)); + ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); + + assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "foo"), is(true)); + assertThat(hasReadMethodForProperty(ebi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true)); @@ -161,11 +172,15 @@ public class ExtendedBeanInfoTests { } BeanInfo bi = Introspector.getBeanInfo(C.class); - ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(bi, "foo"), is(false)); + ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); + + assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "foo"), is(true)); + assertThat(hasReadMethodForProperty(ebi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true)); } @@ -200,7 +215,6 @@ public class ExtendedBeanInfoTests { assertThat(c.getBar(), is(42)); BeanInfo bi = Introspector.getBeanInfo(C.class); - ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(bi, "foo"), is(false)); @@ -208,6 +222,14 @@ public class ExtendedBeanInfoTests { assertThat(hasReadMethodForProperty(bi, "bar"), is(true)); assertThat(hasWriteMethodForProperty(bi, "bar"), is(false)); + ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); + + assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "foo"), is(true)); + + assertThat(hasReadMethodForProperty(bi, "bar"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "bar"), is(true)); + assertThat(hasReadMethodForProperty(ebi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true)); @@ -430,13 +452,19 @@ public class ExtendedBeanInfoTests { } BeanInfo bi = Introspector.getBeanInfo(C.class); - BeanInfo ebi = new ExtendedBeanInfo(Introspector.getBeanInfo(C.class)); assertThat(hasIndexedReadMethodForProperty(bi, "foos"), is(true)); assertThat(hasWriteMethodForProperty(bi, "foos"), is(false)); // again as above, standard Inspector picks up non-void return types on indexed write methods by default assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(true)); + BeanInfo ebi = new ExtendedBeanInfo(Introspector.getBeanInfo(C.class)); + + assertThat(hasIndexedReadMethodForProperty(bi, "foos"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "foos"), is(true)); + // again as above, standard Inspector picks up non-void return types on indexed write methods by default + assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(true)); + assertThat(hasIndexedReadMethodForProperty(ebi, "foos"), is(true)); assertThat(hasWriteMethodForProperty(ebi, "foos"), is(true)); assertThat(hasIndexedWriteMethodForProperty(ebi, "foos"), is(true)); @@ -454,11 +482,15 @@ public class ExtendedBeanInfoTests { } BeanInfo bi = Introspector.getBeanInfo(C.class); - ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(bi, "foo"), is(false)); + ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); + + assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "foo"), is(true)); + assertThat(hasReadMethodForProperty(ebi, "foo"), is(true)); assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true)); @@ -514,13 +546,19 @@ public class ExtendedBeanInfoTests { public Object setDateFormat(int dateStyle, int timeStyle) { return new Object(); } } BeanInfo bi = Introspector.getBeanInfo(C.class); - ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); assertThat(hasReadMethodForProperty(bi, "dateFormat"), is(false)); assertThat(hasWriteMethodForProperty(bi, "dateFormat"), is(false)); assertThat(hasIndexedReadMethodForProperty(bi, "dateFormat"), is(false)); assertThat(hasIndexedWriteMethodForProperty(bi, "dateFormat"), is(true)); + ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); + + assertThat(hasReadMethodForProperty(bi, "dateFormat"), is(false)); + assertThat(hasWriteMethodForProperty(bi, "dateFormat"), is(true)); + assertThat(hasIndexedReadMethodForProperty(bi, "dateFormat"), is(false)); + assertThat(hasIndexedWriteMethodForProperty(bi, "dateFormat"), is(true)); + assertThat(hasReadMethodForProperty(ebi, "dateFormat"), is(false)); assertThat(hasWriteMethodForProperty(ebi, "dateFormat"), is(true)); assertThat(hasIndexedReadMethodForProperty(ebi, "dateFormat"), is(false)); @@ -625,4 +663,75 @@ public class ExtendedBeanInfoTests { return false; } + @Test + public void reproSpr8806_y() throws IntrospectionException, SecurityException, NoSuchMethodException { + Introspector.getBeanInfo(LawLibrary.class); + } + + @Ignore @Test + public void reproSpr8806_x() throws IntrospectionException, SecurityException, NoSuchMethodException { + BeanInfo info = Introspector.getBeanInfo(LawLibrary.class); + for (PropertyDescriptor d : info.getPropertyDescriptors()) { + if (d.getName().equals("book")) { + Method readMethod = d.getReadMethod(); + Method writeMethod = d.getWriteMethod(); + System.out.println(format("READ : %s.%s (bridge:%s)", + readMethod.getDeclaringClass().getSimpleName(), readMethod.getName(), readMethod.isBridge())); + System.out.println(format("WRITE: %s.%s (bridge:%s)", + writeMethod.getDeclaringClass().getSimpleName(), writeMethod.getName(), writeMethod.isBridge())); + new PropertyDescriptor("book", readMethod, writeMethod); + } + } + + Method readMethod = LawLibrary.class.getMethod("getBook"); + Method writeMethod = LawLibrary.class.getMethod("setBook", Book.class); + + System.out.println(format("read : %s.%s (bridge:%s)", + readMethod.getDeclaringClass().getSimpleName(), readMethod.getName(), readMethod.isBridge())); + System.out.println(format("write: %s.%s (bridge:%s)", + writeMethod.getDeclaringClass().getSimpleName(), writeMethod.getName(), writeMethod.isBridge())); + + + System.out.println("--------"); + for (Method m : LawLibrary.class.getMethods()) { + if (m.getDeclaringClass() == Object.class) continue; + System.out.println(format("%s %s.%s(%s) [bridge:%s]", + m.getReturnType().getSimpleName(), m.getDeclaringClass().getSimpleName(), + m.getName(), + m.getParameterTypes().length == 1 ? m.getParameterTypes()[0].getSimpleName() : "", + m.isBridge())); + } + + //new ExtendedBeanInfo(info); + } + + @Test + public void reproSpr8806() throws IntrospectionException { + BeanInfo bi = Introspector.getBeanInfo(LawLibrary.class); + new ExtendedBeanInfo(bi); // throws + } + + interface Book { } + + interface TextBook extends Book { } + + interface LawBook extends TextBook { } + + interface BookOperations { + Book getBook(); + void setBook(Book book); + } + + interface TextBookOperations extends BookOperations { + TextBook getBook(); + } + + abstract class Library { + public Book getBook() { return null; } + public void setBook(Book book) { } + } + + class LawLibrary extends Library implements TextBookOperations { + public LawBook getBook() { return null; } + } }