Browse Source

Geo: deprecate ShapeBuilder in QueryBuilders (#44715)

Removes unnecessary now timeline decompositions from shape builders
and deprecates ShapeBuilders in QueryBuilder in favor of libs/geo
shapes.

Relates to #40908
Igor Motov 6 years ago
parent
commit
34675caef4

+ 9 - 9
server/src/main/java/org/elasticsearch/common/geo/GeoJson.java

@@ -382,17 +382,17 @@ public final class GeoJson {
         return geometry.visit(new GeometryVisitor<>() {
             @Override
             public String visit(Circle circle) {
-                return "Circle";
+                return "circle";
             }
 
             @Override
             public String visit(GeometryCollection<?> collection) {
-                return "GeometryCollection";
+                return "geometrycollection";
             }
 
             @Override
             public String visit(Line line) {
-                return "LineString";
+                return "linestring";
             }
 
             @Override
@@ -402,32 +402,32 @@ public final class GeoJson {
 
             @Override
             public String visit(MultiLine multiLine) {
-                return "MultiLineString";
+                return "multilinestring";
             }
 
             @Override
             public String visit(MultiPoint multiPoint) {
-                return "MultiPoint";
+                return "multipoint";
             }
 
             @Override
             public String visit(MultiPolygon multiPolygon) {
-                return "MultiPolygon";
+                return "multipolygon";
             }
 
             @Override
             public String visit(Point point) {
-                return "Point";
+                return "point";
             }
 
             @Override
             public String visit(Polygon polygon) {
-                return "Polygon";
+                return "polygon";
             }
 
             @Override
             public String visit(Rectangle rectangle) {
-                return "Envelope";
+                return "envelope";
             }
         });
     }

+ 307 - 0
server/src/main/java/org/elasticsearch/common/geo/GeometryIO.java

@@ -0,0 +1,307 @@
+/*
+ * 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;
+
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.geo.geometry.Circle;
+import org.elasticsearch.geo.geometry.Geometry;
+import org.elasticsearch.geo.geometry.GeometryCollection;
+import org.elasticsearch.geo.geometry.GeometryVisitor;
+import org.elasticsearch.geo.geometry.Line;
+import org.elasticsearch.geo.geometry.LinearRing;
+import org.elasticsearch.geo.geometry.MultiLine;
+import org.elasticsearch.geo.geometry.MultiPoint;
+import org.elasticsearch.geo.geometry.MultiPolygon;
+import org.elasticsearch.geo.geometry.Point;
+import org.elasticsearch.geo.geometry.Polygon;
+import org.elasticsearch.geo.geometry.Rectangle;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+
+/**
+ * Utility class for binary serializtion/deserialization of libs/geo classes
+ */
+public final class GeometryIO {
+
+    public static void writeGeometry(StreamOutput out, Geometry geometry) throws IOException {
+        out.writeString(GeoJson.getGeoJsonName(geometry).toLowerCase(Locale.ROOT));
+        geometry.visit(new GeometryVisitor<Void, IOException>() {
+            @Override
+            public Void visit(Circle circle) throws IOException {
+                throw new UnsupportedOperationException("circle is not supported");
+            }
+
+            @Override
+            public Void visit(GeometryCollection<?> collection) throws IOException {
+                out.writeVInt(collection.size());
+                for (Geometry shape : collection) {
+                    writeGeometry(out, shape);
+                }
+                return null;
+            }
+
+            @Override
+            public Void visit(Line line) throws IOException {
+                writeCoordinates(line);
+                return null;
+            }
+
+            @Override
+            public Void visit(LinearRing ring) {
+                throw new UnsupportedOperationException("linear ring is not supported");
+            }
+
+            @Override
+            public Void visit(MultiLine multiLine) throws IOException {
+                out.writeVInt(multiLine.size());
+                for (Line line : multiLine) {
+                    visit(line);
+                }
+                return null;
+            }
+
+            @Override
+            public Void visit(MultiPoint multiPoint) throws IOException {
+                out.writeVInt(multiPoint.size());
+                for (int i = 0; i < multiPoint.size(); i++) {
+                    Point point = multiPoint.get(i);
+                    writeCoordinate(point.getLat(), point.getLon(), point.getAlt());
+                }
+                return null;
+            }
+
+            @Override
+            public Void visit(MultiPolygon multiPolygon) throws IOException {
+                out.writeBoolean(true); // Orientation for BWC with ShapeBuilder
+                out.writeVInt(multiPolygon.size());
+                for (int i = 0; i < multiPolygon.size(); i++) {
+                    visit(multiPolygon.get(i));
+                }
+                return null;
+            }
+
+            @Override
+            public Void visit(Point point) throws IOException {
+                out.writeVInt(1); // Number of points For BWC with Shape Builder
+                writeCoordinate(point.getLat(), point.getLon(), point.getAlt());
+                return null;
+            }
+
+            @Override
+            public Void visit(Polygon polygon) throws IOException {
+                writeCoordinates(polygon.getPolygon());
+                out.writeBoolean(true); // Orientation for BWC with ShapeBuilder
+                out.writeVInt(polygon.getNumberOfHoles());
+                for (int i = 0; i < polygon.getNumberOfHoles(); i++) {
+                    writeCoordinates(polygon.getHole(i));
+                }
+                return null;
+            }
+
+            @Override
+            public Void visit(Rectangle rectangle) throws IOException {
+                writeCoordinate(rectangle.getMaxLat(), rectangle.getMinLon(), rectangle.getMinAlt()); // top left
+                writeCoordinate(rectangle.getMinLat(), rectangle.getMaxLon(), rectangle.getMaxAlt()); // bottom right
+                return null;
+            }
+
+            private void writeCoordinate(double lat, double lon, double alt) throws IOException {
+                out.writeDouble(lon);
+                out.writeDouble(lat);
+                out.writeOptionalDouble(Double.isNaN(alt) ? null : alt);
+            }
+
+            private void writeCoordinates(Line line) throws IOException {
+                out.writeVInt(line.length());
+                for (int i = 0; i < line.length(); i++) {
+                    writeCoordinate(line.getLat(i), line.getLon(i), line.getAlt(i));
+                }
+            }
+
+        });
+    }
+
+    public static Geometry readGeometry(StreamInput in) throws IOException {
+        String type = in.readString();
+        switch (type) {
+            case "geometrycollection":
+                return readGeometryCollection(in);
+            case "polygon":
+                return readPolygon(in);
+            case "point":
+                return readPoint(in);
+            case "linestring":
+                return readLine(in);
+            case "multilinestring":
+                return readMultiLine(in);
+            case "multipoint":
+                return readMultiPoint(in);
+            case "multipolygon":
+                return readMultiPolygon(in);
+            case "envelope":
+                return readRectangle(in);
+            default:
+                throw new UnsupportedOperationException("unsupported shape type " + type);
+        }
+    }
+
+    private static GeometryCollection<Geometry> readGeometryCollection(StreamInput in) throws IOException {
+        int size = in.readVInt();
+        List<Geometry> shapes = new ArrayList<>(size);
+        for (int i = 0; i < size; i++) {
+            shapes.add(readGeometry(in));
+        }
+        return new GeometryCollection<>(shapes);
+    }
+
+    private static Polygon readPolygon(StreamInput in) throws IOException {
+        double[][] shellComponents = readLineComponents(in);
+        boolean orientation = in.readBoolean();
+        LinearRing shell = buildLinearRing(shellComponents, orientation);
+        int numberOfHoles = in.readVInt();
+        if (numberOfHoles > 0) {
+            List<LinearRing> holes = new ArrayList<>(numberOfHoles);
+            for (int i = 0; i < numberOfHoles; i++) {
+                holes.add(buildLinearRing(readLineComponents(in), orientation));
+            }
+            return new Polygon(shell, holes);
+        } else {
+            return new Polygon(shell);
+        }
+    }
+
+    private static double[][] readLineComponents(StreamInput in) throws IOException {
+        int len = in.readVInt();
+        double[] lat = new double[len];
+        double[] lon = new double[len];
+        double[] alt = new double[len];
+        for (int i = 0; i < len; i++) {
+            lon[i] = in.readDouble();
+            lat[i] = in.readDouble();
+            alt[i] = readAlt(in);
+        }
+        if (Double.isNaN(alt[0])) {
+            return new double[][]{lat, lon};
+        } else {
+            return new double[][]{lat, lon, alt};
+        }
+    }
+
+    private static void reverse(double[][] arr) {
+        for (double[] carr : arr) {
+            int len = carr.length;
+            for (int j = 0; j < len / 2; j++) {
+                double temp = carr[j];
+                carr[j] = carr[len - j - 1];
+                carr[len - j - 1] = temp;
+            }
+        }
+    }
+
+    private static LinearRing buildLinearRing(double[][] arr, boolean orientation) {
+        if (orientation == false) {
+            reverse(arr);
+        }
+        if (arr.length == 3) {
+            return new LinearRing(arr[0], arr[1], arr[2]);
+        } else {
+            return new LinearRing(arr[0], arr[1]);
+        }
+    }
+
+    private static Point readPoint(StreamInput in) throws IOException {
+        int size = in.readVInt(); // For BWC with Shape Builder
+        if (size != 1) {
+            throw new IOException("Unexpected point count " + size);
+        }
+        double lon = in.readDouble();
+        double lat = in.readDouble();
+        double alt = readAlt(in);
+        return new Point(lat, lon, alt);
+    }
+
+    private static Line readLine(StreamInput in) throws IOException {
+        double[][] coords = readLineComponents(in);
+        if (coords.length == 3) {
+            return new Line(coords[0], coords[1], coords[2]);
+        } else {
+            return new Line(coords[0], coords[1]);
+        }
+    }
+
+    private static MultiLine readMultiLine(StreamInput in) throws IOException {
+        int size = in.readVInt();
+        List<Line> lines = new ArrayList<>(size);
+        for (int i = 0; i < size; i++) {
+            lines.add(readLine(in));
+        }
+        return new MultiLine(lines);
+    }
+
+    private static MultiPoint readMultiPoint(StreamInput in) throws IOException {
+        int size = in.readVInt();
+        List<Point> points = new ArrayList<>(size);
+        for (int i = 0; i < size; i++) {
+            double lon = in.readDouble();
+            double lat = in.readDouble();
+            double alt = readAlt(in);
+            points.add(new Point(lat, lon, alt));
+        }
+        return new MultiPoint(points);
+    }
+
+
+    private static MultiPolygon readMultiPolygon(StreamInput in) throws IOException {
+        in.readBoolean(); // orientation for BWC
+        int size = in.readVInt();
+        List<Polygon> polygons = new ArrayList<>(size);
+        for (int i = 0; i < size; i++) {
+            polygons.add(readPolygon(in));
+        }
+        return new MultiPolygon(polygons);
+    }
+
+    private static Rectangle readRectangle(StreamInput in) throws IOException {
+        // top left
+        double minLon = in.readDouble();
+        double maxLat = in.readDouble();
+        double minAlt = readAlt(in);
+
+        // bottom right
+        double maxLon = in.readDouble();
+        double minLat = in.readDouble();
+        double maxAlt = readAlt(in);
+
+        return new Rectangle(minLat, maxLat, minLon, maxLon, minAlt, maxAlt);
+    }
+
+    private static double readAlt(StreamInput in) throws IOException {
+        Double alt = in.readOptionalDouble();
+        if (alt == null) {
+            return Double.NaN;
+        } else {
+            return alt;
+        }
+    }
+}

+ 3 - 2
server/src/main/java/org/elasticsearch/common/geo/GeometryIndexer.java

@@ -245,6 +245,7 @@ public final class GeometryIndexer {
 
         for (int i = 1; i < lons.length; i++) {
             double t = intersection(lastLon, lons[i], dateline);
+            lastLon = lons[i];
             if (Double.isNaN(t) == false) {
                 double[] partLons = Arrays.copyOfRange(lons, offset, i + 1);
                 double[] partLats = Arrays.copyOfRange(lats, offset, i + 1);
@@ -330,7 +331,7 @@ public final class GeometryIndexer {
             exterior.add(new Point(shell.getLat(i), shell.getLon(i)));
         }
         for (int i = 0; i < hole.length(); i++) {
-            interior.remove(new Point(hole.getLat(i), hole.getLon(i)));
+            interior.add(new Point(hole.getLat(i), hole.getLon(i)));
         }
         exterior.retainAll(interior);
         if (exterior.size() >= 2) {
@@ -645,7 +646,7 @@ public final class GeometryIndexer {
                 edges[edgeOffset + i - 1].next = edges[edgeOffset + i] = new Edge(nextPoint, null);
                 edges[edgeOffset + i - 1].component = component;
             } else {
-                throw new InvalidShapeException("Provided shape has duplicate consecutive coordinates at: " + nextPoint);
+                throw new InvalidShapeException("Provided shape has duplicate consecutive coordinates at: (" + nextPoint + ")");
             }
         }
 

+ 2 - 26
server/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java

@@ -24,7 +24,6 @@ import org.elasticsearch.common.geo.parsers.ShapeParser;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.geo.geometry.Line;
-import org.elasticsearch.geo.geometry.MultiLine;
 import org.locationtech.jts.geom.Coordinate;
 import org.locationtech.jts.geom.Geometry;
 import org.locationtech.jts.geom.GeometryFactory;
@@ -36,9 +35,6 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
-import static org.elasticsearch.common.geo.GeoUtils.normalizeLat;
-import static org.elasticsearch.common.geo.GeoUtils.normalizeLon;
-
 public class LineStringBuilder extends ShapeBuilder<JtsGeometry, org.elasticsearch.geo.geometry.Geometry, LineStringBuilder> {
     public static final GeoShapeType TYPE = GeoShapeType.LINESTRING;
 
@@ -126,18 +122,8 @@ public class LineStringBuilder extends ShapeBuilder<JtsGeometry, org.elasticsear
 
     @Override
     public org.elasticsearch.geo.geometry.Geometry buildGeometry() {
-        // decompose linestrings crossing dateline into array of Lines
-        Coordinate[] coordinates = this.coordinates.toArray(new Coordinate[this.coordinates.size()]);
-        if (wrapdateline) {
-            List<Line> linestrings = decomposeGeometry(coordinates, new ArrayList<>());
-            if (linestrings.size() == 1) {
-                return linestrings.get(0);
-            } else {
-                return new MultiLine(linestrings);
-            }
-        }
-        return new Line(Arrays.stream(coordinates).mapToDouble(i->normalizeLat(i.y)).toArray(),
-            Arrays.stream(coordinates).mapToDouble(i->normalizeLon(i.x)).toArray());
+        return new Line(coordinates.stream().mapToDouble(i->i.y).toArray(),
+            coordinates.stream().mapToDouble(i->i.x).toArray());
     }
 
     static ArrayList<LineString> decomposeS4J(GeometryFactory factory, Coordinate[] coordinates, ArrayList<LineString> strings) {
@@ -149,16 +135,6 @@ public class LineStringBuilder extends ShapeBuilder<JtsGeometry, org.elasticsear
         return strings;
     }
 
-    static List<Line> decomposeGeometry(Coordinate[] coordinates, List<Line> lines) {
-        for (Coordinate[] part : decompose(+DATELINE, coordinates)) {
-            for (Coordinate[] line : decompose(-DATELINE, part)) {
-                lines.add(new Line(Arrays.stream(line).mapToDouble(i->normalizeLat(i.y)).toArray(),
-                    Arrays.stream(line).mapToDouble(i->normalizeLon(i.x)).toArray()));
-            }
-        }
-        return lines;
-    }
-
     /**
      * Decompose a linestring given as array of coordinates at a vertical line.
      *

+ 0 - 10
server/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java

@@ -154,16 +154,6 @@ public class MultiLineStringBuilder extends ShapeBuilder<JtsGeometry, org.elasti
         if (lines.isEmpty()) {
             return MultiLine.EMPTY;
         }
-        if (wrapdateline) {
-            List<org.elasticsearch.geo.geometry.Line> parts = new ArrayList<>();
-            for (LineStringBuilder line : lines) {
-                LineStringBuilder.decomposeGeometry(line.coordinates(false), parts);
-            }
-            if (parts.size() == 1) {
-                return parts.get(0);
-            }
-            return new MultiLine(parts);
-        }
         List<org.elasticsearch.geo.geometry.Line> linestrings = new ArrayList<>(lines.size());
         for (int i = 0; i < lines.size(); ++i) {
             LineStringBuilder lsb = lines.get(i);

+ 9 - 62
server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java

@@ -19,12 +19,6 @@
 
 package org.elasticsearch.common.geo.builders;
 
-import org.locationtech.jts.geom.Coordinate;
-import org.locationtech.jts.geom.Geometry;
-import org.locationtech.jts.geom.GeometryFactory;
-import org.locationtech.jts.geom.LinearRing;
-import org.locationtech.jts.geom.MultiPolygon;
-import org.locationtech.jts.geom.Polygon;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.geo.GeoShapeType;
 import org.elasticsearch.common.geo.parsers.ShapeParser;
@@ -32,13 +26,18 @@ import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.locationtech.jts.geom.Coordinate;
+import org.locationtech.jts.geom.Geometry;
+import org.locationtech.jts.geom.GeometryFactory;
+import org.locationtech.jts.geom.LinearRing;
+import org.locationtech.jts.geom.MultiPolygon;
+import org.locationtech.jts.geom.Polygon;
 import org.locationtech.spatial4j.exception.InvalidShapeException;
 import org.locationtech.spatial4j.shape.jts.JtsGeometry;
 
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -47,8 +46,6 @@ import java.util.Locale;
 import java.util.Objects;
 import java.util.concurrent.atomic.AtomicBoolean;
 
-import static org.elasticsearch.common.geo.GeoUtils.normalizeLat;
-import static org.elasticsearch.common.geo.GeoUtils.normalizeLon;
 import static org.apache.lucene.geo.GeoUtils.orient;
 
 /**
@@ -235,12 +232,6 @@ public class PolygonBuilder extends ShapeBuilder<JtsGeometry, org.elasticsearch.
 
     @Override
     public org.elasticsearch.geo.geometry.Geometry buildGeometry() {
-        if (wrapdateline) {
-            Coordinate[][][] polygons = coordinates();
-            return polygons.length == 1
-                ? polygonGeometry(polygons[0])
-                : multipolygon(polygons);
-        }
         return toPolygonGeometry();
     }
 
@@ -294,15 +285,12 @@ public class PolygonBuilder extends ShapeBuilder<JtsGeometry, org.elasticsearch.
         for (int i = 0; i < this.holes.size(); ++i) {
             holes.add(linearRing(this.holes.get(i).coordinates));
         }
-        return new org.elasticsearch.geo.geometry.Polygon(
-            new org.elasticsearch.geo.geometry.LinearRing(
-                this.shell.coordinates.stream().mapToDouble(i -> normalizeLat(i.y)).toArray(),
-                this.shell.coordinates.stream().mapToDouble(i -> normalizeLon(i.x)).toArray()), holes);
+        return new org.elasticsearch.geo.geometry.Polygon(linearRing(this.shell.coordinates), holes);
     }
 
     protected static org.elasticsearch.geo.geometry.LinearRing linearRing(List<Coordinate> coordinates) {
-        return new org.elasticsearch.geo.geometry.LinearRing(coordinates.stream().mapToDouble(i -> normalizeLat(i.y)).toArray(),
-            coordinates.stream().mapToDouble(i -> normalizeLon(i.x)).toArray());
+        return new org.elasticsearch.geo.geometry.LinearRing(coordinates.stream().mapToDouble(i -> i.y).toArray(),
+            coordinates.stream().mapToDouble(i -> i.x).toArray());
     }
 
     protected static LinearRing linearRingS4J(GeometryFactory factory, List<Coordinate> coordinates) {
@@ -338,39 +326,6 @@ public class PolygonBuilder extends ShapeBuilder<JtsGeometry, org.elasticsearch.
         return factory.createPolygon(shell, holes);
     }
 
-    protected static org.elasticsearch.geo.geometry.Polygon polygonGeometry(Coordinate[][] polygon) {
-        List<org.elasticsearch.geo.geometry.LinearRing> holes;
-        Coordinate[] shell = polygon[0];
-        if (polygon.length > 1) {
-            holes = new ArrayList<>(polygon.length - 1);
-            for (int i = 1; i < polygon.length; ++i) {
-                Coordinate[] coords = polygon[i];
-                //We do not have holes on the dateline as they get eliminated
-                //when breaking the polygon around it.
-                double[] x = new double[coords.length];
-                double[] y = new double[coords.length];
-                for (int c = 0; c < coords.length; ++c) {
-                    x[c] = normalizeLon(coords[c].x);
-                    y[c] = normalizeLat(coords[c].y);
-                }
-                holes.add(new org.elasticsearch.geo.geometry.LinearRing(y, x));
-            }
-        } else {
-            holes = Collections.emptyList();
-        }
-
-        double[] x = new double[shell.length];
-        double[] y = new double[shell.length];
-        for (int i = 0; i < shell.length; ++i) {
-            //Lucene Tessellator treats different +180 and -180 and we should keep the sign.
-            //normalizeLon method excludes -180.
-            x[i] = Math.abs(shell[i].x) > 180 ? normalizeLon(shell[i].x) : shell[i].x;
-            y[i] = normalizeLat(shell[i].y);
-        }
-
-        return new org.elasticsearch.geo.geometry.Polygon(new org.elasticsearch.geo.geometry.LinearRing(y, x), holes);
-    }
-
     /**
      * Create a Multipolygon from a set of coordinates. Each primary array contains a polygon which
      * in turn contains an array of linestrings. These line Strings are represented as an array of
@@ -389,14 +344,6 @@ public class PolygonBuilder extends ShapeBuilder<JtsGeometry, org.elasticsearch.
         return factory.createMultiPolygon(polygonSet);
     }
 
-    protected static org.elasticsearch.geo.geometry.MultiPolygon multipolygon(Coordinate[][][] polygons) {
-        List<org.elasticsearch.geo.geometry.Polygon> polygonSet = new ArrayList<>(polygons.length);
-        for (int i = 0; i < polygons.length; ++i) {
-            polygonSet.add(polygonGeometry(polygons[i]));
-        }
-        return new org.elasticsearch.geo.geometry.MultiPolygon(polygonSet);
-    }
-
     /**
      * This method sets the component id of all edges in a ring to a given id and shifts the
      * coordinates of this component according to the dateline

+ 157 - 28
server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java

@@ -40,9 +40,21 @@ import org.elasticsearch.client.Client;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParsingException;
+import org.elasticsearch.common.geo.GeoJson;
 import org.elasticsearch.common.geo.GeoShapeType;
+import org.elasticsearch.common.geo.GeometryIO;
+import org.elasticsearch.common.geo.GeometryIndexer;
+import org.elasticsearch.common.geo.GeometryParser;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.geo.SpatialStrategy;
+import org.elasticsearch.common.geo.builders.EnvelopeBuilder;
+import org.elasticsearch.common.geo.builders.GeometryCollectionBuilder;
+import org.elasticsearch.common.geo.builders.LineStringBuilder;
+import org.elasticsearch.common.geo.builders.MultiLineStringBuilder;
+import org.elasticsearch.common.geo.builders.MultiPointBuilder;
+import org.elasticsearch.common.geo.builders.MultiPolygonBuilder;
+import org.elasticsearch.common.geo.builders.PointBuilder;
+import org.elasticsearch.common.geo.builders.PolygonBuilder;
 import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.common.geo.parsers.ShapeParser;
 import org.elasticsearch.common.io.stream.StreamInput;
@@ -62,11 +74,16 @@ import org.elasticsearch.geo.geometry.MultiLine;
 import org.elasticsearch.geo.geometry.MultiPoint;
 import org.elasticsearch.geo.geometry.MultiPolygon;
 import org.elasticsearch.geo.geometry.Point;
+import org.elasticsearch.geo.geometry.Rectangle;
 import org.elasticsearch.index.mapper.BaseGeoShapeFieldMapper;
 import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
+import org.locationtech.jts.geom.Coordinate;
+import org.locationtech.spatial4j.shape.Shape;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Objects;
 import java.util.function.Supplier;
 
@@ -104,8 +121,8 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
 
     private final String fieldName;
 
-    private final ShapeBuilder shape;
-    private final Supplier<ShapeBuilder> supplier;
+    private final Geometry shape;
+    private final Supplier<Geometry> supplier;
 
     private SpatialStrategy strategy;
 
@@ -129,11 +146,28 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
      *            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) {
+        this(fieldName, shape == null ? null : shape.buildGeometry(), null, null);
+    }
+
+    /**
+     * 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
+     */
+    public GeoShapeQueryBuilder(String fieldName, Geometry shape) {
         this(fieldName, shape, null, null);
     }
 
+
     /**
      * Creates a new GeoShapeQueryBuilder whose Query will be against the given
      * field name and will use the Shape found with the given ID
@@ -144,7 +178,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
      *            ID of the indexed Shape that will be used in the Query
      */
     public GeoShapeQueryBuilder(String fieldName, String indexedShapeId) {
-        this(fieldName, (ShapeBuilder) null, indexedShapeId, null);
+        this(fieldName, (Geometry) null, indexedShapeId, null);
     }
 
     /**
@@ -162,10 +196,10 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
      */
     @Deprecated
     public GeoShapeQueryBuilder(String fieldName, String indexedShapeId, String indexedShapeType) {
-        this(fieldName, (ShapeBuilder) null, indexedShapeId, indexedShapeType);
+        this(fieldName, (Geometry) null, indexedShapeId, indexedShapeType);
     }
 
-    private GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape, String indexedShapeId, @Nullable String indexedShapeType) {
+    private GeoShapeQueryBuilder(String fieldName, Geometry shape, String indexedShapeId, @Nullable String indexedShapeType) {
         if (fieldName == null) {
             throw new IllegalArgumentException("fieldName is required");
         }
@@ -179,7 +213,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
         this.supplier = null;
     }
 
-    private GeoShapeQueryBuilder(String fieldName, Supplier<ShapeBuilder> supplier, String indexedShapeId,
+    private GeoShapeQueryBuilder(String fieldName, Supplier<Geometry> supplier, String indexedShapeId,
             @Nullable String indexedShapeType) {
         this.fieldName = fieldName;
         this.shape = null;
@@ -195,7 +229,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
         super(in);
         fieldName = in.readString();
         if (in.readBoolean()) {
-            shape = in.readNamedWriteable(ShapeBuilder.class);
+            shape = GeometryIO.readGeometry(in);
             indexedShapeId = null;
             indexedShapeType = null;
         } else {
@@ -221,7 +255,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
         boolean hasShape = shape != null;
         out.writeBoolean(hasShape);
         if (hasShape) {
-            out.writeNamedWriteable(shape);
+            GeometryIO.writeGeometry(out, shape);;
         } else {
             out.writeOptionalString(indexedShapeId);
             out.writeOptionalString(indexedShapeType);
@@ -244,7 +278,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
     /**
      * @return the shape used in the Query
      */
-    public ShapeBuilder shape() {
+    public Geometry shape() {
         return shape;
     }
 
@@ -397,7 +431,6 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
         if (shape == null || supplier != null) {
             throw new UnsupportedOperationException("query must be rewritten first");
         }
-        final ShapeBuilder shapeToQuery = shape;
         final MappedFieldType fieldType = context.fieldMapper(fieldName);
         if (fieldType == null) {
             if (ignoreUnmapped) {
@@ -425,32 +458,36 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
                 // in this case, execute disjoint as exists && !intersects
                 BooleanQuery.Builder bool = new BooleanQuery.Builder();
                 Query exists = ExistsQueryBuilder.newFilter(context, fieldName);
-                Query intersects = prefixTreeStrategy.makeQuery(getArgs(shapeToQuery, ShapeRelation.INTERSECTS));
+                Query intersects = prefixTreeStrategy.makeQuery(getArgs(shape, ShapeRelation.INTERSECTS));
                 bool.add(exists, BooleanClause.Occur.MUST);
                 bool.add(intersects, BooleanClause.Occur.MUST_NOT);
                 query = new ConstantScoreQuery(bool.build());
             } else {
-                query = new ConstantScoreQuery(prefixTreeStrategy.makeQuery(getArgs(shapeToQuery, relation)));
+                query = new ConstantScoreQuery(prefixTreeStrategy.makeQuery(getArgs(shape, relation)));
             }
         } else {
-            query = new ConstantScoreQuery(getVectorQuery(context, shapeToQuery));
+            query = new ConstantScoreQuery(getVectorQuery(context, shape));
         }
         return query;
     }
 
-    private Query getVectorQuery(QueryShardContext context, ShapeBuilder queryShapeBuilder) {
+    private Query getVectorQuery(QueryShardContext context, Geometry queryShape) {
         // CONTAINS queries are not yet supported by VECTOR strategy
         if (relation == ShapeRelation.CONTAINS) {
             throw new QueryShardException(context,
                 ShapeRelation.CONTAINS + " query relation not supported for Field [" + fieldName + "]");
         }
 
-        // wrap geoQuery as a ConstantScoreQuery
-        return getVectorQueryFromShape(context, queryShapeBuilder.buildGeometry());
-    }
+        // TODO: Move this to QueryShardContext
+        GeometryIndexer geometryIndexer = new GeometryIndexer(true);
+
+        Geometry processedShape = geometryIndexer.prepareForIndexing(queryShape);
 
-    private Query getVectorQueryFromShape(QueryShardContext context, Geometry queryShape) {
-        return queryShape.visit(new GeometryVisitor<Query, RuntimeException>() {
+        if (processedShape == null) {
+            return new MatchNoDocsQuery();
+        }
+
+        return processedShape.visit(new GeometryVisitor<Query, RuntimeException>() {
             @Override
             public Query visit(Circle circle) {
                 throw new QueryShardException(context, "Field [" + fieldName + "] found and unknown shape Circle");
@@ -536,7 +573,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
      *            Name or path of the field in the Shape Document where the
      *            Shape itself is located
      */
-    private void fetch(Client client, GetRequest getRequest, String path, ActionListener<ShapeBuilder> listener) {
+    private void fetch(Client client, GetRequest getRequest, String path, ActionListener<Geometry> listener) {
         getRequest.preference("_local");
         client.get(getRequest, new ActionListener<GetResponse>(){
 
@@ -565,7 +602,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
                                 if (pathElements[currentPathSlot].equals(parser.currentName())) {
                                     parser.nextToken();
                                     if (++currentPathSlot == pathElements.length) {
-                                        listener.onResponse(ShapeParser.parse(parser));
+                                        listener.onResponse(new GeometryParser(true, true, true).parse(parser));
                                         return;
                                     }
                                 } else {
@@ -589,16 +626,16 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
 
     }
 
-    public static SpatialArgs getArgs(ShapeBuilder shape, ShapeRelation relation) {
+    public static SpatialArgs getArgs(Geometry shape, ShapeRelation relation) {
         switch (relation) {
         case DISJOINT:
-            return new SpatialArgs(SpatialOperation.IsDisjointTo, shape.buildS4J());
+            return new SpatialArgs(SpatialOperation.IsDisjointTo, buildS4J(shape));
         case INTERSECTS:
-            return new SpatialArgs(SpatialOperation.Intersects, shape.buildS4J());
+            return new SpatialArgs(SpatialOperation.Intersects, buildS4J(shape));
         case WITHIN:
-            return new SpatialArgs(SpatialOperation.IsWithin, shape.buildS4J());
+            return new SpatialArgs(SpatialOperation.IsWithin, buildS4J(shape));
         case CONTAINS:
-            return new SpatialArgs(SpatialOperation.Contains, shape.buildS4J());
+            return new SpatialArgs(SpatialOperation.Contains, buildS4J(shape));
         default:
             throw new IllegalArgumentException("invalid relation [" + relation + "]");
         }
@@ -616,7 +653,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
 
         if (shape != null) {
             builder.field(SHAPE_FIELD.getPreferredName());
-            shape.toXContent(builder, params);
+            GeoJson.toXContent(shape, builder,params);
         } else {
             builder.startObject(INDEXED_SHAPE_FIELD.getPreferredName())
                     .field(SHAPE_ID_FIELD.getPreferredName(), indexedShapeId);
@@ -797,7 +834,7 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
             return supplier.get() == null ? this : new GeoShapeQueryBuilder(this.fieldName, supplier.get()).relation(relation).strategy
                 (strategy);
         } else if (this.shape == null) {
-            SetOnce<ShapeBuilder> supplier = new SetOnce<>();
+            SetOnce<Geometry> supplier = new SetOnce<>();
             queryRewriteContext.registerAsyncAction((client, listener) -> {
                 GetRequest getRequest;
                 if (indexedShapeType == null) {
@@ -816,4 +853,96 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuil
         }
         return this;
     }
+
+    /**
+     * Builds JTS shape from a geometry
+     *
+     * This method is needed to handle legacy indices and will be removed when we no longer need to build JTS shapes
+     */
+    private static Shape buildS4J(Geometry geometry) {
+        return geometryToShapeBuilder(geometry).buildS4J();
+    }
+
+    public static ShapeBuilder<?, ?, ?> geometryToShapeBuilder(Geometry geometry) {
+        ShapeBuilder<?, ?, ?> shapeBuilder = geometry.visit(new GeometryVisitor<>() {
+            @Override
+            public ShapeBuilder<?, ?, ?> visit(Circle circle) {
+                throw new UnsupportedOperationException("circle is not supported");
+            }
+
+            @Override
+            public ShapeBuilder<?, ?, ?> visit(GeometryCollection<?> collection) {
+                GeometryCollectionBuilder shapes = new GeometryCollectionBuilder();
+                for (Geometry geometry : collection) {
+                    shapes.shape(geometry.visit(this));
+                }
+                return shapes;
+            }
+
+            @Override
+            public ShapeBuilder<?, ?, ?> visit(org.elasticsearch.geo.geometry.Line line) {
+                List<Coordinate> coordinates = new ArrayList<>();
+                for (int i = 0; i < line.length(); i++) {
+                    coordinates.add(new Coordinate(line.getLon(i), line.getLat(i), line.getAlt(i)));
+                }
+                return new LineStringBuilder(coordinates);
+            }
+
+            @Override
+            public ShapeBuilder<?, ?, ?> visit(LinearRing ring) {
+                throw new UnsupportedOperationException("circle is not supported");
+            }
+
+            @Override
+            public ShapeBuilder<?, ?, ?> visit(MultiLine multiLine) {
+                MultiLineStringBuilder lines = new MultiLineStringBuilder();
+                for (int i = 0; i < multiLine.size(); i++) {
+                    lines.linestring((LineStringBuilder) visit(multiLine.get(i)));
+                }
+                return lines;
+            }
+
+            @Override
+            public ShapeBuilder<?, ?, ?> visit(MultiPoint multiPoint) {
+                List<Coordinate> coordinates = new ArrayList<>();
+                for (int i = 0; i < multiPoint.size(); i++) {
+                    Point p = multiPoint.get(i);
+                    coordinates.add(new Coordinate(p.getLon(), p.getLat(), p.getAlt()));
+                }
+                return new MultiPointBuilder(coordinates);
+            }
+
+            @Override
+            public ShapeBuilder<?, ?, ?> visit(MultiPolygon multiPolygon) {
+                MultiPolygonBuilder polygons = new MultiPolygonBuilder();
+                for (int i = 0; i < multiPolygon.size(); i++) {
+                    polygons.polygon((PolygonBuilder) visit(multiPolygon.get(i)));
+                }
+                return polygons;
+            }
+
+            @Override
+            public ShapeBuilder<?, ?, ?> visit(Point point) {
+                return new PointBuilder(point.getLon(), point.getLat());
+            }
+
+            @Override
+            public ShapeBuilder<?, ?, ?> visit(org.elasticsearch.geo.geometry.Polygon polygon) {
+                PolygonBuilder polygonBuilder =
+                    new PolygonBuilder((LineStringBuilder) visit((org.elasticsearch.geo.geometry.Line) polygon.getPolygon()),
+                        ShapeBuilder.Orientation.RIGHT, false);
+                for (int i = 0; i < polygon.getNumberOfHoles(); i++) {
+                    polygonBuilder.hole((LineStringBuilder) visit((org.elasticsearch.geo.geometry.Line) polygon.getHole(i)));
+                }
+                return polygonBuilder;
+            }
+
+            @Override
+            public ShapeBuilder<?, ?, ?> visit(Rectangle rectangle) {
+                return new EnvelopeBuilder(new Coordinate(rectangle.getMinLon(), rectangle.getMaxLat()),
+                    new Coordinate(rectangle.getMaxLon(), rectangle.getMinLat()));
+            }
+        });
+        return shapeBuilder;
+    }
 }

+ 1 - 1
server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java

@@ -470,7 +470,7 @@ public class GeoWKTShapeParserTests extends BaseGeoParsingTestCase {
         } else {
             GeometryCollectionBuilder gcb = RandomShapeGenerator.createGeometryCollection(random());
             assertExpected(gcb.buildS4J(), gcb, true);
-            assertExpected(gcb.buildGeometry(), gcb, false);
+            assertExpected(new GeometryIndexer(true).prepareForIndexing(gcb.buildGeometry()), gcb, false);
         }
     }
 

+ 110 - 0
server/src/test/java/org/elasticsearch/common/geo/GeometryIOTests.java

@@ -0,0 +1,110 @@
+/*
+ * 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;
+
+import org.elasticsearch.common.geo.builders.ShapeBuilder;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.geo.geometry.Geometry;
+import org.elasticsearch.geo.geometry.GeometryCollection;
+import org.elasticsearch.geo.geometry.ShapeType;
+import org.elasticsearch.test.ESTestCase;
+
+import static org.elasticsearch.geo.GeometryTestUtils.randomGeometry;
+import static org.elasticsearch.index.query.GeoShapeQueryBuilder.geometryToShapeBuilder;
+
+public class GeometryIOTests extends ESTestCase {
+
+    public void testRandomSerialization() throws Exception {
+        for (int i = 0; i < randomIntBetween(1, 20); i++) {
+            boolean hasAlt = randomBoolean();
+            Geometry geometry = randomGeometry(hasAlt);
+            if (shapeSupported(geometry) && randomBoolean()) {
+                // Shape builder conversion doesn't support altitude
+                ShapeBuilder<?, ?, ?> shapeBuilder = geometryToShapeBuilder(geometry);
+                if (randomBoolean()) {
+                    Geometry actual = shapeBuilder.buildGeometry();
+                    assertEquals(geometry, actual);
+                }
+                if (randomBoolean()) {
+                    // Test ShapeBuilder -> Geometry Serialization
+                    try (BytesStreamOutput out = new BytesStreamOutput()) {
+                        out.writeNamedWriteable(shapeBuilder);
+                        try (StreamInput in = out.bytes().streamInput()) {
+                            Geometry actual = GeometryIO.readGeometry(in);
+                            assertEquals(geometry, actual);
+                            assertEquals(0, in.available());
+                        }
+                    }
+                } else {
+                    // Test Geometry -> ShapeBuilder Serialization
+                    try (BytesStreamOutput out = new BytesStreamOutput()) {
+                        GeometryIO.writeGeometry(out, geometry);
+                        try (StreamInput in = out.bytes().streamInput()) {
+                            try (StreamInput nin = new NamedWriteableAwareStreamInput(in, this.writableRegistry())) {
+                                ShapeBuilder<?, ?, ?> actual = nin.readNamedWriteable(ShapeBuilder.class);
+                                assertEquals(shapeBuilder, actual);
+                                assertEquals(0, in.available());
+                            }
+                        }
+                    }
+                }
+                // Test Geometry -> Geometry
+                try (BytesStreamOutput out = new BytesStreamOutput()) {
+                    GeometryIO.writeGeometry(out, geometry);
+                    ;
+                    try (StreamInput in = out.bytes().streamInput()) {
+                        Geometry actual = GeometryIO.readGeometry(in);
+                        assertEquals(geometry, actual);
+                        assertEquals(0, in.available());
+                    }
+                }
+
+            }
+        }
+    }
+
+    private boolean shapeSupported(Geometry geometry) {
+        if (geometry.hasAlt()) {
+            return false;
+        }
+
+        if (geometry.type() == ShapeType.CIRCLE) {
+            return false;
+        }
+
+        if (geometry.type() == ShapeType.GEOMETRYCOLLECTION) {
+            GeometryCollection<?> collection = (GeometryCollection<?>) geometry;
+            for (Geometry g : collection) {
+                if (shapeSupported(g) == false) {
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+
+    @Override
+    protected NamedWriteableRegistry writableRegistry() {
+        return new NamedWriteableRegistry(GeoShapeType.getShapeWriteables());
+    }
+}

+ 2 - 2
server/src/test/java/org/elasticsearch/common/geo/GeometryParserTests.java

@@ -51,7 +51,7 @@ public class GeometryParserTests extends ESTestCase {
             assertEquals(new Point(0, 100), format.fromXContent(parser));
             XContentBuilder newGeoJson = XContentFactory.jsonBuilder();
             format.toXContent(new Point(10, 100), newGeoJson, ToXContent.EMPTY_PARAMS);
-            assertEquals("{\"type\":\"Point\",\"coordinates\":[100.0,10.0]}", Strings.toString(newGeoJson));
+            assertEquals("{\"type\":\"point\",\"coordinates\":[100.0,10.0]}", Strings.toString(newGeoJson));
         }
 
         XContentBuilder pointGeoJsonWithZ = XContentFactory.jsonBuilder()
@@ -148,7 +148,7 @@ public class GeometryParserTests extends ESTestCase {
             // if we serialize non-null value - it should be serialized as geojson
             format.toXContent(new Point(10, 100), newGeoJson, ToXContent.EMPTY_PARAMS);
             newGeoJson.endObject();
-            assertEquals("{\"val\":{\"type\":\"Point\",\"coordinates\":[100.0,10.0]}}", Strings.toString(newGeoJson));
+            assertEquals("{\"val\":{\"type\":\"point\",\"coordinates\":[100.0,10.0]}}", Strings.toString(newGeoJson));
 
             newGeoJson = XContentFactory.jsonBuilder().startObject().field("val");
             format.toXContent(null, newGeoJson, ToXContent.EMPTY_PARAMS);

+ 34 - 31
server/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java

@@ -19,12 +19,8 @@
 
 package org.elasticsearch.common.geo;
 
-import org.locationtech.jts.geom.Coordinate;
-import org.locationtech.jts.geom.LineString;
-import org.locationtech.jts.geom.Polygon;
-
-import org.elasticsearch.common.geo.builders.CoordinatesBuilder;
 import org.elasticsearch.common.geo.builders.CircleBuilder;
+import org.elasticsearch.common.geo.builders.CoordinatesBuilder;
 import org.elasticsearch.common.geo.builders.EnvelopeBuilder;
 import org.elasticsearch.common.geo.builders.LineStringBuilder;
 import org.elasticsearch.common.geo.builders.MultiLineStringBuilder;
@@ -32,6 +28,9 @@ import org.elasticsearch.common.geo.builders.PointBuilder;
 import org.elasticsearch.common.geo.builders.PolygonBuilder;
 import org.elasticsearch.common.geo.builders.ShapeBuilder;
 import org.elasticsearch.test.ESTestCase;
+import org.locationtech.jts.geom.Coordinate;
+import org.locationtech.jts.geom.LineString;
+import org.locationtech.jts.geom.Polygon;
 import org.locationtech.spatial4j.exception.InvalidShapeException;
 import org.locationtech.spatial4j.shape.Circle;
 import org.locationtech.spatial4j.shape.Point;
@@ -161,7 +160,7 @@ public class ShapeBuilderTests extends ESTestCase {
             .coordinate(-110.0, 55.0));
 
         lsb.buildS4J();
-        lsb.buildGeometry();
+        buildGeometry(lsb);
 
         // Building a linestring that needs to be wrapped
         lsb = new LineStringBuilder(new CoordinatesBuilder()
@@ -175,7 +174,7 @@ public class ShapeBuilderTests extends ESTestCase {
         .coordinate(130.0, 60.0));
 
         lsb.buildS4J();
-        lsb.buildGeometry();
+        buildGeometry(lsb);
 
         // Building a lineString on the dateline
         lsb = new LineStringBuilder(new CoordinatesBuilder()
@@ -185,7 +184,7 @@ public class ShapeBuilderTests extends ESTestCase {
         .coordinate(-180.0, -80.0));
 
         lsb.buildS4J();
-        lsb.buildGeometry();
+        buildGeometry(lsb);
 
         // Building a lineString on the dateline
         lsb = new LineStringBuilder(new CoordinatesBuilder()
@@ -195,7 +194,7 @@ public class ShapeBuilderTests extends ESTestCase {
         .coordinate(180.0, -80.0));
 
         lsb.buildS4J();
-        lsb.buildGeometry();
+        buildGeometry(lsb);
     }
 
     public void testMultiLineString() {
@@ -215,7 +214,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 )
             );
         mlsb.buildS4J();
-        mlsb.buildGeometry();
+        buildGeometry(mlsb);
 
         // LineString that needs to be wrapped
         new MultiLineStringBuilder()
@@ -235,7 +234,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 );
 
         mlsb.buildS4J();
-        mlsb.buildGeometry();
+        buildGeometry(mlsb);
     }
 
     public void testPolygonSelfIntersection() {
@@ -283,7 +282,7 @@ public class ShapeBuilderTests extends ESTestCase {
             .close());
 
         assertMultiPolygon(pb.buildS4J(), true);
-        assertMultiPolygon(pb.buildGeometry(), false);
+        assertMultiPolygon(buildGeometry(pb), false);
     }
 
     public void testLineStringWrapping() {
@@ -295,7 +294,7 @@ public class ShapeBuilderTests extends ESTestCase {
             .close());
 
         assertMultiLineString(lsb.buildS4J(), true);
-        assertMultiLineString(lsb.buildGeometry(), false);
+        assertMultiLineString(buildGeometry(lsb), false);
     }
 
     public void testDatelineOGC() {
@@ -339,7 +338,7 @@ public class ShapeBuilderTests extends ESTestCase {
             ));
 
         assertMultiPolygon(builder.close().buildS4J(), true);
-        assertMultiPolygon(builder.close().buildGeometry(), false);
+        assertMultiPolygon(buildGeometry(builder.close()), false);
     }
 
     public void testDateline() {
@@ -383,7 +382,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 ));
 
         assertMultiPolygon(builder.close().buildS4J(), true);
-        assertMultiPolygon(builder.close().buildGeometry(), false);
+        assertMultiPolygon(buildGeometry(builder.close()), false);
     }
 
     public void testComplexShapeWithHole() {
@@ -458,7 +457,7 @@ public class ShapeBuilderTests extends ESTestCase {
             )
             );
         assertPolygon(builder.close().buildS4J(), true);
-        assertPolygon(builder.close().buildGeometry(), false);
+        assertPolygon(buildGeometry(builder.close()), false);
      }
 
     public void testShapeWithHoleAtEdgeEndPoints() {
@@ -480,7 +479,7 @@ public class ShapeBuilderTests extends ESTestCase {
             .coordinate(4, 1)
             ));
         assertPolygon(builder.close().buildS4J(), true);
-        assertPolygon(builder.close().buildGeometry(), false);
+        assertPolygon(buildGeometry(builder.close()), false);
      }
 
     public void testShapeWithPointOnDateline() {
@@ -491,7 +490,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 .coordinate(180, 0)
                 );
             assertPolygon(builder.close().buildS4J(), true);
-            assertPolygon(builder.close().buildGeometry(), false);
+            assertPolygon(buildGeometry(builder.close()), false);
      }
 
     public void testShapeWithEdgeAlongDateline() {
@@ -504,7 +503,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 );
 
         assertPolygon(builder.close().buildS4J(), true);
-        assertPolygon(builder.close().buildGeometry(), false);
+        assertPolygon(buildGeometry(builder.close()), false);
 
         // test case 2: test the negative side of the dateline
         builder = new PolygonBuilder(new CoordinatesBuilder()
@@ -515,7 +514,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 );
 
         assertPolygon(builder.close().buildS4J(), true);
-        assertPolygon(builder.close().buildGeometry(), false);
+        assertPolygon(buildGeometry(builder.close()), false);
      }
 
     public void testShapeWithBoundaryHoles() {
@@ -537,7 +536,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 ));
 
         assertMultiPolygon(builder.close().buildS4J(), true);
-        assertMultiPolygon(builder.close().buildGeometry(), false);
+        assertMultiPolygon(buildGeometry(builder.close()), false);
 
         // test case 2: test the negative side of the dateline
         builder = new PolygonBuilder(
@@ -560,7 +559,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 ));
 
         assertMultiPolygon(builder.close().buildS4J(), true);
-        assertMultiPolygon(builder.close().buildGeometry(), false);
+        assertMultiPolygon(buildGeometry(builder.close()), false);
     }
 
     public void testShapeWithTangentialHole() {
@@ -582,7 +581,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 ));
 
         assertMultiPolygon(builder.close().buildS4J(), true);
-        assertMultiPolygon(builder.close().buildGeometry(), false);
+        assertMultiPolygon(buildGeometry(builder.close()), false);
     }
 
     public void testShapeWithInvalidTangentialHole() {
@@ -606,7 +605,7 @@ public class ShapeBuilderTests extends ESTestCase {
 
         e = expectThrows(InvalidShapeException.class, () -> builder.close().buildS4J());
         assertThat(e.getMessage(), containsString("interior cannot share more than one point with the exterior"));
-        e = expectThrows(InvalidShapeException.class, () -> builder.close().buildGeometry());
+        e = expectThrows(IllegalArgumentException.class, () -> buildGeometry(builder.close()));
         assertThat(e.getMessage(), containsString("interior cannot share more than one point with the exterior"));
     }
 
@@ -634,7 +633,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 .coordinate(172, 0)
                 ));
         assertMultiPolygon(builder.close().buildS4J(), true);
-        assertMultiPolygon(builder.close().buildGeometry(), false);
+        assertMultiPolygon(buildGeometry(builder.close()), false);
     }
 
     public void testBoundaryShapeWithInvalidTangentialHole() {
@@ -657,7 +656,7 @@ public class ShapeBuilderTests extends ESTestCase {
         Exception e;
         e = expectThrows(InvalidShapeException.class, () -> builder.close().buildS4J());
         assertThat(e.getMessage(), containsString("interior cannot share more than one point with the exterior"));
-        e = expectThrows(InvalidShapeException.class, () -> builder.close().buildGeometry());
+        e = expectThrows(IllegalArgumentException.class, () -> buildGeometry(builder.close()));
         assertThat(e.getMessage(), containsString("interior cannot share more than one point with the exterior"));
     }
 
@@ -673,7 +672,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 );
 
         assertPolygon(builder.close().buildS4J(), true);
-        assertPolygon(builder.close().buildGeometry(), false);
+        assertPolygon(buildGeometry(builder.close()), false);
     }
 
     public void testShapeWithAlternateOrientation() {
@@ -686,7 +685,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 );
 
         assertPolygon(builder.close().buildS4J(), true);
-        assertPolygon(builder.close().buildGeometry(), false);
+        assertPolygon(buildGeometry(builder.close()), false);
 
         // cw: geo core will convert to ccw across the dateline
         builder = new PolygonBuilder(new CoordinatesBuilder()
@@ -697,7 +696,7 @@ public class ShapeBuilderTests extends ESTestCase {
                 );
 
         assertMultiPolygon(builder.close().buildS4J(), true);
-        assertMultiPolygon(builder.close().buildGeometry(), false);
+        assertMultiPolygon(buildGeometry(builder.close()), false);
      }
 
     public void testInvalidShapeWithConsecutiveDuplicatePoints() {
@@ -711,7 +710,7 @@ public class ShapeBuilderTests extends ESTestCase {
 
         Exception e = expectThrows(InvalidShapeException.class, () -> builder.close().buildS4J());
         assertThat(e.getMessage(), containsString("duplicate consecutive coordinates at: ("));
-        e = expectThrows(InvalidShapeException.class, () -> builder.close().buildGeometry());
+        e = expectThrows(InvalidShapeException.class, () -> buildGeometry(builder.close()));
         assertThat(e.getMessage(), containsString("duplicate consecutive coordinates at: ("));
     }
 
@@ -774,7 +773,11 @@ public class ShapeBuilderTests extends ESTestCase {
         );
         Exception e = expectThrows(InvalidShapeException.class, () -> builder.close().buildS4J());
         assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
-        e = expectThrows(InvalidShapeException.class, () -> builder.close().buildGeometry());
+        e = expectThrows(InvalidShapeException.class, () -> buildGeometry(builder.close()));
         assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
     }
+
+    public Object buildGeometry(ShapeBuilder<?, ?, ?> builder) {
+        return new GeometryIndexer(true).prepareForIndexing(builder.buildGeometry());
+    }
 }

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

@@ -198,7 +198,7 @@ public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<GeoShapeQue
 
     // see #3878
     public void testThatXContentSerializationInsideOfArrayWorks() throws Exception {
-        EnvelopeBuilder envelopeBuilder = new EnvelopeBuilder(new Coordinate(0, 0), new Coordinate(10, 10));
+        EnvelopeBuilder envelopeBuilder = new EnvelopeBuilder(new Coordinate(0, 10), new Coordinate(10, 0));
         GeoShapeQueryBuilder geoQuery = QueryBuilders.geoShapeQuery("searchGeometry", envelopeBuilder);
         JsonXContent.contentBuilder().startArray().value(geoQuery).endArray();
     }

+ 20 - 12
test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java

@@ -161,19 +161,27 @@ public class GeometryTestUtils {
         int size = ESTestCase.randomIntBetween(1, 10);
         List<Geometry> shapes = new ArrayList<>();
         for (int i = 0; i < size; i++) {
-            @SuppressWarnings("unchecked") Function<Boolean, Geometry> geometry = ESTestCase.randomFrom(
-                GeometryTestUtils::randomCircle,
-                GeometryTestUtils::randomLine,
-                GeometryTestUtils::randomPoint,
-                GeometryTestUtils::randomPolygon,
-                GeometryTestUtils::randomMultiLine,
-                GeometryTestUtils::randomMultiPoint,
-                GeometryTestUtils::randomMultiPolygon,
-                hasAlt ? GeometryTestUtils::randomPoint : (b) -> randomRectangle(),
-                level < 3 ? (b) -> randomGeometryCollection(level + 1, b) : GeometryTestUtils::randomPoint // don't build too deep
-            );
-            shapes.add(geometry.apply(hasAlt));
+            shapes.add(randomGeometry(level, hasAlt));
         }
         return new GeometryCollection<>(shapes);
     }
+
+    public static Geometry randomGeometry(boolean hasAlt) {
+        return randomGeometry(0, hasAlt);
+    }
+
+    private static Geometry randomGeometry(int level, boolean hasAlt) {
+        @SuppressWarnings("unchecked") Function<Boolean, Geometry> geometry = ESTestCase.randomFrom(
+            GeometryTestUtils::randomCircle,
+            GeometryTestUtils::randomLine,
+            GeometryTestUtils::randomPoint,
+            GeometryTestUtils::randomPolygon,
+            GeometryTestUtils::randomMultiLine,
+            GeometryTestUtils::randomMultiPoint,
+            GeometryTestUtils::randomMultiPolygon,
+            hasAlt ? GeometryTestUtils::randomPoint : (b) -> randomRectangle(),
+            level < 3 ? (b) -> randomGeometryCollection(level + 1, b) : GeometryTestUtils::randomPoint // don't build too deep
+        );
+        return geometry.apply(hasAlt);
+    }
 }