From f1ecd1ca2a73158bd88dc5059ecbf0c24154f61d Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Thu, 9 Feb 2012 11:55:33 +0100 Subject: [PATCH] Detect overridden boolean getters in ExtendedBeanInfo Prior to this commit, and due to idiosyncracies of java.beans.Introspector, overridden boolean getter methods were not detected by Spring's ExtendedBeanInfo, which relied on too-strict Method equality checks when selecting read and write methods. Now ExtendedBeanInfo uses Spring's ClassUtils#getMostSpecificMethod against the methods returned from java.beans.Introspector in order to ensure that subsequent equality checks are reasonable to make. Issue: SPR-8949 --- .../beans/ExtendedBeanInfo.java | 18 ++++--- .../beans/ExtendedBeanInfoTests.java | 53 ++++++++++++++++++- 2 files changed, 63 insertions(+), 8 deletions(-) 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 205cae1c17..c2551dc284 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,6 +35,7 @@ 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.ClassUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; @@ -159,24 +160,27 @@ class ExtendedBeanInfo implements BeanInfo { // the method is not a setter, but is it a getter? 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 mostSpecificReadMethod = ClassUtils.getMostSpecificMethod(readMethod, method.getDeclaringClass()); for (PropertyDescriptor existingPD : this.propertyDescriptors) { - if (method.equals(pd.getReadMethod()) - && existingPD.getName().equals(pd.getName())) { + if (method.equals(mostSpecificReadMethod) + && existingPD.getName().equals(propertyName)) { if (existingPD.getReadMethod() == null) { // no -> add it now - this.addOrUpdatePropertyDescriptor(pd, pd.getName(), method, pd.getWriteMethod()); + this.addOrUpdatePropertyDescriptor(pd, propertyName, method, pd.getWriteMethod()); } // yes -> do not add a duplicate continue ALL_METHODS; } } - if (method.equals(pd.getReadMethod()) + if (method.equals(mostSpecificReadMethod) || (pd instanceof IndexedPropertyDescriptor && method.equals(((IndexedPropertyDescriptor) pd).getIndexedReadMethod()))) { // yes -> copy it, including corresponding setter method (if any -- may be null) if (pd instanceof IndexedPropertyDescriptor) { - this.addOrUpdatePropertyDescriptor(pd, pd.getName(), pd.getReadMethod(), pd.getWriteMethod(), ((IndexedPropertyDescriptor)pd).getIndexedReadMethod(), ((IndexedPropertyDescriptor)pd).getIndexedWriteMethod()); + this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, pd.getWriteMethod(), ((IndexedPropertyDescriptor)pd).getIndexedReadMethod(), ((IndexedPropertyDescriptor)pd).getIndexedWriteMethod()); } else { - this.addOrUpdatePropertyDescriptor(pd, pd.getName(), pd.getReadMethod(), pd.getWriteMethod()); + this.addOrUpdatePropertyDescriptor(pd, propertyName, readMethod, pd.getWriteMethod()); } continue ALL_METHODS; } 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 7475922c5b..0dfea93a54 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 @@ -22,6 +22,7 @@ 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; @@ -29,9 +30,11 @@ import java.beans.IndexedPropertyDescriptor; import java.beans.IntrospectionException; import java.beans.Introspector; import java.beans.PropertyDescriptor; +import java.lang.reflect.Method; import org.junit.Test; import org.springframework.beans.ExtendedBeanInfo.PropertyDescriptorComparator; +import org.springframework.util.ClassUtils; import test.beans.TestBean; @@ -660,13 +663,14 @@ public class ExtendedBeanInfoTests { return false; } + @Test public void reproSpr8806() throws IntrospectionException { // does not throw Introspector.getBeanInfo(LawLibrary.class); // does not throw after the changes introduced in SPR-8806 - new ExtendedBeanInfo(Introspector.getBeanInfo(LawLibrary.class)); + new ExtendedBeanInfo(Introspector.getBeanInfo(LawLibrary.class)); } interface Book { } @@ -692,4 +696,51 @@ public class ExtendedBeanInfoTests { class LawLibrary extends Library implements TextBookOperations { public LawBook getBook() { return null; } } + + + /** + * 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 + */ + @Test + public void demonstrateCauseSpr8949() throws IntrospectionException { + BeanInfo info = Introspector.getBeanInfo(B.class); + + for (PropertyDescriptor pd : info.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. + } + } + } + + @Test + public void cornerSpr8949() throws IntrospectionException { + BeanInfo bi = Introspector.getBeanInfo(B.class); + ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi); + + assertThat(hasReadMethodForProperty(bi, "targetMethod"), is(true)); + assertThat(hasWriteMethodForProperty(bi, "targetMethod"), is(false)); + + assertThat(hasReadMethodForProperty(ebi, "targetMethod"), is(true)); + assertThat(hasWriteMethodForProperty(ebi, "targetMethod"), is(false)); + } + + static class A { + public boolean isTargetMethod() { + return false; + } + } + + static class B extends A { + @Override + public boolean isTargetMethod() { + return false; + } + } }