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
This commit is contained in:
Chris Beams
2012-02-09 11:55:33 +01:00
parent c1df51a3b4
commit f1ecd1ca2a
2 changed files with 63 additions and 8 deletions

View File

@@ -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;
}

View File

@@ -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;
}
}
}