From ac3c670f704ee56be5b6e0db80a8a32b1d75ff53 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 28 Jan 2014 01:20:27 +0100 Subject: [PATCH] Fixed type resolution in case of inconsistencies between read and write method Issue: SPR-11361 --- .../springframework/beans/BeanWrapper.java | 16 +++--- .../beans/BeanWrapperImpl.java | 54 +++++++++--------- .../beans/BeanWrapperTests.java | 57 ++++++++++++------- 3 files changed, 73 insertions(+), 54 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/BeanWrapper.java b/spring-beans/src/main/java/org/springframework/beans/BeanWrapper.java index 628ee8e78a5..16992e2e16b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/BeanWrapper.java +++ b/spring-beans/src/main/java/org/springframework/beans/BeanWrapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2014 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. @@ -59,7 +59,7 @@ public interface BeanWrapper extends ConfigurablePropertyAccessor { * @return the type of the wrapped bean instance, * or {@code null} if no wrapped object has been set */ - Class getWrappedClass(); + Class getWrappedClass(); /** * Obtain the PropertyDescriptors for the wrapped object @@ -79,11 +79,13 @@ public interface BeanWrapper extends ConfigurablePropertyAccessor { PropertyDescriptor getPropertyDescriptor(String propertyName) throws InvalidPropertyException; /** - * Set whether this BeanWrapper should attempt to "auto-grow" a nested path that contains a null value. - *

If "true", a null path location will be populated with a default object value and traversed - * instead of resulting in a {@link NullValueInNestedPathException}. Turning this flag on also - * enables auto-growth of collection elements when accessing an out-of-bounds index. - *

Default is "false" on a plain BeanWrapper. + * Set whether this BeanWrapper should attempt to "auto-grow" a + * nested path that contains a {@code null} value. + *

If {@code true}, a {@code null} path location will be populated + * with a default object value and traversed instead of resulting in a + * {@link NullValueInNestedPathException}. Turning this flag on also enables + * auto-growth of collection elements when accessing an out-of-bounds index. + *

Default is {@code false} on a plain BeanWrapper. */ void setAutoGrowNestedPaths(boolean autoGrowNestedPaths); diff --git a/spring-beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java b/spring-beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java index df5def12a8d..48782fc084d 100644 --- a/spring-beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java +++ b/spring-beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2014 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. @@ -223,7 +223,7 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra return this.object; } - public final Class getWrappedClass() { + public final Class getWrappedClass() { return (this.object != null ? this.object.getClass() : null); } @@ -246,7 +246,7 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra * Return the class of the root object at the top of the path of this BeanWrapper. * @see #getNestedPath */ - public final Class getRootClass() { + public final Class getRootClass() { return (this.rootObject != null ? this.rootObject.getClass() : null); } @@ -304,7 +304,7 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra * Needs to be called when the target object changes. * @param clazz the class to introspect */ - protected void setIntrospectionClass(Class clazz) { + protected void setIntrospectionClass(Class clazz) { if (this.cachedIntrospectionResults != null && !clazz.equals(this.cachedIntrospectionResults.getBeanClass())) { this.cachedIntrospectionResults = null; @@ -352,7 +352,7 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra } @Override - public Class getPropertyType(String propertyName) throws BeansException { + public Class getPropertyType(String propertyName) throws BeansException { try { PropertyDescriptor pd = getPropertyDescriptorInternal(propertyName); if (pd != null) { @@ -366,7 +366,7 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra } // Check to see if there is a custom editor, // which might give an indication on the desired target type. - Class editorType = guessPropertyTypeFromEditors(propertyName); + Class editorType = guessPropertyTypeFromEditors(propertyName); if (editorType != null) { return editorType; } @@ -485,13 +485,13 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra throw new InvalidPropertyException(getRootClass(), this.nestedPath + propertyName, "No property '" + propertyName + "' found"); } - return convertForProperty(propertyName, null, value, pd); + return convertForProperty(propertyName, null, value, new TypeDescriptor(property(pd))); } - private Object convertForProperty(String propertyName, Object oldValue, Object newValue, PropertyDescriptor pd) + private Object convertForProperty(String propertyName, Object oldValue, Object newValue, TypeDescriptor td) throws TypeMismatchException { - return convertIfNecessary(propertyName, oldValue, newValue, pd.getPropertyType(), new TypeDescriptor(property(pd))); + return convertIfNecessary(propertyName, oldValue, newValue, td.getType(), td); } private Property property(PropertyDescriptor pd) { @@ -699,7 +699,8 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra return nestedBw.getPropertyValue(tokens); } - private Object getPropertyValue(PropertyTokenHolder tokens) throws BeansException { + @SuppressWarnings("unchecked") + private Object getPropertyValue(PropertyTokenHolder tokens) throws BeansException { String propertyName = tokens.canonicalName; String actualName = tokens.actualName; PropertyDescriptor pd = getCachedIntrospectionResults().getPropertyDescriptor(actualName); @@ -766,20 +767,20 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra } else if (value instanceof List) { int index = Integer.parseInt(key); - List list = (List) value; + List list = (List) value; growCollectionIfNecessary(list, index, indexedPropertyName, pd, i + 1); value = list.get(index); } else if (value instanceof Set) { // Apply index to Iterator in case of a Set. - Set set = (Set) value; + Set set = (Set) value; int index = Integer.parseInt(key); if (index < 0 || index >= set.size()) { throw new InvalidPropertyException(getRootClass(), this.nestedPath + propertyName, "Cannot get element with index " + index + " from Set of size " + set.size() + ", accessed using property path '" + propertyName + "'"); } - Iterator it = set.iterator(); + Iterator it = set.iterator(); for (int j = 0; it.hasNext(); j++) { Object elem = it.next(); if (j == index) { @@ -789,11 +790,12 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra } } else if (value instanceof Map) { - Map map = (Map) value; + Map map = (Map) value; Class mapKeyType = GenericCollectionTypeResolver.getMapKeyReturnType(pd.getReadMethod(), i + 1); // IMPORTANT: Do not pass full property name in here - property editors // must not kick in for map keys but rather only for map values. - TypeDescriptor typeDescriptor = mapKeyType != null ? TypeDescriptor.valueOf(mapKeyType) : TypeDescriptor.valueOf(Object.class); + TypeDescriptor typeDescriptor = (mapKeyType != null ? + TypeDescriptor.valueOf(mapKeyType) : TypeDescriptor.valueOf(Object.class)); Object convertedMapKey = convertIfNecessary(null, null, key, mapKeyType, typeDescriptor); value = map.get(convertedMapKey); } @@ -850,16 +852,15 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra } } - @SuppressWarnings("unchecked") - private void growCollectionIfNecessary( - Collection collection, int index, String name, PropertyDescriptor pd, int nestingLevel) { + private void growCollectionIfNecessary(Collection collection, int index, String name, + PropertyDescriptor pd, int nestingLevel) { if (!this.autoGrowNestedPaths) { return; } int size = collection.size(); if (index >= size && index < this.autoGrowCollectionLimit) { - Class elementType = GenericCollectionTypeResolver.getCollectionReturnType(pd.getReadMethod(), nestingLevel); + Class elementType = GenericCollectionTypeResolver.getCollectionReturnType(pd.getReadMethod(), nestingLevel); if (elementType != null) { for (int i = collection.size(); i < index + 1; i++) { collection.add(newValue(elementType, name)); @@ -945,7 +946,7 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra } if (propValue.getClass().isArray()) { PropertyDescriptor pd = getCachedIntrospectionResults().getPropertyDescriptor(actualName); - Class requiredType = propValue.getClass().getComponentType(); + Class requiredType = propValue.getClass().getComponentType(); int arrayIndex = Integer.parseInt(key); Object oldValue = null; try { @@ -963,9 +964,9 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra } else if (propValue instanceof List) { PropertyDescriptor pd = getCachedIntrospectionResults().getPropertyDescriptor(actualName); - Class requiredType = GenericCollectionTypeResolver.getCollectionReturnType( + Class requiredType = GenericCollectionTypeResolver.getCollectionReturnType( pd.getReadMethod(), tokens.keys.length); - List list = (List) propValue; + List list = (List) propValue; int index = Integer.parseInt(key); Object oldValue = null; if (isExtractOldValueForEditor() && index < list.size()) { @@ -1000,11 +1001,11 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra } else if (propValue instanceof Map) { PropertyDescriptor pd = getCachedIntrospectionResults().getPropertyDescriptor(actualName); - Class mapKeyType = GenericCollectionTypeResolver.getMapKeyReturnType( + Class mapKeyType = GenericCollectionTypeResolver.getMapKeyReturnType( pd.getReadMethod(), tokens.keys.length); - Class mapValueType = GenericCollectionTypeResolver.getMapValueReturnType( + Class mapValueType = GenericCollectionTypeResolver.getMapValueReturnType( pd.getReadMethod(), tokens.keys.length); - Map map = (Map) propValue; + Map map = (Map) propValue; // IMPORTANT: Do not pass full property name in here - property editors // must not kick in for map keys but rather only for map values. TypeDescriptor typeDescriptor = (mapKeyType != null ? @@ -1094,7 +1095,8 @@ public class BeanWrapperImpl extends AbstractPropertyAccessor implements BeanWra } } } - valueToApply = convertForProperty(propertyName, oldValue, originalValue, pd); + valueToApply = convertForProperty( + propertyName, oldValue, originalValue, new TypeDescriptor(property(pd))); } pv.getOriginalPropertyValue().conversionNecessary = (valueToApply != originalValue); } diff --git a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java index 17dcfd65345..75d5bc10854 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -16,13 +16,6 @@ package org.springframework.beans; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.beans.PropertyEditorSupport; import java.math.BigDecimal; import java.math.BigInteger; @@ -44,11 +37,16 @@ import java.util.TreeSet; import org.apache.commons.logging.LogFactory; import org.junit.Test; + import org.springframework.beans.factory.annotation.Autowire; import org.springframework.beans.propertyeditors.CustomNumberEditor; import org.springframework.beans.propertyeditors.StringArrayPropertyEditor; import org.springframework.beans.propertyeditors.StringTrimmerEditor; import org.springframework.beans.support.DerivedFromProtectedBaseBean; +import org.springframework.core.convert.ConversionFailedException; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.core.convert.support.GenericConversionService; import org.springframework.tests.Assume; import org.springframework.tests.TestGroup; import org.springframework.tests.sample.beans.BooleanTestBean; @@ -56,15 +54,10 @@ import org.springframework.tests.sample.beans.ITestBean; import org.springframework.tests.sample.beans.IndexedTestBean; import org.springframework.tests.sample.beans.NumberTestBean; import org.springframework.tests.sample.beans.TestBean; -import org.springframework.core.convert.ConversionFailedException; -import org.springframework.core.convert.TypeDescriptor; -import org.springframework.core.convert.support.DefaultConversionService; -import org.springframework.core.convert.support.GenericConversionService; import org.springframework.util.StopWatch; import org.springframework.util.StringUtils; import static org.hamcrest.Matchers.*; - import static org.junit.Assert.*; @@ -1557,17 +1550,15 @@ public final class BeanWrapperTests { @Test public void cornerSpr10115() { Spr10115Bean foo = new Spr10115Bean(); - BeanWrapperImpl bwi = new BeanWrapperImpl(); - bwi.setWrappedInstance(foo); + BeanWrapperImpl bwi = new BeanWrapperImpl(foo); bwi.setPropertyValue("prop1", "val1"); assertEquals("val1", Spr10115Bean.prop1); } @Test - public void testArrayToObject() throws Exception { + public void testArrayToObject() { ArrayToObject foo = new ArrayToObject(); - BeanWrapperImpl bwi = new BeanWrapperImpl(); - bwi.setWrappedInstance(foo); + BeanWrapperImpl bwi = new BeanWrapperImpl(foo); Object[] array = new Object[] {"1","2"}; bwi.setPropertyValue("object", array ); @@ -1576,9 +1567,20 @@ public final class BeanWrapperTests { array = new Object[] {"1"}; bwi.setPropertyValue("object", array ); assertThat(foo.getObject(), equalTo((Object) array)); -} + } + + @Test + public void testPropertyTypeMismatch() { + PropertyTypeMismatch foo = new PropertyTypeMismatch(); + BeanWrapperImpl bwi = new BeanWrapperImpl(foo); + bwi.setPropertyValue("object", "a String"); + assertEquals("a String", foo.value); + assertEquals(8, bwi.getPropertyValue("object")); + } + static class Spr10115Bean { + private static String prop1; public static void setProp1(String prop1) { @@ -1963,11 +1965,10 @@ public final class BeanWrapperTests { } - static class ArrayToObject { + public static class ArrayToObject { private Object object; - public void setObject(Object object) { this.object = object; } @@ -1975,6 +1976,20 @@ public final class BeanWrapperTests { public Object getObject() { return object; } + } + + public static class PropertyTypeMismatch { + + public String value; + + public void setObject(String object) { + this.value = object; + } + + public Integer getObject() { + return (this.value != null ? this.value.length() : null); + } } + }