Allow beans with non-instantiatable collection/map properties to be c…

…loned (#366)

Fixes #365.

rome/src/main/java/com/rometools/rome/feed/impl/CloneableBean.java

@@ -20,13 +20,19 @@
 import java.lang.reflect.Array;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.SortedMap;
+import java.util.SortedSet;
+import java.util.TreeMap;
+import java.util.TreeSet;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -232,16 +238,31 @@ private <T> T cloneArray(final T array) throws Exception {
 
     private <T> Collection<T> cloneCollection(final Collection<T> collection) throws Exception {
         @SuppressWarnings("unchecked")
-        final Collection<T> newCollection = collection.getClass().newInstance();
+        final Collection<T> newCollection = newCollection(collection.getClass());
         for (final T item : collection) {
             newCollection.add(doClone(item));
         }
         return newCollection;
     }
 
+    private static <T extends Collection<E>, E> Collection<E> newCollection(Class<T> type)
+        throws InstantiationException, IllegalAccessException {
+        Collection<E> collection;
+        if (SortedSet.class.isAssignableFrom(type)) {
+            collection = new TreeSet<E>();
+        } else if (Set.class.isAssignableFrom(type)) {
+            collection = new HashSet<E>();
+        } else if (List.class.isAssignableFrom(type)) {
+            collection = new ArrayList<E>();
+        } else {
+            collection = type.newInstance();
+        }
+        return collection;
+    }
+
     private <K, V> Map<K, V> cloneMap(final Map<K, V> map) throws Exception {
         @SuppressWarnings("unchecked")
-        final Map<K, V> newMap = map.getClass().newInstance();
+        final Map<K, V> newMap = newMap(map.getClass());
         for (final Entry<K, V> entry : map.entrySet()) {
             final K clonedKey = doClone(entry.getKey());
             final V clonedValue = doClone(entry.getValue());
@@ -250,6 +271,17 @@ private <K, V> Map<K, V> cloneMap(final Map<K, V> map) throws Exception {
         return newMap;
     }
 
+    private static <T extends Map<K, V>, K, V> Map<K, V> newMap(Class<T> type)
+        throws InstantiationException, IllegalAccessException {
+        Map<K, V> map;
+        if (SortedMap.class.isAssignableFrom(type)) {
+            map = new TreeMap<K, V>();
+        } else {
+            map = new HashMap<K, V>();
+        }
+        return map;
+    }
+
     private boolean isBasicType(final Class<?> vClass) {
         return BASIC_TYPES.contains(vClass);
     }

rome/src/test/java/com/rometools/rome/unittest/CloneableBeanTest.java

@@ -0,0 +1,221 @@
+/*
+ * Licensed 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 com.rometools.rome.unittest;
+
+import java.util.AbstractCollection;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.SortedMap;
+import java.util.SortedSet;
+import java.util.TreeMap;
+import java.util.TreeSet;
+
+import com.rometools.rome.feed.impl.CloneableBean;
+
+import junit.framework.TestCase;
+
+import static java.util.Arrays.asList;
+import static java.util.Collections.unmodifiableList;
+import static java.util.Collections.unmodifiableMap;
+import static java.util.Collections.unmodifiableSet;
+import static java.util.Collections.unmodifiableSortedMap;
+import static java.util.Collections.unmodifiableSortedSet;
+
+public class CloneableBeanTest extends TestCase {
+
+    public static class TestBean {
+
+        private Collection<String> collection;
+
+        private Map<String, String> map;
+
+        public Collection<String> getCollection() {
+            return collection;
+        }
+
+        public void setCollection(Collection<String> collection) {
+            this.collection = collection;
+        }
+
+        public Map<String, String> getMap() {
+            return map;
+        }
+
+        public void setMap(Map<String, String> map) {
+            this.map = map;
+        }
+    }
+
+    public static class TestCollection<E> extends AbstractCollection<E> {
+
+        private final Collection<E> collection;
+
+        public TestCollection() {
+            this(Collections.<E>emptyList());
+        }
+
+        public TestCollection(Collection<E> collection) {
+            this.collection = new ArrayList<E>(collection);
+        }
+
+        @Override
+        public boolean add(E e) {
+            return collection.add(e);
+        }
+
+        @Override
+        public Iterator<E> iterator() {
+            return collection.iterator();
+        }
+
+        @Override
+        public int size() {
+            return collection.size();
+        }
+    }
+
+    public void testCloneSetProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        Set<String> set = new HashSet<String>(asList("x", "y"));
+        bean.setCollection(set);
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        Collection<String> clonedSet = clonedBean.getCollection();
+        assertNotSame(set, clonedSet);
+        assertEquals(new HashSet<String>(asList("x", "y")), clonedSet);
+    }
+
+    public void testCloneNonInstantiatableSetProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        bean.setCollection(unmodifiableSet(new HashSet<String>(asList("x", "y"))));
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        assertEquals(new HashSet<String>(asList("x", "y")), clonedBean.getCollection());
+    }
+
+    public void testCloneSortedSetProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        SortedSet<String> sortedSet = new TreeSet<String>(asList("x", "y"));
+        bean.setCollection(sortedSet);
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        Collection<String> clonedSortedSet = clonedBean.getCollection();
+        assertNotSame(sortedSet, clonedSortedSet);
+        assertTrue(clonedSortedSet instanceof SortedSet);
+        assertEquals(new TreeSet<String>(asList("x", "y")), clonedSortedSet);
+    }
+
+    public void testCloneNonInstantiatableSortedSetProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        bean.setCollection(unmodifiableSortedSet(new TreeSet<String>(asList("x", "y"))));
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        assertEquals(new TreeSet<String>(asList("x", "y")), clonedBean.getCollection());
+    }
+
+    public void testCloneListProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        List<String> list = new ArrayList<String>(asList("x", "y"));
+        bean.setCollection(list);
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        Collection<String> clonedList = clonedBean.getCollection();
+        assertNotSame(list, clonedList);
+        assertEquals(asList("x", "y"), clonedList);
+    }
+
+    public void testCloneNonInstantiatableListProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        bean.setCollection(unmodifiableList(asList("x", "y")));
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        assertEquals(asList("x", "y"), clonedBean.getCollection());
+    }
+
+    public void testCloneMapProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        Map<String, String> map = new HashMap<String, String>(mapOf("x1", "y1", "x2", "y2"));
+        bean.setMap(map);
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        Map<String, String> clonedMap = clonedBean.getMap();
+        assertNotSame(map, clonedMap);
+        assertEquals(mapOf("x1", "y1", "x2", "y2"), clonedMap);
+    }
+
+    public void testCloneNonInstantiatableMapProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        bean.setMap(unmodifiableMap(mapOf("x1", "y1", "x2", "y2")));
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        assertEquals(mapOf("x1", "y1", "x2", "y2"), clonedBean.getMap());
+    }
+
+    public void testCloneSortedMapProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        SortedMap<String, String> sortedMap = new TreeMap<String, String>(mapOf("x1", "y1", "x2", "y2"));
+        bean.setMap(sortedMap);
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        Map<String, String> clonedSortedMap = clonedBean.getMap();
+        assertNotSame(sortedMap, clonedSortedMap);
+        assertTrue(clonedSortedMap instanceof SortedMap);
+        assertEquals(new TreeMap<String, String>(mapOf("x1", "y1", "x2", "y2")), clonedSortedMap);
+    }
+
+    public void testCloneNonInstantiatableSortedMapProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        bean.setMap(unmodifiableSortedMap(new TreeMap<String, String>(mapOf("x1", "y1", "x2", "y2"))));
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        assertEquals(mapOf("x1", "y1", "x2", "y2"), clonedBean.getMap());
+    }
+
+    public void testCloneUnknownCollectionProperty() throws CloneNotSupportedException {
+        TestBean bean = new TestBean();
+        Collection<String> collection = new TestCollection<String>(asList("x", "y"));
+        bean.setCollection(collection);
+
+        TestBean clonedBean = (TestBean) new CloneableBean(bean).beanClone();
+
+        Collection<String> clonedCollection = clonedBean.getCollection();
+        assertNotSame(collection, clonedCollection);
+        assertTrue(clonedCollection instanceof TestCollection);
+        assertEquals(asList("x", "y"), new ArrayList<String>(clonedCollection));
+    }
+
+    private static Map<String, String> mapOf(String key1, String value1, String key2, String value2) {
+        Map<String, String> map = new HashMap<String, String>();
+        map.put(key1, value1);
+        map.put(key2, value2);
+        return map;
+    }
+}