Fixed bugs where methods failed when there is no bean set for the bean map.

Fixed entrySet() to return a set containing Map.Entry objects with readable
properties as keys.  This fixes the test case error, and a couple of the test
case failures (and uncovers a couple more test case failures)

Fixed clone method to allow subclasses to clone properly.  This requires a
non-backwards compatible change where the clone method now declares it throws
CloneNotSupportedException.  See:
http://www.javaworld.com/javaworld/jw-01-1999/jw-01-object.html Seeing how
BeanMap never directly implemented Cloneable anyway, and this is for a major
revision, I don't see this changing being much of a problem.

Since clone() declares it throws CloneNotSupportedException, that exception is
now used to indicate a problem when attempting to clone (rather than
UnsupportedOperationException, or other RuntimeException).

Added a small test for testing BeanMap clone.


git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk@130638 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/commons/collections/BeanMap.java

@@ -1,7 +1,7 @@
 /*
- * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/java/org/apache/commons/collections/BeanMap.java,v 1.5 2002/03/13 04:15:49 mas Exp $
+ * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/java/org/apache/commons/collections/BeanMap.java,v 1.6 2002/03/13 04:36:18 mas Exp $
- * $Revision: 1.5 $
+ * $Revision: 1.6 $
- * $Date: 2002/03/13 04:15:49 $
+ * $Date: 2002/03/13 04:36:18 $
  *
  * ====================================================================
  *
@@ -68,8 +68,10 @@ import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.AbstractMap;
+import java.util.AbstractSet;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
@@ -85,13 +87,13 @@ import java.util.Set;
   * @author <a href="mailto:jstrachan@apache.org">James Strachan</a>
   */
 
