瀏覽代碼

Remove legacy geo code from AbstractGeometryQueryBuilder classes (#74741)

removes references to Legacy ShapeParser and ShapeBuilder in AbstractGeometryQueryBuilder classes 
in favour to Geometry and GeometryParser.
Ignacio Vera 4 年之前
父節點
當前提交
0f30c79e4b
共有 16 個文件被更改,包括 178 次插入172 次删除
  1. 6 3
      modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java
  2. 1 17
      server/src/main/java/org/elasticsearch/index/query/AbstractGeometryQueryBuilder.java
  3. 8 19
      server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java
  4. 1 1
      server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java
  5. 5 6
      server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderGeoPointTests.java
  6. 14 13
      server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderGeoShapeTests.java
  7. 29 34
      server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java
  8. 2 3
      server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java
  9. 4 5
      server/src/test/java/org/elasticsearch/index/query/ScriptQueryBuilderTests.java
  10. 8 5
      server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java
  11. 17 1
      test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java
  12. 21 20
      test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java
  13. 37 26
      test/framework/src/test/java/org/elasticsearch/test/AbstractQueryTestCaseTests.java
  14. 9 19
      x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/query/ShapeQueryBuilder.java
  15. 8 0
      x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/query/GeoShapeWithDocValuesQueryBuilderTests.java
  16. 8 0
      x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/query/ShapeQueryBuilderTests.java

+ 6 - 3
modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java

@@ -43,6 +43,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -188,10 +189,12 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase<PercolateQ
     }
 
     @Override
