From 7e840532c1f7211ebc631ba60d4b4e2ccc73b45b Mon Sep 17 00:00:00 2001
From: javanna <cavannaluca@gmail.com>
Date: Tue, 6 Oct 2015 13:17:42 +0200
Subject: [PATCH] Make strategy optional in GeoShapeQueryBuilder readFrom and
 writeTo

The field is optional everywhere else but in the serialization methods, which causes problems. Also expanded tests so that they can catch this type of problem.

Closes #13963
---
 .../index/query/GeoShapeQueryBuilder.java     | 13 +++++++++---
 .../query/GeoShapeQueryBuilderTests.java      | 20 +++++++++----------
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java
index 31bc889cb91..72c226f8b00 100644
--- a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java
+++ b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java
@@ -70,7 +70,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
     // and Equals so ShapeBuilder can be used here
     private BytesReference shapeBytes;
 
-    private SpatialStrategy strategy = null;
+    private SpatialStrategy strategy;
 
     private final String indexedShapeId;
     private final String indexedShapeType;
@@ -429,7 +429,9 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
             }
         }
         builder.relation = ShapeRelation.DISJOINT.readFrom(in);
-        builder.strategy = SpatialStrategy.RECURSIVE.readFrom(in);
+        if (in.readBoolean()) {
+            builder.strategy = SpatialStrategy.RECURSIVE.readFrom(in);
+        }
         return builder;
     }
 
@@ -447,7 +449,12 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
             out.writeOptionalString(indexedShapePath);
         }
         relation.writeTo(out);
-        strategy.writeTo(out);
+        if (strategy == null) {
+            out.writeBoolean(false);
+        } else {
+            out.writeBoolean(true);
+            strategy.writeTo(out);
+        }
     }
 
     @Override
diff --git a/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java
index a4ac66c658c..0a2034d98d9 100644
--- a/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java
+++ b/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java
@@ -77,10 +77,12 @@ public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<GeoShapeQue
                 builder.indexedShapePath(indexedShapePath);
             }
         }
-        SpatialStrategy strategy = randomFrom(SpatialStrategy.values());
-        builder.strategy(strategy);
-        if (strategy != SpatialStrategy.TERM) {
-            builder.relation(randomFrom(ShapeRelation.values()));
+        if (randomBoolean()) {
+            SpatialStrategy strategy = randomFrom(SpatialStrategy.values());
+            builder.strategy(strategy);
+            if (strategy != SpatialStrategy.TERM) {
+                builder.relation(randomFrom(ShapeRelation.values()));
+            }
         }
         return builder;
     }
@@ -105,9 +107,7 @@ public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<GeoShapeQue
         } catch (IOException ex) {
             throw new ElasticsearchException("boom", ex);
         }
-        GetResponse response = new GetResponse(new GetResult(indexedShapeIndex, indexedShapeType, indexedShapeId, 0, true, new BytesArray(
-                json), null));
-        return response;
+        return new GetResponse(new GetResult(indexedShapeIndex, indexedShapeType, indexedShapeId, 0, true, new BytesArray(json), null));
     }
 
     @After
@@ -149,7 +149,7 @@ public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<GeoShapeQue
     @Test
     public void testNoShape() throws IOException {
         try {
-            GeoShapeQueryBuilder builder = new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, (ShapeBuilder) null);
+            new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, (ShapeBuilder) null);
             fail("exception expected");
         } catch (IllegalArgumentException e) {
             // expected
@@ -158,12 +158,12 @@ public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<GeoShapeQue
 
     @Test(expected = IllegalArgumentException.class)
     public void testNoIndexedShape() throws IOException {
-        new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, (String) null, "type");
+        new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, null, "type");
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void testNoIndexedShapeType() throws IOException {
-        new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, "id", (String) null);
+        new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, "id", null);
     }
 
     @Test(expected=IllegalArgumentException.class)