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

Author: cbeams

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 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 @@ public ExtendedBeanInfo(BeanInfo delegate) throws IntrospectionException {
 			// 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;
 				}

org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java

@@ -22,16 +22,19 @@
 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.Test;
 import org.springframework.beans.ExtendedBeanInfo.PropertyDescriptorComparator;
+import org.springframework.util.ClassUtils;
 
 import test.beans.TestBean;
 
@@ -660,13 +663,14 @@ private boolean hasIndexedReadMethodForProperty(BeanInfo beanInfo, String proper
 		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 void setBook(Book book) { }
 	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;
+		}
+	}
 }