-    protected Set<String> getObjectsHoldingArbitraryContent() {
+    protected Map<String, String> getObjectsHoldingArbitraryContent() {
         //document contains arbitrary content, no error expected when an object is added to it
-        return new HashSet<>(Arrays.asList(PercolateQueryBuilder.DOCUMENT_FIELD.getPreferredName(),
-                PercolateQueryBuilder.DOCUMENTS_FIELD.getPreferredName()));
+        final Map<String, String> objects = new HashMap<>();
+        objects.put(PercolateQueryBuilder.DOCUMENT_FIELD.getPreferredName(), null);
+        objects.put(PercolateQueryBuilder.DOCUMENTS_FIELD.getPreferredName(), null);
+        return objects;
     }
 
     public void testRequiredParameters() {

+ 1 - 17
server/src/main/java/org/elasticsearch/index/query/AbstractGeometryQueryBuilder.java

@@ -22,7 +22,6 @@ import org.elasticsearch.common.geo.GeoJson;
 import org.elasticsearch.common.geo.GeometryIO;
 import org.elasticsearch.common.geo.GeometryParser;
 import org.elasticsearch.common.geo.ShapeRelation;
-import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
@@ -78,21 +77,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
 
     protected boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED;
 
-    /**
-     * Creates a new ShapeQueryBuilder whose Query will be against the given
-     * field name using the given Shape
-     *
-     * @param fieldName
-     *            Name of the field that will be queried
-     * @param shape
-     *            Shape used in the Query
-     * @deprecated use {@link #AbstractGeometryQueryBuilder(String, Geometry)} instead
-     */
-    @Deprecated
-    protected AbstractGeometryQueryBuilder(String fieldName, ShapeBuilder shape) {
-        this(fieldName, shape == null ? null : shape.buildGeometry(), null);
-    }
-
     /**
      * Creates a new AbstractGeometryQueryBuilder whose Query will be against the given
      * field name using the given Shape
@@ -484,7 +468,7 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
     protected abstract static class ParsedGeometryQueryParams {
         public String fieldName;
         public ShapeRelation relation;
-        public ShapeBuilder shape;
+        public Geometry shape;
 
         public String id = null;
         public String index = null;

+ 8 - 19
server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java

@@ -10,12 +10,11 @@ package org.elasticsearch.index.query;
 
 import org.apache.lucene.search.ConstantScoreQuery;
 import org.apache.lucene.search.Query;
+import org.elasticsearch.common.geo.GeometryParser;
 import org.elasticsearch.common.xcontent.ParseField;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.geo.SpatialStrategy;
-import org.elasticsearch.common.geo.builders.ShapeBuilder;
-import org.elasticsearch.common.geo.parsers.ShapeParser;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -25,6 +24,7 @@ import org.elasticsearch.index.mapper.GeoShapeQueryable;
 import org.elasticsearch.index.mapper.MappedFieldType;
 
 import java.io.IOException;
+import java.text.ParseException;
 import java.util.Objects;
 import java.util.function.Supplier;
 
@@ -53,22 +53,6 @@ public class GeoShapeQueryBuilder extends AbstractGeometryQueryBuilder<GeoShapeQ
         super(fieldName, shape);
     }
 
-    /**
-     * Creates a new GeoShapeQueryBuilder whose Query will be against the given
-     * field name using the given Shape
-     *
-     * @param fieldName
-     *            Name of the field that will be queried
-     * @param shape
-     *            Shape used in the Query
-     *
-     * @deprecated use {@link #GeoShapeQueryBuilder(String, Geometry)} instead
-     */
-    @Deprecated
-    public GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape) {
-        super(fieldName, shape);
-    }
-
     /**
      * Creates a new GeoShapeQueryBuilder whose Query will be against the given
      * field name and will use the Shape found with the given shape id and supplier
@@ -199,12 +183,17 @@ public class GeoShapeQueryBuilder extends AbstractGeometryQueryBuilder<GeoShapeQ
 
     private static class ParsedGeoShapeQueryParams extends ParsedGeometryQueryParams {
         SpatialStrategy strategy;
+        private final GeometryParser geometryParser = new GeometryParser(true, true, true);
 
         @Override
         protected boolean parseXContentField(XContentParser parser) throws IOException {
             SpatialStrategy strategy;
             if (SHAPE_FIELD.match(parser.currentName(), parser.getDeprecationHandler())) {
-                this.shape = ShapeParser.parse(parser);
+                try {
+                    this.shape = geometryParser.parse(parser);
+                } catch (ParseException e) {
+                    throw new IOException(e);
+                }
                 return true;
             } else if (STRATEGY_FIELD.match(parser.currentName(), parser.getDeprecationHandler())) {
                 String strategyName = parser.text();

+ 1 - 1
server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java

@@ -645,7 +645,7 @@ public final class QueryBuilders {
      */
     @Deprecated
     public static GeoShapeQueryBuilder geoShapeQuery(String name, ShapeBuilder shape) throws IOException {
-        return new GeoShapeQueryBuilder(name, shape);
+        return new GeoShapeQueryBuilder(name, shape.buildGeometry());
     }
 
     public static GeoShapeQueryBuilder geoShapeQuery(String name, String indexedShapeId) {

+ 5 - 6
server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderGeoPointTests.java

@@ -8,8 +8,8 @@
 package org.elasticsearch.index.query;
 
 import org.elasticsearch.common.geo.ShapeRelation;
-import org.elasticsearch.common.geo.builders.ShapeBuilder;
-import org.elasticsearch.test.geo.RandomShapeGenerator;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geometry.Geometry;
 
 public class GeoShapeQueryBuilderGeoPointTests extends GeoShapeQueryBuilderTests {
 
@@ -18,14 +18,13 @@ public class GeoShapeQueryBuilderGeoPointTests extends GeoShapeQueryBuilderTests
     }
 
     protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) {
-        RandomShapeGenerator.ShapeType shapeType = RandomShapeGenerator.ShapeType.POLYGON;
-        ShapeBuilder<?, ?, ?> shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType);
+        Geometry geometry = GeometryTestUtils.randomPolygon(false);
         GeoShapeQueryBuilder builder;
         clearShapeFields();
         if (indexedShape == false) {
-            builder = new GeoShapeQueryBuilder(fieldName(), shape);
+            builder = new GeoShapeQueryBuilder(fieldName(), geometry);
         } else {
-            indexedShapeToReturn = shape;
+            indexedShapeToReturn = geometry;
             indexedShapeId = randomAlphaOfLengthBetween(3, 20);
             builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId);
             if (randomBoolean()) {

+ 14 - 13
server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderGeoShapeTests.java

@@ -9,8 +9,9 @@ package org.elasticsearch.index.query;
 
 import org.elasticsearch.Version;
 import org.elasticsearch.common.geo.ShapeRelation;
-import org.elasticsearch.common.geo.builders.ShapeBuilder;
-import org.elasticsearch.test.geo.RandomShapeGenerator;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geometry.Geometry;
+import org.elasticsearch.geometry.ShapeType;
 
 public class GeoShapeQueryBuilderGeoShapeTests extends GeoShapeQueryBuilderTests {
 
@@ -19,19 +20,19 @@ public class GeoShapeQueryBuilderGeoShapeTests extends GeoShapeQueryBuilderTests
     }
 
     protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) {
-        RandomShapeGenerator.ShapeType shapeType = randomFrom(
-            RandomShapeGenerator.ShapeType.POINT,
-            RandomShapeGenerator.ShapeType.MULTIPOINT,
-            RandomShapeGenerator.ShapeType.LINESTRING,
-            RandomShapeGenerator.ShapeType.MULTILINESTRING,
-            RandomShapeGenerator.ShapeType.POLYGON);
-        ShapeBuilder<?, ?, ?> shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType);
+        ShapeType shapeType = randomFrom(
+            ShapeType.POINT,
+            ShapeType.MULTIPOINT,
+            ShapeType.LINESTRING,
+            ShapeType.MULTILINESTRING,
+            ShapeType.POLYGON);
+        Geometry geometry = GeometryTestUtils.randomGeometry(shapeType, false);
         GeoShapeQueryBuilder builder;
         clearShapeFields();
         if (indexedShape == false) {
-            builder = new GeoShapeQueryBuilder(fieldName(), shape);
+            builder = new GeoShapeQueryBuilder(fieldName(), geometry);
         } else {
-            indexedShapeToReturn = shape;
+            indexedShapeToReturn = geometry;
             indexedShapeId = randomAlphaOfLengthBetween(3, 20);
             builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId);
             if (randomBoolean()) {
@@ -50,14 +51,14 @@ public class GeoShapeQueryBuilderGeoShapeTests extends GeoShapeQueryBuilderTests
         if (randomBoolean()) {
             SearchExecutionContext context = createSearchExecutionContext();
             if (context.indexVersionCreated().onOrAfter(Version.V_7_5_0)) { // CONTAINS is only supported from version 7.5
-                if (shapeType == RandomShapeGenerator.ShapeType.LINESTRING || shapeType == RandomShapeGenerator.ShapeType.MULTILINESTRING) {
+                if (shapeType == ShapeType.LINESTRING || shapeType == ShapeType.MULTILINESTRING) {
                     builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS, ShapeRelation.CONTAINS));
                 } else {
                     builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS,
                         ShapeRelation.WITHIN, ShapeRelation.CONTAINS));
                 }
             } else {
-                if (shapeType == RandomShapeGenerator.ShapeType.LINESTRING || shapeType == RandomShapeGenerator.ShapeType.MULTILINESTRING) {
+                if (shapeType == ShapeType.LINESTRING || shapeType == ShapeType.MULTILINESTRING) {
                     builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS));
                 } else {
                     builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS, ShapeRelation.WITHIN));

