Ensure constructor and clone method correctly clone multimaps

bug 28972


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

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

@@ -50,7 +50,7 @@ import java.util.Set;
  * <code>list</code> will be a list containing "A", "B", "C".
  *
  * @since Commons Collections 2.0
- * @version $Revision: 1.17 $ $Date: 2004/03/14 17:05:24 $
+ * @version $Revision: 1.18 $ $Date: 2004/05/14 22:33:31 $
  * 
  * @author Christopher Berry
  * @author James Strachan
@@ -94,12 +94,28 @@ public class MultiHashMap extends HashMap implements MultiMap {
     }
 
     /**
-     * Constructor.
+     * Constructor that copies the input map creating an independent copy.
+     * <p>
+     * This method performs different behaviour depending on whether the map
+     * specified is a MultiMap or not. If a MultiMap is specified, each internal
+     * collection is also cloned. If the specified map only implements Map, then
+     * the values are not cloned.
+     * <p>
+     * NOTE: From Commons Collections 3.1 this method correctly copies a MultiMap
+     * to form a truly independent new map.
      * 
      * @param mapToCopy  a Map to copy
      */
     public MultiHashMap(Map mapToCopy) {
         super(mapToCopy);
+        if (mapToCopy instanceof MultiMap) {
+            for (Iterator it = entrySet().iterator(); it.hasNext();) {
+                Map.Entry entry = (Map.Entry) it.next();
+                Collection coll = (Collection) entry.getValue();
+                Collection newColl = createCollection(coll);
+                entry.setValue(newColl);
+            }
+        }
     }
 
     /**
@@ -401,25 +417,25 @@ public class MultiHashMap extends HashMap implements MultiMap {
 
     //-----------------------------------------------------------------------
     /**
-     * Clone the map.
+     * Clones the map creating an independent copy.
      * <p>
      * The clone will shallow clone the collections as well as the map.
      * 
      * @return the cloned map
      */
     public Object clone() {
-        MultiHashMap obj = (MultiHashMap) super.clone();
+        MultiHashMap cloned = (MultiHashMap) super.clone();
 
         // clone each Collection container
-        for (Iterator it = entrySet().iterator(); it.hasNext();) {
+        for (Iterator it = cloned.entrySet().iterator(); it.hasNext();) {
             Map.Entry entry = (Map.Entry) it.next();
             Collection coll = (Collection) entry.getValue();
             Collection newColl = createCollection(coll);
             entry.setValue(newColl);
         }
-        return obj;
+        return cloned;
     }
-    
+
     /** 
      * Creates a new instance of the map value Collection container.
      * <p>

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

@@ -29,7 +29,7 @@ import org.apache.commons.collections.map.AbstractTestMap;
 /**
  * Unit Tests for <code>MultiHashMap</code>.
  * 
- * @version $Revision: 1.18 $ $Date: 2004/03/14 17:05:24 $
+ * @version $Revision: 1.19 $ $Date: 2004/05/14 22:33:58 $
  *
  * @author Unknown
  */
@@ -344,5 +344,50 @@ public class TestMultiHashMap extends AbstractTestMap {
         assertEquals(true, map.containsValue("A", "Z"));
         assertEquals(true, map.containsValue("A", "M"));
     }
-    
+
+    public void testClone() {
+        MultiHashMap map = new MultiHashMap();
+        map.put("A", "1");
+        map.put("A", "2");
+        Collection coll = (Collection) map.get("A");
+        assertEquals(1, map.size());
+        assertEquals(2, coll.size());
+        
+        MultiHashMap cloned = (MultiHashMap) map.clone();
+        Collection clonedColl = (Collection) cloned.get("A");
+        assertNotSame(map, cloned);
+        assertNotSame(coll, clonedColl);
+        assertEquals(1, map.size());
+        assertEquals(2, coll.size());
+        assertEquals(1, cloned.size());
+        assertEquals(2, clonedColl.size());
+        map.put("A", "3");
+        assertEquals(1, map.size());
+        assertEquals(3, coll.size());
+        assertEquals(1, cloned.size());
+        assertEquals(2, clonedColl.size());
+    }
+
+    public void testConstructor() {
+        MultiHashMap map = new MultiHashMap();
+        map.put("A", "1");
+        map.put("A", "2");
+        Collection coll = (Collection) map.get("A");
+        assertEquals(1, map.size());
+        assertEquals(2, coll.size());
+        
+        MultiHashMap newMap = new MultiHashMap(map);
+        Collection newColl = (Collection) newMap.get("A");
+        assertNotSame(map, newMap);
+        assertNotSame(coll, newColl);
+        assertEquals(1, map.size());
+        assertEquals(2, coll.size());
+        assertEquals(1, newMap.size());
+        assertEquals(2, newColl.size());
+        map.put("A", "3");
+        assertEquals(1, map.size());
+        assertEquals(3, coll.size());
+        assertEquals(1, newMap.size());
+        assertEquals(2, newColl.size());
+    }
 }