Making EnvelopeBuilder writable and adding equals/hashCode

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

@@ -171,7 +171,9 @@ public class CircleBuilder extends ShapeBuilder {
     }
 
     @Override
-    public ShapeBuilder readFrom(StreamInput in) throws IOException {
+    public CircleBuilder readFrom(StreamInput in) throws IOException {
-        return new CircleBuilder().center(readCoordinateFrom(in)).radius(in.readDouble(), DistanceUnit.readDistanceUnit(in));
+        return new CircleBuilder()
+                    .center(readCoordinateFrom(in))
+                    .radius(in.readDouble(), DistanceUnit.readDistanceUnit(in));
     }
 }

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

@@ -21,13 +21,19 @@ package org.elasticsearch.common.geo.builders;
 
 import com.spatial4j.core.shape.Rectangle;
 import com.vividsolutions.jts.geom.Coordinate;
+
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
+import java.util.Locale;
+import java.util.Objects;
 
 public class EnvelopeBuilder extends ShapeBuilder {
 
     public static final GeoShapeType TYPE = GeoShapeType.ENVELOPE;
+    public static final EnvelopeBuilder PROTOTYPE = new EnvelopeBuilder();
 
     protected Coordinate topLeft;
     protected Coordinate bottomRight;
@@ -62,6 +68,7 @@ public class EnvelopeBuilder extends ShapeBuilder {
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
         builder.field(FIELD_TYPE, TYPE.shapeName());
+        builder.field(FIELD_ORIENTATION, orientation.name().toLowerCase(Locale.ROOT));
         builder.startArray(FIELD_COORDINATES);
         toXContent(builder, topLeft);
         toXContent(builder, bottomRight);
@@ -78,4 +85,38 @@ public class EnvelopeBuilder extends ShapeBuilder {
     public GeoShapeType type() {
         return TYPE;
     }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(orientation, topLeft, bottomRight);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj == null || getClass() != obj.getClass()) {
+            return false;
+        }
+        EnvelopeBuilder other = (EnvelopeBuilder) obj;
+        return Objects.equals(orientation, other.orientation) &&
+                Objects.equals(topLeft, other.topLeft) &&
+                Objects.equals(bottomRight, other.bottomRight);
+    }
+
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        out.writeBoolean(orientation == Orientation.RIGHT);
+        writeCoordinateTo(topLeft, out);
+        writeCoordinateTo(bottomRight, out);
+    }
+
+    @Override
+    public EnvelopeBuilder readFrom(StreamInput in) throws IOException {
+        Orientation orientation = in.readBoolean() ? Orientation.RIGHT : Orientation.LEFT;
+        return new EnvelopeBuilder(orientation)
+                .topLeft(readCoordinateFrom(in))
+                .bottomRight(readCoordinateFrom(in));
+    }
 }

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

@@ -92,7 +92,7 @@ public class PointBuilder extends ShapeBuilder {
     }
 
     @Override
-    public ShapeBuilder readFrom(StreamInput in) throws IOException {
+    public PointBuilder readFrom(StreamInput in) throws IOException {
         return new PointBuilder().coordinate(readCoordinateFrom(in));
     }
 }

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

@@ -843,13 +843,15 @@ public abstract class ShapeBuilder extends ToXContentToBytes implements NamedWri
 
     @Override
     public String getWriteableName() {
-        return type().shapename;
+        return type().shapeName();
     }
 
+    // NORELEASE this should be deleted as soon as all shape builders implement writable
     @Override
     public void writeTo(StreamOutput out) throws IOException {
     }
 
