Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Overhaul non-void JavaBean write method support

Browse Source 
This change revisits the implementation of ExtendedBeanInfo, simplifying
the overall approach while also ensuring that ExtendedBeanInfo is fully
isolated from the BeanInfo instance it wraps. This includes any existing
PropertyDescriptors in the wrapped BeanInfo - along with being copied
locally into ExtendedBeanInfo, each property descriptor is now also
wrapped with our own new "simple" PropertyDescriptor variants that
bypass the soft/weak reference management that goes on in both
java.beans.PropertyDescriptor and java.beans.IndexedPropertyDescriptor,
maintaining hard references to methods and bean classes instead. This
ensures that changes we make to property descriptors, e.g. adding write
methods, do not cause subtle conflicts during garbage collection (as was
reported and reproduced in SPR-9702).

Eliminating soft/weak reference management means that we must take extra
care to ensure that we do not cause ClassLoader leaks by maintaining
hard references to methods, and therefore transitively to the
ClassLoader in which the bean class was loaded. The forthcoming
SPR-10028 addresses this aspect.

See the updated ExtendedBeanInfo Javadoc for further details.

Issue: SPR-8079, SPR-8175, SPR-8347, SPR-8432, SPR-8491, SPR-8522,
       SPR-8806, SPR-8931, SPR-8937, SPR-8949, SPR-9007, SPR-9059,
       SPR-9414, SPR-9453, SPR-9542, SPR-9584, SPR-9677, SPR-9702,
       SPR-9723, SPR-9943, SPR-9978, SPR-10028, SPR-10029
Backport-Commit: 4a8be69099
Conflicts:
	spring-beans/src/main/java/org/springframework/beans/ExtendedBeanInfoFactory.java
This commit is contained in:
Chris Beams
2012-11-24 12:22:31 +01:00
parent e0b29b708c
commit 0ee12563b0
2 changed files with 675 additions and 330 deletions
Download Patch File Download Diff File Expand all files Collapse all files

150
org.springframework.beans/src/test/java/org/springframework/beans/ExtendedBeanInfoTests.java
View File

