Making CircleBuilder writable and adding equals/hashcode

core/src/main/java/org/elasticsearch/common/geo/builders/CircleBuilder.java

@@ -21,17 +21,23 @@ package org.elasticsearch.common.geo.builders;
 
 import com.spatial4j.core.shape.Circle;
 import com.vividsolutions.jts.geom.Coordinate;
+
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.unit.DistanceUnit.Distance;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
+import java.util.Objects;
 
 public class CircleBuilder extends ShapeBuilder {
 
     public static final String FIELD_RADIUS = "radius";
     public static final GeoShapeType TYPE = GeoShapeType.CIRCLE;
 
+    public static final CircleBuilder PROTOTYPE = new CircleBuilder();
+
     private DistanceUnit unit;
     private double radius;
     private Coordinate center;
@@ -57,6 +63,13 @@ public class CircleBuilder extends ShapeBuilder {
         return center(new Coordinate(lon, lat));
     }
 
+    /**
+     * Get the center of the circle
+     */
+    public Coordinate center() {
+        return center;
+    }
+
     /**
      * Set the radius of the circle. The String value will be parsed by {@link DistanceUnit}
      * @param radius Value and unit of the circle combined in a string
@@ -97,6 +110,20 @@ public class CircleBuilder extends ShapeBuilder {
         return this;
     }
 
+    /**
+     * Get the radius of the circle without unit
+     */
+    public double radius() {
+        return this.radius;
+    }
+
+    /**
+     * Get the radius unit of the circle
+     */
+    public DistanceUnit unit() {
+        return this.unit;
+    }
+
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
@@ -116,4 +143,35 @@ public class CircleBuilder extends ShapeBuilder {
     public GeoShapeType type() {
         return TYPE;
     }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(center, radius, unit.ordinal());
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj == null || getClass() != obj.getClass()) {
+            return false;
+        }
+        CircleBuilder other = (CircleBuilder) obj;
+        return Objects.equals(center, other.center) &&
+                Objects.equals(radius, other.radius) &&
+                Objects.equals(unit.ordinal(), other.unit.ordinal());
+    }
+
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        writeCoordinateTo(center, out);
+        out.writeDouble(radius);
+        DistanceUnit.writeDistanceUnit(out, unit);
+    }
+
+    @Override
+    public ShapeBuilder readFrom(StreamInput in) throws IOException {
+        return new CircleBuilder().center(readCoordinateFrom(in)).radius(in.readDouble(), DistanceUnit.readDistanceUnit(in));
+    }
 }

core/src/main/java/org/elasticsearch/common/geo/builders/PointBuilder.java

@@ -88,12 +88,11 @@ public class PointBuilder extends ShapeBuilder {
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
-        out.writeDouble(coordinate.x);
+        writeCoordinateTo(coordinate, out);
-        out.writeDouble(coordinate.y);
     }
 
     @Override
     public ShapeBuilder readFrom(StreamInput in) throws IOException {
-        return new PointBuilder().coordinate(new Coordinate(in.readDouble(), in.readDouble()));
+        return new PointBuilder().coordinate(readCoordinateFrom(in));
     }
 }

core/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java

@@ -177,6 +177,15 @@ public abstract class ShapeBuilder extends ToXContentToBytes implements NamedWri
         return builder.startArray().value(coordinate.x).value(coordinate.y).endArray();
     }
 
