From 57c579e7b70a98e101489b1cb9a59ee092fcb3ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 16 Dec 2015 12:21:17 +0100 Subject: [PATCH] Geo: Add validation of shapes to shape builders So far the validation of geo shapes was only taking place in the parse methods in ShapeBuilder. With the recent refactoring we no longer can rely on shapes being parsed from json, so the same kind of validation should take place when just using the java api. A lot of validation concerns the number of points a shape needs to have in order to be valid. Since this is not possible with current builders where points can be added one by one, the builder constructors are changed to require the mandatory parameters and validate those already at construction time. To help with constructing longer lists of points, a new utility PointsListBuilder is instroduces which can produce list of coordinates accepted by most of the other shape builder constructors. Also adding tests for invalid shape exceptions to the already existing shape builder tests. --- .../common/geo/builders/CircleBuilder.java | 10 +- .../common/geo/builders/EnvelopeBuilder.java | 25 +-- .../geo/builders/LineStringBuilder.java | 31 ++- .../geo/builders/MultiLineStringBuilder.java | 6 +- .../geo/builders/MultiPointBuilder.java | 17 +- .../geo/builders/MultiPolygonBuilder.java | 5 +- .../common/geo/builders/PointBuilder.java | 8 + .../common/geo/builders/PointCollection.java | 19 +- .../common/geo/builders/PointListBuilder.java | 98 ++++++++ .../common/geo/builders/PolygonBuilder.java | 89 ++++---- .../common/geo/builders/ShapeBuilder.java | 16 +- .../common/geo/builders/ShapeBuilders.java | 26 +-- .../common/geo/GeoJSONShapeParserTests.java | 1 + .../common/geo/ShapeBuilderTests.java | 212 +++++++++++------- .../AbstractShapeBuilderTestCase.java | 2 +- .../geo/builders/CircleBuilderTests.java | 28 ++- .../geo/builders/EnvelopeBuilderTests.java | 31 ++- .../geo/builders/LineStringBuilderTests.java | 25 +++ .../builders/MultiLineStringBuilderTests.java | 37 +-- .../geo/builders/MultiPointBuilderTests.java | 45 +++- .../geo/builders/PolygonBuilderTests.java | 3 +- .../query/GeoShapeQueryBuilderTests.java | 4 +- .../query/QueryDSLDocumentationTests.java | 7 +- .../elasticsearch/search/geo/GeoFilterIT.java | 114 +++++----- .../search/geo/GeoShapeQueryTests.java | 36 +-- .../test/geo/RandomShapeGenerator.java | 9 +- 26 files changed, 599 insertions(+), 305 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/common/geo/builders/PointListBuilder.java diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/CircleBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/CircleBuilder.java index eb77ef7a46a..16641aede20 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/CircleBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/CircleBuilder.java @@ -37,10 +37,18 @@ public class CircleBuilder extends ShapeBuilder { public static final CircleBuilder PROTOTYPE = new CircleBuilder(); - private DistanceUnit unit; + private DistanceUnit unit = DistanceUnit.DEFAULT; private double radius; private Coordinate center; + /** + * Creates a circle centered at [0.0, 0.0]. + * Center can be changed by calling {@link #center(Coordinate)} later. + */ + public CircleBuilder() { + this.center = ZERO_ZERO; + } + /** * Set the center of the circle * diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/EnvelopeBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/EnvelopeBuilder.java index 71b68207e74..708241c6666 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/EnvelopeBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/EnvelopeBuilder.java @@ -32,33 +32,22 @@ public class EnvelopeBuilder extends ShapeBuilder { public static final GeoShapeType TYPE = GeoShapeType.ENVELOPE; - public static final EnvelopeBuilder PROTOTYPE = new EnvelopeBuilder(); + public static final EnvelopeBuilder PROTOTYPE = new EnvelopeBuilder(new Coordinate(-1.0, 1.0), new Coordinate(1.0, -1.0)); private Coordinate topLeft; private Coordinate bottomRight; - public EnvelopeBuilder topLeft(Coordinate topLeft) { + public EnvelopeBuilder(Coordinate topLeft, Coordinate bottomRight) { + Objects.requireNonNull(topLeft, "topLeft of envelope cannot be null"); + Objects.requireNonNull(bottomRight, "bottomRight of envelope cannot be null"); this.topLeft = topLeft; - return this; - } - - public EnvelopeBuilder topLeft(double longitude, double latitude) { - return topLeft(coordinate(longitude, latitude)); + this.bottomRight = bottomRight; } public Coordinate topLeft() { return this.topLeft; } - public EnvelopeBuilder bottomRight(Coordinate bottomRight) { - this.bottomRight = bottomRight; - return this; - } - - public EnvelopeBuilder bottomRight(double longitude, double latitude) { - return bottomRight(coordinate(longitude, latitude)); - } - public Coordinate bottomRight() { return this.bottomRight; } @@ -110,8 +99,6 @@ public class EnvelopeBuilder extends ShapeBuilder { @Override public EnvelopeBuilder readFrom(StreamInput in) throws IOException { - return new EnvelopeBuilder() - .topLeft(readCoordinateFrom(in)) - .bottomRight(readCoordinateFrom(in)); + return new EnvelopeBuilder(readCoordinateFrom(in), readCoordinateFrom(in)); } } diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java index 0bf1ed8fa09..48752d013ae 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/LineStringBuilder.java @@ -33,11 +33,35 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Objects; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; + public class LineStringBuilder extends PointCollection { + /** + * Construct a new LineString. + * Per GeoJSON spec (http://geojson.org/geojson-spec.html#linestring) + * a LineString must contain two or more positions + * @param points the initial list of points + * @throw {@link IllegalArgumentException} if there are less then two points defined + */ + public LineStringBuilder(List points) { + super(points); + if (points.size() < 2) { + throw new IllegalArgumentException("invalid number of points in LineString (found [" + points.size()+ "] - must be >= 2)"); + } + } + public static final GeoShapeType TYPE = GeoShapeType.LINESTRING; - public static final LineStringBuilder PROTOTYPE = new LineStringBuilder(); + public static final LineStringBuilder PROTOTYPE = new LineStringBuilder(new PointListBuilder().point(0.0, 0.0).point(1.0, 1.0).list()); @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { @@ -172,11 +196,12 @@ public class LineStringBuilder extends PointCollection { @Override public LineStringBuilder readFrom(StreamInput in) throws IOException { - LineStringBuilder lineStringBuilder = new LineStringBuilder(); + PointListBuilder pl = new PointListBuilder(); int size = in.readVInt(); for (int i=0; i < size; i++) { - lineStringBuilder.point(readCoordinateFrom(in)); + pl.point(readCoordinateFrom(in)); } + LineStringBuilder lineStringBuilder = new LineStringBuilder(pl.list()); return lineStringBuilder; } } diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java index be09ae81836..4dc13454216 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilder.java @@ -27,6 +27,10 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.XContentBuilder; + import java.io.IOException; import java.util.ArrayList; import java.util.Iterator; @@ -46,7 +50,7 @@ public class MultiLineStringBuilder extends ShapeBuilder { } public MultiLineStringBuilder linestring(Coordinate[] coordinates) { - return this.linestring(new LineStringBuilder().points(coordinates)); + return this.linestring(new LineStringBuilder(new PointListBuilder().points(coordinates).list())); } public Coordinate[][] coordinates() { diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java index b0e86a819aa..ebc5d822d3b 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPointBuilder.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Objects; @@ -36,7 +37,15 @@ public class MultiPointBuilder extends PointCollection { public static final GeoShapeType TYPE = GeoShapeType.MULTIPOINT; - public final static MultiPointBuilder PROTOTYPE = new MultiPointBuilder(); + public final static MultiPointBuilder PROTOTYPE = new MultiPointBuilder(Arrays.asList(new Coordinate[]{new Coordinate(0.0, 0.0)})); + + /** + * Create a new {@link MultiPointBuilder}. + * @param points needs at least two points to be valid, otherwise will throw an exception + */ + public MultiPointBuilder(List points) { + super(points); + } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { @@ -93,11 +102,13 @@ public class MultiPointBuilder extends PointCollection { @Override public MultiPointBuilder readFrom(StreamInput in) throws IOException { - MultiPointBuilder multiPointBuilder = new MultiPointBuilder(); int size = in.readVInt(); + List points = new ArrayList(size); for (int i=0; i < size; i++) { - multiPointBuilder.point(readCoordinateFrom(in)); + points.add(readCoordinateFrom(in)); } + MultiPointBuilder multiPointBuilder = new MultiPointBuilder(points); + return multiPointBuilder; } } diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPolygonBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPolygonBuilder.java index cff06dbfe59..37eafb57e9c 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPolygonBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/MultiPolygonBuilder.java @@ -21,6 +21,7 @@ package org.elasticsearch.common.geo.builders; import com.spatial4j.core.shape.Shape; import com.vividsolutions.jts.geom.Coordinate; + import org.elasticsearch.common.geo.XShapeCollection; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -28,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Objects; @@ -58,8 +60,7 @@ public class MultiPolygonBuilder extends ShapeBuilder { * {@link MultiPolygonBuilder} to the polygon if polygon has different orientation. */ public MultiPolygonBuilder polygon(PolygonBuilder polygon) { - PolygonBuilder pb = new PolygonBuilder(this.orientation); - pb.points(polygon.shell().coordinates(false)); + PolygonBuilder pb = new PolygonBuilder(Arrays.asList(polygon.shell().coordinates(false)), this.orientation); for (LineStringBuilder hole : polygon.holes()) { pb.hole(hole); } diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/PointBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/PointBuilder.java index afb713cb09d..1cee6525e7a 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/PointBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/PointBuilder.java @@ -21,6 +21,7 @@ package org.elasticsearch.common.geo.builders; import com.spatial4j.core.shape.Point; import com.vividsolutions.jts.geom.Coordinate; + import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -35,6 +36,13 @@ public class PointBuilder extends ShapeBuilder { private Coordinate coordinate; + /** + * Create a point at [0.0,0.0] + */ + public PointBuilder() { + this.coordinate = ZERO_ZERO; + } + public PointBuilder coordinate(Coordinate coordinate) { this.coordinate = coordinate; return this; diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/PointCollection.java b/core/src/main/java/org/elasticsearch/common/geo/builders/PointCollection.java index b48aacd857b..58b85fff760 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/PointCollection.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/PointCollection.java @@ -20,25 +20,30 @@ package org.elasticsearch.common.geo.builders; import com.vividsolutions.jts.geom.Coordinate; + import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.List; /** * The {@link PointCollection} is an abstract base implementation for all GeoShapes. It simply handles a set of points. */ public abstract class PointCollection> extends ShapeBuilder { - protected final ArrayList points; + protected final List points; - protected PointCollection() { - this(new ArrayList()); - } - - protected PointCollection(ArrayList points) { + /** + * Construct a new collection of points. + * @param points an initial list of points + * @throws IllegalArgumentException if points is null or empty + */ + protected PointCollection(List points) { + if (points == null || points.size() == 0) { + throw new IllegalArgumentException("cannot create point collection with empty set of points"); + } this.points = points; } diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/PointListBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/PointListBuilder.java new file mode 100644 index 00000000000..febb3dcb5ba --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/PointListBuilder.java @@ -0,0 +1,98 @@ +/* + * 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.builders; + +import com.vividsolutions.jts.geom.Coordinate; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +/** + * A builder for a list of points (of {@link Coordinate} type). + * Enables chaining of individual points either as long/lat pairs + * or as {@link Coordinate} elements, arrays or collections. + */ +public class PointListBuilder { + + private final List points = new ArrayList<>(); + + /** + * Add a new point to the collection + * @param longitude longitude of the coordinate + * @param latitude latitude of the coordinate + * @return this + */ + public PointListBuilder point(double longitude, double latitude) { + return this.point(new Coordinate(longitude, latitude)); + } + + /** + * Add a new point to the collection + * @param coordinate coordinate of the point + * @return this + */ + public PointListBuilder point(Coordinate coordinate) { + this.points.add(coordinate); + return this; + } + + /** + * Add a array of points to the collection + * + * @param coordinates array of {@link Coordinate}s to add + * @return this + */ + public PointListBuilder points(Coordinate...coordinates) { + return this.points(Arrays.asList(coordinates)); + } + + /** + * Add a collection of points to the collection + * + * @param coordinates array of {@link Coordinate}s to add + * @return this + */ + public PointListBuilder points(Collection coordinates) { + this.points.addAll(coordinates); + return this; + } + + /** + * Closes the current list of points by adding the starting point as the end point + * if they are not already the same + */ + public PointListBuilder close() { + Coordinate start = points.get(0); + Coordinate end = points.get(points.size()-1); + if(start.x != end.x || start.y != end.y) { + points.add(start); + } + return this; + } + + /** + * @return the current list of points + */ + public List list() { + return new ArrayList<>(this.points); + } +} diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java index 026fc9aa170..189b2c9ce86 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java @@ -52,7 +52,8 @@ import java.util.concurrent.atomic.AtomicBoolean; public class PolygonBuilder extends ShapeBuilder { public static final GeoShapeType TYPE = GeoShapeType.POLYGON; - public static final PolygonBuilder PROTOTYPE = new PolygonBuilder(); + public static final PolygonBuilder PROTOTYPE = new PolygonBuilder(new PointListBuilder().point(0.0, 0.0).point(0.0, 1.0) + .point(1.0, 0.0).point(0.0, 0.0).list()); private static final Coordinate[][] EMPTY = new Coordinate[0][]; @@ -64,55 +65,40 @@ public class PolygonBuilder extends ShapeBuilder { // List of line strings defining the holes of the polygon private final ArrayList holes = new ArrayList<>(); - public PolygonBuilder() { - this(Orientation.RIGHT); + public PolygonBuilder(List points, Orientation orientation) { + this(points, orientation, false); } - public PolygonBuilder(Orientation orientation) { - this(new ArrayList(), orientation); - } - - public PolygonBuilder(ArrayList points, Orientation orientation) { + public PolygonBuilder(List points, Orientation orientation, boolean coerce) { this.orientation = orientation; - this.shell = new LineStringBuilder().points(points); + this.shell = validateLinearRing(new LineStringBuilder(points), coerce); + } + + public PolygonBuilder(List points) { + this(points, Orientation.RIGHT); } public Orientation orientation() { return this.orientation; } - public PolygonBuilder point(double longitude, double latitude) { - shell.point(longitude, latitude); - return this; - } - - /** - * Add a point to the shell of the polygon - * @param coordinate coordinate of the new point - * @return this - */ - public PolygonBuilder point(Coordinate coordinate) { - shell.point(coordinate); - return this; - } - - /** - * Add an array of points to the shell of the polygon - * @param coordinates coordinates of the new points to add - * @return this - */ - public PolygonBuilder points(Coordinate...coordinates) { - shell.points(coordinates); - return this; - } - /** * Add a new hole to the polygon * @param hole linear ring defining the hole * @return this */ public PolygonBuilder hole(LineStringBuilder hole) { - holes.add(hole); + return this.hole(hole, false); + } + + /** + * Add a new hole to the polygon + * @param hole linear ring defining the hole + * @param coerce if set to true, will close the hole by adding starting point as end point + * @return this + */ + public PolygonBuilder hole(LineStringBuilder hole, boolean coerce) { + holes.add(validateLinearRing(hole, coerce)); return this; } @@ -138,6 +124,30 @@ public class PolygonBuilder extends ShapeBuilder { return this; } + private static LineStringBuilder validateLinearRing(LineStringBuilder linestring, boolean coerce) { + /** + * Per GeoJSON spec (http://geojson.org/geojson-spec.html#linestring) + * A LinearRing is closed LineString with 4 or more positions. The first and last positions + * are equivalent (they represent equivalent points). Though a LinearRing is not explicitly + * represented as a GeoJSON geometry type, it is referred to in the Polygon geometry type definition. + */ + int numValidPts; + List points = linestring.points; + if (points.size() < (numValidPts = (coerce) ? 3 : 4)) { + throw new IllegalArgumentException( + "invalid number of points in LinearRing (found [" + points.size() + "] - must be >= " + numValidPts + ")"); + } + + if (!points.get(0).equals(points.get(points.size() - 1))) { + if (coerce) { + points.add(points.get(0)); + } else { + throw new IllegalArgumentException("invalid LinearRing found (coordinates are not closed)"); + } + } + return linestring; + } + /** * Validates only 1 vertex is tangential (shared) between the interior and exterior of a polygon */ @@ -235,7 +245,7 @@ public class PolygonBuilder extends ShapeBuilder { return factory.createPolygon(shell, holes); } - protected static LinearRing linearRing(GeometryFactory factory, ArrayList coordinates) { + protected static LinearRing linearRing(GeometryFactory factory, List coordinates) { return factory.createLinearRing(coordinates.toArray(new Coordinate[coordinates.size()])); } @@ -711,8 +721,8 @@ public class PolygonBuilder extends ShapeBuilder { @Override public void writeTo(StreamOutput out) throws IOException { - orientation.writeTo(out); shell.writeTo(out); + orientation.writeTo(out); out.writeVInt(holes.size()); for (LineStringBuilder hole : holes) { hole.writeTo(out); @@ -721,8 +731,9 @@ public class PolygonBuilder extends ShapeBuilder { @Override public PolygonBuilder readFrom(StreamInput in) throws IOException { - PolygonBuilder polyBuilder = new PolygonBuilder(Orientation.readFrom(in)); - polyBuilder.shell = LineStringBuilder.PROTOTYPE.readFrom(in); + LineStringBuilder shell = LineStringBuilder.PROTOTYPE.readFrom(in); + Orientation orientation = Orientation.readFrom(in); + PolygonBuilder polyBuilder = new PolygonBuilder(shell.points, orientation); int holes = in.readVInt(); for (int i = 0; i < holes; i++) { polyBuilder.hole(LineStringBuilder.PROTOTYPE.readFrom(in)); diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java b/core/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java index d286237e547..e080ee4e201 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilder.java @@ -64,6 +64,11 @@ public abstract class ShapeBuilder extends ToXContentToBytes implements NamedWri } public static final double DATELINE = 180; + + /** + * coordinate at [0.0, 0.0] + */ + public static final Coordinate ZERO_ZERO = new Coordinate(0.0, 0.0); // TODO how might we use JtsSpatialContextFactory to configure the context (esp. for non-geo)? public static final JtsSpatialContext SPATIAL_CONTEXT = JtsSpatialContext.GEO; public static final GeometryFactory FACTORY = SPATIAL_CONTEXT.getGeometryFactory(); @@ -569,7 +574,7 @@ public abstract class ShapeBuilder extends ToXContentToBytes implements NamedWri uL = new Coordinate(Math.min(uL.x, lR.x), Math.max(uL.y, lR.y)); lR = new Coordinate(Math.max(uLtmp.x, lR.x), Math.min(uLtmp.y, lR.y)); } - return ShapeBuilders.newEnvelope().topLeft(uL).bottomRight(lR); + return ShapeBuilders.newEnvelope(uL, lR); } protected static void validateMultiPointNode(CoordinateNode coordinates) { @@ -589,12 +594,11 @@ public abstract class ShapeBuilder extends ToXContentToBytes implements NamedWri protected static MultiPointBuilder parseMultiPoint(CoordinateNode coordinates) { validateMultiPointNode(coordinates); - - MultiPointBuilder points = new MultiPointBuilder(); + PointListBuilder points = new PointListBuilder(); for (CoordinateNode node : coordinates.children) { points.point(node.coordinate); } - return points; + return new MultiPointBuilder(points.list()); } protected static LineStringBuilder parseLineString(CoordinateNode coordinates) { @@ -607,11 +611,11 @@ public abstract class ShapeBuilder extends ToXContentToBytes implements NamedWri throw new ElasticsearchParseException("invalid number of points in LineString (found [{}] - must be >= 2)", coordinates.children.size()); } - LineStringBuilder line = ShapeBuilders.newLineString(); + PointListBuilder line = new PointListBuilder(); for (CoordinateNode node : coordinates.children) { line.point(node.coordinate); } - return line; + return ShapeBuilders.newLineString(line.list()); } protected static MultiLineStringBuilder parseMultiLine(CoordinateNode coordinates) { diff --git a/core/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilders.java b/core/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilders.java index 61d7a9cd07e..1fb36fe21e1 100644 --- a/core/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilders.java +++ b/core/src/main/java/org/elasticsearch/common/geo/builders/ShapeBuilders.java @@ -21,6 +21,8 @@ package org.elasticsearch.common.geo.builders; import com.vividsolutions.jts.geom.Coordinate; +import java.util.List; + /** * A collection of static methods for creating ShapeBuilders. */ @@ -50,16 +52,16 @@ public class ShapeBuilders { * Create a new set of points * @return new {@link MultiPointBuilder} */ - public static MultiPointBuilder newMultiPoint() { - return new MultiPointBuilder(); + public static MultiPointBuilder newMultiPoint(List points) { + return new MultiPointBuilder(points); } /** * Create a new lineString * @return a new {@link LineStringBuilder} */ - public static LineStringBuilder newLineString() { - return new LineStringBuilder(); + public static LineStringBuilder newLineString(List list) { + return new LineStringBuilder(list); } /** @@ -74,16 +76,8 @@ public class ShapeBuilders { * Create a new Polygon * @return a new {@link PointBuilder} */ - public static PolygonBuilder newPolygon() { - return new PolygonBuilder(); - } - - /** - * Create a new Polygon - * @return a new {@link PointBuilder} - */ - public static PolygonBuilder newPolygon(ShapeBuilder.Orientation orientation) { - return new PolygonBuilder(orientation); + public static PolygonBuilder newPolygon(List shell) { + return new PolygonBuilder(shell); } /** @@ -124,7 +118,7 @@ public class ShapeBuilders { * * @return a new {@link EnvelopeBuilder} */ - public static EnvelopeBuilder newEnvelope() { - return new EnvelopeBuilder(); + public static EnvelopeBuilder newEnvelope(Coordinate topLeft, Coordinate bottomRight) { + return new EnvelopeBuilder(topLeft, bottomRight); } } diff --git a/core/src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java b/core/src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java index d65137b21b9..6e4d3867fde 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java +++ b/core/src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java @@ -34,6 +34,7 @@ import com.vividsolutions.jts.geom.LinearRing; import com.vividsolutions.jts.geom.MultiLineString; import com.vividsolutions.jts.geom.Point; import com.vividsolutions.jts.geom.Polygon; + import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.xcontent.XContentFactory; diff --git a/core/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java b/core/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java index ac439ff12e0..0837361ab15 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java @@ -29,6 +29,7 @@ import com.vividsolutions.jts.geom.Coordinate; import com.vividsolutions.jts.geom.LineString; import com.vividsolutions.jts.geom.Polygon; import org.elasticsearch.common.geo.builders.LineStringBuilder; +import org.elasticsearch.common.geo.builders.PointListBuilder; import org.elasticsearch.common.geo.builders.PolygonBuilder; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.geo.builders.ShapeBuilders; @@ -50,7 +51,7 @@ public class ShapeBuilderTests extends ESTestCase { } public void testNewRectangle() { - Rectangle rectangle = ShapeBuilders.newEnvelope().topLeft(-45, 30).bottomRight(45, -30).build(); + Rectangle rectangle = ShapeBuilders.newEnvelope(new Coordinate(-45, 30), new Coordinate(45, -30)).build(); assertEquals(-45D, rectangle.getMinX(), 0.0d); assertEquals(-30D, rectangle.getMinY(), 0.0d); assertEquals(45D, rectangle.getMaxX(), 0.0d); @@ -58,12 +59,12 @@ public class ShapeBuilderTests extends ESTestCase { } public void testNewPolygon() { - Polygon polygon = ShapeBuilders.newPolygon() + Polygon polygon = ShapeBuilders.newPolygon(new PointListBuilder() .point(-45, 30) .point(45, 30) .point(45, -30) .point(-45, -30) - .point(-45, 30).toPolygon(); + .point(-45, 30).list()).toPolygon(); LineString exterior = polygon.getExteriorRing(); assertEquals(exterior.getCoordinateN(0), new Coordinate(-45, 30)); @@ -73,12 +74,12 @@ public class ShapeBuilderTests extends ESTestCase { } public void testNewPolygon_coordinate() { - Polygon polygon = ShapeBuilders.newPolygon() + Polygon polygon = ShapeBuilders.newPolygon(new PointListBuilder() .point(new Coordinate(-45, 30)) .point(new Coordinate(45, 30)) .point(new Coordinate(45, -30)) .point(new Coordinate(-45, -30)) - .point(new Coordinate(-45, 30)).toPolygon(); + .point(new Coordinate(-45, 30)).list()).toPolygon(); LineString exterior = polygon.getExteriorRing(); assertEquals(exterior.getCoordinateN(0), new Coordinate(-45, 30)); @@ -88,8 +89,9 @@ public class ShapeBuilderTests extends ESTestCase { } public void testNewPolygon_coordinates() { - Polygon polygon = ShapeBuilders.newPolygon() - .points(new Coordinate(-45, 30), new Coordinate(45, 30), new Coordinate(45, -30), new Coordinate(-45, -30), new Coordinate(-45, 30)).toPolygon(); + Polygon polygon = ShapeBuilders.newPolygon(new PointListBuilder() + .points(new Coordinate(-45, 30), new Coordinate(45, 30), new Coordinate(45, -30), new Coordinate(-45, -30), new Coordinate(-45, 30)) + .list()).toPolygon(); LineString exterior = polygon.getExteriorRing(); assertEquals(exterior.getCoordinateN(0), new Coordinate(-45, 30)); @@ -100,7 +102,7 @@ public class ShapeBuilderTests extends ESTestCase { public void testLineStringBuilder() { // Building a simple LineString - ShapeBuilders.newLineString() + ShapeBuilders.newLineString(new PointListBuilder() .point(-130.0, 55.0) .point(-130.0, -40.0) .point(-15.0, -40.0) @@ -108,10 +110,10 @@ public class ShapeBuilderTests extends ESTestCase { .point(-45.0, 50.0) .point(-45.0, -15.0) .point(-110.0, -15.0) - .point(-110.0, 55.0).build(); + .point(-110.0, 55.0).list()).build(); // Building a linestring that needs to be wrapped - ShapeBuilders.newLineString() + ShapeBuilders.newLineString(new PointListBuilder() .point(100.0, 50.0) .point(110.0, -40.0) .point(240.0, -40.0) @@ -120,66 +122,73 @@ public class ShapeBuilderTests extends ESTestCase { .point(200.0, -30.0) .point(130.0, -30.0) .point(130.0, 60.0) + .list()) .build(); // Building a lineString on the dateline - ShapeBuilders.newLineString() + ShapeBuilders.newLineString(new PointListBuilder() .point(-180.0, 80.0) .point(-180.0, 40.0) .point(-180.0, -40.0) .point(-180.0, -80.0) + .list()) .build(); // Building a lineString on the dateline - ShapeBuilders.newLineString() + ShapeBuilders.newLineString(new PointListBuilder() .point(180.0, 80.0) .point(180.0, 40.0) .point(180.0, -40.0) .point(180.0, -80.0) + .list()) .build(); } public void testMultiLineString() { ShapeBuilders.newMultiLinestring() - .linestring(new LineStringBuilder() + .linestring(new LineStringBuilder(new PointListBuilder() .point(-100.0, 50.0) .point(50.0, 50.0) .point(50.0, 20.0) .point(-100.0, 20.0) + .list()) ) - .linestring(new LineStringBuilder() + .linestring(new LineStringBuilder(new PointListBuilder() .point(-100.0, 20.0) .point(50.0, 20.0) .point(50.0, 0.0) .point(-100.0, 0.0) + .list()) ) .build(); // LineString that needs to be wrappped ShapeBuilders.newMultiLinestring() - .linestring(new LineStringBuilder() + .linestring(new LineStringBuilder(new PointListBuilder() .point(150.0, 60.0) .point(200.0, 60.0) .point(200.0, 40.0) .point(150.0, 40.0) + .list()) ) - .linestring(new LineStringBuilder() + .linestring(new LineStringBuilder(new PointListBuilder() .point(150.0, 20.0) .point(200.0, 20.0) .point(200.0, 0.0) .point(150.0, 0.0) + .list()) ) .build(); } public void testPolygonSelfIntersection() { try { - ShapeBuilders.newPolygon() + ShapeBuilders.newPolygon(new PointListBuilder() .point(-40.0, 50.0) .point(40.0, 50.0) .point(-40.0, -50.0) - .point(40.0, -50.0) - .close().build(); + .point(40.0, -50.0).close().list()) + .build(); fail("Expected InvalidShapeException"); } catch (InvalidShapeException e) { assertThat(e.getMessage(), containsString("Self-intersection at or near point (0.0")); @@ -212,22 +221,26 @@ public class ShapeBuilderTests extends ESTestCase { } public void testPolygonWrapping() { - Shape shape = ShapeBuilders.newPolygon() + Shape shape = ShapeBuilders.newPolygon(new PointListBuilder() .point(-150.0, 65.0) .point(-250.0, 65.0) .point(-250.0, -65.0) .point(-150.0, -65.0) - .close().build(); + .close() + .list()) + .build(); assertMultiPolygon(shape); } public void testLineStringWrapping() { - Shape shape = ShapeBuilders.newLineString() + Shape shape = ShapeBuilders.newLineString(new PointListBuilder() .point(-150.0, 65.0) .point(-250.0, 65.0) .point(-250.0, -65.0) .point(-150.0, -65.0) + .close() + .list()) .build(); assertMultiLineString(shape); } @@ -238,7 +251,7 @@ public class ShapeBuilderTests extends ESTestCase { // expected results: 3 polygons, 1 with a hole // a giant c shape - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(174,0) .point(-176,0) .point(-176,3) @@ -247,10 +260,11 @@ public class ShapeBuilderTests extends ESTestCase { .point(-176,5) .point(-176,8) .point(174,8) - .point(174,0); + .point(174,0) + .list()); // 3/4 of an embedded 'c', crossing dateline once - builder.hole(new LineStringBuilder() + builder.hole(new LineStringBuilder(new PointListBuilder() .point(175, 1) .point(175, 7) .point(-178, 7) @@ -259,15 +273,17 @@ public class ShapeBuilderTests extends ESTestCase { .point(176, 2) .point(179, 2) .point(179,1) - .point(175, 1)); + .point(175, 1) + .list())); // embedded hole right of the dateline - builder.hole(new LineStringBuilder() + builder.hole(new LineStringBuilder(new PointListBuilder() .point(-179, 1) .point(-179, 2) .point(-177, 2) .point(-177,1) - .point(-179,1)); + .point(-179,1) + .list())); Shape shape = builder.close().build(); assertMultiPolygon(shape); @@ -279,7 +295,7 @@ public class ShapeBuilderTests extends ESTestCase { // expected results: 3 polygons, 1 with a hole // a giant c shape - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(-186,0) .point(-176,0) .point(-176,3) @@ -288,10 +304,11 @@ public class ShapeBuilderTests extends ESTestCase { .point(-176,5) .point(-176,8) .point(-186,8) - .point(-186,0); + .point(-186,0) + .list()); // 3/4 of an embedded 'c', crossing dateline once - builder.hole(new LineStringBuilder() + builder.hole(new LineStringBuilder(new PointListBuilder() .point(-185,1) .point(-181,1) .point(-181,2) @@ -300,22 +317,24 @@ public class ShapeBuilderTests extends ESTestCase { .point(-178,6) .point(-178,7) .point(-185,7) - .point(-185,1)); + .point(-185,1) + .list())); // embedded hole right of the dateline - builder.hole(new LineStringBuilder() + builder.hole(new LineStringBuilder(new PointListBuilder() .point(-179,1) .point(-177,1) .point(-177,2) .point(-179,2) - .point(-179,1)); + .point(-179,1) + .list())); Shape shape = builder.close().build(); assertMultiPolygon(shape); } public void testComplexShapeWithHole() { - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(-85.0018514,37.1311314) .point(-85.0016645,37.1315293) .point(-85.0016246,37.1317069) @@ -353,9 +372,10 @@ public class ShapeBuilderTests extends ESTestCase { .point(-85.0009461,37.1311684) .point(-85.0011373,37.1311515) .point(-85.0016455,37.1310491) - .point(-85.0018514,37.1311314); + .point(-85.0018514,37.1311314) + .list()); - builder.hole(new LineStringBuilder() + builder.hole(new LineStringBuilder(new PointListBuilder() .point(-85.0000002,37.1317672) .point(-85.0001983,37.1317538) .point(-85.0003378,37.1317582) @@ -381,39 +401,44 @@ public class ShapeBuilderTests extends ESTestCase { .point(-84.9993527,37.1317788) .point(-84.9994931,37.1318061) .point(-84.9996815,37.1317979) - .point(-85.0000002,37.1317672)); + .point(-85.0000002,37.1317672) + .list()) + ); Shape shape = builder.close().build(); assertPolygon(shape); } public void testShapeWithHoleAtEdgeEndPoints() { - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(-4, 2) .point(4, 2) .point(6, 0) .point(4, -2) .point(-4, -2) .point(-6, 0) - .point(-4, 2); + .point(-4, 2) + .list()); - builder.hole(new LineStringBuilder() + builder.hole(new LineStringBuilder(new PointListBuilder() .point(4, 1) .point(4, -1) .point(-4, -1) .point(-4, 1) - .point(4, 1)); + .point(4, 1) + .list())); Shape shape = builder.close().build(); assertPolygon(shape); } public void testShapeWithPointOnDateline() { - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(180, 0) .point(176, 4) .point(176, -4) - .point(180, 0); + .point(180, 0) + .list()); Shape shape = builder.close().build(); assertPolygon(shape); @@ -421,21 +446,23 @@ public class ShapeBuilderTests extends ESTestCase { public void testShapeWithEdgeAlongDateline() { // test case 1: test the positive side of the dateline - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(180, 0) .point(176, 4) .point(180, -4) - .point(180, 0); + .point(180, 0) + .list()); Shape shape = builder.close().build(); assertPolygon(shape); // test case 2: test the negative side of the dateline - builder = ShapeBuilders.newPolygon() + builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(-176, 4) .point(-180, 0) .point(-180, -4) - .point(-176, 4); + .point(-176, 4) + .list()); shape = builder.close().build(); assertPolygon(shape); @@ -443,73 +470,85 @@ public class ShapeBuilderTests extends ESTestCase { public void testShapeWithBoundaryHoles() { // test case 1: test the positive side of the dateline - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(-177, 10) .point(176, 15) .point(172, 0) .point(176, -15) .point(-177, -10) - .point(-177, 10); - builder.hole(new LineStringBuilder() + .point(-177, 10) + .list()); + builder.hole(new LineStringBuilder(new PointListBuilder() .point(176, 10) .point(180, 5) .point(180, -5) .point(176, -10) - .point(176, 10)); + .point(176, 10) + .list())); Shape shape = builder.close().build(); assertMultiPolygon(shape); // test case 2: test the negative side of the dateline - builder = ShapeBuilders.newPolygon() + builder = ShapeBuilders.newPolygon( + new PointListBuilder() .point(-176, 15) .point(179, 10) .point(179, -10) .point(-176, -15) - .point(-172, 0); - builder.hole(new LineStringBuilder() + .point(-172, 0) + .close() + .list()); + builder.hole(new LineStringBuilder( + new PointListBuilder() .point(-176, 10) .point(-176, -10) .point(-180, -5) .point(-180, 5) - .point(-176, 10)); + .point(-176, 10) + .close() + .list())); shape = builder.close().build(); assertMultiPolygon(shape); } public void testShapeWithTangentialHole() { // test a shape with one tangential (shared) vertex (should pass) - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(179, 10) .point(168, 15) .point(164, 0) .point(166, -15) .point(179, -10) - .point(179, 10); - builder.hole(new LineStringBuilder() + .point(179, 10) + .list()); + builder.hole(new LineStringBuilder(new PointListBuilder() .point(-177, 10) .point(-178, -10) .point(-180, -5) .point(-180, 5) - .point(-177, 10)); + .point(-177, 10) + .list())); Shape shape = builder.close().build(); assertMultiPolygon(shape); } public void testShapeWithInvalidTangentialHole() { // test a shape with one invalid tangential (shared) vertex (should throw exception) - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(179, 10) .point(168, 15) .point(164, 0) .point(166, -15) .point(179, -10) - .point(179, 10); - builder.hole(new LineStringBuilder() + .point(179, 10) + .list()); + builder.hole(new LineStringBuilder(new PointListBuilder() .point(164, 0) .point(175, 10) .point(175, 5) .point(179, -10) - .point(164, 0)); + .point(164, 0) + .list())); try { builder.close().build(); fail("Expected InvalidShapeException"); @@ -520,43 +559,48 @@ public class ShapeBuilderTests extends ESTestCase { public void testBoundaryShapeWithTangentialHole() { // test a shape with one tangential (shared) vertex for each hole (should pass) - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(-177, 10) .point(176, 15) .point(172, 0) .point(176, -15) .point(-177, -10) - .point(-177, 10); - builder.hole(new LineStringBuilder() + .point(-177, 10) + .list()); + builder.hole(new LineStringBuilder(new PointListBuilder() .point(-177, 10) .point(-178, -10) .point(-180, -5) .point(-180, 5) - .point(-177, 10)); - builder.hole(new LineStringBuilder() + .point(-177, 10) + .list())); + builder.hole(new LineStringBuilder(new PointListBuilder() .point(172, 0) .point(176, 10) .point(176, -5) - .point(172, 0)); + .point(172, 0) + .list())); Shape shape = builder.close().build(); assertMultiPolygon(shape); } public void testBoundaryShapeWithInvalidTangentialHole() { // test shape with two tangential (shared) vertices (should throw exception) - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(-177, 10) .point(176, 15) .point(172, 0) .point(176, -15) .point(-177, -10) - .point(-177, 10); - builder.hole(new LineStringBuilder() + .point(-177, 10) + .list()); + builder.hole(new LineStringBuilder(new PointListBuilder() .point(-177, 10) .point(172, 0) .point(180, -5) .point(176, -10) - .point(-177, 10)); + .point(-177, 10) + .list())); try { builder.close().build(); fail("Expected InvalidShapeException"); @@ -569,11 +613,12 @@ public class ShapeBuilderTests extends ESTestCase { * Test an enveloping polygon around the max mercator bounds */ public void testBoundaryShape() { - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(-180, 90) .point(180, 90) .point(180, -90) - .point(-180, -90); + .point(-180, 90) + .list()); Shape shape = builder.close().build(); @@ -582,21 +627,23 @@ public class ShapeBuilderTests extends ESTestCase { public void testShapeWithAlternateOrientation() { // cw: should produce a multi polygon spanning hemispheres - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(180, 0) .point(176, 4) .point(-176, 4) - .point(180, 0); + .point(180, 0) + .list()); Shape shape = builder.close().build(); assertPolygon(shape); // cw: geo core will convert to ccw across the dateline - builder = ShapeBuilders.newPolygon() + builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(180, 0) .point(-176, 4) .point(176, 4) - .point(180, 0); + .point(180, 0) + .list()); shape = builder.close().build(); @@ -604,12 +651,13 @@ public class ShapeBuilderTests extends ESTestCase { } public void testInvalidShapeWithConsecutiveDuplicatePoints() { - PolygonBuilder builder = ShapeBuilders.newPolygon() + PolygonBuilder builder = ShapeBuilders.newPolygon(new PointListBuilder() .point(180, 0) .point(176, 4) .point(176, 4) .point(-176, 4) - .point(180, 0); + .point(180, 0) + .list()); try { builder.close().build(); fail("Expected InvalidShapeException"); diff --git a/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java b/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java index 279e31aadd4..03294b8e49c 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java +++ b/core/src/test/java/org/elasticsearch/common/geo/builders/AbstractShapeBuilderTestCase.java @@ -40,7 +40,7 @@ import static org.hamcrest.Matchers.not; public abstract class AbstractShapeBuilderTestCase extends ESTestCase { - private static final int NUMBER_OF_TESTBUILDERS = 20; + private static final int NUMBER_OF_TESTBUILDERS = 100; private static NamedWriteableRegistry namedWriteableRegistry; /** diff --git a/core/src/test/java/org/elasticsearch/common/geo/builders/CircleBuilderTests.java b/core/src/test/java/org/elasticsearch/common/geo/builders/CircleBuilderTests.java index bd90fefc922..3c9ca34c6ea 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/builders/CircleBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/common/geo/builders/CircleBuilderTests.java @@ -42,9 +42,18 @@ public class CircleBuilderTests extends AbstractShapeBuilderTestCase 0.0 || original.center().y > 0.0) { + mutation.center(new Coordinate(original.center().x/2, original.center().y/2)); + } else { + // original center was 0.0, 0.0 + mutation.center(randomDouble() + 0.1, randomDouble() + 0.1); + } } else if (randomBoolean()) { - radius = radius/2; + if (radius > 0) { + radius = radius/2; + } else { + radius = randomDouble() + 0.1; + } } else { DistanceUnit newRandom = unit; while (newRandom == unit) { @@ -56,10 +65,15 @@ public class CircleBuilderTests extends AbstractShapeBuilderTestCase { + public void testInvalidConstructorArgs() { + try { + new EnvelopeBuilder(null, new Coordinate(1.0, -1.0)); + fail("Exception expected"); + } catch (NullPointerException e) { + assertThat("topLeft of envelope cannot be null", equalTo(e.getMessage())); + } + + try { + new EnvelopeBuilder(new Coordinate(1.0, -1.0), null); + fail("Exception expected"); + } catch (NullPointerException e) { + assertThat("bottomRight of envelope cannot be null", equalTo(e.getMessage())); + } + } + @Override protected EnvelopeBuilder createTestShapeBuilder() { return createRandomShape(); @@ -42,26 +60,25 @@ public class EnvelopeBuilderTests extends AbstractShapeBuilderTestCase { + public void testInvalidConstructorArgs() { + try { + new LineStringBuilder(null); + fail("Exception expected"); + } catch (IllegalArgumentException e) { + assertThat("cannot create point collection with empty set of points", equalTo(e.getMessage())); + } + + try { + new LineStringBuilder(new PointListBuilder().list()); + fail("Exception expected"); + } catch (IllegalArgumentException e) { + assertThat("cannot create point collection with empty set of points", equalTo(e.getMessage())); + } + + try { + new LineStringBuilder(new PointListBuilder().point(0.0, 0.0).list()); + fail("Exception expected"); + } catch (IllegalArgumentException e) { + assertThat("invalid number of points in LineString (found [1] - must be >= 2)", equalTo(e.getMessage())); + } + } + @Override protected LineStringBuilder createTestShapeBuilder() { return createRandomShape(); diff --git a/core/src/test/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilderTests.java b/core/src/test/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilderTests.java index c2224ae6d68..3c618fd3696 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/common/geo/builders/MultiLineStringBuilderTests.java @@ -40,30 +40,37 @@ public class MultiLineStringBuilderTests extends AbstractShapeBuilderTestCase 0) { + int lineToChange = randomInt(coordinates.length - 1); + for (int i = 0; i < coordinates.length; i++) { + Coordinate[] line = coordinates[i]; + if (i == lineToChange) { + Coordinate coordinate = randomFrom(line); + if (randomBoolean()) { + if (coordinate.x != 0.0) { + coordinate.x = coordinate.x / 2; + } else { + coordinate.x = randomDoubleBetween(-180.0, 180.0, true); + } } else { - coordinate.x = randomDoubleBetween(-180.0, 180.0, true); - } - } else { - if (coordinate.y != 0.0) { - coordinate.y = coordinate.y / 2; - } else { - coordinate.y = randomDoubleBetween(-90.0, 90.0, true); + if (coordinate.y != 0.0) { + coordinate.y = coordinate.y / 2; + } else { + coordinate.y = randomDoubleBetween(-90.0, 90.0, true); + } } } } + } else { + mutation.linestring((LineStringBuilder) RandomShapeGenerator.createShape(getRandom(), ShapeType.LINESTRING)); } return mutation; } static MultiLineStringBuilder createRandomShape() { + if (true) { + return new MultiLineStringBuilder(); + } return (MultiLineStringBuilder) RandomShapeGenerator.createShape(getRandom(), ShapeType.MULTILINESTRING); } } diff --git a/core/src/test/java/org/elasticsearch/common/geo/builders/MultiPointBuilderTests.java b/core/src/test/java/org/elasticsearch/common/geo/builders/MultiPointBuilderTests.java index fb365df0122..6135f326b6e 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/builders/MultiPointBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/common/geo/builders/MultiPointBuilderTests.java @@ -25,8 +25,29 @@ import org.elasticsearch.test.geo.RandomShapeGenerator.ShapeType; import java.io.IOException; +import static org.hamcrest.Matchers.equalTo; + public class MultiPointBuilderTests extends AbstractShapeBuilderTestCase { + public void testInvalidBuilderException() { + try { + new MultiPointBuilder(null); + fail("IllegalArgumentException expected"); + } catch (IllegalArgumentException e) { + assertThat("cannot create point collection with empty set of points", equalTo(e.getMessage())); + } + + try { + new MultiPointBuilder(new PointListBuilder().list()); + fail("IllegalArgumentException expected"); + } catch (IllegalArgumentException e) { + assertThat("cannot create point collection with empty set of points", equalTo(e.getMessage())); + } + + // one point is minimum + new MultiPointBuilder(new PointListBuilder().point(0.0, 0.0).list()); + } + @Override protected MultiPointBuilder createTestShapeBuilder() { return createRandomShape(); @@ -40,19 +61,23 @@ public class MultiPointBuilderTests extends AbstractShapeBuilderTestCase 0) { + Coordinate coordinate = randomFrom(coordinates); + if (randomBoolean()) { + if (coordinate.x != 0.0) { + coordinate.x = coordinate.x / 2; + } else { + coordinate.x = randomDoubleBetween(-180.0, 180.0, true); + } } else { - coordinate.x = randomDoubleBetween(-180.0, 180.0, true); + if (coordinate.y != 0.0) { + coordinate.y = coordinate.y / 2; + } else { + coordinate.y = randomDoubleBetween(-90.0, 90.0, true); + } } } else { - if (coordinate.y != 0.0) { - coordinate.y = coordinate.y / 2; - } else { - coordinate.y = randomDoubleBetween(-90.0, 90.0, true); - } + coordinates = new Coordinate[]{new Coordinate(1.0, 1.0)}; } return mutation.points(coordinates); } diff --git a/core/src/test/java/org/elasticsearch/common/geo/builders/PolygonBuilderTests.java b/core/src/test/java/org/elasticsearch/common/geo/builders/PolygonBuilderTests.java index ea83359c1f0..c7b4673a466 100644 --- a/core/src/test/java/org/elasticsearch/common/geo/builders/PolygonBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/common/geo/builders/PolygonBuilderTests.java @@ -77,8 +77,7 @@ public class PolygonBuilderTests extends AbstractShapeBuilderTestCase