Support cloning of primitive arrays

3 files changed, 83 insertions, 39 deletions

diff --git a/processing/src/main/java/com/yahoo/processing/request/CloneHelper.java b/processing/src/main/java/com/yahoo/processing/request/CloneHelper.java
index 8b639855764..bc866c8a538 100644
--- a/processing/src/main/java/com/yahoo/processing/request/CloneHelper.java
+++ b/processing/src/main/java/com/yahoo/processing/request/CloneHelper.java
@@ -24,24 +24,50 @@ import java.util.HashMap;
  * The rest has the slow path with reflection,
  * though using a fast thread safe method cache for speedup.
  *
- * @author : baldersheim
+ * @author bratseth
+ * @author baldersheim
  */
 public class CloneHelper {
+
     private static Logger log = Logger.getLogger(CloneHelper.class.getName());
     private static final MethodCache cloneMethodCache = new MethodCache("clone");
+
     /**
      * Clones this object if it is clonable, and the clone is public. Returns null if not
      */
     public final Object clone(Object object) {
         if (object == null) return null;
-        if (!(object instanceof Cloneable)) return null;
-        if (object instanceof Object[])
-            return arrayClone((Object[]) object);
+        if ( ! (object instanceof Cloneable)) return null;
+        if (object.getClass().isArray())
+            return arrayClone(object);
         else
             return objectClone(object);
     }
 
-    private final Object arrayClone(Object[] object) {
+    private Object arrayClone(Object array) {
+        if (array instanceof Object[])
+            return objectArrayClone((Object[]) array);
+        else if (array instanceof byte[])
+            return Arrays.copyOf((byte[])array, ((byte[])array).length);
+        else if (array instanceof char[])
+            return Arrays.copyOf((char[])array, ((char[])array).length);
+        else if (array instanceof short[])
+            return Arrays.copyOf((short[])array, ((short[])array).length);
+        else if (array instanceof int[])
+            return Arrays.copyOf((int[])array, ((int[])array).length);
+        else if (array instanceof long[])
+            return Arrays.copyOf((long[])array, ((long[])array).length);
+        else if (array instanceof float[])
+            return Arrays.copyOf((float[])array, ((float[])array).length);
+        else if (array instanceof double[])
+            return Arrays.copyOf((double[])array, ((double[])array).length);
+        else if (array instanceof boolean[])
+            return Arrays.copyOf((boolean[])array, ((boolean[])array).length);
+        else
+            return new IllegalArgumentException("Unexpected primitive arrat type " + array.getClass());
+    }
+    
+    private Object objectArrayClone(Object[] object) {
         Object[] arrayClone = Arrays.copyOf(object, object.length);
         // deep clone
         for (int i = 0; i < arrayClone.length; i++) {
@@ -53,35 +79,33 @@ public class CloneHelper {
     }
 
     protected Object objectClone(Object object) {
-        // Fastpath for our own commonly used classes
-        if (object instanceof FreezableClass) {
-            // List common superclass of 'com.yahoo.search.result.Hit'
-            return ((FreezableClass) object).clone();
-        }
-        else if (object instanceof PublicCloneable) {
+        // Fastpath for our commonly used classes
+        if (object instanceof FreezableClass)
+            return ((FreezableClass)object).clone();
+        else if (object instanceof PublicCloneable)
             return ((PublicCloneable)object).clone();
-        }
-        else if (object instanceof LinkedList) { // TODO: Why? Somebody's infatuation with LinkedList knows no limits
+        else if (object instanceof LinkedList)
             return ((LinkedList) object).clone();
-        }
-        else if (object instanceof ArrayList) { // TODO: Why? Likewise
+        else if (object instanceof ArrayList)
             return ((ArrayList) object).clone();
-        }
 
         try {
             Method cloneMethod = cloneMethodCache.get(object);
             if (cloneMethod == null) {
-                log.warning("'" + object + "' is Cloneable, but has no clone method - will use the same instance in all requests");
+                log.warning("'" + object + "' of class " + object.getClass() + 
+                            " is Cloneable, but has no clone method - will use the same instance in all requests");
                 return null;
             }
             return cloneMethod.invoke(object);
         } catch (IllegalAccessException e) {
-            log.warning("'" + object + "' is Cloneable, but clone method cannot be accessed - will use the same instance in all requests");
+            log.warning("'" + object + "' of class " + object.getClass() + 
+                        " is Cloneable, but clone method cannot be accessed - will use the same instance in all requests");
             return null;
         } catch (InvocationTargetException e) {
             throw new RuntimeException("Exception cloning '" + object + "'", e);
         }
     }
+
     /**
      * Clones a map by deep cloning each value which is cloneable and shallow copying all other values.
      */
diff --git a/processing/src/test/java/com/yahoo/processing/request/test/PropertyMapTestCase.java b/processing/src/test/java/com/yahoo/processing/request/test/PropertyMapTestCase.java
index 812ce2fae65..3822286daac 100644
--- a/processing/src/test/java/com/yahoo/processing/request/test/PropertyMapTestCase.java
+++ b/processing/src/test/java/com/yahoo/processing/request/test/PropertyMapTestCase.java
@@ -3,23 +3,30 @@ package com.yahoo.processing.request.test;
 
 import com.yahoo.processing.request.properties.PropertyMap;
 import com.yahoo.processing.request.properties.PublicCloneable;
+import org.junit.Test;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 /**
  * @author  bratseth
  */
-public class PropertyMapTestCase extends junit.framework.TestCase {
-
-    public void testCloning() {
-        PropertyMap map=new PropertyMap();
-        map.set("clonable",new ClonableObject());
-        map.set("publicClonable",new PublicClonableObject());
-        map.set("nonclonable",new NonClonableObject());
-        map.set("clonableArray",new ClonableObject[] {new ClonableObject()});
-        map.set("publicClonableArray",new ClonableObject[] {new ClonableObject()});
-        map.set("nonclonableArray",new NonClonableObject[] {new NonClonableObject()});
+public class PropertyMapTestCase {
+
+    @Test
+    public void testObjectCloning() {
+        PropertyMap map = new PropertyMap();
+        map.set("clonable", new ClonableObject());
+        map.set("publicClonable", new PublicClonableObject());
+        map.set("nonclonable", new NonClonableObject());
+        map.set("clonableArray", new ClonableObject[] {new ClonableObject()});
+        map.set("publicClonableArray", new ClonableObject[] {new ClonableObject()});
+        map.set("nonclonableArray", new NonClonableObject[] {new NonClonableObject()});
         map.set("clonableList", Collections.singletonList(new ClonableObject()));
         map.set("nonclonableList", Collections.singletonList(new NonClonableObject()));
         assertNotNull(map.get("clonable"));
@@ -36,6 +43,17 @@ public class PropertyMapTestCase extends junit.framework.TestCase {
         assertTrue(first(map.get("publicClonableArray")) != first(mapClone.get("publicClonableArray")));
         assertTrue(first(map.get("nonclonableArray")) == first(mapClone.get("nonclonableArray")));
     }
+    
+    @Test
+    public void testArrayCloning() {
+        PropertyMap map = new PropertyMap();
+        byte[] byteArray = new byte[] {2, 4, 7};
+        map.set("byteArray", byteArray);
+
+        PropertyMap mapClone = map.clone();
+        assertArrayEquals(byteArray, (byte[])mapClone.get("byteArray"));
+        assertTrue("Array was cloned", mapClone.get("byteArray") != byteArray);
+    }
 
     private Object first(Object object) {
         if (object instanceof Object[])
diff --git a/processing/src/test/java/com/yahoo/processing/request/test/RequestTestCase.java b/processing/src/test/java/com/yahoo/processing/request/test/RequestTestCase.java
index 7045ea1efbd..a2e26494113 100644
--- a/processing/src/test/java/com/yahoo/processing/request/test/RequestTestCase.java
+++ b/processing/src/test/java/com/yahoo/processing/request/test/RequestTestCase.java
@@ -8,6 +8,8 @@ import com.yahoo.processing.request.Properties;
 import com.yahoo.processing.request.properties.PropertyMap;
 import org.junit.Test;
 
+import java.util.Arrays;
+
 /**
  * Tests using requests
  *
@@ -117,17 +119,17 @@ public class RequestTestCase extends junit.framework.TestCase {
         request.properties().set("d", "d1");
         request.errors().add(new ErrorMessage("boz"));
 
-        assertEquals("a1",request.properties().get("a"));
-        assertEquals("a1",rcloned.properties().get("a"));
-        assertEquals("b1",request.properties().get("b"));
-        assertEquals("b1",rcloned.properties().get("b"));
-        assertEquals(null,request.properties().get("c"));
-        assertEquals("c1",rcloned.properties().get("c"));
-        assertEquals("d1",request.properties().get("d"));
-        assertEquals(null,rcloned.properties().get("d"));
-
-        assertEquals(3,request.errors().size());
-        assertEquals(1,rcloned.errors().size());
+        assertEquals("a1", request.properties().get("a"));
+        assertEquals("a1", rcloned.properties().get("a"));
+        assertEquals("b1", request.properties().get("b"));
+        assertEquals("b1", rcloned.properties().get("b"));
+        assertEquals(null, request.properties().get("c"));
+        assertEquals("c1", rcloned.properties().get("c"));
+        assertEquals("d1", request.properties().get("d"));
+        assertEquals(null, rcloned.properties().get("d"));
+
+        assertEquals(3, request.errors().size());
+        assertEquals(1, rcloned.errors().size());
         assertEquals("foo",request.errors().get(0).getMessage());
         assertEquals("bar",request.errors().get(1).getMessage());
         assertEquals("boz",request.errors().get(2).getMessage());