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 971f009ac6..7322ad3848 100644 --- a/spring-beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java +++ b/spring-beans/src/main/java/org/springframework/beans/ExtendedBeanInfo.java @@ -41,7 +41,7 @@ import static org.springframework.beans.PropertyDescriptorUtils.*; * Decorator for a standard {@link BeanInfo} object, e.g. as created by * {@link Introspector#getBeanInfo(Class)}, designed to discover and register static * and/or non-void returning setter methods. For example: - *
{@code
+ * 
  * public class Bean {
  *     private Foo foo;
  *
@@ -53,7 +53,7 @@ import static org.springframework.beans.PropertyDescriptorUtils.*;
  *         this.foo = foo;
  *         return this;
  *     }
- * }}
+ * }
* The standard JavaBeans {@code Introspector} will discover the {@code getFoo} read * method, but will bypass the {@code #setFoo(Foo)} write method, because its non-void * returning signature does not comply with the JavaBeans specification. @@ -206,31 +206,31 @@ class ExtendedBeanInfo implements BeanInfo { } public BeanInfo[] getAdditionalBeanInfo() { - return delegate.getAdditionalBeanInfo(); + return this.delegate.getAdditionalBeanInfo(); } public BeanDescriptor getBeanDescriptor() { - return delegate.getBeanDescriptor(); + return this.delegate.getBeanDescriptor(); } public int getDefaultEventIndex() { - return delegate.getDefaultEventIndex(); + return this.delegate.getDefaultEventIndex(); } public int getDefaultPropertyIndex() { - return delegate.getDefaultPropertyIndex(); + return this.delegate.getDefaultPropertyIndex(); } public EventSetDescriptor[] getEventSetDescriptors() { - return delegate.getEventSetDescriptors(); + return this.delegate.getEventSetDescriptors(); } public Image getIcon(int iconKind) { - return delegate.getIcon(iconKind); + return this.delegate.getIcon(iconKind); } public MethodDescriptor[] getMethodDescriptors() { - return delegate.getMethodDescriptors(); + return this.delegate.getMethodDescriptors(); } } @@ -284,7 +284,7 @@ class SimplePropertyDescriptor extends PropertyDescriptor { this.propertyType = findPropertyType(this.readMethod, this.writeMethod); } catch (IntrospectionException ex) { - // ignore, as does PropertyDescriptor#getPropertyType + // Ignore, as does PropertyDescriptor#getPropertyType } } return this.propertyType; @@ -374,7 +374,7 @@ class SimpleIndexedPropertyDescriptor extends IndexedPropertyDescriptor { this.propertyType = findPropertyType(this.readMethod, this.writeMethod); } catch (IntrospectionException ex) { - // ignore, as does IndexedPropertyDescriptor#getPropertyType + // Ignore, as does IndexedPropertyDescriptor#getPropertyType } } return this.propertyType; @@ -408,7 +408,7 @@ class SimpleIndexedPropertyDescriptor extends IndexedPropertyDescriptor { getName(), getPropertyType(), this.indexedReadMethod, this.indexedWriteMethod); } catch (IntrospectionException ex) { - // ignore, as does IndexedPropertyDescriptor#getIndexedPropertyType + // Ignore, as does IndexedPropertyDescriptor#getIndexedPropertyType } } return this.indexedPropertyType; @@ -473,14 +473,14 @@ class PropertyDescriptorUtils { target.setShortDescription(source.getShortDescription()); target.setDisplayName(source.getDisplayName()); - // copy all attributes (emulating behavior of private FeatureDescriptor#addTable) + // Copy all attributes (emulating behavior of private FeatureDescriptor#addTable) Enumeration keys = source.attributeNames(); while (keys.hasMoreElements()) { String key = keys.nextElement(); target.setValue(key, source.getValue(key)); } - // see java.beans.PropertyDescriptor#PropertyDescriptor(PropertyDescriptor) + // See java.beans.PropertyDescriptor#PropertyDescriptor(PropertyDescriptor) target.setPropertyEditorClass(source.getPropertyEditorClass()); target.setBound(source.isBound()); target.setConstrained(source.isConstrained()); @@ -494,24 +494,34 @@ class PropertyDescriptorUtils { if (readMethod != null) { Class[] params = readMethod.getParameterTypes(); if (params.length != 0) { - throw new IntrospectionException("bad read method arg count: " + readMethod); + throw new IntrospectionException("Bad read method arg count: " + readMethod); } propertyType = readMethod.getReturnType(); if (propertyType == Void.TYPE) { - throw new IntrospectionException("read method " - + readMethod.getName() + " returns void"); + throw new IntrospectionException("Read method returns void: " + readMethod); } } if (writeMethod != null) { Class params[] = writeMethod.getParameterTypes(); if (params.length != 1) { - throw new IntrospectionException("bad write method arg count: " + writeMethod); + throw new IntrospectionException("Bad write method arg count: " + writeMethod); } - if (propertyType != null - && !params[0].isAssignableFrom(propertyType)) { - throw new IntrospectionException("type mismatch between read and write methods"); + if (propertyType != null) { + if (propertyType.isAssignableFrom(params[0])) { + // Write method's property type potentially more specific + propertyType = params[0]; + } + else if (params[0].isAssignableFrom(propertyType)) { + // Proceed with read method's property type + } + else { + throw new IntrospectionException( + "Type mismatch between read and write methods: " + readMethod + " - " + writeMethod); + } + } + else { + propertyType = params[0]; } - propertyType = params[0]; } return propertyType; } @@ -523,44 +533,48 @@ class PropertyDescriptorUtils { Method indexedReadMethod, Method indexedWriteMethod) throws IntrospectionException { Class indexedPropertyType = null; - if (indexedReadMethod != null) { Class params[] = indexedReadMethod.getParameterTypes(); if (params.length != 1) { - throw new IntrospectionException( - "bad indexed read method arg count"); + throw new IntrospectionException("Bad indexed read method arg count: " + indexedReadMethod); } if (params[0] != Integer.TYPE) { - throw new IntrospectionException( - "non int index to indexed read method"); + throw new IntrospectionException("Non int index to indexed read method: " + indexedReadMethod); } indexedPropertyType = indexedReadMethod.getReturnType(); if (indexedPropertyType == Void.TYPE) { - throw new IntrospectionException( - "indexed read method returns void"); + throw new IntrospectionException("Indexed read method returns void: " + indexedReadMethod); } } if (indexedWriteMethod != null) { Class params[] = indexedWriteMethod.getParameterTypes(); if (params.length != 2) { - throw new IntrospectionException( - "bad indexed write method arg count"); + throw new IntrospectionException("Bad indexed write method arg count: " + indexedWriteMethod); } if (params[0] != Integer.TYPE) { - throw new IntrospectionException( - "non int index to indexed write method"); + throw new IntrospectionException("Non int index to indexed write method: " + indexedWriteMethod); } - if (indexedPropertyType != null && indexedPropertyType != params[1]) { - throw new IntrospectionException( - "type mismatch between indexed read and indexed write methods: " + name); + if (indexedPropertyType != null) { + if (indexedPropertyType.isAssignableFrom(params[1])) { + // Write method's property type potentially more specific + indexedPropertyType = params[1]; + } + else if (params[1].isAssignableFrom(indexedPropertyType)) { + // Proceed with read method's property type + } + else { + throw new IntrospectionException("Type mismatch between indexed read and write methods: " + + indexedReadMethod + " - " + indexedWriteMethod); + } + } + else { + indexedPropertyType = params[1]; } - indexedPropertyType = params[1]; } - if (propertyType != null - && (!propertyType.isArray() || - propertyType.getComponentType() != indexedPropertyType)) { - throw new IntrospectionException( - "type mismatch between indexed and non-indexed methods: " + name); + if (propertyType != null && (!propertyType.isArray() || + propertyType.getComponentType() != indexedPropertyType)) { + throw new IntrospectionException("Type mismatch between indexed and non-indexed methods: " + + indexedReadMethod + " - " + indexedWriteMethod); } return indexedPropertyType; } @@ -581,15 +595,12 @@ class PropertyDescriptorUtils { if (!compareMethods(pd1.getReadMethod(), pd2.getReadMethod())) { return false; } - if (!compareMethods(pd1.getWriteMethod(), pd2.getWriteMethod())) { return false; } - - if (pd1.getPropertyType() == pd2.getPropertyType() - && pd1.getPropertyEditorClass() == pd2.getPropertyEditorClass() - && pd1.isBound() == pd2.isBound() - && pd1.isConstrained() == pd2.isConstrained()) { + if (pd1.getPropertyType() == pd2.getPropertyType() && + pd1.getPropertyEditorClass() == pd2.getPropertyEditorClass() && + pd1.isBound() == pd2.isBound() && pd1.isConstrained() == pd2.isConstrained()) { return true; } } @@ -603,7 +614,7 @@ class PropertyDescriptorUtils { if ((a == null) != (b == null)) { return false; } - if (a != null && b != null) { + if (a != null) { if (!a.equals(b)) { return false; } 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 30b2013d07..67f3b8cb5e 100644 --- a/spring-beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java @@ -21,29 +21,19 @@ import java.beans.IndexedPropertyDescriptor; import java.beans.IntrospectionException; import java.beans.Introspector; import java.beans.PropertyDescriptor; - -import java.lang.reflect.Method; import java.math.BigDecimal; import org.junit.Test; import org.springframework.core.JdkVersion; import org.springframework.tests.sample.beans.TestBean; -import org.springframework.util.ClassUtils; - - -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.lessThan; - import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; - +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; /** - * Unit tests for {@link ExtendedBeanInfo}. - * * @author Chris Beams * @since 3.1 */ @@ -207,10 +197,6 @@ public class ExtendedBeanInfoTests { } } - interface Spr9453 { - T getProp(); - } - @Test public void cornerSpr9453() throws IntrospectionException { final class Bean implements Spr9453> { @@ -335,10 +321,8 @@ public class ExtendedBeanInfoTests { BeanInfo ebi = new ExtendedBeanInfo(bi); assertThat(hasReadMethodForProperty(bi, "foo"), is(true)); - assertThat(hasWriteMethodForProperty(bi, "foo"), is(false)); - assertThat(hasReadMethodForProperty(ebi, "foo"), is(true)); - assertThat(hasWriteMethodForProperty(ebi, "foo"), is(false)); + assertEquals(hasWriteMethodForProperty(bi, "foo"), hasWriteMethodForProperty(ebi, "foo")); } @Test @@ -352,10 +336,8 @@ public class ExtendedBeanInfoTests { BeanInfo ebi = new ExtendedBeanInfo(bi); assertThat(hasIndexedReadMethodForProperty(bi, "foos"), is(true)); - assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(false)); - assertThat(hasIndexedReadMethodForProperty(ebi, "foos"), is(true)); - assertThat(hasIndexedWriteMethodForProperty(ebi, "foos"), is(false)); + assertEquals(hasIndexedWriteMethodForProperty(bi, "foos"), hasIndexedWriteMethodForProperty(ebi, "foos")); } /** @@ -595,7 +577,6 @@ public class ExtendedBeanInfoTests { new ExtendedBeanInfo(Introspector.getBeanInfo(BigDecimal.class)); } - @Test public void subclassWriteMethodWithCovariantReturnType() throws IntrospectionException { @SuppressWarnings("unused") class B { @@ -792,6 +773,7 @@ public class ExtendedBeanInfoTests { @Test public void propertyDescriptorComparator() throws IntrospectionException { PropertyDescriptorComparator c = new PropertyDescriptorComparator(); + assertThat(c.compare(new PropertyDescriptor("a", null, null), new PropertyDescriptor("a", null, null)), equalTo(0)); assertThat(c.compare(new PropertyDescriptor("abc", null, null), new PropertyDescriptor("abc", null, null)), equalTo(0)); assertThat(c.compare(new PropertyDescriptor("a", null, null), new PropertyDescriptor("b", null, null)), lessThan(0)); @@ -863,31 +845,6 @@ public class ExtendedBeanInfoTests { new ExtendedBeanInfo(Introspector.getBeanInfo(LawLibrary.class)); } - 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; } - } - - @Test public void cornerSpr8949() throws IntrospectionException { class A { @@ -905,23 +862,10 @@ public class ExtendedBeanInfoTests { BeanInfo bi = Introspector.getBeanInfo(B.class); - /* first, demonstrate the 'problem': - * java.beans.Introspector returns the "wrong" declaring class for overridden read - * methods, which in turn violates expectations in {@link ExtendedBeanInfo} regarding - * method equality. Spring's {@link ClassUtils#getMostSpecificMethod(Method, Class)} - * helps out here, and is now put into use in ExtendedBeanInfo as well - */ - for (PropertyDescriptor pd : bi.getPropertyDescriptors()) { - if ("targetMethod".equals(pd.getName())) { - Method readMethod = pd.getReadMethod(); - assertTrue(readMethod.getDeclaringClass().equals(A.class)); // we expected B! - - Method msReadMethod = ClassUtils.getMostSpecificMethod(readMethod, B.class); - assertTrue(msReadMethod.getDeclaringClass().equals(B.class)); // and now we get it. - } - } - - // and now demonstrate that we've indeed fixed the problem + // java.beans.Introspector returns the "wrong" declaring class for overridden read + // methods, which in turn violates expectations in {@link ExtendedBeanInfo} regarding + // method equality. Spring's {@link ClassUtils#getMostSpecificMethod(Method, Class)} + // helps out here, and is now put into use in ExtendedBeanInfo as well. BeanInfo ebi = new ExtendedBeanInfo(bi); assertThat(hasReadMethodForProperty(bi, "targetMethod"), is(true)); @@ -973,8 +917,56 @@ public class ExtendedBeanInfoTests { } } + + interface Spr9453 { + + T getProp(); + } + + interface Book { + } + + interface TextBook extends Book { + } + + interface LawBook extends TextBook { + } + + interface BookOperations { + + Book getBook(); + + void setBook(Book book); + } + + interface TextBookOperations extends BookOperations { + + @Override + TextBook getBook(); + } + + abstract class Library { + + public Book getBook() { + return null; + } + + public void setBook(Book book) { + } + } + + class LawLibrary extends Library implements TextBookOperations { + + @Override + public LawBook getBook() { + return null; + } + } + static class WithStaticWriteMethod { + public static void setProp1(String prop1) { } } + }