+ 29 - 34
server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java

@@ -17,20 +17,22 @@ import org.elasticsearch.action.get.GetResponse;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.geo.builders.EnvelopeBuilder;
-import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geometry.Geometry;
+import org.elasticsearch.geometry.utils.WellKnownText;
 import org.elasticsearch.index.get.GetResult;
 import org.elasticsearch.test.AbstractQueryTestCase;
-import org.elasticsearch.test.geo.RandomShapeGenerator;
-import org.elasticsearch.test.geo.RandomShapeGenerator.ShapeType;
 import org.junit.After;
 import org.locationtech.jts.geom.Coordinate;
 
 import java.io.IOException;
+import java.util.Collections;
+import java.util.Map;
 
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.notNullValue;
@@ -44,7 +46,7 @@ public abstract class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<Ge
     protected static String indexedShapePath;
     protected static String indexedShapeIndex;
     protected static String indexedShapeRouting;
-    protected static ShapeBuilder<?, ?, ?> indexedShapeToReturn;
+    protected static Geometry indexedShapeToReturn;
 
     protected abstract String fieldName();
 
@@ -70,7 +72,7 @@ public abstract class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<Ge
         try {
             XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint();
             builder.startObject();
-            builder.field(expectedShapePath, indexedShapeToReturn);
+            builder.field(expectedShapePath, WellKnownText.toWKT(indexedShapeToReturn));
             builder.field(randomAlphaOfLengthBetween(10, 20), "something");
             builder.endObject();
             json = Strings.toString(builder);
@@ -100,23 +102,23 @@ public abstract class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<Ge
     }
 
     public void testNoFieldName() throws Exception {
-        ShapeBuilder<?, ?, ?> shape = RandomShapeGenerator.createShapeWithin(random(), null);
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new GeoShapeQueryBuilder(null, shape));
+        Geometry geometry = GeometryTestUtils.randomGeometry(false);
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new GeoShapeQueryBuilder(null, geometry));
         assertEquals("fieldName is required", e.getMessage());
     }
 
-    public void testNoShape() throws IOException {
-        expectThrows(IllegalArgumentException.class, () -> new GeoShapeQueryBuilder(fieldName(), (ShapeBuilder) null));
+    public void testNoShape() {
+        expectThrows(IllegalArgumentException.class, () -> new GeoShapeQueryBuilder(fieldName(), (Geometry) null));
     }
 
-    public void testNoIndexedShape() throws IOException {
+    public void testNoIndexedShape() {
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
             () -> new GeoShapeQueryBuilder(fieldName(), null, null));
         assertEquals("either shape or indexedShapeId is required", e.getMessage());
     }
 
-    public void testNoRelation() throws IOException {
-        ShapeBuilder<?, ?, ?> shape = RandomShapeGenerator.createShapeWithin(random(), null);
+    public void testNoRelation() {
+        Geometry shape = GeometryTestUtils.randomGeometry(false);
         GeoShapeQueryBuilder builder = new GeoShapeQueryBuilder(fieldName(), shape);
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> builder.relation(null));
         assertEquals("No Shape Relation defined", e.getMessage());
@@ -159,25 +161,20 @@ public abstract class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<Ge
             () -> query.toQuery(createSearchExecutionContext()));
         assertEquals("query must be rewritten first", e.getMessage());
         QueryBuilder rewrite = rewriteAndFetch(query, createSearchExecutionContext());
-        GeoShapeQueryBuilder geoShapeQueryBuilder = randomBoolean() ?
-            new GeoShapeQueryBuilder(fieldName(), indexedShapeToReturn) :
-            new GeoShapeQueryBuilder(fieldName(), indexedShapeToReturn.buildGeometry());
+        GeoShapeQueryBuilder geoShapeQueryBuilder = new GeoShapeQueryBuilder(fieldName(), indexedShapeToReturn);
         geoShapeQueryBuilder.strategy(query.strategy());
         geoShapeQueryBuilder.relation(query.relation());
         assertEquals(geoShapeQueryBuilder, rewrite);
     }
 