+    protected static void writeCoordinateTo(Coordinate coordinate, StreamOutput out) throws IOException {
+        out.writeDouble(coordinate.x);
+        out.writeDouble(coordinate.y);
+    }
+
+    protected Coordinate readCoordinateFrom(StreamInput in) throws IOException {
+        return new Coordinate(in.readDouble(), in.readDouble());
+    }
+
     public static Orientation orientationFromString(String orientation) {
         orientation = orientation.toLowerCase(Locale.ROOT);
         switch (orientation) {

core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java

@@ -33,7 +33,7 @@ import static org.hamcrest.Matchers.*;
 
 public abstract class AbstractShapeBuilderTestCase<SB extends ShapeBuilder> extends ESTestCase {
 
-    private static final int NUMBER_OF_TESTBUILDERS = 1;
+    private static final int NUMBER_OF_TESTBUILDERS = 20;
     private static final NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry();
 
     /**
@@ -42,6 +42,7 @@ public abstract class AbstractShapeBuilderTestCase<SB extends ShapeBuilder> exte
     @BeforeClass
     public static void init() {
         namedWriteableRegistry.registerPrototype(ShapeBuilder.class, PointBuilder.PROTOTYPE);
+        namedWriteableRegistry.registerPrototype(ShapeBuilder.class, CircleBuilder.PROTOTYPE);
     }
 
     /**
@@ -52,7 +53,7 @@ public abstract class AbstractShapeBuilderTestCase<SB extends ShapeBuilder> exte
     /**
      * mutate the given query so the returned query is different
      */
-    protected abstract SB mutate(SB original);
+    protected abstract SB mutate(SB original) throws IOException;
 
     /**
      * Generic test that creates new shape from a random test shape and checks both for equality
@@ -61,8 +62,10 @@ public abstract class AbstractShapeBuilderTestCase<SB extends ShapeBuilder> exte
         for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
             SB testShape = createTestShapeBuilder();
             XContentBuilder builder = toXContent(testShape, randomFrom(XContentType.values()));
+            builder = toXContent(testShape, randomFrom(XContentType.values()));
 
             XContentParser shapeParser = XContentHelper.createParser(builder.bytes());
+            XContentHelper.createParser(builder.bytes());
             shapeParser.nextToken();
             ShapeBuilder parsedShape = ShapeBuilder.parse(shapeParser);
             assertNotSame(testShape, parsedShape);
@@ -120,7 +123,7 @@ public abstract class AbstractShapeBuilderTestCase<SB extends ShapeBuilder> exte
         }
     }
 
-    private SB copyShape(SB original) throws IOException {
+    protected SB copyShape(SB original) throws IOException {
         try (BytesStreamOutput output = new BytesStreamOutput()) {
             original.writeTo(output);
             try (StreamInput in = new NamedWriteableAwareStreamInput(StreamInput.wrap(output.bytes()), namedWriteableRegistry)) {

core/src/test/java/org/elasticsearch/common/geo/builders/CirlceBuilderTests.java

@@ -0,0 +1,58 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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.elasticsearch.common.geo.builders;
+
+import com.vividsolutions.jts.geom.Coordinate;
+
+import org.elasticsearch.common.unit.DistanceUnit;
+
+import java.io.IOException;
+
+public class CirlceBuilderTests extends AbstractShapeBuilderTestCase<CircleBuilder> {
+
+    @Override
+    protected CircleBuilder createTestShapeBuilder() {
+        double centerX = randomDoubleBetween(-180, 180, false);
+        double centerY = randomDoubleBetween(-90, 90, false);
+        return new CircleBuilder()
+                .center(new Coordinate(centerX, centerY))
+                .radius(randomDoubleBetween(0.1, 10.0, false), randomFrom(DistanceUnit.values()));
+    }
+
+    @Override
+    protected CircleBuilder mutate(CircleBuilder original) throws IOException {
+        CircleBuilder mutation = copyShape(original);
+        double radius = original.radius();
+        DistanceUnit unit = original.unit();
+
+        if (randomBoolean()) {
+            mutation.center(new Coordinate(original.center().x/2, original.center().y/2));
+        } else if (randomBoolean()) {
+            radius = radius/2;
+        } else {
+            DistanceUnit newRandom = unit;
+            while (newRandom == unit) {
+                newRandom = randomFrom(DistanceUnit.values());
+            };
+            unit = newRandom;
+        }
+        return mutation.radius(radius, unit);
+    }
+}

core/src/test/java/org/elasticsearch/common/unit/DistanceUnitTests.java

@@ -57,4 +57,20 @@ public class DistanceUnitTests extends ESTestCase {
             assertThat("Value can be parsed from '" + testValue + unit.toString() + "'", DistanceUnit.Distance.parseDistance(unit.toString(testValue)).value, equalTo(testValue));
         }
     }
+
+    /**
+     * This test ensures that we are aware of accidental reordering in the distance unit ordinals,
+     * since equality in e.g. CircleShapeBuilder, hashCode and serialization rely on them
+     */
+    public void testDistanceUnitNames() {
+        assertEquals(0, DistanceUnit.INCH.ordinal());
+        assertEquals(1, DistanceUnit.YARD.ordinal());
+        assertEquals(2, DistanceUnit.FEET.ordinal());
+        assertEquals(3, DistanceUnit.KILOMETERS.ordinal());
+        assertEquals(4, DistanceUnit.NAUTICALMILES.ordinal());
+        assertEquals(5, DistanceUnit.MILLIMETERS.ordinal());
+        assertEquals(6, DistanceUnit.CENTIMETERS.ordinal());
+        assertEquals(7, DistanceUnit.MILES.ordinal());
+        assertEquals(8, DistanceUnit.METERS.ordinal());
+    }
 }