From cc40f20544020c67c2fe3cb1d26d73af708392ac Mon Sep 17 00:00:00 2001
From: Simon Willnauer <simonw@apache.org>
Date: Thu, 8 Oct 2015 12:33:14 +0200
Subject: [PATCH] Remove unnecessary Writeable implementation from GeoPoint

GeoPoint now has native support in StreamOutput/StreamInput
impementing Writable is not necessary. This also adds tests
for XContentBuilder rendering GeoPoint
---
 .../elasticsearch/common/geo/GeoPoint.java    | 28 +------------------
 .../query/GeoBoundingBoxQueryBuilder.java     |  8 +++---
 .../index/query/GeoDistanceQueryBuilder.java  |  4 +--
 .../query/GeoDistanceRangeQueryBuilder.java   |  4 +--
 .../index/query/GeoPolygonQueryBuilder.java   |  4 +--
 .../builder/XContentBuilderTests.java         | 13 +++++++++
 6 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/common/geo/GeoPoint.java b/core/src/main/java/org/elasticsearch/common/geo/GeoPoint.java
index c50b85a835c..0bc9455bea3 100644
--- a/core/src/main/java/org/elasticsearch/common/geo/GeoPoint.java
+++ b/core/src/main/java/org/elasticsearch/common/geo/GeoPoint.java
@@ -19,12 +19,6 @@
 
 package org.elasticsearch.common.geo;
 
-import org.elasticsearch.common.io.stream.StreamInput;
-import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.io.stream.Writeable;
-
-import java.io.IOException;
-
 import org.apache.lucene.util.BitUtil;
 import org.apache.lucene.util.XGeoHashUtils;
 import org.apache.lucene.util.XGeoUtils;
@@ -32,15 +26,12 @@ import org.apache.lucene.util.XGeoUtils;
 /**
  *
  */
