craigmcc 2003/02/03 23:28:14
Modified: beanutils/src/java/org/apache/commons/beanutils
BeanUtils.java
beanutils/src/test/org/apache/commons/beanutils
BeanUtilsTestCase.java DynaBeanUtilsTestCase.java
Log:
Enhance BeanUtils.copyProperty() to deal with about 80% of the use cases for
copying indexed, mapped, and nested properties with type conversions. The
remaining restrictions are documented in the Javadocs for this method.
This is a partial response to Bugzilla #16525, which documents some
restrictions in the functionality of copyProperty() that leads people
to try setProperty() instead -- which they should not do.
It is much more conservative than the proposed patch, which (as the reporter
acknowledges) is more appropriate in a minor update (1.7) versus a bugfix
patch (1.6.1) which is currently contemplated.
PR: Bugzilla #16525
Submitted by: Tim Vernum <tpv at spamcop.net>
Revision Changes Path
1.36 +147 -62
jakarta-commons/beanutils/src/java/org/apache/commons/beanutils/BeanUtils.java
Index: BeanUtils.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/beanutils/src/java/org/apache/commons/beanutils/BeanUtils.java,v
retrieving revision 1.35
retrieving revision 1.36
diff -u -r1.35 -r1.36
--- BeanUtils.java 15 Jan 2003 21:59:38 -0000 1.35
+++ BeanUtils.java 4 Feb 2003 07:28:14 -0000 1.36
@@ -153,7 +153,7 @@
* @exception InvocationTargetException if the property accessor method
* throws an exception
* @exception NoSuchMethodException if an accessor method for this
- * propety cannot be found
+ * property cannot be found
*/
public static Object cloneBean(Object bean)
throws IllegalAccessException, InstantiationException,
@@ -275,18 +275,28 @@
/**
* <p>Copy the specified property value to the specified destination bean,
* performing any type conversion that is required. If the specified
- * bean does not have a property of the specified name, return without
+ * bean does not have a property of the specified name, or the property
+ * is read only on the destination bean, return without
* doing anything. If you have custom destination property types, register
* {@link Converter}s for them by calling the <code>register()</code>
* method of {@link ConvertUtils}.</p>
*
- * <p><strong>FIXME</strong> - Indexed and mapped properties that do not
- * have getter and setter methods for the underlying array or Map are not
- * copied by this method.</p>
+ * <p><strong>IMPLEMENTATION RESTRICTIONS</strong>:</p>
+ * <ul>
+ * <li>Does not support destination properties that are indexed,
+ * but only an indexed setter (as opposed to an array setter)
+ * is available.</li>
+ * <li>Does not support destination properties that are mapped,
+ * but only a keyed setter (as opposed to a Map setter)
+ * is available.</li>
+ * <li>The desired property type of a mapped setter cannot be
+ * determined (since Maps support any data type), so no conversion
+ * will be performed.</li>
+ * </ul>
*
* @param bean Bean on which setting is to be performed
- * @param name Simple property name of the property to be set
- * @param value Property value to be set
+ * @param name Property name (can be nested/indexed/mapped/combo)
+ * @param value Value to be set
*
* @exception IllegalAccessException if the caller does not have
* access to the property accessor method
@@ -296,6 +306,7 @@
public static void copyProperty(Object bean, String name, Object value)
throws IllegalAccessException, InvocationTargetException {
+ // Trace logging (if enabled)
if (log.isTraceEnabled()) {
StringBuffer sb = new StringBuffer(" copyProperty(");
sb.append(bean);
@@ -323,55 +334,123 @@
log.trace(sb.toString());
}
- if (bean instanceof DynaBean) {
- DynaProperty propDescriptor =
- ((DynaBean) bean).getDynaClass().getDynaProperty(name);
- if (propDescriptor != null) {
- Converter converter =
- ConvertUtils.lookup(propDescriptor.getType());
- if (converter != null) {
- value = converter.convert(propDescriptor.getType(), value);
- }
- try {
- PropertyUtils.setSimpleProperty(bean, name, value);
- } catch (NoSuchMethodException e) {
- log.error("-->Should not have happened", e);
- ; // Silently ignored
+ // Resolve any nested expression to get the actual target bean
+ Object target = bean;
+ int delim = name.lastIndexOf(PropertyUtils.NESTED_DELIM);
+ if (delim >= 0) {
+ try {
+ target =
+ PropertyUtils.getProperty(bean, name.substring(0, delim));
+ } catch (NoSuchMethodException e) {
+ return; // Skip this property setter
+ }
+ name = name.substring(delim + 1);
+ if (log.isTraceEnabled()) {
+ log.trace(" Target bean = " + target);
+ log.trace(" Target name = " + name);
+ }
+ }
+
+ // Declare local variables we will require
+ String propName = null; // Simple name of target property
+ Class type = null; // Java type of target property
+ int index = -1; // Indexed subscript value (if any)
+ String key = null; // Mapped key value (if any)
+
+ // Calculate the target property name, index, and key values
+ propName = name;
+ int i = propName.indexOf(PropertyUtils.INDEXED_DELIM);
+ if (i >= 0) {
+ int k = propName.indexOf(PropertyUtils.INDEXED_DELIM2);
+ try {
+ index =
+ Integer.parseInt(propName.substring(i + 1, k));
+ } catch (NumberFormatException e) {
+ ;
+ }
+ propName = propName.substring(0, i);
+ }
+ int j = propName.indexOf(PropertyUtils.MAPPED_DELIM);
+ if (j >= 0) {
+ int k = propName.indexOf(PropertyUtils.MAPPED_DELIM2);
+ try {
+ key = propName.substring(j + 1, k);
+ } catch (IndexOutOfBoundsException e) {
+ ;
+ }
+ propName = propName.substring(0, j);
+ }
+
+ // Calculate the target property type
+ if (target instanceof DynaBean) {
+ DynaClass dynaClass = ((DynaBean) target).getDynaClass();
+ DynaProperty dynaProperty = dynaClass.getDynaProperty(propName);
+ if (dynaProperty == null) {
+ return; // Skip this property setter
+ }
+ type = dynaProperty.getType();
+ } else {
+ PropertyDescriptor descriptor = null;
+ try {
+ descriptor =
+ PropertyUtils.getPropertyDescriptor(target, name);
+ if (descriptor == null) {
+ return; // Skip this property setter
}
- } else {
+ } catch (NoSuchMethodException e) {
+ return; // Skip this property setter
+ }
+ type = descriptor.getPropertyType();
+ if (type == null) {
+ // Most likely an indexed setter on a POJB only
if (log.isTraceEnabled()) {
- log.trace("-->No setter on 'to' DynaBean, skipping");
+ log.trace(" target type for property '" +
+ propName + "' is null, so skipping ths setter");
}
+ return;
+ }
+ }
+ if (log.isTraceEnabled()) {
+ log.trace(" target propName=" + propName + ", type=" +
+ type + ", index=" + index + ", key=" + key);
+ }
+
+ // Convert the specified value to the required type and store it
+ if (index >= 0) { // Destination must be indexed
+ Converter converter = ConvertUtils.lookup(type.getComponentType());
+ if (converter != null) {
+ log.trace(" USING CONVERTER " + converter);
+ value = converter.convert(type, value);
}
- } else /* if (!(bean instanceof DynaBean)) */ {
- PropertyDescriptor propDescriptor = null;
try {
- propDescriptor =
- PropertyUtils.getPropertyDescriptor(bean, name);
+ PropertyUtils.setIndexedProperty(target, propName,
+ index, value);
} catch (NoSuchMethodException e) {
- propDescriptor = null;
+ throw new InvocationTargetException
+ (e, "Cannot set " + propName);
}
- if ((propDescriptor != null) &&
- (propDescriptor.getWriteMethod() == null)) {
- propDescriptor = null;
- }
- if (propDescriptor != null) {
- Converter converter =
- ConvertUtils.lookup(propDescriptor.getPropertyType());
- if (converter != null) {
- value = converter.convert
- (propDescriptor.getPropertyType(), value);
- }
- try {
- PropertyUtils.setSimpleProperty(bean, name, value);
- } catch (NoSuchMethodException e) {
- log.error("-->Should not have happened", e);
- ; // Silently ignored
- }
- } else {
- if (log.isTraceEnabled()) {
- log.trace("-->No setter on JavaBean, skipping");
- }
+ } else if (key != null) { // Destination must be mapped
+ // Maps do not know what the preferred data type is,
+ // so perform no conversions at all
+ // FIXME - should we create or support a TypedMap?
+ try {
+ PropertyUtils.setMappedProperty(target, propName,
+ key, value);
+ } catch (NoSuchMethodException e) {
+ throw new InvocationTargetException
+ (e, "Cannot set " + propName);
+ }
+ } else { // Destination must be simple
+ Converter converter = ConvertUtils.lookup(type);
+ if (converter != null) {
+ log.trace(" USING CONVERTER " + converter);
+ value = converter.convert(type, value);
+ }
+ try {
+ PropertyUtils.setSimpleProperty(target, propName, value);
+ } catch (NoSuchMethodException e) {
+ throw new InvocationTargetException
+ (e, "Cannot set " + propName);
}
}
@@ -392,7 +471,7 @@
* @exception InvocationTargetException if the property accessor method
* throws an exception
* @exception NoSuchMethodException if an accessor method for this
- * propety cannot be found
+ * property cannot be found
*/
public static Map describe(Object bean)
throws IllegalAccessException, InvocationTargetException,
@@ -441,7 +520,7 @@
* @exception InvocationTargetException if the property accessor method
* throws an exception
* @exception NoSuchMethodException if an accessor method for this
- * propety cannot be found
+ * property cannot be found
*/
public static String[] getArrayProperty(Object bean, String name)
throws IllegalAccessException, InvocationTargetException,
@@ -499,7 +578,7 @@
* @exception InvocationTargetException if the property accessor method
* throws an exception
* @exception NoSuchMethodException if an accessor method for this
- * propety cannot be found
+ * property cannot be found
*/
public static String getIndexedProperty(Object bean, String name)
throws IllegalAccessException, InvocationTargetException,
@@ -525,7 +604,7 @@
* @exception InvocationTargetException if the property accessor method
* throws an exception
* @exception NoSuchMethodException if an accessor method for this
- * propety cannot be found
+ * property cannot be found
*/
public static String getIndexedProperty(Object bean,
String name, int index)
@@ -554,7 +633,7 @@
* @exception InvocationTargetException if the property accessor method
* throws an exception
* @exception NoSuchMethodException if an accessor method for this
- * propety cannot be found
+ * property cannot be found
*/
public static String getMappedProperty(Object bean, String name)
throws IllegalAccessException, InvocationTargetException,
@@ -580,7 +659,7 @@
* @exception InvocationTargetException if the property accessor method
* throws an exception
* @exception NoSuchMethodException if an accessor method for this
- * propety cannot be found
+ * property cannot be found
*/
public static String getMappedProperty(Object bean,
String name, String key)
@@ -607,7 +686,7 @@
* @exception InvocationTargetException if the property accessor method
* throws an exception
* @exception NoSuchMethodException if an accessor method for this
- * propety cannot be found
+ * property cannot be found
*/
public static String getNestedProperty(Object bean, String name)
throws IllegalAccessException, InvocationTargetException,
@@ -632,7 +711,7 @@
* @exception InvocationTargetException if the property accessor method
* throws an exception
* @exception NoSuchMethodException if an accessor method for this
- * propety cannot be found
+ * property cannot be found
*/
public static String getProperty(Object bean, String name)
throws IllegalAccessException, InvocationTargetException,
@@ -655,7 +734,7 @@
* @exception InvocationTargetException if the property accessor method
* throws an exception
* @exception NoSuchMethodException if an accessor method for this
- * propety cannot be found
+ * property cannot be found
*/
public static String getSimpleProperty(Object bean, String name)
throws IllegalAccessException, InvocationTargetException,
@@ -747,6 +826,12 @@
* to meet the needs of <code>populate()</code>, and is probably not what
* you want for general property copying with type conversion. For that
* purpose, check out the <code>copyProperty()</code> method instead.</p>
+ *
+ * <p><strong>WARNING</strong> - PLEASE do not modify the behavior of this
+ * method without consulting with the Struts developer community. There
+ * are some subtleties to its functionality that are not documented in the
+ * Javadoc description above, yet are vital to the way that Struts utilizes
+ * this method.</p>
*
* @param bean Bean on which setting is to be performed
* @param name Property name (can be nested/indexed/mapped/combo)
1.20 +125 -4
jakarta-commons/beanutils/src/test/org/apache/commons/beanutils/BeanUtilsTestCase.java
Index: BeanUtilsTestCase.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/beanutils/src/test/org/apache/commons/beanutils/BeanUtilsTestCase.java,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -r1.19 -r1.20
--- BeanUtilsTestCase.java 1 Feb 2003 08:14:33 -0000 1.19
+++ BeanUtilsTestCase.java 4 Feb 2003 07:28:14 -0000 1.20
@@ -64,6 +64,7 @@
import java.lang.reflect.InvocationTargetException;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.Map;
import junit.framework.Test;
@@ -1112,6 +1113,126 @@
BeanUtils.copyProperty(bean, "shortProperty", new Short((short) 123));
assertEquals((short) 123, bean.getShortProperty());
+ }
+
+
+ /**
+ * Test copying a property using a nested indexed array expression,
+ * with and without conversions.
+ */
+ public void testCopyPropertyNestedIndexedArray() throws Exception {
+
+ int origArray[] = { 0, 10, 20, 30, 40 };
+ int intArray[] = { 0, 0, 0 };
+ bean.getNested().setIntArray(intArray);
+ int intChanged[] = { 0, 0, 0 };
+
+ // No conversion required
+ BeanUtils.copyProperty(bean, "nested.intArray[1]", new Integer(1));
+ checkIntArray(bean.getIntArray(), origArray);
+ intChanged[1] = 1;
+ checkIntArray(bean.getNested().getIntArray(), intChanged);
+
+ // Widening conversion required
+ BeanUtils.copyProperty(bean, "nested.intArray[1]", new Byte((byte) 2));
+ checkIntArray(bean.getIntArray(), origArray);
+ intChanged[1] = 2;
+ checkIntArray(bean.getNested().getIntArray(), intChanged);
+
+ // Narrowing conversion required
+ BeanUtils.copyProperty(bean, "nested.intArray[1]", new Long((long) 3));
+ checkIntArray(bean.getIntArray(), origArray);
+ intChanged[1] = 3;
+ checkIntArray(bean.getNested().getIntArray(), intChanged);
+
+ // String conversion required
+ BeanUtils.copyProperty(bean, "nested.intArray[1]", "4");
+ checkIntArray(bean.getIntArray(), origArray);
+ intChanged[1] = 4;
+ checkIntArray(bean.getNested().getIntArray(), intChanged);
+
+ }
+
+
+ /**
+ * Test copying a property using a nested mapped map property.
+ */
+ public void testCopyPropertyNestedMappedMap() throws Exception {
+
+ Map origMap = new HashMap();
+ origMap.put("First Key", "First Value");
+ origMap.put("Second Key", "Second Value");
+ Map changedMap = new HashMap();
+ changedMap.put("First Key", "First Value");
+ changedMap.put("Second Key", "Second Value");
+
+ // No conversion required
+ BeanUtils.copyProperty(bean, "nested.mapProperty(Second Key)",
+ "New Second Value");
+ checkMap(bean.getMapProperty(), origMap);
+ changedMap.put("Second Key", "New Second Value");
+ checkMap(bean.getNested().getMapProperty(), changedMap);
+
+ }
+
+
+ /**
+ * Test copying a property using a nested simple expression, with and
+ * without conversions.
+ */
+ public void testCopyPropertyNestedSimple() throws Exception {
+
+ bean.setIntProperty(0);
+ bean.getNested().setIntProperty(0);
+
+ // No conversion required
+ BeanUtils.copyProperty(bean, "nested.intProperty", new Integer(1));
+ assertNotNull(bean.getNested());
+ assertEquals(0, bean.getIntProperty());
+ assertEquals(1, bean.getNested().getIntProperty());
+
+ // Widening conversion required
+ BeanUtils.copyProperty(bean, "nested.intProperty", new Byte((byte) 2));
+ assertNotNull(bean.getNested());
+ assertEquals(0, bean.getIntProperty());
+ assertEquals(2, bean.getNested().getIntProperty());
+
+ // Narrowing conversion required
+ BeanUtils.copyProperty(bean, "nested.intProperty", new Long((long) 3));
+ assertNotNull(bean.getNested());
+ assertEquals(0, bean.getIntProperty());
+ assertEquals(3, bean.getNested().getIntProperty());
+
+ // String conversion required
+ BeanUtils.copyProperty(bean, "nested.intProperty", "4");
+ assertNotNull(bean.getNested());
+ assertEquals(0, bean.getIntProperty());
+ assertEquals(4, bean.getNested().getIntProperty());
+
+ }
+
+
+ // Ensure that the actual int[] matches the expected int[]
+ protected void checkIntArray(int actual[], int expected[]) {
+ assertNotNull("actual array not null", actual);
+ assertEquals("actual array length", expected.length, actual.length);
+ for (int i = 0; i < actual.length; i++) {
+ assertEquals("actual array value[" + i + "]",
+ expected[i], actual[i]);
+ }
+ }
+
+
+ // Ensure that the actual Map matches the expected Map
+ protected void checkMap(Map actual, Map expected) {
+ assertNotNull("actual map not null", actual);
+ assertEquals("actual map size", expected.size(), actual.size());
+ Iterator keys = expected.keySet().iterator();
+ while (keys.hasNext()) {
+ Object key = keys.next();
+ assertEquals("actual map value(" + key + ")",
+ expected.get(key), actual.get(key));
+ }
}
1.17 +131 -4
jakarta-commons/beanutils/src/test/org/apache/commons/beanutils/DynaBeanUtilsTestCase.java
Index: DynaBeanUtilsTestCase.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/beanutils/src/test/org/apache/commons/beanutils/DynaBeanUtilsTestCase.java,v
retrieving revision 1.16
retrieving revision 1.17
diff -u -r1.16 -r1.17
--- DynaBeanUtilsTestCase.java 1 Feb 2003 08:14:33 -0000 1.16
+++ DynaBeanUtilsTestCase.java 4 Feb 2003 07:28:14 -0000 1.17
@@ -65,6 +65,7 @@
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -113,6 +114,7 @@
"intProperty",
"listIndexed",
"longProperty",
+ "mapProperty",
"mappedProperty",
"mappedIntProperty",
"nested",
@@ -173,6 +175,10 @@
listIndexed.add("String 4");
bean.set("listIndexed", listIndexed);
bean.set("longProperty", new Long((long) 321));
+ HashMap mapProperty = new HashMap();
+ mapProperty.put("First Key", "First Value");
+ mapProperty.put("Second Key", "Second Value");
+ bean.set("mapProperty", mapProperty);
HashMap mappedProperty = new HashMap();
mappedProperty.put("First Key", "First Value");
mappedProperty.put("Second Key", "Second Value");
@@ -1037,9 +1043,129 @@
}
+ /**
+ * Test copying a property using a nested indexed array expression,
+ * with and without conversions.
+ */
+ public void testCopyPropertyNestedIndexedArray() throws Exception {
+
+ int origArray[] = { 0, 10, 20, 30, 40};
+ int intArray[] = { 0, 0, 0 };
+ ((TestBean) bean.get("nested")).setIntArray(intArray);
+ int intChanged[] = { 0, 0, 0 };
+
+ // No conversion required
+ BeanUtils.copyProperty(bean, "nested.intArray[1]", new Integer(1));
+ checkIntArray((int[]) bean.get("intArray"), origArray);
+ intChanged[1] = 1;
+ checkIntArray(((TestBean) bean.get("nested")).getIntArray(),
+ intChanged);
+
+ // Widening conversion required
+ BeanUtils.copyProperty(bean, "nested.intArray[1]", new Byte((byte) 2));
+ checkIntArray((int[]) bean.get("intArray"), origArray);
+ intChanged[1] = 2;
+ checkIntArray(((TestBean) bean.get("nested")).getIntArray(),
+ intChanged);
+
+ // Narrowing conversion required
+ BeanUtils.copyProperty(bean, "nested.intArray[1]", new Long((long) 3));
+ checkIntArray((int[]) bean.get("intArray"), origArray);
+ intChanged[1] = 3;
+ checkIntArray(((TestBean) bean.get("nested")).getIntArray(),
+ intChanged);
+
+ // String conversion required
+ BeanUtils.copyProperty(bean, "nested.intArray[1]", "4");
+ checkIntArray((int[]) bean.get("intArray"), origArray);
+ intChanged[1] = 4;
+ checkIntArray(((TestBean) bean.get("nested")).getIntArray(),
+ intChanged);
+
+ }
+
+
+ /**
+ * Test copying a property using a nested mapped map property.
+ */
+ public void testCopyPropertyNestedMappedMap() throws Exception {
+
+ Map origMap = new HashMap();
+ origMap.put("First Key", "First Value");
+ origMap.put("Second Key", "Second Value");
+ Map changedMap = new HashMap();
+ changedMap.put("First Key", "First Value");
+ changedMap.put("Second Key", "Second Value");
+
+ // No conversion required
+ BeanUtils.copyProperty(bean, "nested.mapProperty(Second Key)",
+ "New Second Value");
+ checkMap((Map) bean.get("mapProperty"), origMap);
+ changedMap.put("Second Key", "New Second Value");
+ checkMap(((TestBean) bean.get("nested")).getMapProperty(), changedMap);
+
+ }
+
+
+ /**
+ * Test copying a property using a nested simple expression, with and
+ * without conversions.
+ */
+ public void testCopyPropertyNestedSimple() throws Exception {
+
+ bean.set("intProperty", new Integer(0));
+ nested.setIntProperty(0);
+
+ // No conversion required
+ BeanUtils.copyProperty(bean, "nested.intProperty", new Integer(1));
+ assertEquals(0, ((Integer) bean.get("intProperty")).intValue());
+ assertEquals(1, nested.getIntProperty());
+
+ // Widening conversion required
+ BeanUtils.copyProperty(bean, "nested.intProperty", new Byte((byte) 2));
+ assertEquals(0, ((Integer) bean.get("intProperty")).intValue());
+ assertEquals(2, nested.getIntProperty());
+
+ // Narrowing conversion required
+ BeanUtils.copyProperty(bean, "nested.intProperty", new Long((long) 3));
+ assertEquals(0, ((Integer) bean.get("intProperty")).intValue());
+ assertEquals(3, nested.getIntProperty());
+
+ // String conversion required
+ BeanUtils.copyProperty(bean, "nested.intProperty", "4");
+ assertEquals(0, ((Integer) bean.get("intProperty")).intValue());
+ assertEquals(4, nested.getIntProperty());
+
+ }
+
+
// ------------------------------------------------------ Protected Methods
+ // Ensure that the nested intArray matches the specified values
+ protected void checkIntArray(int actual[], int expected[]) {
+ assertNotNull("actual array not null", actual);
+ assertEquals("actual array length", expected.length, actual.length);
+ for (int i = 0; i < actual.length; i++) {
+ assertEquals("actual array value[" + i + "]",
+ expected[i], actual[i]);
+ }
+ }
+
+
+ // Ensure that the actual Map matches the expected Map
+ protected void checkMap(Map actual, Map expected) {
+ assertNotNull("actual map not null", actual);
+ assertEquals("actual map size", expected.size(), actual.size());
+ Iterator keys = expected.keySet().iterator();
+ while (keys.hasNext()) {
+ Object key = keys.next();
+ assertEquals("actual map value(" + key + ")",
+ expected.get(key), actual.get(key));
+ }
+ }
+
+
/**
* Create and return a <code>DynaClass</code> instance for our test
* <code>DynaBean</code>.
@@ -1063,6 +1189,7 @@
new DynaProperty("intProperty", Integer.TYPE),
new DynaProperty("listIndexed", List.class),
new DynaProperty("longProperty", Long.TYPE),
+ new DynaProperty("mapProperty", Map.class),
new DynaProperty("mappedProperty", Map.class),
new DynaProperty("mappedIntProperty", Map.class),
new DynaProperty("nested", TestBean.class),
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
