Merge pull request #13963 from javanna/fix/geo_shape_strategy_optional

Make strategy optional in GeoShapeQueryBuilder readFrom and writeTo

Author: javanna

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 @@ protected GeoShapeQueryBuilder doReadFrom(StreamInput in) throws IOException {
             }
         }
         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 @@ protected void doWriteTo(StreamOutput out) throws IOException {
             out.writeOptionalString(indexedShapePath);
         }
         relation.writeTo(out);
-        strategy.writeTo(out);
+        if (strategy == null) {
+            out.writeBoolean(false);
+        } else {
+            out.writeBoolean(true);
+            strategy.writeTo(out);
+        }
     }
 
     @Override

core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java

@@ -77,10 +77,12 @@ protected GeoShapeQueryBuilder doCreateTestQueryBuilder() {
                 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 @@ protected GetResponse executeGet(GetRequest getRequest) {
         } 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 void testNoFieldName() throws Exception {
     @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 void testNoShape() throws IOException {
 
     @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)