-    public void testMultipleRewrite() throws IOException {
+    public void testMultipleRewrite() {
         GeoShapeQueryBuilder shape = doCreateTestQueryBuilder(true);
         QueryBuilder builder = new BoolQueryBuilder()
             .should(shape)
             .should(shape);
 
         builder = rewriteAndFetch(builder, createSearchExecutionContext());
-
-        GeoShapeQueryBuilder expectedShape = randomBoolean() ?
-            new GeoShapeQueryBuilder(fieldName(), indexedShapeToReturn) :
-            new GeoShapeQueryBuilder(fieldName(), indexedShapeToReturn.buildGeometry());
+        GeoShapeQueryBuilder expectedShape = new GeoShapeQueryBuilder(fieldName(), indexedShapeToReturn);
         expectedShape.strategy(shape.strategy());
         expectedShape.relation(shape.relation());
         QueryBuilder expected = new BoolQueryBuilder()
@@ -187,30 +184,22 @@ public abstract class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<Ge
     }
 
     public void testIgnoreUnmapped() throws IOException {
-        ShapeType shapeType = ShapeType.randomType(random());
-        ShapeBuilder<?, ?, ?> shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType);
-        final GeoShapeQueryBuilder queryBuilder = randomBoolean() ?
-            new GeoShapeQueryBuilder("unmapped", shape) :
-            new GeoShapeQueryBuilder("unmapped", shape.buildGeometry());
+        Geometry geometry = GeometryTestUtils.randomGeometry(false);
+        final GeoShapeQueryBuilder queryBuilder = new GeoShapeQueryBuilder("unmapped", geometry);
         queryBuilder.ignoreUnmapped(true);
         Query query = queryBuilder.toQuery(createSearchExecutionContext());
         assertThat(query, notNullValue());
         assertThat(query, instanceOf(MatchNoDocsQuery.class));
 
-        final GeoShapeQueryBuilder failingQueryBuilder = randomBoolean() ?
-            new GeoShapeQueryBuilder("unmapped", shape) :
-            new GeoShapeQueryBuilder("unmapped", shape.buildGeometry());
+        final GeoShapeQueryBuilder failingQueryBuilder = new GeoShapeQueryBuilder("unmapped", geometry);
         failingQueryBuilder.ignoreUnmapped(false);
         QueryShardException e = expectThrows(QueryShardException.class, () -> failingQueryBuilder.toQuery(createSearchExecutionContext()));
         assertThat(e.getMessage(), containsString("failed to find type for field [unmapped]"));
     }
 
-    public void testWrongFieldType() throws IOException {
-        ShapeType shapeType = ShapeType.randomType(random());
-        ShapeBuilder<?, ?, ?> shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType);
-        final GeoShapeQueryBuilder queryBuilder = randomBoolean() ?
-            new GeoShapeQueryBuilder(TEXT_FIELD_NAME, shape) :
-            new GeoShapeQueryBuilder(TEXT_FIELD_NAME, shape.buildGeometry());
+    public void testWrongFieldType() {
+        Geometry geometry = GeometryTestUtils.randomGeometry(false);
+        final GeoShapeQueryBuilder queryBuilder = new GeoShapeQueryBuilder(TEXT_FIELD_NAME, geometry);
         QueryShardException e = expectThrows(QueryShardException.class, () -> queryBuilder.toQuery(createSearchExecutionContext()));
         assertThat(e.getMessage(), containsString("Field [mapped_string] is of unsupported type [text] for [geo_shape] query"));
     }
@@ -230,4 +219,10 @@ public abstract class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<Ge
         assertThat(query, instanceOf(GeoShapeQueryBuilder.class));
         return query;
     }
+
+    @Override
+    protected Map<String, String> getObjectsHoldingArbitraryContent() {
+        // shape field can accept any element but expects a type
+       return Collections.singletonMap("shape", "Required [type]");
+    }
 }

+ 2 - 3
server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java

@@ -45,7 +45,6 @@ import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Set;
 import java.util.stream.Stream;
 
 import static org.elasticsearch.index.query.QueryBuilders.moreLikeThisQuery;
@@ -204,9 +203,9 @@ public class MoreLikeThisQueryBuilderTests extends AbstractQueryTestCase<MoreLik
     }
 
     @Override
-    protected Set<String> getObjectsHoldingArbitraryContent() {
+    protected Map<String, String> getObjectsHoldingArbitraryContent() {
         //doc contains arbitrary content, anything can be added to it and no exception will be thrown
-        return Collections.singleton(MoreLikeThisQueryBuilder.DOC.getPreferredName());
+        return Collections.singletonMap(MoreLikeThisQueryBuilder.DOC.getPreferredName(), null);
     }
 
     @Override

+ 4 - 5
server/src/test/java/org/elasticsearch/index/query/ScriptQueryBuilderTests.java

@@ -19,7 +19,6 @@ import org.elasticsearch.test.AbstractQueryTestCase;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.Map;
-import java.util.Set;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.instanceOf;
@@ -103,10 +102,10 @@ public class ScriptQueryBuilderTests extends AbstractQueryTestCase<ScriptQueryBu
     }
 
     @Override
