When code is duplicated is will only be fixed in one of the places.

At least now the cloning magic is only present in one place.
Shall we deprecate/deduplicate any of this processing/search code ?

3 files changed, 94 insertions, 80 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
new file mode 100644
index 00000000000..9bc1c89135c
--- /dev/null
+++ b/processing/src/main/java/com/yahoo/processing/request/CloneHelper.java
@@ -0,0 +1,85 @@
+package com.yahoo.processing.request;
+
+import com.yahoo.collections.MethodCache;
+import com.yahoo.component.provider.FreezableClass;
+import com.yahoo.processing.request.properties.PublicCloneable;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.*;
+import java.util.logging.Logger;
+
+/**
+ * @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);
+        else
+            return objectClone(object);
+    }
+
+    private final Object arrayClone(Object[] object) {
+        Object[] arrayClone = Arrays.copyOf(object, object.length);
+        // deep clone
+        for (int i = 0; i < arrayClone.length; i++) {
+            Object elementClone = clone(arrayClone[i]);
+            if (elementClone != null)
+                arrayClone[i] = elementClone;
+        }
+        return arrayClone;
+    }
+
+    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) {
+            return ((PublicCloneable)object).clone();
+        }
+        else if (object instanceof LinkedList) { // TODO: Why? Somebody's infatuation with LinkedList knows no limits
+            return ((LinkedList) object).clone();
+        }
+        else if (object instanceof ArrayList) { // TODO: Why? Likewise
+            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");
+                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");
+            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.
+     */
+    public Map<CompoundName, Object> cloneMap(Map<CompoundName, Object> map) {
+        Map<CompoundName, Object> cloneMap = new HashMap<>();
+        for (Map.Entry<CompoundName, Object> entry : map.entrySet()) {
+            Object cloneValue = clone(entry.getValue());
+            if (cloneValue == null)
+                cloneValue = entry.getValue(); // Shallow copy objects which does not support cloning
+            cloneMap.put(entry.getKey(), cloneValue);
+        }
+        return cloneMap;
+    }
+
+}
diff --git a/processing/src/main/java/com/yahoo/processing/request/Properties.java b/processing/src/main/java/com/yahoo/processing/request/Properties.java
index 2c603b6c7a0..00be00f9794 100644
--- a/processing/src/main/java/com/yahoo/processing/request/Properties.java
+++ b/processing/src/main/java/com/yahoo/processing/request/Properties.java
@@ -1,9 +1,9 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.processing.request;
 
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
+import java.util.HashMap;
+import java.util.Collections;
 
 /**
  * The properties of a request
@@ -12,6 +12,7 @@ import java.util.Map;
  */
 public class Properties implements Cloneable {
 
+    private final static CloneHelper cloneHelper = new CloneHelper();
     private Properties chained = null;
 
     /**
@@ -570,4 +571,10 @@ public class Properties implements Cloneable {
         }
     }
 
+    /**
+     * Clones a map by deep cloning each value which is cloneable and shallow copying all other values.
+     */
+    public static Map<CompoundName, Object> cloneMap(Map<CompoundName, Object> map) {
+        return cloneHelper.cloneMap(map);
+    }
 }
diff --git a/processing/src/main/java/com/yahoo/processing/request/properties/PropertyMap.java b/processing/src/main/java/com/yahoo/processing/request/properties/PropertyMap.java
index 66e49cff51a..105de5f5347 100644
--- a/processing/src/main/java/com/yahoo/processing/request/properties/PropertyMap.java
+++ b/processing/src/main/java/com/yahoo/processing/request/properties/PropertyMap.java
@@ -1,19 +1,12 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.processing.request.properties;
 
-import com.yahoo.collections.MethodCache;
-import com.yahoo.component.provider.FreezableClass;
 import com.yahoo.processing.request.CompoundName;
 import com.yahoo.processing.request.Properties;
 
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.util.LinkedList;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Arrays;
-import java.util.logging.Logger;
 
 /**
  * A HashMap backing of Properties.
@@ -30,9 +23,6 @@ import java.util.logging.Logger;
  */
 public class PropertyMap extends Properties {
 
-    private static Logger log = Logger.getLogger(PropertyMap.class.getName());
-    private static final MethodCache cloneMethodCache = new MethodCache("clone");
-
     /**
      * The properties of this
      */
@@ -69,74 +59,6 @@ public class PropertyMap extends Properties {
         return clone;
     }
 
-    /**
-     * Clones a map by deep cloning each value which is cloneable and shallow copying all other values.
-     */
-    public static Map<CompoundName, Object> cloneMap(Map<CompoundName, Object> map) {
-        Map<CompoundName, Object> cloneMap = new HashMap<>();
-        for (Map.Entry<CompoundName, Object> entry : map.entrySet()) {
-            Object cloneValue = clone(entry.getValue());
-            if (cloneValue == null)
-                cloneValue = entry.getValue(); // Shallow copy objects which does not support cloning
-            cloneMap.put(entry.getKey(), cloneValue);
-        }
-        return cloneMap;
-    }
-
-    /**
-     * Clones this object if it is clonable, and the clone is public. Returns null if not
-     */
-    public static Object clone(Object object) {
-        if (object == null) return null;
-        if (!(object instanceof Cloneable)) return null;
-        if (object instanceof Object[])
-            return arrayClone((Object[]) object);
-        else
-            return objectClone(object);
-    }
-
-    private static Object arrayClone(Object[] object) {
-        Object[] arrayClone = Arrays.copyOf(object, object.length);
-        // deep clone
-        for (int i = 0; i < arrayClone.length; i++) {
-            Object elementClone = clone(arrayClone[i]);
-            if (elementClone != null)
-                arrayClone[i] = elementClone;
-        }
-        return arrayClone;
-    }
-
-    private static 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) {
-            return ((PublicCloneable)object).clone();
-        }
-        else if (object instanceof LinkedList) { // TODO: Why? Somebody's infatuation with LinkedList knows no limits
-            return ((LinkedList) object).clone();
-        }
-        else if (object instanceof ArrayList) { // TODO: Why? Likewise
-            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");
-                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");
-            return null;
-        } catch (InvocationTargetException e) {
-            throw new RuntimeException("Exception cloning '" + object + "'", e);
-        }
-    }
-
     @Override
     public Map<String, Object> listProperties(CompoundName path, Map<String, String> context, Properties substitution) {
         Map<String, Object> map = super.listProperties(path, context, substitution);