Add missing cloning

10 files changed, 105 insertions, 85 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/Location.java b/container-search/src/main/java/com/yahoo/prelude/Location.java
index d3c4daab9a0..c686e869536 100644
--- a/container-search/src/main/java/com/yahoo/prelude/Location.java
+++ b/container-search/src/main/java/com/yahoo/prelude/Location.java
@@ -8,11 +8,12 @@ import java.util.StringTokenizer;
 
 /**
  * Location data for a geographical query.
+ * This is mutable and clonable. It's identifty is decided by its content.
  *
  * @author Steinar Knutsen
  * @author arnej27959
  */
-public class Location {
+public class Location implements Cloneable {
 
     // 1 or 2
     private int dimensions = 0;
@@ -34,67 +35,46 @@ public class Location {
 
     private String attribute;
 
-    public boolean equals(Object other) {
-        if (! (other instanceof Location)) return false;
-        Location l = (Location)other;
-        return dimensions == l.dimensions
-            && renderCircle == l.renderCircle
-            && renderRectangle == l.renderRectangle
-            && this.aspect == l.aspect
-            && this.x1 == l.x1
-            && this.x2 == l.x2
-            && this.y1 == l.y1
-            && this.y2 == l.y2
-            && this.x == l.x
-            && this.y == l.y
-            && this.r == l.r;
-    }
-
     public boolean hasDimensions() {
         return dimensions != 0;
     }
+
     public void setDimensions(int d) {
-        if (hasDimensions() && dimensions != d) {
-            throw new IllegalArgumentException("already has dimensions="+dimensions+", cannot change it to "+d);
-        }
-        if (d == 2) {
+        if (hasDimensions() && dimensions != d)
+            throw new IllegalStateException("already has dimensions " + dimensions + ", cannot change to " + d);
+        if (d == 2)
             dimensions = d;
-        } else {
-            throw new IllegalArgumentException("Illegal location, dimensions must be 2, but was: "+d);
-        }
+        else
+            throw new IllegalArgumentException("Illegal location, dimensions must be 2, but was: " + d);
     }
+
     public int getDimensions() {
         return dimensions;
     }
 
     // input data are degrees n/e (if positive) or s/w (if negative)
-    public void setBoundingBox(double n, double s,
-                               double e, double w)
-    {
+    public void setBoundingBox(double n, double s, double e, double w) {
         setDimensions(2);
-        if (hasBoundingBox()) {
-            throw new IllegalArgumentException("can only set bounding box once");
-        }
+        if (hasBoundingBox())
+            throw new IllegalStateException("Can only set bounding box once");
         int px1 = (int) (Math.round(w * 1000000));
         int px2 = (int) (Math.round(e * 1000000));
         int py1 = (int) (Math.round(s * 1000000));
         int py2 = (int) (Math.round(n * 1000000));
-        if (px1 > px2) {
-            throw new IllegalArgumentException("cannot have w > e");
-        }
+        if (px1 > px2)
+            throw new IllegalArgumentException("Cannot have w > e");
         this.x1 = px1;
         this.x2 = px2;
-        if (py1 > py2) {
-            throw new IllegalArgumentException("cannot have s > n");
-        }
+        if (py1 > py2)
+            throw new IllegalArgumentException("Cannot have s > n");
         this.y1 = py1;
         this.y2 = py2;
         renderRectangle = true;
     }
 
     private void adjustAspect() {
-        //calculate aspect based on latitude (elevation angle)
-        //no need to "optimize" for special cases, exactly 0, 30, 45, 60, or 90 degrees won't be input anyway
+        // calculate aspect based on latitude (elevation angle)
+        // no need to "optimize" for special cases, exactly 0, 30, 45, 60, or 90 degrees won't be input anyway
         double degrees = (double) y / 1000000d;
         if (degrees <= -90.0 || degrees >= +90.0) {
             this.aspect = 0;
@@ -107,21 +87,17 @@ public class Location {
 
     public void setGeoCircle(double ns, double ew, double radius_in_degrees) {
         setDimensions(2);
-        if (isGeoCircle()) {
-            throw new IllegalArgumentException("can only set geo circle once");
-        }
+        if (isGeoCircle())
+            throw new IllegalStateException("Can only set geo circle once");
         int px = (int) (ew * 1000000);
         int py = (int) (ns * 1000000);
         int pr = (int) (radius_in_degrees * 1000000);
-        if (ew < -180.1 || ew > +180.1) {
+        if (ew < -180.1 || ew > +180.1)
             throw new IllegalArgumentException("e/w location must be in range [-180,+180]");
-        }
-        if (ns < -90.1 || ns > +90.1) {
+        if (ns < -90.1 || ns > +90.1)
             throw new IllegalArgumentException("n/s location must be in range [-90,+90]");
-        }
-        if (radius_in_degrees < 0) {
+        if (radius_in_degrees < 0)
             pr = -1;
-        }
         this.x = px;
         this.y = py;
         this.r = pr;
@@ -131,12 +107,10 @@ public class Location {
 
     public void setXyCircle(int px, int py, int radius_in_units) {
         setDimensions(2);
-        if (isGeoCircle()) {
-            throw new IllegalArgumentException("can only set geo circle once");
-        }
-        if (radius_in_units < 0) {
+        if (isGeoCircle())
+            throw new IllegalStateException("can only set geo circle once");
+        if (radius_in_units < 0)
             radius_in_units = -1;
-        }
         this.x = px;
         this.y = py;
         this.r = radius_in_units;
@@ -145,9 +119,8 @@ public class Location {
 
     private void parseRectangle(String rectangle) {
         int endof = rectangle.indexOf(']');
-        if (endof == -1) {
-            throw new IllegalArgumentException("Illegal location syntax: "+rectangle);
-        }
+        if (endof == -1)
+            throw new IllegalArgumentException("Illegal location syntax: " + rectangle);
         String rectPart = rectangle.substring(1,endof);
         StringTokenizer tokens = new StringTokenizer(rectPart, ",");
         setDimensions(Integer.parseInt(tokens.nextToken()));
@@ -155,21 +128,18 @@ public class Location {
         this.y1 = Integer.parseInt(tokens.nextToken());
         this.x2 = Integer.parseInt(tokens.nextToken());
         this.y2 = Integer.parseInt(tokens.nextToken());
-        if (tokens.hasMoreTokens()) {
-            throw new IllegalArgumentException("Illegal location syntax: "+rectangle);
-        }
+        if (tokens.hasMoreTokens())
+            throw new IllegalArgumentException("Illegal location syntax: " + rectangle);
         renderRectangle = true;
         String theRest = rectangle.substring(endof+1).trim();
-        if (theRest.length() >= 15 && theRest.charAt(0) == '(') {
+        if (theRest.length() >= 15 && theRest.charAt(0) == '(')
             parseCircle(theRest);
-        }
     }
 
     private void parseCircle(String circle) {
         int endof = circle.indexOf(')');
-        if (endof == -1) {
-            throw new IllegalArgumentException("Illegal location syntax: "+circle);
-        }
+        if (endof == -1)
+            throw new IllegalArgumentException("Illegal location syntax: " + circle);
         String circlePart = circle.substring(1,endof);
         StringTokenizer tokens = new StringTokenizer(circlePart, ",");
         setDimensions(Integer.parseInt(tokens.nextToken()));
@@ -187,18 +157,18 @@ public class Location {
                 try {
                     aspect = Long.parseLong(aspectToken);
                 } catch (NumberFormatException nfe) {
-                    throw new IllegalArgumentException("Aspect "+aspectToken+" for location must be an integer or 'CalcLatLon' for automatic aspect calculation.", nfe);
-                }
-                if (aspect > 4294967295L || aspect < 0) {
-                    throw new IllegalArgumentException("Aspect "+aspect+" for location parameter must be less than 4294967296 (2^32)");
+                    throw new IllegalArgumentException("Aspect "+aspectToken+" for location must be an integer or " +
+                                                       "'CalcLatLon' for automatic aspect calculation.", nfe);
                 }
+                if (aspect > 4294967295L || aspect < 0)
+                    throw new IllegalArgumentException("Aspect " + aspect + " for location parameter must be " +
+                                                       "less than 4294967296 (2^32)");
             }
         }
         renderCircle = true;
         String theRest = circle.substring(endof+1).trim();
-        if (theRest.length() > 5 && theRest.charAt(0) == '[') {
+        if (theRest.length() > 5 && theRest.charAt(0) == '[')
             parseRectangle(theRest);
-        }
     }
 
     public Location() {}
@@ -225,9 +195,11 @@ public class Location {
         }
     }
 
+    @Override
     public String toString() {
         return render(false);
     }
+
     public String backendString() {
         return render(true);
     }
@@ -284,6 +256,35 @@ public class Location {
         }
     }
 
+    @Override
+    public Location clone() {
+        try {
+            return (Location) super.clone();
+        }
+        catch (CloneNotSupportedException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    @Override
+    public boolean equals(Object other) {
+        if (other == this) return true;
+        if (! (other instanceof Location)) return false;
+        Location l = (Location)other;
+        return dimensions == l.dimensions
+               && renderCircle == l.renderCircle
+               && renderRectangle == l.renderRectangle
+               && this.aspect == l.aspect
+               && this.x1 == l.x1
+               && this.x2 == l.x2
+               && this.y1 == l.y1
+               && this.y2 == l.y2
+               && this.x == l.x
+               && this.y == l.y
+               && this.r == l.r;
+    }
+
+    @Override
     public int hashCode() {
         return toString().hashCode();
     }
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/AndItem.java b/container-search/src/main/java/com/yahoo/prelude/query/AndItem.java
index d5fbb193cfc..ef571794383 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/AndItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/AndItem.java
@@ -9,16 +9,14 @@ package com.yahoo.prelude.query;
  */
 public class AndItem extends CompositeItem {
 
+    @Override
     public ItemType getItemType() {
         return ItemType.AND;
     }
 
+    @Override
     public String getName() {
         return "AND";
     }
 
-    public boolean equals(Object p) {
-        return super.equals(p);
-    }
-
 }
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/BlockItem.java b/container-search/src/main/java/com/yahoo/prelude/query/BlockItem.java
index c3962c97356..bde5fd2c2f9 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/BlockItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/BlockItem.java
@@ -9,10 +9,7 @@ package com.yahoo.prelude.query;
  */
 public interface BlockItem extends HasIndexItem {
 
-    /**
-     * The untransformed raw text from the user serving as base for
-     * this item.
-     */
+    /** The untransformed raw text from the user serving as base for this item. */
     String getRawWord();
 
     /** Returns the substring which is the origin of this item, or null if none */
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/GeoLocationItem.java b/container-search/src/main/java/com/yahoo/prelude/query/GeoLocationItem.java
index a4aacd9d6a5..712dbbfc489 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/GeoLocationItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/GeoLocationItem.java
@@ -8,11 +8,12 @@ import java.nio.ByteBuffer;
 /**
  * This represents a geo-location in the query tree.
  * Used for closeness(fieldname) and distance(fieldname) rank features.
+ *
  * @author arnej
  */
 public class GeoLocationItem extends TermItem {
 
-    private final Location location;
+    private Location location;
 
     /**
      * Construct from a Location, which must be geo circle with an attribute set.
@@ -89,6 +90,13 @@ public class GeoLocationItem extends TermItem {
     }
 
     @Override
+    public GeoLocationItem clone() {
+        var clone = (GeoLocationItem)super.clone();
+        clone.location = this.location.clone();
+        return clone;
+    }
+
+    @Override
     public String getIndexedString() {
         return location.toString();
     }
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/Item.java b/container-search/src/main/java/com/yahoo/prelude/query/Item.java
index e8b1580848d..47efed323a9 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/Item.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/Item.java
@@ -17,7 +17,9 @@ import java.util.Optional;
 /**
  * An item in the tree which defines which documents will match a query.
  * Item subclasses can be composed freely to create arbitrary complex matching trees.
- * Items are in general mutable and not thread safe. Their identity is defined by their content
+ * Items are in general mutable and not thread safe.
+ * They can be deeply cloned by calling clone().
+ * Their identity is defined by their content
  * (i.e the field value of two items decide if they are equal).
  *
  * @author bratseth
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/NonReducibleCompositeItem.java b/container-search/src/main/java/com/yahoo/prelude/query/NonReducibleCompositeItem.java
index 7483863e459..d9c903de374 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/NonReducibleCompositeItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/NonReducibleCompositeItem.java
@@ -6,9 +6,7 @@ import java.util.Optional;
 /**
  * A composite item which specifies semantics which are not maintained
  * if an instance with a single child is replaced by the single child.
- * <p>
  * Most composites, like AND and OR, are reducible as e.g (AND a) is semantically equal to (a).
- * <p>
  * This type functions as a marker type for query rewriters.
  *
  * @author bratseth
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/PredicateQueryItem.java b/container-search/src/main/java/com/yahoo/prelude/query/PredicateQueryItem.java
index e32c817ceaf..5b15dd1556f 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/PredicateQueryItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/PredicateQueryItem.java
@@ -187,6 +187,7 @@ public class PredicateQueryItem extends SimpleTaggableItem {
         return Objects.hash(super.hashCode(), fieldName, features, rangeFeatures);
     }
 
+    /** An entry in a predicate item. This is immutable. */
     public abstract static class EntryBase {
 
         private final String key;
@@ -205,10 +206,6 @@ public class PredicateQueryItem extends SimpleTaggableItem {
             return subQueryBitmap;
         }
 
-        public void setSubQueryBitmap(long subQueryBitmap) {
-            this.subQueryBitmap = subQueryBitmap;
-        }
-
         public abstract void encode(ByteBuffer buffer);
 
         @Override
@@ -228,6 +225,7 @@ public class PredicateQueryItem extends SimpleTaggableItem {
 
     }
 
+    /** A unique entry in a predicate item. This is immutable. */
     public static class Entry extends EntryBase {
 
         private final String value;
@@ -265,6 +263,7 @@ public class PredicateQueryItem extends SimpleTaggableItem {
 
     }
 
+    /** A range entry in a predicate item. This is immutable. */
     public static class RangeEntry extends EntryBase {
 
         private final long value;
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java b/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java
index 907055d435b..84a176c2f7d 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java
@@ -38,6 +38,7 @@ public class WeightedSetItem extends SimpleTaggableItem {
         }
         set = new CopyOnWriteHashMap<>(1000);
     }
+
     public WeightedSetItem(String indexName, Map<Object, Integer> map) {
         if (indexName == null) {
             this.indexName = "";
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/WordAlternativesItem.java b/container-search/src/main/java/com/yahoo/prelude/query/WordAlternativesItem.java
index 1e9a27f237d..59dad29ab5c 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/WordAlternativesItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/WordAlternativesItem.java
@@ -141,6 +141,13 @@ public class WordAlternativesItem extends TermItem {
     }
 
     @Override
+    public WordAlternativesItem clone() {
+        var clone = (WordAlternativesItem)super.clone();
+        clone.alternatives = new ArrayList(this.alternatives);
+        return clone;
+    }
+
+    @Override
     public boolean equals(Object other) {
         if ( ! super.equals(other)) return false;
         return this.alternatives.equals(((WordAlternativesItem)other).alternatives);
diff --git a/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java
index 526f520a583..d9e3cf84726 100644
--- a/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/test/QueryTestCase.java
@@ -8,6 +8,7 @@ import com.yahoo.prelude.Index;
 import com.yahoo.prelude.IndexFacts;
 import com.yahoo.prelude.IndexModel;
 import com.yahoo.prelude.SearchDefinition;
+import com.yahoo.prelude.query.BoolItem;
 import com.yahoo.prelude.query.WordItem;
 import com.yahoo.search.Query;
 import com.yahoo.search.query.Sorting;
@@ -32,6 +33,14 @@ import static org.junit.Assert.*;
 public class QueryTestCase {
 
     @Test
+    public void testBoolItem() {
+        var original = new BoolItem(false);
+        var cloned = original.clone();
+        assertNotSame(original, cloned);
+        assertEquals(original, cloned);
+    }
+
+    @Test
     public void testSimpleQueryParsing () {
         Query q = newQuery("/search?query=foobar&offset=10&hits=20");
         assertEquals("foobar",((WordItem) q.getModel().getQueryTree().getRoot()).getWord());