-    protected Set<String> getObjectsHoldingArbitraryContent() {
-        //script_score.script.params can contain arbitrary parameters. no error is expected when
-        //adding additional objects within the params object.
-        return Collections.singleton(Script.PARAMS_PARSE_FIELD.getPreferredName());
+    protected Map<String, String> getObjectsHoldingArbitraryContent() {
+        // script_score.script.params can contain arbitrary parameters. no error is expected when
+        // adding additional objects within the params object.
+        return Collections.singletonMap(Script.PARAMS_PARSE_FIELD.getPreferredName(), null);
     }
 
     /**

+ 8 - 5
server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java

@@ -61,10 +61,9 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import static java.util.Collections.singletonList;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -116,11 +115,15 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
     }
 
     @Override
-    protected Set<String> getObjectsHoldingArbitraryContent() {
+    protected Map<String, String> getObjectsHoldingArbitraryContent() {
         //script_score.script.params can contain arbitrary parameters. no error is expected when adding additional objects
         //within the params object. Score functions get parsed in the data nodes, so they are not validated in the coord node.
-        return new HashSet<>(Arrays.asList(Script.PARAMS_PARSE_FIELD.getPreferredName(), ExponentialDecayFunctionBuilder.NAME,
-                LinearDecayFunctionBuilder.NAME, GaussDecayFunctionBuilder.NAME));
+        final Map<String, String> objects = new HashMap<>();
+        objects.put(Script.PARAMS_PARSE_FIELD.getPreferredName(), null);
+        objects.put(ExponentialDecayFunctionBuilder.NAME, null);
+        objects.put(LinearDecayFunctionBuilder.NAME, null);
+        objects.put(GaussDecayFunctionBuilder.NAME, null);
+        return objects;
     }
 
     /**

+ 17 - 1
test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java

@@ -21,6 +21,7 @@ import org.elasticsearch.geometry.MultiPolygon;
 import org.elasticsearch.geometry.Point;
 import org.elasticsearch.geometry.Polygon;
 import org.elasticsearch.geometry.Rectangle;
+import org.elasticsearch.geometry.ShapeType;
 import org.elasticsearch.test.ESTestCase;
 
 import java.util.ArrayList;
@@ -187,6 +188,21 @@ public class GeometryTestUtils {
         return new GeometryCollection<>(shapes);
     }
 
+    public static Geometry randomGeometry(ShapeType type, boolean hasAlt) {
+       switch (type) {
+           case GEOMETRYCOLLECTION: return randomGeometryCollection(0, hasAlt);
+           case MULTILINESTRING: return randomMultiLine(hasAlt);
+           case ENVELOPE: return randomRectangle();
+           case LINESTRING: return randomLine(hasAlt);
+           case POLYGON: return randomPolygon(hasAlt);
+           case MULTIPOLYGON: return randomMultiPolygon(hasAlt);
+           case CIRCLE: return randomCircle(hasAlt);
+           case MULTIPOINT: return randomMultiPoint(hasAlt);
+           case POINT: return randomPoint(hasAlt);
+           default: throw new IllegalArgumentException("Ussuported shape type [" + type + "]");
+       }
+    }
+
     public static Geometry randomGeometry(boolean hasAlt) {
         return randomGeometry(0, hasAlt);
     }
@@ -216,7 +232,7 @@ public class GeometryTestUtils {
             GeometryTestUtils::randomMultiPolygon,
             hasAlt ? GeometryTestUtils::randomPoint : (b) -> randomRectangle(),
             level < 3 ? (b) ->
-                randomGeometryCollectionWithoutCircle(level + 1, hasAlt) : GeometryTestUtils::randomPoint // don't build too deep
+                randomGeometryWithoutCircleCollection(level + 1, hasAlt) : GeometryTestUtils::randomPoint // don't build too deep
         );
         return geometry.apply(hasAlt);
     }

+ 21 - 20
test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java

@@ -160,25 +160,25 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
         // Adds the alternates versions of the query too
         candidates.addAll(getAlternateVersions().keySet());
 
-        List<Tuple<String, Boolean>> testQueries = alterateQueries(candidates, getObjectsHoldingArbitraryContent());
-        for (Tuple<String, Boolean> testQuery : testQueries) {
-            boolean expectedException = testQuery.v2();
+        List<Tuple<String, String>> testQueries = alterateQueries(candidates, getObjectsHoldingArbitraryContent());
+        for (Tuple<String, String> testQuery : testQueries) {
+            String expectedException = testQuery.v2();
             try {
                 parseQuery(testQuery.v1());
-                if (expectedException) {
+                if (expectedException != null) {
                     fail("some parsing exception expected for query: " + testQuery);
                 }
             } catch (ParsingException | ElasticsearchParseException | XContentParseException e) {
                 // different kinds of exception wordings depending on location
                 // of mutation, so no simple asserts possible here
-                if (expectedException == false) {
+                if (expectedException == null) {
                     throw new AssertionError("unexpected exception when parsing query:\n" + testQuery, e);
                 }
             } catch (IllegalArgumentException e) {
-                if (expectedException == false) {
+                if (expectedException == null) {
                     throw new AssertionError("unexpected exception when parsing query:\n" + testQuery, e);
                 }
-                assertThat(e.getMessage(), containsString("unknown field [newField]"));
+                assertThat(e.getMessage(), containsString(expectedException));
             }
         }
     }
@@ -221,8 +221,8 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
      * for the mutation. Some specific objects do not cause any exception as they can hold arbitrary content; they are passed using the
      * arbitraryMarkers parameter.
      */
-    static List<Tuple<String, Boolean>> alterateQueries(Set<String> queries, Set<String> arbitraryMarkers) throws IOException {
-        List<Tuple<String, Boolean>> results = new ArrayList<>();
+    static List<Tuple<String, String>> alterateQueries(Set<String> queries, Map<String, String> arbitraryMarkers) throws IOException {
+        List<Tuple<String, String>> results = new ArrayList<>();
 
         // Indicate if a part of the query can hold any arbitrary content
         boolean hasArbitraryContent = (arbitraryMarkers != null && arbitraryMarkers.isEmpty() == false);
@@ -232,7 +232,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
             int mutation = 0;
 
             while (true) {
-                boolean expectException = true;
+                String exception = "unknown field [newField]";
 
                 BytesStreamOutput out = new BytesStreamOutput();
                 try (
@@ -262,9 +262,9 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
                                 if (hasArbitraryContent) {
                                     // The query has one or more fields that hold arbitrary content. If the current
                                     // field is one (or a child) of those, no exception is expected when parsing the mutated query.
-                                    for (String marker : arbitraryMarkers) {
+                                    for (String marker : arbitraryMarkers.keySet()) {
                                         if (levels.contains(marker)) {
-                                            expectException = false;
+                                            exception = arbitraryMarkers.get(marker);
                                             break;
                                         }
                                     }
@@ -290,21 +290,22 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
                     }
                 }
 
-                results.add(new Tuple<>(out.bytes().utf8ToString(), expectException));
+                results.add(new Tuple<>(out.bytes().utf8ToString(), exception));
             }
         }
         return results;
     }
 
     /**
-     * Returns a set of object names that won't trigger any exception (uncluding their children) when testing that unknown
-     * objects cause parse exceptions through {@link #testUnknownObjectException()}. Default is an empty set. Can be overridden
-     * by subclasses that test queries which contain objects that get parsed on the data nodes (e.g. score functions) or objects
-     * that can contain arbitrary content (e.g. documents for percolate or more like this query, params for scripts). In such
-     * cases no exception would get thrown.
+     * Returns a map where the keys are object names that won't trigger a standard exception (an exception that contains the string
+     * "unknown field [newField]") through {@link #testUnknownObjectException()}. The value is a string that is contained in the thrown
+     * exception or null in the case that no exception is thrown (including their children).
+     * Default is an empty Map. Can be overridden by subclasses that test queries which contain objects that get parsed on the data nodes
+     * (e.g. score functions) or objects that can contain arbitrary content (e.g. documents for percolate or more like this query, params
+     * for scripts) and/or expect some content(e.g documents with geojson geometries).
      */
-    protected Set<String> getObjectsHoldingArbitraryContent() {
-        return Collections.emptySet();
+    protected Map<String, String> getObjectsHoldingArbitraryContent() {
+        return Collections.emptyMap();
     }
 
     /**

+ 37 - 26
test/framework/src/test/java/org/elasticsearch/test/AbstractQueryTestCaseTests.java

@@ -13,10 +13,10 @@ import org.elasticsearch.common.util.set.Sets;
 import org.hamcrest.Matcher;
 
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 import static java.util.Collections.singleton;
 import static org.elasticsearch.test.AbstractQueryTestCase.alterateQueries;
@@ -29,65 +29,76 @@ import static org.hamcrest.Matchers.notNullValue;
  */
 public class AbstractQueryTestCaseTests extends ESTestCase {
 
+    private final String STANDARD_ERROR = "unknown field [newField]";
+
     public void testAlterateQueries() throws IOException {
-        List<Tuple<String, Boolean>> alterations = alterateQueries(singleton("{\"field\": \"value\"}"), null);
-        assertAlterations(alterations, allOf(notNullValue(), hasEntry("{\"newField\":{\"field\":\"value\"}}", true)));
+        List<Tuple<String, String>> alterations = alterateQueries(singleton("{\"field\": \"value\"}"), null);
+        assertAlterations(alterations, allOf(notNullValue(), hasEntry("{\"newField\":{\"field\":\"value\"}}", STANDARD_ERROR)));
 
         alterations = alterateQueries(singleton("{\"term\":{\"field\": \"value\"}}"), null);
         assertAlterations(alterations, allOf(
-            hasEntry("{\"newField\":{\"term\":{\"field\":\"value\"}}}", true),
-            hasEntry("{\"term\":{\"newField\":{\"field\":\"value\"}}}", true))
+            hasEntry("{\"newField\":{\"term\":{\"field\":\"value\"}}}", STANDARD_ERROR),
+            hasEntry("{\"term\":{\"newField\":{\"field\":\"value\"}}}", STANDARD_ERROR))
         );
 
         alterations = alterateQueries(singleton("{\"bool\":{\"must\": [{\"match\":{\"field\":\"value\"}}]}}"), null);
         assertAlterations(alterations, allOf(
-                hasEntry("{\"newField\":{\"bool\":{\"must\":[{\"match\":{\"field\":\"value\"}}]}}}", true),
-                hasEntry("{\"bool\":{\"newField\":{\"must\":[{\"match\":{\"field\":\"value\"}}]}}}", true),
-                hasEntry("{\"bool\":{\"must\":[{\"newField\":{\"match\":{\"field\":\"value\"}}}]}}", true),
-                hasEntry("{\"bool\":{\"must\":[{\"match\":{\"newField\":{\"field\":\"value\"}}}]}}", true)
+                hasEntry("{\"newField\":{\"bool\":{\"must\":[{\"match\":{\"field\":\"value\"}}]}}}", STANDARD_ERROR),
+                hasEntry("{\"bool\":{\"newField\":{\"must\":[{\"match\":{\"field\":\"value\"}}]}}}", STANDARD_ERROR),
+                hasEntry("{\"bool\":{\"must\":[{\"newField\":{\"match\":{\"field\":\"value\"}}}]}}", STANDARD_ERROR),
+                hasEntry("{\"bool\":{\"must\":[{\"match\":{\"newField\":{\"field\":\"value\"}}}]}}", STANDARD_ERROR)
         ));
 
         alterations = alterateQueries(singleton("{\"function_score\":" +
                 "{\"query\": {\"term\":{\"foo\": \"bar\"}}, \"script_score\": {\"script\":\"a + 1\", \"params\": {\"a\":0}}}}"), null);
         assertAlterations(alterations, allOf(
                 hasEntry("{\"newField\":{\"function_score\":{\"query\":{\"term\":{\"foo\":\"bar\"}},\"script_score\":{\"script\":\"a + " +
-                        "1\",\"params\":{\"a\":0}}}}}", true),
+                        "1\",\"params\":{\"a\":0}}}}}", STANDARD_ERROR),
                 hasEntry("{\"function_score\":{\"newField\":{\"query\":{\"term\":{\"foo\":\"bar\"}},\"script_score\":{\"script\":\"a + " +
-                        "1\",\"params\":{\"a\":0}}}}}", true),
+                        "1\",\"params\":{\"a\":0}}}}}", STANDARD_ERROR),
                 hasEntry("{\"function_score\":{\"query\":{\"newField\":{\"term\":{\"foo\":\"bar\"}}},\"script_score\":{\"script\":\"a + " +
-                        "1\",\"params\":{\"a\":0}}}}", true),
+                        "1\",\"params\":{\"a\":0}}}}", STANDARD_ERROR),
                 hasEntry("{\"function_score\":{\"query\":{\"term\":{\"newField\":{\"foo\":\"bar\"}}},\"script_score\":{\"script\":\"a + " +
-                        "1\",\"params\":{\"a\":0}}}}", true),
+                        "1\",\"params\":{\"a\":0}}}}", STANDARD_ERROR),
                 hasEntry("{\"function_score\":{\"query\":{\"term\":{\"foo\":\"bar\"}},\"script_score\":{\"newField\":{\"script\":\"a + " +
-                        "1\",\"params\":{\"a\":0}}}}}", true),
+                        "1\",\"params\":{\"a\":0}}}}}", STANDARD_ERROR),
                 hasEntry("{\"function_score\":{\"query\":{\"term\":{\"foo\":\"bar\"}},\"script_score\":{\"script\":\"a + 1\"," +
-                        "\"params\":{\"newField\":{\"a\":0}}}}}", true)
+                        "\"params\":{\"newField\":{\"a\":0}}}}}", STANDARD_ERROR)
         ));
     }
 
     public void testAlterateQueriesWithArbitraryContent() throws IOException {
-        Set<String> arbitraryContentHolders = Sets.newHashSet("params", "doc");
+        Map<String, String> arbitraryContentHolders = new HashMap<>();
+        arbitraryContentHolders.put("params", null); // no exception expected
+        arbitraryContentHolders.put("doc", "my own error");
         Set<String> queries = Sets.newHashSet(
                 "{\"query\":{\"script\":\"test\",\"params\":{\"foo\":\"bar\"}}}",
                 "{\"query\":{\"more_like_this\":{\"fields\":[\"a\",\"b\"],\"like\":{\"doc\":{\"c\":\"d\"}}}}}"
         );
 
-        List<Tuple<String, Boolean>> alterations = alterateQueries(queries, arbitraryContentHolders);
+        List<Tuple<String, String>> alterations = alterateQueries(queries, arbitraryContentHolders);
         assertAlterations(alterations, allOf(
-            hasEntry("{\"newField\":{\"query\":{\"script\":\"test\",\"params\":{\"foo\":\"bar\"}}}}", true),
-            hasEntry("{\"query\":{\"newField\":{\"script\":\"test\",\"params\":{\"foo\":\"bar\"}}}}", true),
-            hasEntry("{\"query\":{\"script\":\"test\",\"params\":{\"newField\":{\"foo\":\"bar\"}}}}", false)
+            hasEntry("{\"newField\":{\"query\":{\"script\":\"test\",\"params\":{\"foo\":\"bar\"}}}}", STANDARD_ERROR),
+            hasEntry("{\"query\":{\"newField\":{\"script\":\"test\",\"params\":{\"foo\":\"bar\"}}}}", STANDARD_ERROR),
+            hasEntry("{\"query\":{\"script\":\"test\",\"params\":{\"newField\":{\"foo\":\"bar\"}}}}", null)
         ));
         assertAlterations(alterations, allOf(
-            hasEntry("{\"newField\":{\"query\":{\"more_like_this\":{\"fields\":[\"a\",\"b\"],\"like\":{\"doc\":{\"c\":\"d\"}}}}}}", true),
-            hasEntry("{\"query\":{\"newField\":{\"more_like_this\":{\"fields\":[\"a\",\"b\"],\"like\":{\"doc\":{\"c\":\"d\"}}}}}}", true),
-            hasEntry("{\"query\":{\"more_like_this\":{\"newField\":{\"fields\":[\"a\",\"b\"],\"like\":{\"doc\":{\"c\":\"d\"}}}}}}", true),
-            hasEntry("{\"query\":{\"more_like_this\":{\"fields\":[\"a\",\"b\"],\"like\":{\"newField\":{\"doc\":{\"c\":\"d\"}}}}}}", true),
-            hasEntry("{\"query\":{\"more_like_this\":{\"fields\":[\"a\",\"b\"],\"like\":{\"doc\":{\"newField\":{\"c\":\"d\"}}}}}}", false)
+            hasEntry("{\"newField\":{\"query\":{\"more_like_this\":{\"fields\":[\"a\",\"b\"],\"like\":{\"doc\":{\"c\":\"d\"}}}}}}",
+                STANDARD_ERROR),
+            hasEntry("{\"query\":{\"newField\":{\"more_like_this\":{\"fields\":[\"a\",\"b\"],\"like\":{\"doc\":{\"c\":\"d\"}}}}}}",
+                STANDARD_ERROR),
+            hasEntry("{\"query\":{\"more_like_this\":{\"newField\":{\"fields\":[\"a\",\"b\"],\"like\":{\"doc\":{\"c\":\"d\"}}}}}}",
+                STANDARD_ERROR),
+            hasEntry("{\"query\":{\"more_like_this\":{\"fields\":[\"a\",\"b\"],\"like\":{\"newField\":{\"doc\":{\"c\":\"d\"}}}}}}",
+                STANDARD_ERROR),
+            hasEntry("{\"query\":{\"more_like_this\":{\"fields\":[\"a\",\"b\"],\"like\":{\"doc\":{\"newField\":{\"c\":\"d\"}}}}}}",
+                "my own error")
         ));
     }
 
     private static <K, V> void assertAlterations(List<Tuple<K, V>> alterations, Matcher<Map<K, V>> matcher) {
-        assertThat(alterations.stream().collect(Collectors.toMap(Tuple::v1, Tuple::v2)), matcher);
+        Map<K, V> alterationsMap = new HashMap<>();
+        alterations.forEach(t -> alterationsMap.put(t.v1(), t.v2()));
+        assertThat(alterationsMap, matcher);
     }
 }