-public final class GeoPoint implements Writeable<GeoPoint> {
+public final class GeoPoint {
 
     private double lat;
     private double lon;
     private final static double TOLERANCE = XGeoUtils.TOLERANCE;
     
-    // for serialization purposes
-    private static final GeoPoint PROTOTYPE = new GeoPoint(Double.NaN, Double.NaN);
-
     public GeoPoint() {
     }
 
@@ -179,21 +170,4 @@ public final class GeoPoint implements Writeable<GeoPoint> {
     public static GeoPoint fromIndexLong(long indexLong) {
         return new GeoPoint().resetFromIndexHash(indexLong);
     }
-
-    @Override
-    public GeoPoint readFrom(StreamInput in) throws IOException {
-        double lat = in.readDouble();
-        double lon = in.readDouble();
-        return new GeoPoint(lat, lon);
-    }
-
-    public static GeoPoint readGeoPointFrom(StreamInput in) throws IOException {
-        return PROTOTYPE.readFrom(in);
-    }
-
-    @Override
-    public void writeTo(StreamOutput out) throws IOException {
-        out.writeDouble(lat);
-        out.writeDouble(lon);
-    }
 }
diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java
index 4ad63ebc1fc..59d20cef611 100644
--- a/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java
+++ b/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java
@@ -314,8 +314,8 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
     public GeoBoundingBoxQueryBuilder doReadFrom(StreamInput in) throws IOException {
         String fieldName = in.readString();
         GeoBoundingBoxQueryBuilder geo = new GeoBoundingBoxQueryBuilder(fieldName);
-        geo.topLeft = geo.topLeft.readFrom(in);
-        geo.bottomRight = geo.bottomRight.readFrom(in);
+        geo.topLeft = in.readGeoPoint();
+        geo.bottomRight = in.readGeoPoint();
         geo.type = GeoExecType.readTypeFrom(in);
         geo.validationMethod = GeoValidationMethod.readGeoValidationMethodFrom(in);
         return geo;
@@ -324,8 +324,8 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
     @Override
     public void doWriteTo(StreamOutput out) throws IOException {
         out.writeString(fieldName);
-        topLeft.writeTo(out);
-        bottomRight.writeTo(out);
+        out.writeGeoPoint(topLeft);
+        out.writeGeoPoint(bottomRight);
         type.writeTo(out);
         validationMethod.writeTo(out);
     }
diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryBuilder.java
index 7d6066eb968..056027230d2 100644
--- a/core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryBuilder.java
+++ b/core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryBuilder.java
@@ -261,7 +261,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
         GeoDistanceQueryBuilder result = new GeoDistanceQueryBuilder(fieldName);
         result.distance = in.readDouble();
         result.validationMethod = GeoValidationMethod.readGeoValidationMethodFrom(in);
-        result.center = GeoPoint.readGeoPointFrom(in);
+        result.center = in.readGeoPoint();
         result.optimizeBbox = in.readString();
         result.geoDistance = GeoDistance.readGeoDistanceFrom(in);
         return result;
@@ -272,7 +272,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
         out.writeString(fieldName);
         out.writeDouble(distance);
         validationMethod.writeTo(out);
-        center.writeTo(out);
+        out.writeGeoPoint(center);
         out.writeString(optimizeBbox);
         geoDistance.writeTo(out);
     }
diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeQueryBuilder.java
index a265427b312..ee3d8bcf75d 100644
--- a/core/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeQueryBuilder.java
+++ b/core/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeQueryBuilder.java
@@ -277,7 +277,7 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
 
     @Override
     protected GeoDistanceRangeQueryBuilder doReadFrom(StreamInput in) throws IOException {
-        GeoDistanceRangeQueryBuilder queryBuilder = new GeoDistanceRangeQueryBuilder(in.readString(), GeoPoint.readGeoPointFrom(in));
+        GeoDistanceRangeQueryBuilder queryBuilder = new GeoDistanceRangeQueryBuilder(in.readString(), in.readGeoPoint());
         queryBuilder.from = in.readGenericValue();
         queryBuilder.to = in.readGenericValue();
         queryBuilder.includeLower = in.readBoolean();
@@ -292,7 +292,7 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
     @Override
     protected void doWriteTo(StreamOutput out) throws IOException {
         out.writeString(fieldName);
-        point.writeTo(out);
+        out.writeGeoPoint(point);
         out.writeGenericValue(from);
         out.writeGenericValue(to);
         out.writeBoolean(includeLower);
diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java
index e832be5123e..192fd867d0c 100644
--- a/core/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java
+++ b/core/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java
@@ -164,7 +164,7 @@ public class GeoPolygonQueryBuilder extends AbstractQueryBuilder<GeoPolygonQuery
         List<GeoPoint> shell = new ArrayList<>();
         int size = in.readVInt();
         for (int i = 0; i < size; i++) {
-            shell.add(GeoPoint.readGeoPointFrom(in));
+            shell.add(in.readGeoPoint());
         }
         GeoPolygonQueryBuilder builder = new GeoPolygonQueryBuilder(fieldName, shell);
         builder.validationMethod = GeoValidationMethod.readGeoValidationMethodFrom(in);
@@ -176,7 +176,7 @@ public class GeoPolygonQueryBuilder extends AbstractQueryBuilder<GeoPolygonQuery
         out.writeString(fieldName);
         out.writeVInt(shell.size());
         for (GeoPoint point : shell) {
-            point.writeTo(out);
+            out.writeGeoPoint(point);
         }
         validationMethod.writeTo(out);
     }
diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/builder/XContentBuilderTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/builder/XContentBuilderTests.java
index 23cf83b0d1c..4ce1a7c4630 100644
--- a/core/src/test/java/org/elasticsearch/common/xcontent/builder/XContentBuilderTests.java
+++ b/core/src/test/java/org/elasticsearch/common/xcontent/builder/XContentBuilderTests.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.common.xcontent.builder;
 
 import org.elasticsearch.common.bytes.BytesArray;
+import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.io.FastCharArrayWriter;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
@@ -351,4 +352,16 @@ public class XContentBuilderTests extends ESTestCase {
                 "  foobar: \"boom\"\n", string);
     }
 
+    public void testRenderGeoPoint() throws IOException {
+        XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON).prettyPrint();
+        builder.startObject().field("foo").value(new GeoPoint(1,2)).endObject();
+        String string = builder.string();
+        assertEquals("{\n" +
+                "  \"foo\" : {\n" +
+                "    \"lat\" : 1.0,\n" +
+                "    \"lon\" : 2.0\n" +
+                "  }\n" +
+                "}", string.trim());
+    }
+
 }