-public class BeanMap extends AbstractMap {
+public class BeanMap extends AbstractMap implements Cloneable {
 
-    private Object bean;
+    private transient Object bean;
 
-    private HashMap readMethods = new HashMap();
+    private transient HashMap readMethods = new HashMap();
-    private HashMap writeMethods = new HashMap();
+    private transient HashMap writeMethods = new HashMap();
-    private HashMap types = new HashMap();
+    private transient HashMap types = new HashMap();
 
     public static final Object[] NULL_ARGUMENTS = {};
     public static HashMap defaultTransformers = new HashMap();
@@ -177,19 +179,78 @@ public class BeanMap extends AbstractMap {
     // Map interface
     //-------------------------------------------------------------------------
 
-    public Object clone() {
+    /**
+     *  Clone this bean map using the following process: 
+     *
+     *  <ul>
+
+     *  <li>If there is no underlying bean, return a cloned BeanMap without a
+     *  bean.
+     *
+     *  <li>Since there is an underlying bean, try to instantiate a new bean of
+     *  the same type using Class.newInstance().
+     * 
+     *  <li>If the instantiation fails, throw a CloneNotSupportedException
+     *
+     *  <li>Clone the bean map and set the newly instantiated bean as the
+     *  underyling bean for the bean map.
+     *
+     *  <li>Copy each property that is both readable and writable from the
+     *  existing object to a cloned bean map.  
+     *
+     *  <li>If anything fails along the way, throw a
+     *  CloneNotSupportedException.
+     *
+     *  <ul>
+     **/
+    public Object clone() throws CloneNotSupportedException {
+        BeanMap newMap = (BeanMap)super.clone();
+
+        if(bean == null) {
+            // no bean, just an empty bean map at the moment.  return a newly
+            // cloned and empty bean map.
+            return newMap;
+        }
+
+        Object newBean = null;            
         Class beanClass = null;
         try {
             beanClass = bean.getClass();
-            Object newBean = beanClass.newInstance();
+            newBean = beanClass.newInstance();
-            Map newMap = new BeanMap( newBean );
+        } catch (Exception e) {
-            newMap.putAll( this );
+            // unable to instantiate
+            throw new CloneNotSupportedException
+                ("Unable to instantiate the underlying bean \"" +
+                 beanClass.getName() + "\": " + e);
+        }
+            
+        try {
+            newMap.setBean(newBean);
+        } catch (Exception exception) {
+            throw new CloneNotSupportedException
+                ("Unable to set bean in the cloned bean map: " + 
+                 exception);
+        }
+            
+        try {
+            // copy only properties that are readable and writable.  If its
+            // not readable, we can't get the value from the old map.  If
+            // its not writable, we can't write a value into the new map.
+            Iterator readableKeys = readMethods.keySet().iterator();
+            while(readableKeys.hasNext()) {
+                Object key = readableKeys.next();
+                if(getWriteMethod(key) != null) {
+                    newMap.put(key, get(key));
+                }
+            }
+        } catch (Exception exception) {
+            throw new CloneNotSupportedException
+                ("Unable to copy bean values to cloned bean map: " +
+                 exception);
+        }
+
         return newMap;
     }
-        catch (Exception e) {
-            throw new UnsupportedOperationException( "Could not create new instance of class: " + beanClass );
-        }
-    }
 
     /**
      *  This method reinitializes the bean map to have default values for the
@@ -200,6 +261,8 @@ public class BeanMap extends AbstractMap {
      *  BeanMap are fixed).
      **/
     public void clear() {
+        if(bean == null) return;
+
         Class beanClass = null;
         try {
             beanClass = bean.getClass();
@@ -281,7 +344,33 @@ public class BeanMap extends AbstractMap {
     }
 
     public Set entrySet() {
-        return readMethods.keySet();
+        return Collections.unmodifiableSet(new AbstractSet() {
+            public Iterator iterator() {
+                return new Iterator() {
+
+                    Iterator methodIter = 
+                      BeanMap.this.readMethods.keySet().iterator();
+
+                    public boolean hasNext() {
+                        return methodIter.hasNext();
+                    }
+
+                    public Object next() {
+                        Object key = (Object)methodIter.next();
+                        return new DefaultMapEntry(key, get(key));
+                    }
+
+                    public void remove() {
+                      throw new UnsupportedOperationException
+                        ("remove() not supported from BeanMap.entrySet()");
+                    }
+                };
+            }
+
+            public int size() {
+              return BeanMap.this.readMethods.size();
+            }
+        });
     }
 
     public Collection values() {
@@ -369,6 +458,8 @@ public class BeanMap extends AbstractMap {
     }
 
     private void initialise() {
+        if(getBean() == null) return;
+
         Class  beanClass = getBean().getClass();
         try {
             //BeanInfo beanInfo = Introspector.getBeanInfo( bean, null );

src/test/org/apache/commons/collections/TestBeanMap.java

@@ -1,7 +1,7 @@
 /*
- * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/TestBeanMap.java,v 1.3 2002/02/22 07:00:30 mas Exp $
+ * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/TestBeanMap.java,v 1.4 2002/03/13 04:36:18 mas Exp $
- * $Revision: 1.3 $
+ * $Revision: 1.4 $
- * $Date: 2002/02/22 07:00:30 $
+ * $Date: 2002/03/13 04:36:18 $
  *
  * ====================================================================
  *
@@ -279,4 +279,22 @@ public class TestBeanMap extends TestMap {
         //TODO: make sure a call to BeanMap.clear returns the bean to its
         //default initialization values.
     }
+
+    public void testBeanMapClone() {
+        BeanMap map = (BeanMap)makeFullMap();
+        try {
+            BeanMap map2 = (BeanMap)((BeanMap)map).clone();
+
+            // make sure containsKey is working to verify the bean was cloned
+            // ok, and the read methods were properly initialized
+            Object[] keys = getSampleKeys();
+            for(int i = 0; i < keys.length; i++) {
+                assertTrue("Cloned BeanMap should contain the same keys",
+                           map2.containsKey(keys[i]));
+            }
+        } catch (CloneNotSupportedException exception) {
+            fail("BeanMap.clone() should not throw a " +
+                 "CloneNotSupportedException when clone should succeed.");
+        }
+    }
 }