You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sk...@apache.org on 2005/05/29 08:55:50 UTC

svn commit: r178929 - in /jakarta/commons/proper/beanutils/trunk/src: java/org/apache/commons/beanutils/PropertyUtilsBean.java test/org/apache/commons/beanutils/PropertyUtilsTestCase.java test/org/apache/commons/beanutils/PropsFirstPropertyUtilsBean.java

Author: skitching
Date: Sat May 28 23:55:48 2005
New Revision: 178929

URL: http://svn.apache.org/viewcvs?rev=178929&view=rev
Log:
bugzilla #23815. 

Create new methods getPropertyOfMapBean and setPropertyOfMapBean that the existing
setNestedProperty and getNestedProperty methods now call when they discover the
bean they are accessing implements Map. This makes it much easier for users to
subclass and customise this behaviour of PropertyUtilsBean, eg in order to
restore pre-1.5 behaviour.

This patch also causes an exception to be thrown when the propertyName passed to
getPropertyOfMapBean or setPropertyOfMapBean has MAPPED_DELIM or INDEXED_DELIM
chars in it. This never worked as expected before (the whole string was treated
literally as the propertyName), so throwing an exception here should not break
any existing code. It should be of help to future developers who make this
mistake though...


Added:
    jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropsFirstPropertyUtilsBean.java   (with props)
Modified:
    jakarta/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/PropertyUtilsBean.java
    jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropertyUtilsTestCase.java

