Skip to content

Commit 569426d

Browse files
jhoellerjhoeller
committed
restored DataBinder's ability to bind to an auto-growing List with unknown element type (SPR-8828)
1 parent f9144ea commit 569426d

File tree

8 files changed

+207
-66
lines changed

8 files changed

+207
-66
lines changed

‎org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -512,11 +512,15 @@ public Object convertForProperty(Object value, String propertyName) throws TypeM
512512

513513
private Object convertForProperty(String propertyName, Object oldValue, Object newValue, PropertyDescriptor pd)
514514
throws TypeMismatchException {
515-
GenericTypeAwarePropertyDescriptor gpd = (GenericTypeAwarePropertyDescriptor) pd;
516-
Class<?> beanClass = gpd.getBeanClass();
515+
517516
return convertIfNecessary(propertyName, oldValue, newValue, pd.getPropertyType(), new TypeDescriptor(property(pd)));
518517
}
519518

519+
private Property property(PropertyDescriptor pd) {
520+
GenericTypeAwarePropertyDescriptor typeAware = (GenericTypeAwarePropertyDescriptor) pd;
521+
return new Property(typeAware.getBeanClass(), typeAware.getReadMethod(), typeAware.getWriteMethod());
522+
}
523+
520524

521525
//---------------------------------------------------------------------
522526
// Implementation methods
@@ -970,7 +974,8 @@ private void setPropertyValue(PropertyTokenHolder tokens, PropertyValue pv) thro
970974
if (isExtractOldValueForEditor() && arrayIndex < Array.getLength(propValue)) {
971975
oldValue = Array.get(propValue, arrayIndex);
972976
}
973-
Object convertedValue = convertIfNecessary(propertyName, oldValue, pv.getValue(), requiredType, TypeDescriptor.nested(property(pd), tokens.keys.length));
977+
Object convertedValue = convertIfNecessary(propertyName, oldValue, pv.getValue(),
978+
requiredType, TypeDescriptor.nested(property(pd), tokens.keys.length));
974979
Array.set(propValue, arrayIndex, convertedValue);
975980
}
976981
catch (IndexOutOfBoundsException ex) {
@@ -984,12 +989,13 @@ else if (propValue instanceof List) {
984989
pd.getReadMethod(), tokens.keys.length);
985990
List list = (List) propValue;
986991
int index = Integer.parseInt(key);
987-
int size = list.size();
988992
Object oldValue = null;
989-
if (isExtractOldValueForEditor() && index < size) {
993+
if (isExtractOldValueForEditor() && index < list.size()) {
990994
oldValue = list.get(index);
991995
}
992-
Object convertedValue = convertIfNecessary(propertyName, oldValue, pv.getValue(), requiredType, TypeDescriptor.nested(property(pd), tokens.keys.length));
996+
Object convertedValue = convertIfNecessary(propertyName, oldValue, pv.getValue(),
997+
requiredType, TypeDescriptor.nested(property(pd), tokens.keys.length));
998+
int size = list.size();
993999
if (index >= size && index < this.autoGrowCollectionLimit) {
9941000
for (int i = size; i < index; i++) {
9951001
try {
@@ -1023,16 +1029,17 @@ else if (propValue instanceof Map) {
10231029
Map map = (Map) propValue;
10241030
// IMPORTANT: Do not pass full property name in here - property editors
10251031
// must not kick in for map keys but rather only for map values.
1026-
TypeDescriptor typeDescriptor = mapKeyType != null ? TypeDescriptor.valueOf(mapKeyType) : TypeDescriptor.valueOf(Object.class);
1032+
TypeDescriptor typeDescriptor = (mapKeyType != null ?
1033+
TypeDescriptor.valueOf(mapKeyType) : TypeDescriptor.valueOf(Object.class));
10271034
Object convertedMapKey = convertIfNecessary(null, null, key, mapKeyType, typeDescriptor);
10281035
Object oldValue = null;
10291036
if (isExtractOldValueForEditor()) {
10301037
oldValue = map.get(convertedMapKey);
10311038
}
10321039
// Pass full property name and old value in here, since we want full
10331040
// conversion ability for map values.
1034-
Object convertedMapValue = convertIfNecessary(
1035-
propertyName, oldValue, pv.getValue(), mapValueType, TypeDescriptor.nested(property(pd), tokens.keys.length));
1041+
Object convertedMapValue = convertIfNecessary(propertyName, oldValue, pv.getValue(),
1042+
mapValueType, TypeDescriptor.nested(property(pd), tokens.keys.length));
10361043
map.put(convertedMapKey, convertedMapValue);
10371044
}
10381045
else {
@@ -1194,10 +1201,5 @@ private static class PropertyTokenHolder {
11941201

11951202
public String[] keys;
11961203
}
1197-
1198-
private Property property(PropertyDescriptor pd) {
1199-
GenericTypeAwarePropertyDescriptor typeAware = (GenericTypeAwarePropertyDescriptor) pd;
1200-
return new Property(typeAware.getBeanClass(), typeAware.getReadMethod(), typeAware.getWriteMethod());
1201-
}
12021204

12031205
}

‎org.springframework.beans/src/main/java/org/springframework/beans/GenericTypeAwarePropertyDescriptor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public GenericTypeAwarePropertyDescriptor(Class beanClass, String propertyName,
9393
}
9494

9595
public Class<?> getBeanClass() {
96-
return beanClass;
96+
return this.beanClass;
9797
}
9898

9999
@Override

‎org.springframework.context/src/test/java/org/springframework/validation/DataBinderTests.java

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,13 @@
2222
import java.io.ByteArrayOutputStream;
2323
import java.io.ObjectInputStream;
2424
import java.io.ObjectOutputStream;
25+
import java.util.AbstractList;
26+
import java.util.ArrayList;
27+
import java.util.Collection;
28+
import java.util.HashMap;
29+
import java.util.Iterator;
2530
import java.util.List;
31+
import java.util.ListIterator;
2632
import java.util.Locale;
2733
import java.util.Map;
2834
import java.util.Set;
@@ -45,6 +51,7 @@
4551
import org.springframework.context.i18n.LocaleContextHolder;
4652
import org.springframework.context.support.ResourceBundleMessageSource;
4753
import org.springframework.context.support.StaticMessageSource;
54+
import org.springframework.core.convert.support.ConversionServiceFactory;
4855
import org.springframework.core.convert.support.DefaultConversionService;
4956
import org.springframework.format.number.NumberFormatter;
5057
import org.springframework.format.support.FormattingConversionService;
@@ -1552,6 +1559,20 @@ public void testAutoGrowBeyondCustomLimit() throws Exception {
15521559
}
15531560
}
15541561

1562+
public void testNestedGrowingList() {
1563+
Form form = new Form();
1564+
DataBinder binder = new DataBinder(form, "form");
1565+
MutablePropertyValues mpv = new MutablePropertyValues();
1566+
mpv.add("f[list][0]", "firstValue");
1567+
mpv.add("f[list][1]", "secondValue");
1568+
binder.bind(mpv);
1569+
assertFalse(binder.getBindingResult().hasErrors());
1570+
List<Object> list = (List<Object>) form.getF().get("list");
1571+
assertEquals("firstValue", list.get(0));
1572+
assertEquals("secondValue", list.get(1));
1573+
assertEquals(2, list.size());
1574+
}
1575+
15551576

15561577
private static class BeanWithIntegerList {
15571578

@@ -1646,4 +1667,94 @@ public void validate(Object obj, Errors errors) {
16461667
}
16471668
}
16481669

1670+
1671+
private static class GrowingList<E> extends AbstractList<E> {
1672+
1673+
private List<E> list;
1674+
1675+
public GrowingList() {
1676+
this.list = new ArrayList<E>();
1677+
}
1678+
1679+
public List<E> getWrappedList() {
1680+
return list;
1681+
}
1682+
1683+
public E get(int index) {
1684+
if (index >= list.size()) {
1685+
for (int i = list.size(); i < index; i++) {
1686+
list.add(null);
1687+
}
1688+
list.add(null);
1689+
return null;
1690+
}
1691+
else {
1692+
return list.get(index);
1693+
}
1694+
}
1695+
1696+
public int size() {
1697+
return list.size();
1698+
}
1699+
1700+
public boolean add(E o) {
1701+
return list.add(o);
1702+
}
1703+
1704+
public void add(int index, E element) {
1705+
list.add(index, element);
1706+
}
1707+
1708+
public boolean addAll(int index, Collection<? extends E> c) {
1709+
return list.addAll(index, c);
1710+
}
1711+
1712+
public void clear() {
1713+
list.clear();
1714+
}
1715+
1716+
public int indexOf(Object o) {
1717+
return list.indexOf(o);
1718+
}
1719+
1720+
public Iterator<E> iterator() {
1721+
return list.iterator();
1722+
}
1723+
1724+
public int lastIndexOf(Object o) {
1725+
return list.lastIndexOf(o);
1726+
}
1727+
1728+
public ListIterator<E> listIterator() {
1729+
return list.listIterator();
1730+
}
1731+
1732+
public ListIterator<E> listIterator(int index) {
1733+
return list.listIterator(index);
1734+
}
1735+
1736+
public E remove(int index) {
1737+
return list.remove(index);
1738+
}
1739+
1740+
public E set(int index, E element) {
1741+
return list.set(index, element);
1742+
}
1743+
}
1744+
1745+
1746+
private static class Form {
1747+
1748+
private final Map<String, Object> f;
1749+
1750+
public Form() {
1751+
f = new HashMap<String, Object>();
1752+
f.put("list", new GrowingList<Object>());
1753+
}
1754+
1755+
public Map<String, Object> getF() {
1756+
return f;
1757+
}
1758+
}
1759+
16491760
}

‎org.springframework.core/src/main/java/org/springframework/core/convert/AbstractDescriptor.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import java.util.Collection;
2121
import java.util.Map;
2222

23+
import org.springframework.util.Assert;
24+
2325
/**
2426
* @author Keith Donald
2527
* @since 3.1
@@ -28,15 +30,15 @@ abstract class AbstractDescriptor {
2830

2931
private final Class<?> type;
3032

33+
3134
protected AbstractDescriptor(Class<?> type) {
32-
if (type == null) {
33-
throw new IllegalArgumentException("type cannot be null");
34-
}
35+
Assert.notNull(type, "Type must not be null");
3536
this.type = type;
3637
}
3738

39+
3840
public Class<?> getType() {
39-
return type;
41+
return this.type;
4042
}
4143

4244
public TypeDescriptor getElementTypeDescriptor() {
@@ -73,27 +75,33 @@ public TypeDescriptor getMapValueTypeDescriptor() {
7375
}
7476
}
7577

76-
public abstract Annotation[] getAnnotations();
77-
7878
public AbstractDescriptor nested() {
7979
if (isCollection()) {
8080
Class<?> elementType = resolveCollectionElementType();
81-
return elementType != null ? nested(elementType, 0) : null;
81+
return (elementType != null ? nested(elementType, 0) : null);
8282
}
8383
else if (isArray()) {
8484
return nested(getType().getComponentType(), 0);
8585
}
8686
else if (isMap()) {
8787
Class<?> mapValueType = resolveMapValueType();
88-
return mapValueType != null ? nested(mapValueType, 1) : null;
88+
return (mapValueType != null ? nested(mapValueType, 1) : null);
89+
}
90+
else if (Object.class.equals(getType())) {
91+
// could be a collection type but we don't know about its element type,
92+
// so let's just assume there is an element type of type Object
93+
return this;
8994
}
9095
else {
9196
throw new IllegalStateException("Not a collection, array, or map: cannot resolve nested value types");
9297
}
9398
}
9499

100+
95101
// subclassing hooks
96102

103+
public abstract Annotation[] getAnnotations();
104+
97105
protected abstract Class<?> resolveCollectionElementType();
98106

99107
protected abstract Class<?> resolveMapKeyType();
@@ -102,6 +110,7 @@ else if (isMap()) {
102110

103111
protected abstract AbstractDescriptor nested(Class<?> type, int typeIndex);
104112

113+
105114
// internal helpers
106115

107116
private boolean isCollection() {

‎org.springframework.core/src/main/java/org/springframework/core/convert/BeanPropertyDescriptor.java

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@
2020
import org.springframework.core.GenericCollectionTypeResolver;
2121
import org.springframework.core.MethodParameter;
2222

23+
/**
24+
* @author Keith Donald
25+
* @since 3.1
26+
*/
2327
class BeanPropertyDescriptor extends AbstractDescriptor {
2428

2529
private final Property property;
@@ -28,41 +32,44 @@ class BeanPropertyDescriptor extends AbstractDescriptor {
2832

2933
private final Annotation[] annotations;
3034

35+
3136
public BeanPropertyDescriptor(Property property) {
3237
super(property.getType());
3338
this.property = property;
3439
this.methodParameter = property.getMethodParameter();
3540
this.annotations = property.getAnnotations();
3641
}
3742

43+
3844
@Override
3945
public Annotation[] getAnnotations() {
40-
return annotations;
46+
return this.annotations;
4147
}
4248

4349
@Override
4450
protected Class<?> resolveCollectionElementType() {
45-
return GenericCollectionTypeResolver.getCollectionParameterType(methodParameter);
51+
return GenericCollectionTypeResolver.getCollectionParameterType(this.methodParameter);
4652
}
4753

4854
@Override
4955
protected Class<?> resolveMapKeyType() {
50-
return GenericCollectionTypeResolver.getMapKeyParameterType(methodParameter);
56+
return GenericCollectionTypeResolver.getMapKeyParameterType(this.methodParameter);
5157
}
5258

5359
@Override
5460
protected Class<?> resolveMapValueType() {
55-
return GenericCollectionTypeResolver.getMapValueParameterType(methodParameter);
61+
return GenericCollectionTypeResolver.getMapValueParameterType(this.methodParameter);
5662
}
5763

5864
@Override
5965
protected AbstractDescriptor nested(Class<?> type, int typeIndex) {
6066
MethodParameter methodParameter = new MethodParameter(this.methodParameter);
6167
methodParameter.increaseNestingLevel();
6268
methodParameter.setTypeIndexForCurrentLevel(typeIndex);
63-
return new BeanPropertyDescriptor(type, property, methodParameter, annotations);
69+
return new BeanPropertyDescriptor(type, this.property, methodParameter, this.annotations);
6470
}
6571

72+
6673
// internal
6774

6875
private BeanPropertyDescriptor(Class<?> type, Property propertyDescriptor, MethodParameter methodParameter, Annotation[] annotations) {
@@ -72,4 +79,4 @@ private BeanPropertyDescriptor(Class<?> type, Property propertyDescriptor, Metho
7279
this.annotations = annotations;
7380
}
7481

75-
}
82+
}

‎org.springframework.core/src/main/java/org/springframework/core/convert/FieldDescriptor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ private FieldDescriptor(Class<?> type, Field field, int nestingLevel, int typeIn
5353

5454
@Override
5555
public Annotation[] getAnnotations() {
56-
return TypeDescriptor.nullSafeAnnotations(field.getAnnotations());
56+
return TypeDescriptor.nullSafeAnnotations(this.field.getAnnotations());
5757
}
5858

5959
@Override

0 commit comments

Comments
 (0)