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
This commit is contained in:
Chris Beams
2012-11-24 12:22:31 +01:00
parent 2d8bacace7
commit 4a8be69099
4 changed files with 824 additions and 338 deletions
Download Patch File Download Diff File Expand all files Collapse all files

146
spring-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,11 +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()));
}
@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");
}
/**
* Corners the bug revealed by SPR-8522, in which an (apparently) indexed write method
* without a corresponding indexed read method would fail to be processed correctly by
@@ -645,7 +720,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 +728,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 +736,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 +748,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 +767,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 +814,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 +826,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 +909,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 +934,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));
}
}
}

150
spring-beans/src/test/java/org/springframework/beans/SimplePropertyDescriptorTests.java Normal file
View File

@@ -0,0 +1,150 @@
/*
* 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.beans;
import java.beans.IndexedPropertyDescriptor;
import java.beans.IntrospectionException;
import java.beans.PropertyDescriptor;
import java.lang.reflect.Method;
import org.junit.Test;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
/**
* Unit tests for {@link SimpleNonIndexedPropertyDescriptor} and
* {@link SimpleIndexedPropertyDescriptor}.
*
* @author Chris Beams
* @see ExtendedBeanInfoTests
*/
public class SimplePropertyDescriptorTests {
@Test
public void toStringOutput() throws IntrospectionException, SecurityException, NoSuchMethodException {
{
Object pd = new SimpleNonIndexedPropertyDescriptor("foo", null, null);
assertThat(pd.toString(), containsString(
"PropertyDescriptor[name=foo, propertyType=null, readMethod=null"));
}
{
class C {
@SuppressWarnings("unused")
public Object setFoo(String foo) { return null; }
}
Method m = C.class.getMethod("setFoo", String.class);
Object pd = new SimpleNonIndexedPropertyDescriptor("foo", null, m);
assertThat(pd.toString(), allOf(
containsString("PropertyDescriptor[name=foo"),
containsString("propertyType=class java.lang.String"),
containsString("readMethod=null, writeMethod=public java.lang.Object")));
}
{
Object pd = new SimpleIndexedPropertyDescriptor("foo", null, null, null, null);
assertThat(pd.toString(), containsString(
"PropertyDescriptor[name=foo, propertyType=null, indexedPropertyType=null"));
}
{
class C {
@SuppressWarnings("unused")
public Object setFoo(int i, String foo) { return null; }
}
Method m = C.class.getMethod("setFoo", int.class, String.class);
Object pd = new SimpleIndexedPropertyDescriptor("foo", null, null, null, m);
assertThat(pd.toString(), allOf(
containsString("PropertyDescriptor[name=foo, propertyType=null"),
containsString("indexedPropertyType=class java.lang.String"),
containsString("indexedWriteMethod=public java.lang.Object")));
}
}
@Test
public void nonIndexedEquality() throws IntrospectionException, SecurityException, NoSuchMethodException {
Object pd1 = new SimpleNonIndexedPropertyDescriptor("foo", null, null);
assertThat(pd1, equalTo(pd1));
Object pd2 = new SimpleNonIndexedPropertyDescriptor("foo", null, null);
assertThat(pd1, equalTo(pd2));
assertThat(pd2, equalTo(pd1));
@SuppressWarnings("unused")
class C {
public Object setFoo(String foo) { return null; }
public String getFoo() { return null; }
}
Method wm1 = C.class.getMethod("setFoo", String.class);
Object pd3 = new SimpleNonIndexedPropertyDescriptor("foo", null, wm1);
assertThat(pd1, not(equalTo(pd3)));
assertThat(pd3, not(equalTo(pd1)));
Method rm1 = C.class.getMethod("getFoo");
Object pd4 = new SimpleNonIndexedPropertyDescriptor("foo", rm1, null);
assertThat(pd1, not(equalTo(pd4)));
assertThat(pd4, not(equalTo(pd1)));
Object pd5 = new PropertyDescriptor("foo", null, null);
assertThat(pd1, equalTo(pd5));
assertThat(pd5, equalTo(pd1));
Object pd6 = "not a PD";
assertThat(pd1, not(equalTo(pd6)));
assertThat(pd6, not(equalTo(pd1)));
Object pd7 = null;
assertThat(pd1, not(equalTo(pd7)));
assertThat(pd7, not(equalTo(pd1)));
}
@Test
public void indexedEquality() throws IntrospectionException, SecurityException, NoSuchMethodException {
Object pd1 = new SimpleIndexedPropertyDescriptor("foo", null, null, null, null);
assertThat(pd1, equalTo(pd1));
Object pd2 = new SimpleIndexedPropertyDescriptor("foo", null, null, null, null);
assertThat(pd1, equalTo(pd2));
assertThat(pd2, equalTo(pd1));
@SuppressWarnings("unused")
class C {
public Object setFoo(int i, String foo) { return null; }
public String getFoo(int i) { return null; }
}
Method wm1 = C.class.getMethod("setFoo", int.class, String.class);
Object pd3 = new SimpleIndexedPropertyDescriptor("foo", null, null, null, wm1);
assertThat(pd1, not(equalTo(pd3)));
assertThat(pd3, not(equalTo(pd1)));
Method rm1 = C.class.getMethod("getFoo", int.class);
Object pd4 = new SimpleIndexedPropertyDescriptor("foo", null, null, rm1, null);
assertThat(pd1, not(equalTo(pd4)));
assertThat(pd4, not(equalTo(pd1)));
Object pd5 = new IndexedPropertyDescriptor("foo", null, null, null, null);
assertThat(pd1, equalTo(pd5));
assertThat(pd5, equalTo(pd1));
Object pd6 = "not a PD";
assertThat(pd1, not(equalTo(pd6)));
assertThat(pd6, not(equalTo(pd1)));
Object pd7 = null;
assertThat(pd1, not(equalTo(pd7)));
assertThat(pd7, not(equalTo(pd1)));
}
}