Modified: jakarta/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/PropertyUtilsBean.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/PropertyUtilsBean.java?rev=178929&r1=178928&r2=178929&view=diff
==============================================================================
--- jakarta/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/PropertyUtilsBean.java (original)
+++ jakarta/commons/proper/beanutils/trunk/src/java/org/apache/commons/beanutils/PropertyUtilsBean.java Sat May 28 23:55:48 2005
@@ -659,7 +659,7 @@
             indexOfINDEXED_DELIM = next.indexOf(PropertyUtils.INDEXED_DELIM);
             indexOfMAPPED_DELIM = next.indexOf(PropertyUtils.MAPPED_DELIM);
             if (bean instanceof Map) {
-                bean = ((Map) bean).get(next);
+                bean = getPropertyOfMapBean((Map) bean, next);
             } else if (indexOfMAPPED_DELIM >= 0) {
                 bean = getMappedProperty(bean, next);
             } else if (indexOfINDEXED_DELIM >= 0) {
@@ -679,7 +679,7 @@
         indexOfMAPPED_DELIM = name.indexOf(PropertyUtils.MAPPED_DELIM);
 
         if (bean instanceof Map) {
-            bean = ((Map) bean).get(name);
+            bean = getPropertyOfMapBean((Map) bean, name);
         } else if (indexOfMAPPED_DELIM >= 0) {
             bean = getMappedProperty(bean, name);
         } else if (indexOfINDEXED_DELIM >= 0) {
@@ -691,6 +691,42 @@
 
     }
 
+    /**
+     * This method is called by getNestedProperty and setNestedProperty to
+     * define what it means to get a property from an object which implements
+     * Map. See setPropertyOfMapBean for more information.
+     * 
+     * @throws IllegalArgumentException when the propertyName is regarded as
+     * being invalid.
+     * 
+     * @throws IllegalAccessException just in case subclasses override this
+     * method to try to access real getter methods and find permission is denied.
+     * 
+     * @throws InvocationTargetException just in case subclasses override this
+     * method to try to access real getter methods, and find it throws an
+     * exception when invoked.
+     * 
+     * @throws NoSuchMethodException just in case subclasses override this
+     * method to try to access real getter methods, and want to fail if
+     * no simple method is available.
+     */
+    protected Object getPropertyOfMapBean(Map bean, String propertyName) 
+        throws IllegalArgumentException, IllegalAccessException, 
+        InvocationTargetException, NoSuchMethodException {
+
+        int indexOfINDEXED_DELIM = propertyName.indexOf(PropertyUtils.INDEXED_DELIM);
+        int indexOfMAPPED_DELIM = propertyName.indexOf(PropertyUtils.MAPPED_DELIM);
+        
+        if ((indexOfINDEXED_DELIM >= 0) || (indexOfMAPPED_DELIM >= 0)) {
+            throw new IllegalArgumentException(
+                    "Indexed or mapped properties are not supported on"
+                    + " objects of type Map: " + propertyName);
+        }
+
+        return bean.get(propertyName);
+    }
+
+
 
     /**
      * Return the value of the specified property of the specified bean,
@@ -1646,6 +1682,17 @@
     /**
      * Set the value of the (possibly nested) property of the specified
      * name, for the specified bean, with no type conversions.
+     * <p>
+     * Example values for parameter "name" are:
+     * <ul>
+     * <li> "a" -- sets the value of property a of the specified bean </li>
+     * <li> "a.b" -- gets the value of property a of the specified bean,
+     * then on that object sets the value of property b.</li>
+     * <li> "a(key)" -- sets a value of mapped-property a on the specified
+     * bean. This effectively means bean.setA("key").</li>
+     * <li> "a[3]" -- sets a value of indexed-property a on the specified
+     * bean. This effectively means bean.setA(3).</li>
+     * </ul>
      *
      * @param bean Bean whose property is to be modified
      * @param name Possibly nested name of the property to be modified
@@ -1685,7 +1732,7 @@
             indexOfINDEXED_DELIM = next.indexOf(PropertyUtils.INDEXED_DELIM);
             indexOfMAPPED_DELIM = next.indexOf(PropertyUtils.MAPPED_DELIM);
             if (bean instanceof Map) {
-                bean = ((Map) bean).get(next);
+                bean = getPropertyOfMapBean((Map)bean, next);
             } else if (indexOfMAPPED_DELIM >= 0) {
                 bean = getMappedProperty(bean, next);
             } else if (indexOfINDEXED_DELIM >= 0) {
@@ -1705,7 +1752,7 @@
         indexOfMAPPED_DELIM = name.indexOf(PropertyUtils.MAPPED_DELIM);
 
         if (bean instanceof Map) {
-            ((Map) bean).put(name, value);
+            setPropertyOfMapBean((Map) bean, name, value);
         } else if (indexOfMAPPED_DELIM >= 0) {
             setMappedProperty(bean, name, value);
         } else if (indexOfINDEXED_DELIM >= 0) {
@@ -1715,6 +1762,74 @@
         }
 
     }
+
+    /**
+     * This method is called by method setNestedProperty when the current bean
+     * is found to be a Map object, and defines how to deal with setting
+     * a property on a Map.
+     * <p>
+     * The standard implementation here is to:
+     * <ul>
+     * <li>call bean.set(propertyName) for all propertyName values.</li>
+     * <li>throw an IllegalArgumentException if the property specifier
+     * contains MAPPED_DELIM or INDEXED_DELIM, as Map entries are essentially
+     * simple properties; mapping and indexing operations do not make sense
+     * when accessing a map (even thought the returned object may be a Map
+     * or an Array).</li>
+     * </ul>
+     * <p>
+     * The default behaviour of beanutils 1.7.1 or later is for assigning to
+     * "a.b" to mean a.put(b, obj) always. However the behaviour of beanutils 
+     * version 1.6.0, 1.6.1, 1.7.0 was for "a.b" to mean a.setB(obj) if such
+     * a method existed, and a.put(b, obj) otherwise. In version 1.5 it meant
+     * a.put(b, obj) always (ie the same as the behaviour in the current version).
+     * In versions prior to 1.5 it meant a.setB(obj) always. [yes, this is 
+     * all <i>very</i> unfortunate]
+     * <p>
+     * Users who would like to customise the meaning of "a.b" in method 
+     * setNestedProperty when a is a Map can create a custom subclass of
+     * this class and override this method to implement the behaviour of 
+     * their choice, such as restoring the pre-1.4 behaviour of this class
+     * if they wish. When overriding this method, do not forget to deal 
+     * with MAPPED_DELIM and INDEXED_DELIM characters in the propertyName.
+     * <p>
+     * Note, however, that the recommended solution for objects that
+     * implement Map but want their simple properties to come first is
+     * for <i>those</i> objects to override their get/put methods to implement
+     * that behaviour, and <i>not</i> to solve the problem by modifying the
+     * default behaviour of the PropertyUtilsBean class by overriding this
+     * method.
+     * 
+     * @throws IllegalArgumentException when the propertyName is regarded as
+     * being invalid.
+     * 
+     * @throws IllegalAccessException just in case subclasses override this
+     * method to try to access real setter methods and find permission is denied.
+     * 
+     * @throws InvocationTargetException just in case subclasses override this
+     * method to try to access real setter methods, and find it throws an
+     * exception when invoked.
+     * 
+     * @throws NoSuchMethodException just in case subclasses override this
+     * method to try to access real setter methods, and want to fail if
+     * no simple method is available.
+     */
+    protected void setPropertyOfMapBean(Map bean, String propertyName, Object value)
+        throws IllegalArgumentException, IllegalAccessException, 
+        InvocationTargetException, NoSuchMethodException {
+
+        int indexOfINDEXED_DELIM = propertyName.indexOf(PropertyUtils.INDEXED_DELIM);
+        int indexOfMAPPED_DELIM = propertyName.indexOf(PropertyUtils.MAPPED_DELIM);
+        
+        if ((indexOfINDEXED_DELIM >= 0) || (indexOfMAPPED_DELIM >= 0)) {
+            throw new IllegalArgumentException(
+                    "Indexed or mapped properties are not supported on"
+                    + " objects of type Map: " + propertyName);
+        }
+
+        bean.put(propertyName, value);
+    }
+
 
 
     /**

Modified: jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropertyUtilsTestCase.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropertyUtilsTestCase.java?rev=178929&r1=178928&r2=178929&view=diff
==============================================================================
--- jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropertyUtilsTestCase.java (original)
+++ jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropertyUtilsTestCase.java Sat May 28 23:55:48 2005
@@ -3647,6 +3647,93 @@
     }
     
     /**
+     * There is an issue in setNestedProperty/getNestedProperty when the
+     * target bean is a map and the name string requests mapped or indexed
+     * operations on a field. These are not supported for fields of a Map,
+     * but it's an easy mistake to make and this test case ensures that an
+     * appropriate exception is thrown when a user does this.
+     * <p>
+     * The problem is with passing strings of form "a(b)" or "a[3]" to
+     * setNestedProperty or getNestedProperty when the target bean they
+     * are applied to implements Map. These strings are actually requesting
+     * "the result of calling mapped method a on the target object with
+     * a parameter of b" or "the result of calling indexed method a on the
+     * target object with a parameter of 3". And these requests are not valid
+     * when the target is a Map as a Map only supports calling get(fieldName)
+     * or put(fieldName), neither of which can be further indexed with a
+     * string or an integer.
+     * <p>
+     * However it is likely that some users will assume that "a[3]" when applied
+     * to a map will be equivalent to (map.get("a"))[3] with the appropriate
+     * typecasting, or for "a(b)" to be equivalent to map.get("a").get("b").
+     * <p>
+     * Here we verify that an exception is thrown if the user makes this
+     * mistake.
+     */
+    public void testNestedPropertyKeyOrIndexOnBeanImplementingMap() throws Exception {
+        HashMap map = new HashMap();
+        HashMap submap = new HashMap();
+        BetaBean betaBean1 = new BetaBean("test1");
+        BetaBean betaBean2 = new BetaBean("test2");
+        
+        // map.put("submap", submap)
+        PropertyUtils.setNestedProperty(map, "submap", submap);
+        
+        // map.get("submap").put("beta1", betaBean1)
+        PropertyUtils.setNestedProperty(map, "submap.beta1", betaBean1);
+        assertEquals("Unexpected keys in map", "submap", keysToString(map));
+        assertEquals("Unexpected keys in submap", "beta1", keysToString(submap));
+
+        try {
+            // One would expect that the command below would be equivalent to
+            //   Map m = (Map) map.get("submap");
+            //   m.put("beta2", betaBean2)
+            // However this isn't how javabeans property methods work. A map
+            // only effectively has "simple" properties, even when the
+            // returned object is a Map or Array.
+            PropertyUtils.setNestedProperty(map, "submap(beta2)", betaBean2);
+
+            // What, no exception? In that case, setNestedProperties has 
+            // probably just tried to do 
+            //    map.set("submap(beta2)", betaBean2)
+            // which is almost certainly not what the used expected. This is
+            // what beanutils 1.5.0 to 1.7.1 did....
+            fail("Exception not thrown for invalid setNestedProperty syntax");
+        } catch(IllegalArgumentException ex) {
+            // ok, getting an exception was expected. As it is of a generic
+            // type, let's check the message string to make sure it really
+            // was caused by the issue we expected.
+            int index = ex.getMessage().indexOf(
+                    "Indexed or mapped properties are not supported");
+            assertTrue("Unexpected exception message", index>=0);
+        }
+
+        try {
+            // One would expect that "submap[3]" would be equivalent to
+            //   Object[] objects = (Object[]) map.get("submap");
+            //   return objects[3];
+            // However this isn't how javabeans property methods work. A map
+            // only effectively has "simple" properties, even when the
+            // returned object is a Map or Array.
+            Object o = PropertyUtils.getNestedProperty(map, "submap[3]");
+            
+            // What, no exception? In that case, getNestedProperties has 
+            // probably just tried to do 
+            //    map.get("submap[3]")
+            // which is almost certainly not what the used expected. This is
+            // what beanutils 1.5.0 to 1.7.1 did....
+            fail("Exception not thrown for invalid setNestedProperty syntax");
+        } catch(IllegalArgumentException ex) {
+            // ok, getting an exception was expected. As it is of a generic
+            // type, let's check the message string to make sure it really
+            // was caused by the issue we expected.
+            int index = ex.getMessage().indexOf(
+                    "Indexed or mapped properties are not supported");
+            assertTrue("Unexpected exception message", index>=0);
+        }
+    }
+
+    /**
      * Returns a single string containing all the keys in the map,
      * sorted in alphabetical order and separated by ", ".
      * <p>
@@ -3699,5 +3786,42 @@
             "Set nested property on map unexpected affected simple property", 
             "new value", 
             bean.getUnusuallyNamedProperty());
+    }
+
+    /** 
+     * This tests to see that it is possible to subclass PropertyUtilsBean
+     * and change the behaviour of setNestedProperty/getNestedProperty when
+     * dealing with objects that implement Map. 
+     */
+    public void testMapExtensionCustom() throws Exception {
+        PropsFirstPropertyUtilsBean utilsBean = new PropsFirstPropertyUtilsBean();
+        ExtendMapBean bean = new ExtendMapBean();
+        
+        // hardly worth testing this, really :-)
+        bean.setUnusuallyNamedProperty("bean value");
+        assertEquals("Set property direct failed", "bean value", bean.getUnusuallyNamedProperty());
+
+        // setSimpleProperty should affect the simple property
+        utilsBean.setSimpleProperty(bean, "unusuallyNamedProperty", "new value");
+        assertEquals("Set property on map failed (1)", "new value", bean.getUnusuallyNamedProperty());
+
+        // setNestedProperty with setter should affect the simple property
+        // getNestedProperty with getter should obtain the simple property
+        utilsBean.setProperty(bean, "unusuallyNamedProperty", "next value");
+        assertEquals("Set property on map failed (2)", "next value", bean.getUnusuallyNamedProperty());
+        assertEquals("setNestedProperty on non-simple property failed", 
+                "next value",
+                utilsBean.getNestedProperty(bean, "unusuallyNamedProperty"));
+
+        // setting property without setter should update the map
+        // getting property without setter should fetch from the map
+        utilsBean.setProperty(bean, "mapProperty", "value1");
+        assertEquals("setNestedProperty on non-simple property failed", 
+                "value1", utilsBean.getNestedProperty(bean, "mapProperty"));
+        
+        HashMap myMap = new HashMap();
+        myMap.put("thebean", bean);
+        utilsBean.getNestedProperty(myMap, "thebean.mapitem");
+        utilsBean.getNestedProperty(myMap, "thebean(mapitem)");
     }
 }