+ 9 - 19
x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/query/ShapeQueryBuilder.java

@@ -8,8 +8,7 @@ package org.elasticsearch.xpack.spatial.index.query;
 
 import org.apache.lucene.search.ConstantScoreQuery;
 import org.apache.lucene.search.Query;
-import org.elasticsearch.common.geo.builders.ShapeBuilder;
-import org.elasticsearch.common.geo.parsers.ShapeParser;
+import org.elasticsearch.common.geo.GeometryParser;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -23,6 +22,7 @@ import org.elasticsearch.index.query.QueryShardException;
 import org.elasticsearch.xpack.spatial.index.mapper.ShapeQueryable;
 
 import java.io.IOException;
+import java.text.ParseException;
 import java.util.Objects;
 import java.util.function.Supplier;
 
@@ -35,22 +35,6 @@ import java.util.function.Supplier;
 public class ShapeQueryBuilder extends AbstractGeometryQueryBuilder<ShapeQueryBuilder> {
     public static final String NAME = "shape";
 
-    /**
-     * Creates a new ShapeQueryBuilder whose Query will be against the given
-     * field name using the given Shape
-     *
-     * @param fieldName
-     *            Name of the field that will be queried
-     * @param shape
-     *            Shape used in the Query
-     * @deprecated use {@link #ShapeQueryBuilder(String, Geometry)} instead
-     */
-    @Deprecated
-    @SuppressWarnings({ "rawtypes" })
-    protected ShapeQueryBuilder(String fieldName, ShapeBuilder shape) {
-        super(fieldName, shape);
-    }
-
     /**
      * Creates a new ShapeQueryBuilder whose Query will be against the given
      * field name using the given Shape
@@ -137,10 +121,16 @@ public class ShapeQueryBuilder extends AbstractGeometryQueryBuilder<ShapeQueryBu
     }
 
     private static class ParsedShapeQueryParams extends ParsedGeometryQueryParams {
+        private final GeometryParser geometryParser = new GeometryParser(true, true, true);
+
         @Override
         protected boolean parseXContentField(XContentParser parser) throws IOException {
             if (SHAPE_FIELD.match(parser.currentName(), parser.getDeprecationHandler())) {
-                this.shape = ShapeParser.parse(parser);
+                try {
+                    this.shape = geometryParser.parse(parser);
+                } catch (ParseException e) {
+                    throw new IOException(e);
+                }
                 return true;
             }
             return false;

+ 8 - 0
x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/query/GeoShapeWithDocValuesQueryBuilderTests.java

@@ -26,6 +26,8 @@ import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
 
 import static org.hamcrest.Matchers.equalTo;
 
@@ -64,4 +66,10 @@ public class GeoShapeWithDocValuesQueryBuilderTests extends AbstractQueryTestCas
         boolean IndexOrDocValuesQuery = fieldType.hasDocValues();
         assertThat(IndexOrDocValuesQuery, equalTo(geoShapeQuery instanceof IndexOrDocValuesQuery));
     }
+
+    @Override
+    protected Map<String, String> getObjectsHoldingArbitraryContent() {
+        // shape field can accept any element but expects a type
+        return Collections.singletonMap("shape", "Required [type]");
+    }
 }

+ 8 - 0
x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/query/ShapeQueryBuilderTests.java

@@ -38,6 +38,8 @@ import org.junit.After;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.instanceOf;
@@ -266,4 +268,10 @@ public abstract class ShapeQueryBuilderTests extends AbstractQueryTestCase<Shape
         return new GetResponse(new GetResult(indexedShapeIndex, indexedShapeId, 0, 1, 0, true, new BytesArray(json),
             null, null));
     }
+
+    @Override
+    protected Map<String, String> getObjectsHoldingArbitraryContent() {
+        // shape field can accept any element but expects a type
+        return Collections.singletonMap("shape", "Required [type]");
+    }
 }