From 0355d0c40dde80e305ae1bd4e303fa33b2a1b572 Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Mon, 22 Jul 2019 12:01:44 -0400 Subject: [PATCH] Geo: deprecate ShapeBuilder in QueryBuilders Removes unnecessary now timeline decompositions from shape builders and deprecates ShapeBuilders in QueryBuilder in favor of libs/geo shapes. Relates to #40908 --- .../org/elasticsearch/common/geo/GeoJson.java | 18 +- .../elasticsearch/common/geo/GeometryIO.java | 307 ++++++++++++++++++ .../common/geo/GeometryIndexer.java | 5 +- .../geo/builders/LineStringBuilder.java | 28 +- .../geo/builders/MultiLineStringBuilder.java | 10 - .../common/geo/builders/PolygonBuilder.java | 71 +--- .../index/query/GeoShapeQueryBuilder.java | 185 +++++++++-- .../common/geo/GeoWKTShapeParserTests.java | 2 +- .../common/geo/GeometryIOTests.java | 110 +++++++ .../common/geo/GeometryParserTests.java | 4 +- .../common/geo/ShapeBuilderTests.java | 65 ++-- .../query/GeoShapeQueryBuilderTests.java | 2 +- .../elasticsearch/geo/GeometryTestUtils.java | 32 +- 13 files changed, 656 insertions(+), 183 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/geo/GeometryIO.java create mode 100644 server/src/test/java/org/elasticsearch/common/geo/GeometryIOTests.java diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java b/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java index ec5600836f1e4..032fddce6b77f 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoJson.java @@ -382,17 +382,17 @@ public static String getGeoJsonName(Geometry geometry) { 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 String visit(LinearRing ring) { @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"; } }); } diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeometryIO.java b/server/src/main/java/org/elasticsearch/common/geo/GeometryIO.java new file mode 100644 index 0000000000000..fb8b2327f3951 --- /dev/null +++ b/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() { + @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 readGeometryCollection(StreamInput in) throws IOException { + int size = in.readVInt(); + List 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 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 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 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 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; + } + } +} diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeometryIndexer.java b/server/src/main/java/org/elasticsearch/common/geo/GeometryIndexer.java index 48f84b1211b92..6d6270e49bbfb 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeometryIndexer.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeometryIndexer.java @@ -245,6 +245,7 @@ private List decompose(double dateline, double[] lons, double[] lats) { 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 @@ private void validateHole(LinearRing shell, LinearRing hole) { 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 @@ private static Edge[] concat(int component, boolean direction, Point[] points, f 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 + ")"); } } diff --git a/server/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java b/server/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java index 8e1e9d7a993b2..3b8e0522d4f4e 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java +++ b/server/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java @@ -24,7 +24,6 @@ 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.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 { public static final GeoShapeType TYPE = GeoShapeType.LINESTRING; @@ -126,18 +122,8 @@ public JtsGeometry buildS4J() { @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 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 decomposeS4J(GeometryFactory factory, Coordinate[] coordinates, ArrayList strings) { @@ -149,16 +135,6 @@ static ArrayList decomposeS4J(GeometryFactory factory, Coordinate[] return strings; } - static List decomposeGeometry(Coordinate[] coordinates, List 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. * diff --git a/server/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java b/server/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java index 24a8b3b226f36..558f74b1c997e 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java +++ b/server/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java @@ -154,16 +154,6 @@ public org.elasticsearch.geo.geometry.Geometry buildGeometry() { if (lines.isEmpty()) { return MultiLine.EMPTY; } - if (wrapdateline) { - List 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 linestrings = new ArrayList<>(lines.size()); for (int i = 0; i < lines.size(); ++i) { LineStringBuilder lsb = lines.get(i); diff --git a/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java b/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java index 97503efc033b0..2dc717cd7f5e0 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java +++ b/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.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,9 +46,9 @@ import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; +import static org.apache.lucene.geo.GeoUtils.orient; import static org.elasticsearch.common.geo.GeoUtils.normalizeLat; import static org.elasticsearch.common.geo.GeoUtils.normalizeLon; -import static org.apache.lucene.geo.GeoUtils.orient; /** * The {@link PolygonBuilder} implements the groundwork to create polygons. This contains @@ -235,12 +234,6 @@ public JtsGeometry buildS4J() { @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 +287,12 @@ public org.elasticsearch.geo.geometry.Polygon toPolygonGeometry() { 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 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 coordinates) { @@ -338,39 +328,6 @@ protected static Polygon polygonS4J(GeometryFactory factory, Coordinate[][] poly return factory.createPolygon(shell, holes); } - protected static org.elasticsearch.geo.geometry.Polygon polygonGeometry(Coordinate[][] polygon) { - List 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 +346,6 @@ protected static MultiPolygon multipolygonS4J(GeometryFactory factory, Coordinat return factory.createMultiPolygon(polygonSet); } - protected static org.elasticsearch.geo.geometry.MultiPolygon multipolygon(Coordinate[][][] polygons) { - List 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 diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java index b651a26d7e280..654aaf9751ca0 100644 --- a/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java @@ -40,9 +40,21 @@ 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.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 supplier; + private final Geometry shape; + private final Supplier supplier; private SpatialStrategy strategy; @@ -129,11 +146,28 @@ public class GeoShapeQueryBuilder extends AbstractQueryBuilder supplier, String indexedShapeId, + private GeoShapeQueryBuilder(String fieldName, Supplier supplier, String indexedShapeId, @Nullable String indexedShapeType) { this.fieldName = fieldName; this.shape = null; @@ -195,7 +229,7 @@ public GeoShapeQueryBuilder(StreamInput in) throws IOException { 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 @@ protected void doWriteTo(StreamOutput out) throws IOException { 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 String fieldName() { /** * @return the shape used in the Query */ - public ShapeBuilder shape() { + public Geometry shape() { return shape; } @@ -397,7 +431,6 @@ protected Query doToQuery(QueryShardContext context) { 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 @@ protected Query doToQuery(QueryShardContext context) { // 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() { + if (processedShape == null) { + return new MatchNoDocsQuery(); + } + + return processedShape.visit(new GeometryVisitor() { @Override public Query visit(Circle circle) { throw new QueryShardException(context, "Field [" + fieldName + "] found and unknown shape Circle"); @@ -536,7 +573,7 @@ public Query visit(org.elasticsearch.geo.geometry.Rectangle r) { * 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 listener) { + private void fetch(Client client, GetRequest getRequest, String path, ActionListener listener) { getRequest.preference("_local"); client.get(getRequest, new ActionListener(){ @@ -565,7 +602,7 @@ public void onResponse(GetResponse response) { 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 void onFailure(Exception e) { } - 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 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep 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 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws return supplier.get() == null ? this : new GeoShapeQueryBuilder(this.fieldName, supplier.get()).relation(relation).strategy (strategy); } else if (this.shape == null) { - SetOnce supplier = new SetOnce<>(); + SetOnce supplier = new SetOnce<>(); queryRewriteContext.registerAsyncAction((client, listener) -> { GetRequest getRequest; if (indexedShapeType == null) { @@ -816,4 +853,96 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws } 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 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 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; + } } diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java index 44016145750ed..d8559b3b1260e 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java @@ -470,7 +470,7 @@ public void testParseGeometryCollection() throws IOException, ParseException { } 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); } } diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeometryIOTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeometryIOTests.java new file mode 100644 index 0000000000000..14fc710e2683c --- /dev/null +++ b/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()); + } +} diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeometryParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeometryParserTests.java index 4cef86b1d570e..68492317f4791 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeometryParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeometryParserTests.java @@ -51,7 +51,7 @@ public void testGeoJsonParsing() throws Exception { 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 void testNullParsing() throws Exception { // 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); diff --git a/server/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java b/server/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java index 3c653db2d1537..bd6c4a2da557f 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java +++ b/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.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 void testLineStringBuilder() { .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 void testLineStringBuilder() { .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 void testLineStringBuilder() { .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 void testLineStringBuilder() { .coordinate(180.0, -80.0)); lsb.buildS4J(); - lsb.buildGeometry(); + buildGeometry(lsb); } public void testMultiLineString() { @@ -215,7 +214,7 @@ public void testMultiLineString() { ) ); mlsb.buildS4J(); - mlsb.buildGeometry(); + buildGeometry(mlsb); // LineString that needs to be wrapped new MultiLineStringBuilder() @@ -235,7 +234,7 @@ public void testMultiLineString() { ); mlsb.buildS4J(); - mlsb.buildGeometry(); + buildGeometry(mlsb); } public void testPolygonSelfIntersection() { @@ -283,7 +282,7 @@ public void testPolygonWrapping() { .close()); assertMultiPolygon(pb.buildS4J(), true); - assertMultiPolygon(pb.buildGeometry(), false); + assertMultiPolygon(buildGeometry(pb), false); } public void testLineStringWrapping() { @@ -295,7 +294,7 @@ public void testLineStringWrapping() { .close()); assertMultiLineString(lsb.buildS4J(), true); - assertMultiLineString(lsb.buildGeometry(), false); + assertMultiLineString(buildGeometry(lsb), false); } public void testDatelineOGC() { @@ -339,7 +338,7 @@ public void testDatelineOGC() { )); assertMultiPolygon(builder.close().buildS4J(), true); - assertMultiPolygon(builder.close().buildGeometry(), false); + assertMultiPolygon(buildGeometry(builder.close()), false); } public void testDateline() { @@ -383,7 +382,7 @@ public void testDateline() { )); assertMultiPolygon(builder.close().buildS4J(), true); - assertMultiPolygon(builder.close().buildGeometry(), false); + assertMultiPolygon(buildGeometry(builder.close()), false); } public void testComplexShapeWithHole() { @@ -458,7 +457,7 @@ public void testComplexShapeWithHole() { ) ); assertPolygon(builder.close().buildS4J(), true); - assertPolygon(builder.close().buildGeometry(), false); + assertPolygon(buildGeometry(builder.close()), false); } public void testShapeWithHoleAtEdgeEndPoints() { @@ -480,7 +479,7 @@ public void testShapeWithHoleAtEdgeEndPoints() { .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 void testShapeWithPointOnDateline() { .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 void testShapeWithEdgeAlongDateline() { ); 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 void testShapeWithEdgeAlongDateline() { ); assertPolygon(builder.close().buildS4J(), true); - assertPolygon(builder.close().buildGeometry(), false); + assertPolygon(buildGeometry(builder.close()), false); } public void testShapeWithBoundaryHoles() { @@ -537,7 +536,7 @@ public void testShapeWithBoundaryHoles() { )); 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 void testShapeWithBoundaryHoles() { )); assertMultiPolygon(builder.close().buildS4J(), true); - assertMultiPolygon(builder.close().buildGeometry(), false); + assertMultiPolygon(buildGeometry(builder.close()), false); } public void testShapeWithTangentialHole() { @@ -582,7 +581,7 @@ public void testShapeWithTangentialHole() { )); assertMultiPolygon(builder.close().buildS4J(), true); - assertMultiPolygon(builder.close().buildGeometry(), false); + assertMultiPolygon(buildGeometry(builder.close()), false); } public void testShapeWithInvalidTangentialHole() { @@ -606,7 +605,7 @@ public void testShapeWithInvalidTangentialHole() { 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 void testBoundaryShapeWithTangentialHole() { .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 void testBoundaryShapeWithInvalidTangentialHole() { 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 void testBoundaryShape() { ); assertPolygon(builder.close().buildS4J(), true); - assertPolygon(builder.close().buildGeometry(), false); + assertPolygon(buildGeometry(builder.close()), false); } public void testShapeWithAlternateOrientation() { @@ -686,7 +685,7 @@ public void testShapeWithAlternateOrientation() { ); 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 void testShapeWithAlternateOrientation() { ); assertMultiPolygon(builder.close().buildS4J(), true); - assertMultiPolygon(builder.close().buildGeometry(), false); + assertMultiPolygon(buildGeometry(builder.close()), false); } public void testInvalidShapeWithConsecutiveDuplicatePoints() { @@ -711,7 +710,7 @@ public void testInvalidShapeWithConsecutiveDuplicatePoints() { 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 void testInvalidSelfCrossingPolygon() { ); 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()); + } } diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java index a5311ed157c62..64f136a327269 100644 --- a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java @@ -198,7 +198,7 @@ public void testNoRelation() throws IOException { // 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(); } diff --git a/test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java b/test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java index 626f9b618d7b7..468d3bc0412f7 100644 --- a/test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java @@ -161,19 +161,27 @@ private static GeometryCollection randomGeometryCollection(int level, int size = ESTestCase.randomIntBetween(1, 10); List shapes = new ArrayList<>(); for (int i = 0; i < size; i++) { - @SuppressWarnings("unchecked") Function 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 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); + } }