Added: jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropsFirstPropertyUtilsBean.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropsFirstPropertyUtilsBean.java?rev=178929&view=auto
==============================================================================
--- jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropsFirstPropertyUtilsBean.java (added)
+++ jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropsFirstPropertyUtilsBean.java Sat May 28 23:55:48 2005
@@ -0,0 +1,60 @@
+/*
+ * Created on 29/05/2005
+ *
+ * TODO To change the template for this generated file go to
+ * Window - Preferences - Java - Code Style - Code Templates
+ */
+package org.apache.commons.beanutils;
+
+import java.util.Map;
+import java.beans.PropertyDescriptor;
+import java.lang.reflect.InvocationTargetException;
+
+/**
+ * A PropertyUtilsBean which customises the behaviour of the
+ * setNestedProperty and getNestedProperty methods to look for
+ * simple properties in preference to map entries.
+ */
+public class PropsFirstPropertyUtilsBean extends PropertyUtilsBean {
+
+    public PropsFirstPropertyUtilsBean() {
+        super();
+    }
+    
+    /**
+     * Note: this is a *very rough* override of this method. In particular,
+     * it does not handle MAPPED_DELIM and INDEXED_DELIM chars in the
+     * propertyName, so propertyNames like "a(b)" or "a[3]" will not
+     * be correctly handled.
+     */
+    protected Object getPropertyOfMapBean(Map bean, String propertyName)
+    throws IllegalAccessException, InvocationTargetException, NoSuchMethodException {
+        
+        PropertyDescriptor descriptor = getPropertyDescriptor(bean, propertyName);
+        if (descriptor == null) {
+            // no simple property exists so return the value from the map
+            return bean.get(propertyName);
+        } else {
+            // a simple property exists so return its value instead.
+            return getSimpleProperty(bean, propertyName);
+        }
+    }
+
+    /**
+     * Note: this is a *very rough* override of this method. In particular,
+     * it does not handle MAPPED_DELIM and INDEXED_DELIM chars in the
+     * propertyName, so propertyNames like "a(b)" or "a[3]" will not
+     * be correctly handled.
+     */
+    protected void setPropertyOfMapBean(Map bean, String propertyName, Object value)
+        throws IllegalAccessException, InvocationTargetException, NoSuchMethodException {
+        PropertyDescriptor descriptor = getPropertyDescriptor(bean, propertyName);
+        if (descriptor == null) {
+            // no simple property exists so put the value into the map
+            bean.put(propertyName, value);
+        } else {
+            // a simple property exists so set that instead.
+            setSimpleProperty(bean, propertyName, value);
+        }
+    }
+}

Propchange: jakarta/commons/proper/beanutils/trunk/src/test/org/apache/commons/beanutils/PropsFirstPropertyUtilsBean.java
------------------------------------------------------------------------------
    svn:keywords = Id



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org