[COLLECTIONS-580] Do not use InstantiateFactory anymore for MultiValuedMaps: different MultiValuedMap implementations are now fully typed for the used underlying map and value collection class being used. This has pros and cons, but it is certainly safer to do it that way.

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

src/main/java/org/apache/commons/collections4/MultiMapUtils.java

@@ -23,7 +23,8 @@ import java.util.List;
 import java.util.Set;
 
 import org.apache.commons.collections4.bag.HashBag;
-import org.apache.commons.collections4.multimap.MultiValuedHashMap;
+import org.apache.commons.collections4.multimap.ArrayListValuedHashMap;
+import org.apache.commons.collections4.multimap.HashSetValuedHashMap;
 import org.apache.commons.collections4.multimap.TransformedMultiValuedMap;
 import org.apache.commons.collections4.multimap.UnmodifiableMultiValuedMap;
 
@@ -52,7 +53,7 @@ public class MultiMapUtils {
      */
     @SuppressWarnings({ "rawtypes", "unchecked" })
     public static final MultiValuedMap EMPTY_MULTI_VALUED_MAP =
-            UnmodifiableMultiValuedMap.unmodifiableMultiValuedMap(new MultiValuedHashMap());
+            UnmodifiableMultiValuedMap.unmodifiableMultiValuedMap(new ArrayListValuedHashMap(0, 0));
 
     /**
      * Returns immutable EMPTY_MULTI_VALUED_MAP with generic type safety.
@@ -188,21 +189,7 @@ public class MultiMapUtils {
      * @return a new <code>ListValuedMap</code>
      */
     public static <K, V> ListValuedMap<K, V> newListValuedHashMap() {
-        return MultiValuedHashMap.<K, V>listValuedHashMap();
+        return new ArrayListValuedHashMap<K, V>();
-    }
-
-    /**
-     * Creates a {@link ListValuedMap} with a {@link java.util.HashMap HashMap} as its internal
-     * storage which maps the keys to list of type <code>listClass</code>.
-     *
-     * @param <K> the key type
-     * @param <V> the value type
-     * @param <C> the List class type
-     * @param listClass the class of the list
-     * @return a new {@link ListValuedMap}
-     */
-    public static <K, V, C extends List<V>> ListValuedMap<K, V> newListValuedHashMap(final Class<C> listClass) {
-        return MultiValuedHashMap.<K, V, C>listValuedHashMap(listClass);
     }
 
     /**
@@ -214,21 +201,7 @@ public class MultiMapUtils {
      * @return a new {@link SetValuedMap}
      */
     public static <K, V> SetValuedMap<K, V> newSetValuedHashMap() {
-        return MultiValuedHashMap.<K, V>setValuedHashMap();
+        return new HashSetValuedHashMap<K, V>();
-    }
-
-    /**
-     * Creates a {@link SetValuedMap} with a {@link java.util.HashMap HashMap} as its internal
-     * storage which maps the keys to a set of type <code>setClass</code>
-     *
-     * @param <K> the key type
-     * @param <V> the value type
-     * @param <C> the Set class type
-     * @param setClass  the class of the set
-     * @return a new {@link SetValuedMap}
-     */
-    public static <K, V, C extends Set<V>> SetValuedMap<K, V> newSetValuedHashMap(final Class<C> setClass) {
-        return MultiValuedHashMap.<K, V, C>setValuedHashMap(setClass);
     }
 
     // MultiValuedMap Decorators

src/main/java/org/apache/commons/collections4/multimap/AbstractListValuedMap.java

@@ -38,34 +38,28 @@ import org.apache.commons.collections4.ListValuedMap;
 public abstract class AbstractListValuedMap<K, V> extends AbstractMultiValuedMap<K, V>
         implements ListValuedMap<K, V> {
 
-    /** The serialization version */
-    private static final long serialVersionUID = 20150612L;
-
     /**
-     * A constructor that wraps, not copies
+     * Constructor needed for subclass serialisation.
-     *
-     * @param <C> the list type
-     * @param map  the map to wrap, must not be null
-     * @param listClazz  the collection class
-     * @throws NullPointerException if the map is null
      */
-    protected <C extends List<V>> AbstractListValuedMap(final Map<K, ? super C> map, Class<C> listClazz) {
+    protected AbstractListValuedMap() {
-        super(map, listClazz);
+        super();
     }
 
     /**
      * A constructor that wraps, not copies
      *
-     * @param <C> the list type
      * @param map  the map to wrap, must not be null
-     * @param listClazz  the collection class
-     * @param initialListCapacity  the initial size of the values list
      * @throws NullPointerException if the map is null
-     * @throws IllegalArgumentException  if initialListCapacity is negative
      */
-    protected <C extends List<V>> AbstractListValuedMap(final Map<K, ? super C> map, Class<C> listClazz,
+    protected AbstractListValuedMap(final Map<K, ? extends List<V>> map) {
-                                                        final int initialListCapacity) {
+        super(map);
-        super(map, listClazz, initialListCapacity);
+    }
+
+    // -----------------------------------------------------------------------
+    @Override
+    @SuppressWarnings("unchecked")
+    protected Map<K, List<V>> getMap() {
+        return (Map<K, List<V>>) super.getMap();
     }
 
     /**
@@ -73,10 +67,9 @@ public abstract class AbstractListValuedMap<K, V> extends AbstractMultiValuedMap
      * @return a new list
      */
     @Override
-    protected List<V> createCollection() {
+    protected abstract List<V> createCollection();
-        return (List<V>) super.createCollection();
-    }
 
+    // -----------------------------------------------------------------------
     /**
      * Gets the list of values associated with the specified key. This would
      * return an empty list in case the mapping is not present
@@ -100,25 +93,10 @@ public abstract class AbstractListValuedMap<K, V> extends AbstractMultiValuedMap
      */
     @Override
     public List<V> remove(Object key) {
-        return ListUtils.emptyIfNull((List<V>) getMap().remove(key));
+        return ListUtils.emptyIfNull(getMap().remove(key));
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-        if (this == obj) {
-            return true;
-        }
-        if (obj instanceof ListValuedMap) {
-            return asMap().equals(((ListValuedMap<?, ?>) obj).asMap());
-        }
-        return false;
-    }
-
-    @Override
-    public int hashCode() {
-        return asMap().hashCode();
     }
 
+    // -----------------------------------------------------------------------
     /**
      * Wrapped list to handle add and remove on the list returned by get(object)
      */
@@ -130,7 +108,7 @@ public abstract class AbstractListValuedMap<K, V> extends AbstractMultiValuedMap
 
         @Override
         protected List<V> getMapping() {
-            return (List<V>) getMap().get(key);
+            return getMap().get(key);
         }
 
         @Override
@@ -237,13 +215,13 @@ public abstract class AbstractListValuedMap<K, V> extends AbstractMultiValuedMap
 
         public ValuesListIterator(final K key) {
             this.key = key;
-            this.values = ListUtils.emptyIfNull((List<V>) getMap().get(key));
+            this.values = ListUtils.emptyIfNull(getMap().get(key));
             this.iterator = values.listIterator();
         }
 
         public ValuesListIterator(final K key, int index) {
             this.key = key;
-            this.values = ListUtils.emptyIfNull((List<V>) getMap().get(key));
+            this.values = ListUtils.emptyIfNull(getMap().get(key));
             this.iterator = values.listIterator(index);
         }
 

src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMap.java

@@ -16,7 +16,9 @@
  */
 package org.apache.commons.collections4.multimap;
 
-import java.io.Serializable;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.lang.reflect.Array;
 import java.util.AbstractCollection;
 import java.util.ArrayList;
@@ -27,13 +29,11 @@ import java.util.Map.Entry;
 import java.util.Set;
 
 import org.apache.commons.collections4.CollectionUtils;
-import org.apache.commons.collections4.Factory;
 import org.apache.commons.collections4.IteratorUtils;
 import org.apache.commons.collections4.MapIterator;
 import org.apache.commons.collections4.MultiSet;
 import org.apache.commons.collections4.MultiValuedMap;
 import org.apache.commons.collections4.Transformer;
-import org.apache.commons.collections4.functors.InstantiateFactory;
 import org.apache.commons.collections4.iterators.EmptyMapIterator;
 import org.apache.commons.collections4.iterators.IteratorChain;
 import org.apache.commons.collections4.iterators.LazyIteratorChain;
@@ -50,13 +50,7 @@ import org.apache.commons.collections4.set.UnmodifiableSet;
  * @since 4.1
  * @version $Id$
  */
-public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K, V>, Serializable {
+public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K, V> {
-
-    /** Serialization Version */
-    private static final long serialVersionUID = 20150612L;
-
-    /** The factory for creating value collections. */
-    private final Factory<? extends Collection<V>> collectionFactory;
 
     /** The values view */
     private transient Collection<V> valuesView;
@@ -68,24 +62,13 @@ public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K,
     private transient KeysMultiSet keysMultiSetView;
 
     /** The map used to store the data */
-    private final Map<K, Collection<V>> map;
+    private transient Map<K, Collection<V>> map;
 
     /**
-     * Constructor that wraps (not copies).
+     * Constructor needed for subclass serialisation.
-     *
-     * @param <C> the collection type
-     * @param map  the map to wrap, must not be null
-     * @param collectionClazz  the collection class
-     * @throws NullPointerException if the map is null
      */
-    @SuppressWarnings("unchecked")
+    protected AbstractMultiValuedMap() {
-    protected <C extends Collection<V>> AbstractMultiValuedMap(final Map<K, ? super C> map,
+        super();
-                                                               final Class<C> collectionClazz) {
-        if (map == null) {
-            throw new NullPointerException("Map must not be null.");
-        }
-        this.map = (Map<K, Collection<V>>) map;
-        this.collectionFactory = new InstantiateFactory<C>(collectionClazz);
     }
 
     /**
@@ -93,35 +76,41 @@ public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K,
      *
      * @param <C> the collection type
      * @param map  the map to wrap, must not be null
-     * @param collectionClazz  the collection class
-     * @param initialCollectionCapacity  the initial capacity of the collection
      * @throws NullPointerException if the map is null
-     * @throws IllegalArgumentException  if initialCollectionCapacity is negative
      */
     @SuppressWarnings("unchecked")
-    protected <C extends Collection<V>> AbstractMultiValuedMap(final Map<K, ? super C> map,
+    protected AbstractMultiValuedMap(final Map<K, ? extends Collection<V>> map) {
-            final Class<C> collectionClazz, final int initialCollectionCapacity) {
         if (map == null) {
             throw new NullPointerException("Map must not be null.");
         }
-        if (initialCollectionCapacity < 0) {
-            throw new IllegalArgumentException("InitialCapacity must not be negative.");
-        }
         this.map = (Map<K, Collection<V>>) map;
-        this.collectionFactory = new InstantiateFactory<C>(collectionClazz,
-                new Class[] { Integer.TYPE },
-                new Object[] { Integer.valueOf(initialCollectionCapacity) });
     }
 
+    // -----------------------------------------------------------------------
     /**
      * Gets the map being wrapped.
      *
      * @return the wrapped map
      */
-    protected Map<K, Collection<V>> getMap() {
+    protected Map<K, ? extends Collection<V>> getMap() {
         return map;
     }
 
+    /**
+     * Sets the map being wrapped.
+     * <p>
+     * <b>NOTE:</b> this method should only be used during deserialization
+     *
+     * @param map the map to wrap
+     */
+    @SuppressWarnings("unchecked")
+    protected void setMap(Map<K, ? extends Collection<V>> map) {
+        this.map = (Map<K, Collection<V>>) map;
+    }
+
+    protected abstract Collection<V> createCollection();
+
+    // -----------------------------------------------------------------------
     @Override
     public boolean containsKey(Object key) {
         return getMap().containsKey(key);
@@ -250,7 +239,7 @@ public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K,
         if (coll == null) {
             coll = createCollection();
             if (coll.add(value)) {
-                getMap().put(key, coll);
+                map.put(key, coll);
                 return true;
             } else {
                 return false;
@@ -325,8 +314,10 @@ public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K,
     }
 
     @Override
+    @SuppressWarnings("unchecked")
     public Map<K, Collection<V>> asMap() {
-        return getMap();
+        // TODO: return a view of the map
+        return (Map<K, Collection<V>>) getMap();
     }
 
     /**
@@ -373,18 +364,12 @@ public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K,
 
     @Override
     public int hashCode() {
-        return getMap().hashCode();
+        return asMap().hashCode();
     }
 
     @Override
     public String toString() {
-        return getMap().toString();
+        return asMap().toString();
-    }
-
-    // -----------------------------------------------------------------------
-
-    protected Collection<V> createCollection() {
-        return collectionFactory.create();
     }
 
     // -----------------------------------------------------------------------
@@ -906,4 +891,44 @@ public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K,
         }
     }
 
+    //-----------------------------------------------------------------------
+    /**
+     * Write the map out using a custom routine.
+     * @param out the output stream
+     * @throws IOException any of the usual I/O related exceptions
+     */
+    protected void doWriteObject(final ObjectOutputStream out) throws IOException {
+        out.writeInt(map.size());
+        for (final Map.Entry<K, Collection<V>> entry : map.entrySet()) {
+            out.writeObject(entry.getKey());
+            out.writeInt(entry.getValue().size());
+            for (final V value : entry.getValue()) {
+                out.writeObject(value);
+            }
+        }
+    }
+
+    /**
+     * Read the map in using a custom routine.
+     * @param in the input stream
+     * @throws IOException any of the usual I/O related exceptions
+     * @throws ClassNotFoundException if the stream contains an object which class can not be loaded
+     * @throws ClassCastException if the stream does not contain the correct objects
+     */
+    protected void doReadObject(final ObjectInputStream in)
+            throws IOException, ClassNotFoundException {
+        final int entrySize = in.readInt();
+        for (int i = 0; i < entrySize; i++) {
+            @SuppressWarnings("unchecked") // This will fail at runtime if the stream is incorrect
+            final K key = (K) in.readObject();
+            final Collection<V> values = get(key);
+            final int valueSize = in.readInt();
+            for (int j = 0; j < valueSize; j++) {
+                @SuppressWarnings("unchecked") // see above
+                V value = (V) in.readObject();
+                values.add(value);
+            }
+        }
+    }
+
 }

src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMapDecorator.java

@@ -61,6 +61,7 @@ public abstract class AbstractMultiValuedMapDecorator<K, V>
         this.map = map;
     }
 
+    // -----------------------------------------------------------------------
     /**
      * The decorated multi-valued map.
      *
@@ -70,6 +71,7 @@ public abstract class AbstractMultiValuedMapDecorator<K, V>
         return map;
     }
 
+    // -----------------------------------------------------------------------
     @Override
     public int size() {
         return decorated().size();

src/main/java/org/apache/commons/collections4/multimap/AbstractSetValuedMap.java

@@ -36,36 +36,38 @@ import org.apache.commons.collections4.SetValuedMap;
 public abstract class AbstractSetValuedMap<K, V> extends AbstractMultiValuedMap<K, V>
     implements SetValuedMap<K, V> {
 
-    /** Serialization version */
-    private static final long serialVersionUID = 20150612L;
-
     /**
-     * A constructor that wraps, not copies
+     * Constructor needed for subclass serialisation.
-     *
-     * @param <C> the set type
-     * @param map  the map to wrap, must not be null
-     * @param setClazz  the collection class
-     * @throws NullPointerException if the map is null
      */
-    protected <C extends Set<V>> AbstractSetValuedMap(Map<K, ? super C> map, Class<C> setClazz) {
+    protected AbstractSetValuedMap() {
-        super(map, setClazz);
+        super();
     }
 
     /**
      * A constructor that wraps, not copies
      *
-     * @param <C> the set type
      * @param map  the map to wrap, must not be null
-     * @param setClazz  the collection class
-     * @param initialSetCapacity  the initial size of the values set
      * @throws NullPointerException if the map is null
-     * @throws IllegalArgumentException if initialSetCapacity is negative
      */
-    protected <C extends Set<V>> AbstractSetValuedMap(Map<K, ? super C> map, Class<C> setClazz,
+    protected AbstractSetValuedMap(Map<K, ? extends Set<V>> map) {
-            int initialSetCapacity) {
+        super(map);
-        super(map, setClazz, initialSetCapacity);
     }
 
+    // -----------------------------------------------------------------------
+    @Override
+    @SuppressWarnings("unchecked")
+    protected Map<K, Set<V>> getMap() {
+        return (Map<K, Set<V>>) super.getMap();
+    }
+
+    /**
+     * Creates a new value collection using the provided factory.
+     * @return a new list
+     */
+    @Override
+    protected abstract Set<V> createCollection();
+
+    // -----------------------------------------------------------------------
     /**
      * Gets the set of values associated with the specified key. This would
      * return an empty set in case the mapping is not present
@@ -90,25 +92,10 @@ public abstract class AbstractSetValuedMap<K, V> extends AbstractMultiValuedMap<
      */
     @Override
     public Set<V> remove(Object key) {
-        return SetUtils.emptyIfNull((Set<V>) getMap().remove(key));
+        return SetUtils.emptyIfNull(getMap().remove(key));
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-        if (this == obj) {
-            return true;
-        }
-        if (obj instanceof SetValuedMap) {
-            return asMap().equals(((SetValuedMap<?, ?>) obj).asMap());
-        }
-        return false;
-    }
-
-    @Override
-    public int hashCode() {
-        return asMap().hashCode();
     }
 
+    // -----------------------------------------------------------------------
     /**
      * Wrapped set to handle add and remove on the collection returned by
      * {@code get(Object)}.

src/main/java/org/apache/commons/collections4/multimap/ArrayListValuedHashMap.java

@@ -0,0 +1,128 @@
+package org.apache.commons.collections4.multimap;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.commons.collections4.ListValuedMap;
+import org.apache.commons.collections4.MultiValuedMap;
+
+/**
+ * Implements a {@link ListValuedMap}, using a {@link HashMap} to provide data
+ * storage and {@link ArrayList}s as value collections. This is the standard
+ * implementation of a ListValuedMap.
+ * <p>
+ * <strong>Note that ArrayListValuedHashMap is not synchronized and is not
+ * thread-safe.</strong> If you wish to use this map from multiple threads
+ * concurrently, you must use appropriate synchronization. This class may throw
+ * exceptions when accessed by concurrent threads without synchronization.
+ *
+ * @since 4.1
+ * @version $Id$
+ */
+public class ArrayListValuedHashMap<K, V> extends AbstractListValuedMap<K, V>
+    implements Serializable {
+
+    /** Serialization Version */
+    private static final long serialVersionUID = 20151118L;
+
+    /**
+     * The initial map capacity used when none specified in constructor.
+     */
+    private static final int DEFAULT_INITIAL_MAP_CAPACITY = 16;
+
+    /**
+     * The initial list capacity when using none specified in constructor.
+     */
+    private static final int DEFAULT_INITIAL_LIST_CAPACITY = 3;
+
+    /**
+     * The initial list capacity when creating a new value collection.
+     */
+    private final int initialListCapacity;
+
+    /**
+     * Creates an empty ArrayListValuedHashMap with the default initial
+     * map capacity (16) and the default initial list capacity (3). 
+     */
+    public ArrayListValuedHashMap() {
+        this(DEFAULT_INITIAL_MAP_CAPACITY, DEFAULT_INITIAL_LIST_CAPACITY);
+    }
+
+    /**
+     * Creates an empty ArrayListValuedHashMap with the default initial
+     * map capacity (16) and the specified initial list capacity. 
+     *
+     * @param initialListCapacity  the initial capacity used for value collections
+     */
+    public ArrayListValuedHashMap(int initialListCapacity) {
+        this(DEFAULT_INITIAL_MAP_CAPACITY, initialListCapacity);
+    }
+
+    /**
+     * Creates an empty ArrayListValuedHashMap with the specified initial
+     * map and list capacities. 
+     *
+     * @param initialMapCapacity  the initial hashmap capacity
+     * @param initialListCapacity  the initial capacity used for value collections
+     */
+    public ArrayListValuedHashMap(int initialMapCapacity, int initialListCapacity) {
+        super(new HashMap<K, ArrayList<V>>(initialMapCapacity));
+        this.initialListCapacity = initialListCapacity;
+    }
+
+    /**
+     * Creates an ArrayListValuedHashMap copying all the mappings of the given map.
+     *
+     * @param map a <code>MultiValuedMap</code> to copy into this map
+     */
+    public ArrayListValuedHashMap(final MultiValuedMap<? extends K, ? extends V> map) {
+        this(map.size(), DEFAULT_INITIAL_LIST_CAPACITY);
+        super.putAll(map);
+    }
+
+    /**
+     * Creates an ArrayListValuedHashMap copying all the mappings of the given map.
+     *
+     * @param map a <code>Map</code> to copy into this map
+     */
+    public ArrayListValuedHashMap(final Map<? extends K, ? extends V> map) {
+        this(map.size(), DEFAULT_INITIAL_LIST_CAPACITY);
+        super.putAll(map);
+    }
+
+    // -----------------------------------------------------------------------
+    @Override
+    protected ArrayList<V> createCollection() {
+        return new ArrayList<V>(initialListCapacity);
+    }
+
+    // -----------------------------------------------------------------------
+    /**
+     * Trims the capacity of all value collections to their current size.
+     */
+    public void trimToSize() {
+        for (Collection<V> coll : getMap().values()) {
+            final ArrayList<V> list = (ArrayList<V>) coll;
+            list.trimToSize();
+        }
+    }
+
+    // -----------------------------------------------------------------------
+    private void writeObject(ObjectOutputStream oos) throws IOException {
+        oos.defaultWriteObject();
+        doWriteObject(oos);
+    }
+    
+    private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {
+        ois.defaultReadObject();
+        setMap(new HashMap<K, ArrayList<V>>());
+        doReadObject(ois);
+    }
+
+}

src/main/java/org/apache/commons/collections4/multimap/HashSetValuedHashMap.java

@@ -0,0 +1,116 @@
+package org.apache.commons.collections4.multimap;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+
+import org.apache.commons.collections4.MultiValuedMap;
+import org.apache.commons.collections4.SetValuedMap;
+
+/**
+ * Implements a {@link SetValuedMap}, using a {@link HashMap} to provide data
+ * storage and {@link HashSet}s as value collections. This is the standard
+ * implementation of a SetValuedMap.
+ * <p>
+ * <strong>Note that HashSetValuedHashMap is not synchronized and is not
+ * thread-safe.</strong> If you wish to use this map from multiple threads
+ * concurrently, you must use appropriate synchronization. This class may throw
+ * exceptions when accessed by concurrent threads without synchronization.
+ *
+ * @since 4.1
+ * @version $Id$
+ */
+public class HashSetValuedHashMap<K, V> extends AbstractSetValuedMap<K, V>
+    implements Serializable {
+
+    /** Serialization Version */
+    private static final long serialVersionUID = 20151118L;
+
+    /**
+     * The initial map capacity used when none specified in constructor.
+     */
+    private static final int DEFAULT_INITIAL_MAP_CAPACITY = 16;
+
+    /**
+     * The initial set capacity when using none specified in constructor.
+     */
+    private static final int DEFAULT_INITIAL_SET_CAPACITY = 3;
+
+    /**
+     * The initial list capacity when creating a new value collection.
+     */
+    private final int initialSetCapacity;
+
+    /**
+     * Creates an empty HashSetValuedHashMap with the default initial
+     * map capacity (16) and the default initial set capacity (3). 
+     */
+    public HashSetValuedHashMap() {
+        this(DEFAULT_INITIAL_MAP_CAPACITY, DEFAULT_INITIAL_SET_CAPACITY);
+    }
+
+    /**
+     * Creates an empty HashSetValuedHashMap with the default initial
+     * map capacity (16) and the specified initial set capacity. 
+     *
+     * @param initialSetCapacity  the initial capacity used for value collections
+     */
+    public HashSetValuedHashMap(int initialSetCapacity) {
+        this(DEFAULT_INITIAL_MAP_CAPACITY, initialSetCapacity);
+    }
+
+    /**
+     * Creates an empty HashSetValuedHashMap with the specified initial
+     * map and list capacities. 
+     *
+     * @param initialMapCapacity  the initial hashmap capacity
+     * @param initialSetCapacity  the initial capacity used for value collections
+     */
+    public HashSetValuedHashMap(int initialMapCapacity, int initialSetCapacity) {
+        super(new HashMap<K, HashSet<V>>(initialMapCapacity));
+        this.initialSetCapacity = initialSetCapacity;
+    }
+
+    /**
+     * Creates an HashSetValuedHashMap copying all the mappings of the given map.
+     *
+     * @param map a <code>MultiValuedMap</code> to copy into this map
+     */
+    public HashSetValuedHashMap(final MultiValuedMap<? extends K, ? extends V> map) {
+        this(map.size(), DEFAULT_INITIAL_SET_CAPACITY);
+        super.putAll(map);
+    }
+
+    /**
+     * Creates an HashSetValuedHashMap copying all the mappings of the given map.
+     *
+     * @param map a <code>Map</code> to copy into this map
+     */
+    public HashSetValuedHashMap(final Map<? extends K, ? extends V> map) {
+        this(map.size(), DEFAULT_INITIAL_SET_CAPACITY);
+        super.putAll(map);
+    }
+
+    // -----------------------------------------------------------------------
+    @Override
+    protected HashSet<V> createCollection() {
+        return new HashSet<V>(initialSetCapacity);
+    }
+
+    // -----------------------------------------------------------------------
+    private void writeObject(ObjectOutputStream oos) throws IOException {
+        oos.defaultWriteObject();
+        doWriteObject(oos);
+    }
+    
+    private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {
+        ois.defaultReadObject();
+        setMap(new HashMap<K, HashSet<V>>());
+        doReadObject(ois);
+    }
+
+}

src/main/java/org/apache/commons/collections4/multimap/MultiValuedHashMap.java

@@ -1,259 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.commons.collections4.multimap;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
-import org.apache.commons.collections4.ListValuedMap;
-import org.apache.commons.collections4.MultiValuedMap;
-import org.apache.commons.collections4.SetValuedMap;
-
-/**
- * Implements a {@link MultiValuedMap}, using a {@link HashMap} to provide data
- * storage. This is the standard implementation of a MultiValuedMap
- * <p>
- * A <code>MultiValuedMap</code> is a Map with slightly different semantics.
- * Putting a value into the map will add the value to a Collection at that key.
- * Getting a value will return a Collection, holding all the values put to that
- * key
- * <p>
- * In addition, this implementation allows the type of collection used for the
- * values to be controlled. By default, an <code>ArrayList</code> is used,
- * however a <code>Class<? extends Collection></code> to instantiate the value
- * collection may be specified.
- * <p>
- * <strong>Note that MultiValuedHashMap is not synchronized and is not
- * thread-safe.</strong> If you wish to use this map from multiple threads
- * concurrently, you must use appropriate synchronization. This class may throw
- * exceptions when accessed by concurrent threads without synchronization.
- *
- * @since 4.1
- * @version $Id$
- */
-public class MultiValuedHashMap<K, V> extends AbstractMultiValuedMap<K, V> {
-
-    /** Serialization Version */
-    private static final long serialVersionUID = 20150612L;
-
-    /**
-     * The initial capacity used when none specified in constructor.
-     */
-    static final int DEFAULT_INITIAL_CAPACITY = 16;
-
-    /**
-     * The load factor used when none specified in constructor.
-     */
-    static final float DEFAULT_LOAD_FACTOR = 0.75f;
-
-    /**
-     * Creates a {@link ListValuedMap} with a {@link HashMap} as its internal
-     * storage
-     *
-     * @param <K> the key type
-     * @param <V> the value type
-     * @return a new <code>ListValuedMap</code>
-     */
-    @SuppressWarnings({ "unchecked", "rawtypes" })
-    public static <K, V> ListValuedMap<K, V> listValuedHashMap() {
-        return new ListValuedHashMap(ArrayList.class);
-    }
-
-    /**
-     * Creates a {@link ListValuedMap} with a {@link HashMap} as its internal
-     * storage which maps the keys to list of type <code>listClass</code>
-     *
-     * @param <K> the key type
-     * @param <V> the value type
-     * @param <C> the List class type
-     * @param listClass the class of the list
-     * @return a new <code>ListValuedMap</code>
-     */
-    @SuppressWarnings({ "unchecked", "rawtypes" })
-    public static <K, V, C extends List<V>> ListValuedMap<K, V> listValuedHashMap(final Class<C> listClass) {
-        return new ListValuedHashMap(listClass);
-    }
-
-    /**
-     * Creates a {@link SetValuedMap} with a {@link HashMap} as its internal
-     * storage
-     *
-     * @param <K> the key type
-     * @param <V> the value type
-     * @return a new <code>SetValuedMap</code>
-     */
-    @SuppressWarnings({ "unchecked", "rawtypes" })
-    public static <K, V> SetValuedMap<K, V> setValuedHashMap() {
-        return new SetValuedHashMap(HashSet.class);
-    }
-
-    /**
-     * Creates a {@link SetValuedMap} with a {@link HashMap} as its internal
-     * storage which maps the keys to a set of type <code>setClass</code>
-     *
-     * @param <K> the key type
-     * @param <V> the value type
-     * @param <C> the Set class type
-     * @param setClass the class of the set
-     * @return a new <code>SetValuedMap</code>
-     */
-    @SuppressWarnings({ "unchecked", "rawtypes" })
-    public static <K, V, C extends Set<V>> SetValuedMap<K, V> setValuedHashMap(final Class<C> setClass) {
-        return new SetValuedHashMap(setClass);
-    }
-
-    /**
-     * Creates a MultiValueMap based on a <code>HashMap</code> with the default
-     * initial capacity (16) and the default load factor (0.75), which stores
-     * the multiple values in an <code>ArrayList</code>.
-     */
-    @SuppressWarnings("unchecked")
-    public MultiValuedHashMap() {
-        this(DEFAULT_INITIAL_CAPACITY, DEFAULT_LOAD_FACTOR, ArrayList.class);
-    }
-
-    /**
-     * Creates a MultiValueMap based on a <code>HashMap</code> with the initial
-     * capacity and the default load factor (0.75), which stores the multiple
-     * values in an <code>ArrayList</code>.
-     *
-     * @param initialCapacity the initial capacity of the underlying hash map
-     */
-    @SuppressWarnings("unchecked")
-    public MultiValuedHashMap(int initialCapacity) {
-        this(initialCapacity, DEFAULT_LOAD_FACTOR, ArrayList.class);
-    }
-
-    /**
-     * Creates a MultiValueMap based on a <code>HashMap</code> with the initial
-     * capacity and the load factor, which stores the multiple values in an
-     * <code>ArrayList</code>.
-     *
-     * @param initialCapacity the initial capacity of the underlying hash map
-     * @param loadFactor the load factor of the underlying hash map
-     */
-    @SuppressWarnings("unchecked")
-    public MultiValuedHashMap(int initialCapacity, float loadFactor) {
-        this(initialCapacity, loadFactor, ArrayList.class);
-    }
-
-    /**
-     * Creates a MultiValueMap based on a <code>HashMap</code> with the initial
-     * capacity and the load factor, which stores the multiple values in an
-     * <code>ArrayList</code> with the initial collection capacity.
-     *
-     * @param initialCapacity the initial capacity of the underlying hash map
-     * @param loadFactor the load factor of the underlying hash map
-     * @param initialCollectionCapacity the initial capacity of the Collection of values
-     */
-    @SuppressWarnings("unchecked")
-    public MultiValuedHashMap(int initialCapacity, float loadFactor, int initialCollectionCapacity) {
-        this(initialCapacity, loadFactor, ArrayList.class, initialCollectionCapacity);
-    }
-
-    /**
-     * Creates a MultiValuedHashMap copying all the mappings of the given map.
-     *
-     * @param map a <code>MultiValuedMap</code> to copy into this map
-     */
-    @SuppressWarnings("unchecked")
-    public MultiValuedHashMap(final MultiValuedMap<? extends K, ? extends V> map) {
-        this(DEFAULT_INITIAL_CAPACITY, DEFAULT_LOAD_FACTOR, ArrayList.class);
-        super.putAll(map);
-    }
-
-    /**
-     * Creates a MultiValuedHashMap copying all the mappings of the given map.
-     *
-     * @param map a <code>Map</code> to copy into this map
-     */
-    @SuppressWarnings("unchecked")
-    public MultiValuedHashMap(final Map<? extends K, ? extends V> map) {
-        this(DEFAULT_INITIAL_CAPACITY, DEFAULT_LOAD_FACTOR, ArrayList.class);
-        super.putAll(map);
-    }
-
-    /**
-     * Creates a MultiValuedHashMap which creates the value collections using
-     * the supplied <code>collectionClazz</code>.
-     *
-     * @param initialCapacity the initial capacity of the underlying
-     *        <code>HashMap</code>
-     * @param loadFactor the load factor of the underlying <code>HashMap</code>
-     * @param <C> the collection type
-     * @param collectionClazz the class of the <code>Collection</code> to use to
-     *        create the value collections
-     */
-    protected <C extends Collection<V>> MultiValuedHashMap(int initialCapacity, float loadFactor,
-            final Class<C> collectionClazz) {
-        super(new HashMap<K, Collection<V>>(initialCapacity, loadFactor), collectionClazz);
-    }
-
-    /**
-     * Creates a MultiValuedHashMap which creates the value collections using
-     * the supplied <code>collectionClazz</code> and the initial collection capacity.
-     *
-     * @param <C> the collection type
-     * @param initialCapacity the initial capacity of the underlying <code>HashMap</code>
-     * @param loadFactor the load factor of the underlying <code>HashMap</code>
-     * @param initialCollectionCapacity the initial capacity of the <code>Collection</code>
-     * @param collectionClazz the class of the <code>Collection</code> to use to create the value collections
-     */
-    protected <C extends Collection<V>> MultiValuedHashMap(int initialCapacity, float loadFactor,
-            final Class<C> collectionClazz, int initialCollectionCapacity) {
-        super(new HashMap<K, Collection<V>>(initialCapacity, loadFactor), collectionClazz,
-                                            initialCollectionCapacity);
-    }
-
-    /** Inner class for ListValuedMap */
-    private static class ListValuedHashMap<K, V> extends AbstractListValuedMap<K, V> {
-
-        private static final long serialVersionUID = 20150612L;
-
-        public <C extends List<V>> ListValuedHashMap(Class<C> listClazz) {
-            super(new HashMap<K, List<V>>(DEFAULT_INITIAL_CAPACITY, DEFAULT_LOAD_FACTOR), listClazz);
-        }
-
-        public <C extends List<V>> ListValuedHashMap(Class<C> listClazz, int initialListCapacity) {
-            super(new HashMap<K, List<V>>(DEFAULT_INITIAL_CAPACITY, DEFAULT_LOAD_FACTOR), listClazz,
-                    initialListCapacity);
-        }
-
-    }
-
-    /** Inner class for SetValuedMap */
-    private static class SetValuedHashMap<K, V> extends AbstractSetValuedMap<K, V> {
-
-        private static final long serialVersionUID = 20150612L;
-
-        public <C extends Set<V>> SetValuedHashMap(Class<C> setClazz) {
-            super(new HashMap<K, Set<V>>(DEFAULT_INITIAL_CAPACITY, DEFAULT_LOAD_FACTOR), setClazz);
-        }
-
-        public <C extends Set<V>> SetValuedHashMap(Class<C> setClazz, int initialSetCapacity) {
-            super(new HashMap<K, Set<V>>(DEFAULT_INITIAL_CAPACITY, DEFAULT_LOAD_FACTOR), setClazz,
-                    initialSetCapacity);
-        }
-
-    }
-
-}

src/main/java/org/apache/commons/collections4/multimap/TransformedMultiValuedMap.java

@@ -91,7 +91,7 @@ public class TransformedMultiValuedMap<K, V> extends AbstractMultiValuedMapDecor
         final TransformedMultiValuedMap<K, V> decorated =
                 new TransformedMultiValuedMap<K, V>(map, keyTransformer, valueTransformer);
         if (!map.isEmpty()) {
-            final MultiValuedMap<K, V> mapCopy = new MultiValuedHashMap<K, V>(map);
+            final MultiValuedMap<K, V> mapCopy = new ArrayListValuedHashMap<K, V>(map);
             decorated.clear();
             decorated.putAll(mapCopy);
         }

src/main/java/org/apache/commons/collections4/multimap/package-info.java

@@ -20,9 +20,8 @@
  * <p>
  * The following implementations are provided in the package:
  * <ul>
- *   <li>MultiValuedHashMap - implementation that uses a HashMap to store the data
+ *   <li>ArrayListValuedHashMap - ListValuedMap implementation using a HashMap/ArrayList
- *   <li>ListValuedHashMap  - implementation of a ListValuedMap using a HashMap as data store
+ *   <li>HashSetValuedHashMap   - SetValuedMap implementation using a HashMap/HashSet
- *   <li>SetValuedHashMap   - implementation of a SetValuedMap using a HashMap as data store
  * </ul>
  * <p>
  * The following decorators are provided in the package:

src/test/java/org/apache/commons/collections4/MultiMapUtilsTest.java

@@ -26,7 +26,7 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Set;
 
-import org.apache.commons.collections4.multimap.MultiValuedHashMap;
+import org.apache.commons.collections4.multimap.ArrayListValuedHashMap;
 import org.junit.Test;
 
 /**
@@ -64,20 +64,20 @@ public class MultiMapUtilsTest {
     public void testEmptyIfNull() {
         assertTrue(MultiMapUtils.emptyIfNull(null).isEmpty());
 
-        final MultiValuedMap<String, String> map = new MultiValuedHashMap<String, String>();
+        final MultiValuedMap<String, String> map = new ArrayListValuedHashMap<String, String>();
         map.put("item", "value");
         assertFalse(MultiMapUtils.emptyIfNull(map).isEmpty());
     }
 
     @Test
     public void testIsEmptyWithEmptyMap() {
-        final MultiValuedMap<Object, Object> map = new MultiValuedHashMap<Object, Object>();
+        final MultiValuedMap<Object, Object> map = new ArrayListValuedHashMap<Object, Object>();
         assertEquals(true, MultiMapUtils.isEmpty(map));
     }
 
     @Test
     public void testIsEmptyWithNonEmptyMap() {
-        final MultiValuedMap<String, String> map = new MultiValuedHashMap<String, String>();
+        final MultiValuedMap<String, String> map = new ArrayListValuedHashMap<String, String>();
         map.put("item", "value");
         assertEquals(false, MultiMapUtils.isEmpty(map));
     }
@@ -93,7 +93,7 @@ public class MultiMapUtilsTest {
         assertNull(MultiMapUtils.getCollection(null, "key1"));
 
         String values[] = { "v1", "v2", "v3" };
-        final MultiValuedMap<String, String> map = new MultiValuedHashMap<String, String>();
+        final MultiValuedMap<String, String> map = new ArrayListValuedHashMap<String, String>();
         for (String val : values) {
             map.put("key1", val);
         }
@@ -109,7 +109,7 @@ public class MultiMapUtilsTest {
         assertNull(MultiMapUtils.getValuesAsList(null, "key1"));
 
         String values[] = { "v1", "v2", "v3" };
-        final MultiValuedMap<String, String> map = new MultiValuedHashMap<String, String>();
+        final MultiValuedMap<String, String> map = new ArrayListValuedHashMap<String, String>();
         for (String val : values) {
             map.put("key1", val);
         }
@@ -126,7 +126,7 @@ public class MultiMapUtilsTest {
         assertNull(MultiMapUtils.getValuesAsList(null, "key1"));
 
         String values[] = { "v1", "v2", "v3" };
-        final MultiValuedMap<String, String> map = new MultiValuedHashMap<String, String>();
+        final MultiValuedMap<String, String> map = new ArrayListValuedHashMap<String, String>();
         for (String val : values) {
             map.put("key1", val);
             map.put("key1", val);
@@ -144,7 +144,7 @@ public class MultiMapUtilsTest {
         assertNull(MultiMapUtils.getValuesAsBag(null, "key1"));
 
         String values[] = { "v1", "v2", "v3" };
-        final MultiValuedMap<String, String> map = new MultiValuedHashMap<String, String>();
+        final MultiValuedMap<String, String> map = new ArrayListValuedHashMap<String, String>();
         for (String val : values) {
             map.put("key1", val);
             map.put("key1", val);

src/test/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMapTest.java

@@ -33,6 +33,7 @@ import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.collections4.MapIterator;
 import org.apache.commons.collections4.MultiSet;
 import org.apache.commons.collections4.MultiValuedMap;
+import org.apache.commons.collections4.SetValuedMap;
 import org.apache.commons.collections4.bag.AbstractBagTest;
 import org.apache.commons.collections4.bag.HashBag;
 import org.apache.commons.collections4.collection.AbstractCollectionTest;
@@ -104,10 +105,9 @@ public abstract class AbstractMultiValuedMapTest<K, V> extends AbstractObjectTes
         return true;
     }
 
-    // FIXME: tests ignore to fix serialization issues
     @Override
     public boolean isTestSerialization() {
-        return false;
+        return true;
     }
 
     /**
@@ -156,13 +156,13 @@ public abstract class AbstractMultiValuedMapTest<K, V> extends AbstractObjectTes
     }
 
     /**
-     * Override to return a MultiValuedMap other than MultiValuedHashMap as the
+     * Override to return a MultiValuedMap other than ArrayListValuedHashMap
-     * confirmed map.
+     * as the confirmed map.
      *
      * @return a MultiValuedMap that is known to be valid
      */
     public MultiValuedMap<K, V> makeConfirmedMap() {
-        return new MultiValuedHashMap<K, V>();
+        return new ArrayListValuedHashMap<K, V>();
     }
 
     public MultiValuedMap<K, V> getConfirmed() {
@@ -762,17 +762,15 @@ public abstract class AbstractMultiValuedMapTest<K, V> extends AbstractObjectTes
     // extend the AbstractTestMap
     // -----------------------------------------------------------------------
 
-    // FIXME: tests ignore to fix serialization issues
+    public void testEmptyMapCompatibility() throws Exception {
-    public void xtestEmptyMapCompatibility() throws Exception {
         final MultiValuedMap<?, ?> map = makeObject();
         final MultiValuedMap<?, ?> map2 =
                 (MultiValuedMap<?, ?>) readExternalFormFromDisk(getCanonicalEmptyCollectionName(map));
         assertEquals("Map is empty", 0, map2.size());
     }
 
-    // FIXME: tests ignore to fix serialization issues
     @SuppressWarnings({ "rawtypes", "unchecked" })
-    public void xtestFullMapCompatibility() throws Exception {
+    public void testFullMapCompatibility() throws Exception {
         final MultiValuedMap map = makeFullMap();
         final MultiValuedMap map2 =
                 (MultiValuedMap) readExternalFormFromDisk(getCanonicalFullCollectionName(map));
@@ -1113,10 +1111,12 @@ public abstract class AbstractMultiValuedMapTest<K, V> extends AbstractObjectTes
         @Override
         @SuppressWarnings("unchecked")
         public Collection<V>[] getSampleValues() {
+            boolean isSetValuedMap = AbstractMultiValuedMapTest.this.getMap() instanceof SetValuedMap;
             V[] sampleValues = AbstractMultiValuedMapTest.this.getSampleValues();
             Collection<V>[] colArr = new Collection[3];
             for(int i = 0; i < 3; i++) {
-                colArr[i] = Arrays.asList(sampleValues[i*2], sampleValues[i*2 + 1]);
+                Collection<V> coll = Arrays.asList(sampleValues[i*2], sampleValues[i*2 + 1]);
+                colArr[i] = isSetValuedMap ? new HashSet<V>(coll) : coll;
             }
             return colArr;
         }
@@ -1124,10 +1124,12 @@ public abstract class AbstractMultiValuedMapTest<K, V> extends AbstractObjectTes
         @Override
         @SuppressWarnings("unchecked")
         public Collection<V>[] getNewSampleValues() {
+            boolean isSetValuedMap = AbstractMultiValuedMapTest.this.getMap() instanceof SetValuedMap;
             Object[] sampleValues = { "ein", "ek", "zwei", "duey", "drei", "teen" };
             Collection<V>[] colArr = new Collection[3];
             for (int i = 0; i < 3; i++) {
-                colArr[i] = Arrays.asList((V) sampleValues[i * 2], (V) sampleValues[i * 2 + 1]);
+                Collection<V> coll = Arrays.asList((V) sampleValues[i * 2], (V) sampleValues[i * 2 + 1]);
+                colArr[i] = isSetValuedMap ? new HashSet<V>(coll) : coll;
             }
             return colArr;
         }

src/test/java/org/apache/commons/collections4/multimap/ArrayListValuedHashMapTest.java

@@ -16,95 +16,41 @@
  */
 package org.apache.commons.collections4.multimap;
 
-import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
-import java.util.Set;
 
 import junit.framework.Test;
 
 import org.apache.commons.collections4.BulkTest;
 import org.apache.commons.collections4.ListValuedMap;
 import org.apache.commons.collections4.MultiValuedMap;
-import org.apache.commons.collections4.SetValuedMap;
 
 /**
- * Test MultValuedHashMap
+ * Test ArrayListValuedHashMap
  *
  * @since 4.1
  * @version $Id$
  */
-public class MultiValuedHashMapTest<K, V> extends AbstractMultiValuedMapTest<K, V> {
+public class ArrayListValuedHashMapTest<K, V> extends AbstractMultiValuedMapTest<K, V> {
 
-    public MultiValuedHashMapTest(String testName) {
+    public ArrayListValuedHashMapTest(String testName) {
         super(testName);
     }
 
     public static Test suite() {
-        return BulkTest.makeSuite(MultiValuedHashMapTest.class);
+        return BulkTest.makeSuite(ArrayListValuedHashMapTest.class);
     }
 
+    // -----------------------------------------------------------------------
     @Override
-    public MultiValuedMap<K, V> makeObject() {
+    public ListValuedMap<K, V> makeObject() {
-        final MultiValuedMap<K, V> m = new MultiValuedHashMap<K, V>();
+        return new ArrayListValuedHashMap<K, V>();
-        return m;
-    }
-
-    @SuppressWarnings("unchecked")
-    public void testSetValuedMapAdd() {
-        final SetValuedMap<K, V> setMap = MultiValuedHashMap.setValuedHashMap();
-        assertTrue(setMap.get((K) "whatever") instanceof Set);
-
-        Set<V> set = setMap.get((K) "A");
-        assertTrue(set.add((V) "a1"));
-        assertTrue(set.add((V) "a2"));
-        assertFalse(set.add((V) "a1"));
-        assertEquals(2, setMap.size());
-        assertTrue(setMap.containsKey("A"));
-    }
-
-    @SuppressWarnings("unchecked")
-    public void testSetValuedMapRemove() {
-        final SetValuedMap<K, V> setMap = MultiValuedHashMap.setValuedHashMap();
-        assertTrue(setMap.get((K) "whatever") instanceof Set);
-
-        Set<V> set = setMap.get((K) "A");
-        assertTrue(set.add((V) "a1"));
-        assertTrue(set.add((V) "a2"));
-        assertFalse(set.add((V) "a1"));
-        assertEquals(2, setMap.size());
-        assertTrue(setMap.containsKey("A"));
-
-        assertTrue(set.remove("a1"));
-        assertTrue(set.remove("a2"));
-        assertFalse(set.remove("a1"));
-
-        assertEquals(0, setMap.size());
-        assertFalse(setMap.containsKey("A"));
-    }
-
-    @SuppressWarnings("unchecked")
-    public void testSetValuedMapRemoveViaIterator() {
-        final SetValuedMap<K, V> setMap = MultiValuedHashMap.setValuedHashMap();
-        assertTrue(setMap.get((K) "whatever") instanceof Set);
-
-        Set<V> set = setMap.get((K) "A");
-        set.add((V) "a1");
-        set.add((V) "a2");
-        set.add((V) "a1");
-
-        Iterator<V> it = set.iterator();
-        while (it.hasNext()) {
-            it.next();
-            it.remove();
-        }
-        assertEquals(0, setMap.size());
-        assertFalse(setMap.containsKey("A"));
     }
 
+    // -----------------------------------------------------------------------
     @SuppressWarnings("unchecked")
     public void testListValuedMapAdd() {
-        final ListValuedMap<K, V> listMap = MultiValuedHashMap.listValuedHashMap();
+        final ListValuedMap<K, V> listMap = makeObject();
         assertTrue(listMap.get((K) "whatever") instanceof List);
         List<V> list = listMap.get((K) "A");
         list.add((V) "a1");
@@ -114,7 +60,7 @@ public class MultiValuedHashMapTest<K, V> extends AbstractMultiValuedMapTest<K,
 
     @SuppressWarnings("unchecked")
     public void testListValuedMapAddViaListIterator() {
-        final ListValuedMap<K, V> listMap = MultiValuedHashMap.listValuedHashMap();
+        final ListValuedMap<K, V> listMap = makeObject();
         ListIterator<V> listIt = listMap.get((K) "B").listIterator();
         assertFalse(listIt.hasNext());
         listIt.add((V) "b1");
@@ -128,7 +74,7 @@ public class MultiValuedHashMapTest<K, V> extends AbstractMultiValuedMapTest<K,
 
     @SuppressWarnings("unchecked")
     public void testListValuedMapRemove() {
-        final ListValuedMap<K, V> listMap = MultiValuedHashMap.listValuedHashMap();
+        final ListValuedMap<K, V> listMap = makeObject();
         List<V> list = listMap.get((K) "A");
         list.add((V) "a1");
         list.add((V) "a2");
@@ -145,7 +91,7 @@ public class MultiValuedHashMapTest<K, V> extends AbstractMultiValuedMapTest<K,
 
     @SuppressWarnings("unchecked")
     public void testListValuedMapRemoveViaListIterator() {
-        final ListValuedMap<K, V> listMap = MultiValuedHashMap.listValuedHashMap();
+        final ListValuedMap<K, V> listMap = makeObject();
         ListIterator<V> listIt = listMap.get((K) "B").listIterator();
         listIt.add((V) "b1");
         listIt.add((V) "b2");
@@ -165,8 +111,8 @@ public class MultiValuedHashMapTest<K, V> extends AbstractMultiValuedMapTest<K,
 
     @SuppressWarnings({ "unchecked", "rawtypes" })
     public void testEqualsHashCodeContract() {
-        MultiValuedMap map1 = new MultiValuedHashMap();
+        MultiValuedMap map1 = makeObject();
-        MultiValuedMap map2 = new MultiValuedHashMap();
+        MultiValuedMap map2 = makeObject();
 
         map1.put("a", "a1");
         map1.put("a", "a2");
@@ -182,8 +128,8 @@ public class MultiValuedHashMapTest<K, V> extends AbstractMultiValuedMapTest<K,
 
     @SuppressWarnings({ "unchecked", "rawtypes" })
     public void testListValuedMapEqualsHashCodeContract() {
-        ListValuedMap map1 = MultiValuedHashMap.listValuedHashMap();
+        ListValuedMap map1 = makeObject();
-        ListValuedMap map2 = MultiValuedHashMap.listValuedHashMap();
+        ListValuedMap map2 = makeObject();
 
         map1.put("a", "a1");
         map1.put("a", "a2");
@@ -200,32 +146,11 @@ public class MultiValuedHashMapTest<K, V> extends AbstractMultiValuedMapTest<K,
         assertNotSame(map1.hashCode(), map2.hashCode());
     }
 
-    @SuppressWarnings({ "unchecked", "rawtypes" })
+    public void testCreate() throws Exception {
-    public void testSetValuedMapEqualsHashCodeContract() {
+        writeExternalFormToDisk((java.io.Serializable) makeObject(),
-        SetValuedMap map1 = MultiValuedHashMap.setValuedHashMap();
+                "src/test/resources/data/test/ArrayListValuedHashMap.emptyCollection.version4.1.obj");
-        SetValuedMap map2 = MultiValuedHashMap.setValuedHashMap();
+        writeExternalFormToDisk((java.io.Serializable) makeFullMap(),
-
+                "src/test/resources/data/test/ArrayListValuedHashMap.fullCollection.version4.1.obj");
-        map1.put("a", "a1");
-        map1.put("a", "a2");
-        map2.put("a", "a2");
-        map2.put("a", "a1");
-        assertEquals(map1, map2);
-        assertEquals(map1.hashCode(), map2.hashCode());
-
-        map2.put("a", "a2");
-        assertEquals(map1, map2);
-        assertEquals(map1.hashCode(), map2.hashCode());
-
-        map2.put("a", "a3");
-        assertNotSame(map1, map2);
-        assertNotSame(map1.hashCode(), map2.hashCode());
     }
 
-//    public void testCreate() throws Exception {
-//        writeExternalFormToDisk((java.io.Serializable) makeObject(),
-//                "src/test/resources/data/test/MultiValuedHashMap.emptyCollection.version4.1.obj");
-//        writeExternalFormToDisk((java.io.Serializable) makeFullMap(),
-//                "src/test/resources/data/test/MultiValuedHashMap.fullCollection.version4.1.obj");
-//    }
-
 }

src/test/java/org/apache/commons/collections4/multimap/HashSetValuedHashMapTest.java

@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.collections4.multimap;
+
+import java.util.Iterator;
+import java.util.Set;
+
+import junit.framework.Test;
+
+import org.apache.commons.collections4.BulkTest;
+import org.apache.commons.collections4.MultiValuedMap;
+import org.apache.commons.collections4.SetValuedMap;
+
+/**
+ * Test HashSetValuedHashMap
+ *
+ * @since 4.1
+ * @version $Id$
+ */
+public class HashSetValuedHashMapTest<K, V> extends AbstractMultiValuedMapTest<K, V> {
+
+    public HashSetValuedHashMapTest(String testName) {
+        super(testName);
+    }
+
+    public static Test suite() {
+        return BulkTest.makeSuite(HashSetValuedHashMapTest.class);
+    }
+
+    // -----------------------------------------------------------------------
+    @Override
+    public SetValuedMap<K, V> makeObject() {
+        return new HashSetValuedHashMap<K, V>();
+    }
+
+    @Override
+    public MultiValuedMap<K, V> makeConfirmedMap() {
+        return new HashSetValuedHashMap<K, V>();
+    }
+
+    // -----------------------------------------------------------------------
+    @SuppressWarnings("unchecked")
+    public void testSetValuedMapAdd() {
+        final SetValuedMap<K, V> setMap = makeObject();
+        assertTrue(setMap.get((K) "whatever") instanceof Set);
+
+        Set<V> set = setMap.get((K) "A");
+        assertTrue(set.add((V) "a1"));
+        assertTrue(set.add((V) "a2"));
+        assertFalse(set.add((V) "a1"));
+        assertEquals(2, setMap.size());
+        assertTrue(setMap.containsKey("A"));
+    }
+
+    @SuppressWarnings("unchecked")
+    public void testSetValuedMapRemove() {
+        final SetValuedMap<K, V> setMap = makeObject();
+        assertTrue(setMap.get((K) "whatever") instanceof Set);
+
+        Set<V> set = setMap.get((K) "A");
+        assertTrue(set.add((V) "a1"));
+        assertTrue(set.add((V) "a2"));
+        assertFalse(set.add((V) "a1"));
+        assertEquals(2, setMap.size());
+        assertTrue(setMap.containsKey("A"));
+
+        assertTrue(set.remove("a1"));
+        assertTrue(set.remove("a2"));
+        assertFalse(set.remove("a1"));
+
+        assertEquals(0, setMap.size());
+        assertFalse(setMap.containsKey("A"));
+    }
+
+    @SuppressWarnings("unchecked")
+    public void testSetValuedMapRemoveViaIterator() {
+        final SetValuedMap<K, V> setMap = makeObject();
+        assertTrue(setMap.get((K) "whatever") instanceof Set);
+
+        Set<V> set = setMap.get((K) "A");
+        set.add((V) "a1");
+        set.add((V) "a2");
+        set.add((V) "a1");
+
+        Iterator<V> it = set.iterator();
+        while (it.hasNext()) {
+            it.next();
+            it.remove();
+        }
+        assertEquals(0, setMap.size());
+        assertFalse(setMap.containsKey("A"));
+    }
+
+    @SuppressWarnings({ "unchecked", "rawtypes" })
+    public void testSetValuedMapEqualsHashCodeContract() {
+        SetValuedMap map1 = makeObject();
+        SetValuedMap map2 = makeObject();
+
+        map1.put("a", "a1");
+        map1.put("a", "a2");
+        map2.put("a", "a2");
+        map2.put("a", "a1");
+        assertEquals(map1, map2);
+        assertEquals(map1.hashCode(), map2.hashCode());
+
+        map2.put("a", "a2");
+        assertEquals(map1, map2);
+        assertEquals(map1.hashCode(), map2.hashCode());
+
+        map2.put("a", "a3");
+        assertNotSame(map1, map2);
+        assertNotSame(map1.hashCode(), map2.hashCode());
+    }
+
+    public void testCreate() throws Exception {
+        writeExternalFormToDisk((java.io.Serializable) makeObject(),
+                "src/test/resources/data/test/HashSetValuedHashMap.emptyCollection.version4.1.obj");
+        writeExternalFormToDisk((java.io.Serializable) makeFullMap(),
+                "src/test/resources/data/test/HashSetValuedHashMap.fullCollection.version4.1.obj");
+    }
+
+}

src/test/java/org/apache/commons/collections4/multimap/TransformedMultiValuedMapTest.java

@@ -42,18 +42,20 @@ public class TransformedMultiValuedMapTest<K, V> extends AbstractMultiValuedMapT
         return BulkTest.makeSuite(TransformedMultiValuedMapTest.class);
     }
 
+    // -----------------------------------------------------------------------
     @Override
     public MultiValuedMap<K, V> makeObject() {
-        return TransformedMultiValuedMap.transformingMap(new MultiValuedHashMap<K, V>(),
+        return TransformedMultiValuedMap.transformingMap(new ArrayListValuedHashMap<K, V>(),
                 TransformerUtils.<K> nopTransformer(), TransformerUtils.<V> nopTransformer());
     }
 
+    // -----------------------------------------------------------------------
     @SuppressWarnings("unchecked")
     public void testKeyTransformedMap() {
         final Object[] els = new Object[] { "1", "3", "5", "7", "2", "4", "6" };
 
         MultiValuedMap<K, V> map = TransformedMultiValuedMap.transformingMap(
-                new MultiValuedHashMap<K, V>(),
+                new ArrayListValuedHashMap<K, V>(),
                 (Transformer<? super K, ? extends K>) TransformedCollectionTest.STRING_TO_INTEGER_TRANSFORMER,
                 null);
         assertEquals(0, map.size());
@@ -77,7 +79,7 @@ public class TransformedMultiValuedMapTest<K, V> extends AbstractMultiValuedMapT
         final Object[] els = new Object[] { "1", "3", "5", "7", "2", "4", "6" };
 
         MultiValuedMap<K, V> map = TransformedMultiValuedMap.transformingMap(
-                new MultiValuedHashMap<K, V>(), null,
+                new ArrayListValuedHashMap<K, V>(), null,
                 (Transformer<? super V, ? extends V>) TransformedCollectionTest.STRING_TO_INTEGER_TRANSFORMER);
         assertEquals(0, map.size());
         for (int i = 0; i < els.length; i++) {
@@ -94,7 +96,7 @@ public class TransformedMultiValuedMapTest<K, V> extends AbstractMultiValuedMapT
     // -----------------------------------------------------------------------
     @SuppressWarnings("unchecked")
     public void testFactory_Decorate() {
-        final MultiValuedMap<K, V> base = new MultiValuedHashMap<K, V>();
+        final MultiValuedMap<K, V> base = new ArrayListValuedHashMap<K, V>();
         base.put((K) "A", (V) "1");
         base.put((K) "B", (V) "2");
         base.put((K) "C", (V) "3");
@@ -114,7 +116,7 @@ public class TransformedMultiValuedMapTest<K, V> extends AbstractMultiValuedMapT
 
     @SuppressWarnings("unchecked")
     public void testFactory_decorateTransform() {
-        final MultiValuedMap<K, V> base = new MultiValuedHashMap<K, V>();
+        final MultiValuedMap<K, V> base = new ArrayListValuedHashMap<K, V>();
         base.put((K) "A", (V) "1");
         base.put((K) "B", (V) "2");
         base.put((K) "C", (V) "3");
@@ -132,11 +134,11 @@ public class TransformedMultiValuedMapTest<K, V> extends AbstractMultiValuedMapT
         assertEquals(true, trans.get((K) "D").contains(Integer.valueOf(4)));
     }
 
-//    public void testCreate() throws Exception {
+    public void testCreate() throws Exception {
-//        writeExternalFormToDisk((java.io.Serializable) makeObject(),
+        writeExternalFormToDisk((java.io.Serializable) makeObject(),
-//                "src/test/resources/data/test/TransformedMultiValuedMap.emptyCollection.version4.1.obj");
+                "src/test/resources/data/test/TransformedMultiValuedMap.emptyCollection.version4.1.obj");
-//        writeExternalFormToDisk((java.io.Serializable) makeFullMap(),
+        writeExternalFormToDisk((java.io.Serializable) makeFullMap(),
-//                "src/test/resources/data/test/TransformedMultiValuedMap.fullCollection.version4.1.obj");
+                "src/test/resources/data/test/TransformedMultiValuedMap.fullCollection.version4.1.obj");
-//    }
+    }
 
 }

src/test/java/org/apache/commons/collections4/multimap/UnmodifiableMultiValuedMapTest.java

@@ -47,6 +47,7 @@ public class UnmodifiableMultiValuedMapTest<K, V> extends AbstractMultiValuedMap
         return BulkTest.makeSuite(UnmodifiableMultiValuedMapTest.class);
     }
     
+    // -----------------------------------------------------------------------
     @Override
     public boolean isAddSupported() {
         return false;
@@ -59,16 +60,18 @@ public class UnmodifiableMultiValuedMapTest<K, V> extends AbstractMultiValuedMap
 
     @Override
     public MultiValuedMap<K, V> makeObject() {
-        return UnmodifiableMultiValuedMap.<K, V> unmodifiableMultiValuedMap(new MultiValuedHashMap<K, V>());
+        return UnmodifiableMultiValuedMap.<K, V> unmodifiableMultiValuedMap(
+                new ArrayListValuedHashMap<K, V>());
     }
 
     @Override
     protected MultiValuedMap<K, V> makeFullMap() {
-        final MultiValuedMap<K, V> map = new MultiValuedHashMap<K, V>();
+        final MultiValuedMap<K, V> map = new ArrayListValuedHashMap<K, V>();
         addSampleMappings(map);
         return UnmodifiableMultiValuedMap.<K, V> unmodifiableMultiValuedMap(map);
     }
 
+    // -----------------------------------------------------------------------
     public void testUnmodifiable() {
         assertTrue(makeObject() instanceof Unmodifiable);
         assertTrue(makeFullMap() instanceof Unmodifiable);
@@ -259,11 +262,11 @@ public class UnmodifiableMultiValuedMapTest<K, V> extends AbstractMultiValuedMap
         }
     }
 
-//    public void testCreate() throws Exception {
+    public void testCreate() throws Exception {
-//        writeExternalFormToDisk((java.io.Serializable) makeObject(),
+        writeExternalFormToDisk((java.io.Serializable) makeObject(),
-//                "src/test/resources/data/test/UnmodifiableMultiValuedMap.emptyCollection.version4.1.obj");
+                "src/test/resources/data/test/UnmodifiableMultiValuedMap.emptyCollection.version4.1.obj");
-//        writeExternalFormToDisk((java.io.Serializable) makeFullMap(),
+        writeExternalFormToDisk((java.io.Serializable) makeFullMap(),
-//                "src/test/resources/data/test/UnmodifiableMultiValuedMap.fullCollection.version4.1.obj");
+                "src/test/resources/data/test/UnmodifiableMultiValuedMap.fullCollection.version4.1.obj");
-//    }
+    }
 
 }