@@ -24,22 +24,22 @@ import java.beans.PropertyDescriptor;
import java.lang.reflect.Method;
import org.junit.Ignore;
import org.junit.Test;
import org.springframework.beans.ExtendedBeanInfo.PropertyDescriptorComparator;
import org.springframework.core.JdkVersion;
import org.springframework.util.ClassUtils;
import test.beans.TestBean;
import static org.junit.Assert.*;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
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.junit.Assert.*;
/**
* Unit tests for {@link ExtendedBeanInfo}.
*
@@ -128,12 +128,32 @@ public class ExtendedBeanInfoTests {
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
assertThat(hasReadMethodForProperty(ebi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true));
}
@Test
public void standardReadAndNonStandardIndexedWriteMethod() throws IntrospectionException {
@SuppressWarnings("unused") class C {
public String[] getFoo() { return null; }
public C setFoo(int i, String foo) { return this; }
}
BeanInfo bi = Introspector.getBeanInfo(C.class);
assertThat(hasReadMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
assertThat(hasIndexedWriteMethodForProperty(bi, "foo"), is(trueUntilJdk17()));
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(ebi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(ebi, "foo"), is(false));
assertThat(hasIndexedWriteMethodForProperty(ebi, "foo"), is(true));
}
@Test
public void standardReadMethodsAndOverloadedNonStandardWriteMethods() throws Exception {
@SuppressWarnings("unused") class C {
@@ -150,7 +170,7 @@ public class ExtendedBeanInfoTests {
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(trueUntilJdk17()));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
assertThat(hasReadMethodForProperty(ebi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true));
@@ -225,7 +245,7 @@ public class ExtendedBeanInfoTests {
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
assertThat(hasReadMethodForProperty(ebi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true));
@@ -268,13 +288,13 @@ public class ExtendedBeanInfoTests {
assertThat(hasReadMethodForProperty(bi, "bar"), is(true));
assertThat(hasWriteMethodForProperty(bi, "bar"), is(false));
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
assertThat(hasReadMethodForProperty(bi, "bar"), is(true));
assertThat(hasWriteMethodForProperty(bi, "bar"), is(true));
assertThat(hasWriteMethodForProperty(bi, "bar"), is(false));
assertThat(hasReadMethodForProperty(ebi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true));
@@ -291,7 +311,7 @@ public class ExtendedBeanInfoTests {
}
BeanInfo bi = Introspector.getBeanInfo(C.class);
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(false));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
@@ -312,7 +332,7 @@ public class ExtendedBeanInfoTests {
}
BeanInfo bi = Introspector.getBeanInfo(C.class);
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
@@ -329,7 +349,7 @@ public class ExtendedBeanInfoTests {
}
BeanInfo bi = Introspector.getBeanInfo(C.class);
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasIndexedReadMethodForProperty(bi, "foos"), is(true));
assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(false));
@@ -350,7 +370,7 @@ public class ExtendedBeanInfoTests {
}
BeanInfo bi = Introspector.getBeanInfo(C.class);
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
@@ -367,7 +387,7 @@ public class ExtendedBeanInfoTests {
}
BeanInfo bi = Introspector.getBeanInfo(C.class);
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasIndexedReadMethodForProperty(bi, "foos"), is(true));
assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(false));
@@ -508,15 +528,14 @@ public class ExtendedBeanInfoTests {
BeanInfo ebi = new ExtendedBeanInfo(Introspector.getBeanInfo(C.class));
assertThat(hasIndexedReadMethodForProperty(bi, "foos"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foos"), is(true));
assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foos"), is(false));
assertThat(hasIndexedWriteMethodForProperty(bi, "foos"), is(trueUntilJdk17()));
assertThat(hasIndexedReadMethodForProperty(ebi, "foos"), is(true));
assertThat(hasWriteMethodForProperty(ebi, "foos"), is(true));
assertThat(hasIndexedWriteMethodForProperty(ebi, "foos"), is(true));
}
@Ignore // see comments at SPR-9702
@Test
public void cornerSpr9702() throws IntrospectionException {
{ // baseline with standard write method
@@ -579,10 +598,10 @@ public class ExtendedBeanInfoTests {
assertThat(hasReadMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
assertThat(hasReadMethodForProperty(ebi, "foo"), is(true));
assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true));
@@ -598,7 +617,7 @@ public class ExtendedBeanInfoTests {
}
BeanInfo bi = Introspector.getBeanInfo(C.class);
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(false));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(true));
@@ -619,9 +638,67 @@ public class ExtendedBeanInfoTests {
}
BeanInfo bi = Introspector.getBeanInfo(C.class);
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(ebi.getPropertyDescriptors(), equalTo(bi.getPropertyDescriptors()));
assertThat(ebi.getPropertyDescriptors().length, equalTo(bi.getPropertyDescriptors().length));
assertThat(ebi.getPropertyDescriptors().length, equalTo(1));
assertThat(ebi.getPropertyDescriptors()[0].getName(), equalTo("class"));
}
@Test
public void overloadedNonStandardWriteMethodsOnly_orderA() throws IntrospectionException, SecurityException, NoSuchMethodException {
@SuppressWarnings("unused") class C {
public Object setFoo(String p) { return new Object(); }
public Object setFoo(int p) { return new Object(); }
}
BeanInfo bi = Introspector.getBeanInfo(C.class);
assertThat(hasReadMethodForProperty(bi, "foo"), is(false));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(false));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
assertThat(hasReadMethodForProperty(ebi, "foo"), is(false));
assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true));
for (PropertyDescriptor pd : ebi.getPropertyDescriptors()) {
if (pd.getName().equals("foo")) {
assertThat(pd.getWriteMethod(), is(C.class.getMethod("setFoo", String.class)));
return;
}
}
fail("never matched write method");
}
@Test
public void overloadedNonStandardWriteMethodsOnly_orderB() throws IntrospectionException, SecurityException, NoSuchMethodException {
@SuppressWarnings("unused") class C {
public Object setFoo(int p) { return new Object(); }
public Object setFoo(String p) { return new Object(); }
}
BeanInfo bi = Introspector.getBeanInfo(C.class);
assertThat(hasReadMethodForProperty(bi, "foo"), is(false));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "foo"), is(false));
assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));
assertThat(hasReadMethodForProperty(ebi, "foo"), is(false));
assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true));
for (PropertyDescriptor pd : ebi.getPropertyDescriptors()) {
if (pd.getName().equals("foo")) {
assertThat(pd.getWriteMethod(), is(C.class.getMethod("setFoo", int.class)));
return;
}
}
fail("never matched write method");
}
/**
@@ -645,7 +722,7 @@ public class ExtendedBeanInfoTests {
assertThat(hasIndexedReadMethodForProperty(bi, "dateFormat"), is(false));
assertThat(hasIndexedWriteMethodForProperty(bi, "dateFormat"), is(trueUntilJdk17()));
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "dateFormat"), is(false));
assertThat(hasWriteMethodForProperty(bi, "dateFormat"), is(false));
@@ -653,7 +730,7 @@ public class ExtendedBeanInfoTests {
assertThat(hasIndexedWriteMethodForProperty(bi, "dateFormat"), is(trueUntilJdk17()));
assertThat(hasReadMethodForProperty(ebi, "dateFormat"), is(false));
assertThat(hasWriteMethodForProperty(ebi, "dateFormat"), is(false));
assertThat(hasWriteMethodForProperty(ebi, "dateFormat"), is(true));
assertThat(hasIndexedReadMethodForProperty(ebi, "dateFormat"), is(false));
assertThat(hasIndexedWriteMethodForProperty(ebi, "dateFormat"), is(true));
}
@@ -661,7 +738,7 @@ public class ExtendedBeanInfoTests {
@Test
public void propertyCountsMatch() throws IntrospectionException {
BeanInfo bi = Introspector.getBeanInfo(TestBean.class);
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(ebi.getPropertyDescriptors().length, equalTo(bi.getPropertyDescriptors().length));
}
@@ -673,7 +750,7 @@ public class ExtendedBeanInfoTests {
public ExtendedTestBean setFoo(String s) { return this; }
}
BeanInfo bi = Introspector.getBeanInfo(ExtendedTestBean.class);
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
boolean found = false;
for (PropertyDescriptor pd : ebi.getPropertyDescriptors()) {
@@ -692,7 +769,7 @@ public class ExtendedBeanInfoTests {
@Test
public void propertyDescriptorOrderIsEqual() throws IntrospectionException {
BeanInfo bi = Introspector.getBeanInfo(TestBean.class);
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
for (int i = 0; i < bi.getPropertyDescriptors().length; i++) {
assertThat("element " + i + " in BeanInfo and ExtendedBeanInfo propertyDescriptor arrays do not match",
@@ -739,7 +816,9 @@ public class ExtendedBeanInfoTests {
private boolean hasIndexedWriteMethodForProperty(BeanInfo beanInfo, String propertyName) {
for (PropertyDescriptor pd : beanInfo.getPropertyDescriptors()) {
if (pd.getName().equals(propertyName)) {
assertThat(propertyName + " property is not indexed", pd, instanceOf(IndexedPropertyDescriptor.class));
if (!(pd instanceof IndexedPropertyDescriptor)) {
return false;
}
return ((IndexedPropertyDescriptor)pd).getIndexedWriteMethod() != null;
}
}
@@ -749,7 +828,9 @@ public class ExtendedBeanInfoTests {
private boolean hasIndexedReadMethodForProperty(BeanInfo beanInfo, String propertyName) {
for (PropertyDescriptor pd : beanInfo.getPropertyDescriptors()) {
if (pd.getName().equals(propertyName)) {
assertThat(propertyName + " property is not indexed", pd, instanceOf(IndexedPropertyDescriptor.class));
if (!(pd instanceof IndexedPropertyDescriptor)) {
return false;
}
return ((IndexedPropertyDescriptor)pd).getIndexedReadMethod() != null;
}
}
@@ -830,7 +911,7 @@ public class ExtendedBeanInfoTests {
}
// and now demonstrate that we've indeed fixed the problem
ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);
BeanInfo ebi = new ExtendedBeanInfo(bi);
assertThat(hasReadMethodForProperty(bi, "targetMethod"), is(true));
assertThat(hasWriteMethodForProperty(bi, "targetMethod"), is(false));
@@ -855,12 +936,11 @@ public class ExtendedBeanInfoTests {
assertThat(hasIndexedWriteMethodForProperty(bi, "address"), is(true));
}
{
ExtendedBeanInfo bi = new ExtendedBeanInfo(Introspector.getBeanInfo(A.class));
BeanInfo bi = new ExtendedBeanInfo(Introspector.getBeanInfo(A.class));
assertThat(hasReadMethodForProperty(bi, "address"), is(false));
assertThat(hasWriteMethodForProperty(bi, "address"), is(false));
assertThat(hasIndexedReadMethodForProperty(bi, "address"), is(true));
assertThat(hasIndexedWriteMethodForProperty(bi, "address"), is(true));
}
}
}