+    // NORELEASE this should be deleted as soon as all shape builders implement writable
     @Override
     public ShapeBuilder readFrom(StreamInput in) throws IOException {
         return null;

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

@@ -25,6 +25,7 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.xcontent.*;
 import org.elasticsearch.test.ESTestCase;
+import org.junit.AfterClass;
 import org.junit.BeforeClass;
 
 import java.io.IOException;
@@ -34,36 +35,47 @@ import static org.hamcrest.Matchers.*;
 public abstract class AbstractShapeBuilderTestCase<SB extends ShapeBuilder> extends ESTestCase {
 
     private static final int NUMBER_OF_TESTBUILDERS = 20;
-    private static final NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry();
+    private static NamedWriteableRegistry namedWriteableRegistry;
 
     /**
-     * Setup for the whole base test class.
+     * setup for the whole base test class
      */
     @BeforeClass
     public static void init() {
+        if (namedWriteableRegistry == null) {
+            namedWriteableRegistry = new NamedWriteableRegistry();
             namedWriteableRegistry.registerPrototype(ShapeBuilder.class, PointBuilder.PROTOTYPE);
             namedWriteableRegistry.registerPrototype(ShapeBuilder.class, CircleBuilder.PROTOTYPE);
+            namedWriteableRegistry.registerPrototype(ShapeBuilder.class, EnvelopeBuilder.PROTOTYPE);
+        }
+    }
+
+    @AfterClass
+    public static void afterClass() throws Exception {
+        namedWriteableRegistry = null;
     }
 
     /**
-     * Create the shape that under test
+     * create random shape that is put under test
      */
     protected abstract SB createTestShapeBuilder();
 
     /**
-     * mutate the given query so the returned query is different
+     * mutate the given shape so the returned shape is different
      */
     protected abstract SB mutate(SB original) throws IOException;
 
     /**
-     * Generic test that creates new shape from a random test shape and checks both for equality
+     * Test that creates new shape from a random test shape and checks both for equality
      */
     public void testFromXContent() throws IOException {
         for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
             SB testShape = createTestShapeBuilder();
-            XContentBuilder builder = toXContent(testShape, randomFrom(XContentType.values()));
+            XContentBuilder contentBuilder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));
-            builder = toXContent(testShape, randomFrom(XContentType.values()));
+            if (randomBoolean()) {
-
+                contentBuilder.prettyPrint();
+            }
+            XContentBuilder builder = testShape.toXContent(contentBuilder, ToXContent.EMPTY_PARAMS);
             XContentParser shapeParser = XContentHelper.createParser(builder.bytes());
             XContentHelper.createParser(builder.bytes());
             shapeParser.nextToken();
@@ -74,14 +86,6 @@ public abstract class AbstractShapeBuilderTestCase<SB extends ShapeBuilder> exte
         }
     }
 
-    protected static XContentBuilder toXContent(ShapeBuilder shape, XContentType contentType) throws IOException {
-        XContentBuilder builder = XContentFactory.contentBuilder(contentType);
-        if (randomBoolean()) {
-            builder.prettyPrint();
-        }
-        return shape.toXContent(builder, ToXContent.EMPTY_PARAMS);
-    }
-
     /**
      * Test serialization and deserialization of the test shape.
      */
@@ -95,6 +99,9 @@ public abstract class AbstractShapeBuilderTestCase<SB extends ShapeBuilder> exte
         }
     }
 
+    /**
+     * Test equality and hashCode properties
+     */
     public void testEqualsAndHashcode() throws IOException {
         for (int runs = 0; runs < NUMBER_OF_TESTBUILDERS; runs++) {
             SB firstShape = createTestShapeBuilder();

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

@@ -0,0 +1,66 @@
+/*
+ * 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.spatial4j.core.shape.Rectangle;
+import com.vividsolutions.jts.geom.Coordinate;
+
+import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
+import org.elasticsearch.test.geo.RandomShapeGenerator;
+
+import java.io.IOException;
+
+public class EnvelopeBuilderTests extends AbstractShapeBuilderTestCase<EnvelopeBuilder> {
+
+    @Override
+    protected EnvelopeBuilder createTestShapeBuilder() {
+        EnvelopeBuilder envelope = new EnvelopeBuilder(randomFrom(Orientation.values()));
+        Rectangle box = RandomShapeGenerator.xRandomRectangle(getRandom(), RandomShapeGenerator.xRandomPoint(getRandom()));
+        envelope.topLeft(box.getMinX(), box.getMaxY())
+                .bottomRight(box.getMaxX(), box.getMinY());
+        return envelope;
+    }
+
+    @Override
+    protected EnvelopeBuilder mutate(EnvelopeBuilder original) throws IOException {
+        EnvelopeBuilder mutation = copyShape(original);
+        if (randomBoolean()) {
+            // toggle orientation
+            mutation.orientation = (original.orientation == Orientation.LEFT ? Orientation.RIGHT : Orientation.LEFT);
+        } else {
+            // move one corner to the middle of original
+            switch (randomIntBetween(0, 3)) {
+            case 0:
+                mutation.topLeft(new Coordinate(randomDoubleBetween(-180.0, original.bottomRight.x, true), original.topLeft.y));
+                break;
+            case 1:
+                mutation.topLeft(new Coordinate(original.topLeft.x, randomDoubleBetween(original.bottomRight.y, 90.0, true)));
+                break;
+            case 2:
+                mutation.bottomRight(new Coordinate(randomDoubleBetween(original.topLeft.x, 180.0, true), original.bottomRight.y));
+                break;
+            case 3:
+                mutation.bottomRight(new Coordinate(original.bottomRight.x, randomDoubleBetween(-90.0, original.topLeft.y, true)));
+                break;
+            }
+        }
+        return mutation;
+    }
+}