honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-28 02:48:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

[GEO] Throw helpful exception for Polygons with holes outside the shell

Browse Source 
A recent situation occured where a MultiPolygon coordinate array was accidentally defined as a single polygon with multiple holes. Since the intent was a MultiPolygon, the holes of the unintended Polygon fell outside the outer shell.  This exposed a bug in the contains logic inside BasePolygonBuilder. An ArrayIndexOutOfBoundsException was being thrown instead of a more useful ElasticsearchParseException( "hole is not within polygon" ).  This pull request fixes the bug and adds additional unit tests for verifying proper MultiPolygon type parsing.

closes
This commit is contained in:
Nicholas Knize 2014-12-30 11:14:59 -06:00
parent 93dddcdfd9
commit b21024b5f9
2 changed files with 87 additions and 2 deletions
src
main/java/org/elasticsearch/common/geo/builders
BasePolygonBuilder.java
test/java/org/elasticsearch/common/geo
GeoJSONShapeParserTests.java

7
src/main/java/org/elasticsearch/common/geo/builders/BasePolygonBuilder.java

@ -361,8 +361,11 @@ public abstract class BasePolygonBuilder<E extends BasePolygonBuilder<E>> extend
// will get the correct position in the edge list and therefore the correct component to add the hole
current.intersect = current.coordinate;
final int intersections = intersections(current.coordinate.x, edges);
final int pos = Arrays.binarySearch(edges, 0, intersections, current, INTERSECTION_ORDER);
if (pos >= 0) {
// if no intersection is found then the hole is not within the polygon, so
// don't waste time calling a binary search
final int pos;
if (intersections == 0 ||
(pos = Arrays.binarySearch(edges, 0, intersections, current, INTERSECTION_ORDER)) >= 0) {
throw new ElasticsearchParseException("Invaild shape: Hole is not within polygon");
}
final int index = -(pos+2);

82
src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java

@ -243,6 +243,42 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
}
@Test
public void testParse_invalidMultiPolygon() throws IOException {
// test invalid multipolygon (an "accidental" polygon with inner rings outside outer ring)
String multiPolygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "MultiPolygon")
.startArray("coordinates")
.startArray()//one poly (with two holes)
.startArray()
.startArray().value(102.0).value(2.0).endArray()
.startArray().value(103.0).value(2.0).endArray()
.startArray().value(103.0).value(3.0).endArray()
.startArray().value(102.0).value(3.0).endArray()
.startArray().value(102.0).value(2.0).endArray()
.endArray()
.startArray()// first hole
.startArray().value(100.0).value(0.0).endArray()
.startArray().value(101.0).value(0.0).endArray()
.startArray().value(101.0).value(1.0).endArray()
.startArray().value(100.0).value(1.0).endArray()
.startArray().value(100.0).value(0.0).endArray()
.endArray()
.startArray()//second hole
.startArray().value(100.2).value(0.8).endArray()
.startArray().value(100.2).value(0.2).endArray()
.startArray().value(100.8).value(0.2).endArray()
.startArray().value(100.8).value(0.8).endArray()
.startArray().value(100.2).value(0.8).endArray()
.endArray()
.endArray()
.endArray()
.endObject().string();
XContentParser parser = JsonXContent.jsonXContent.createParser(multiPolygonGeoJson);
parser.nextToken();
ElasticsearchGeoAssertions.assertValidException(parser, ElasticsearchParseException.class);
}
@Test
public void testParse_OGCPolygonWithoutHoles() throws IOException {
// test 1: ccw poly not crossing dateline
@ -595,6 +631,7 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
@Test
public void testParse_multiPolygon() throws IOException {
// test #1: two polygons; one without hole, one with hole
String multiPolygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "MultiPolygon")
.startArray("coordinates")
.startArray()//first poly (without holes)
@ -658,6 +695,51 @@ public class GeoJSONShapeParserTests extends ElasticsearchTestCase {
Shape expected = shapeCollection(withoutHoles, withHoles);
assertGeometryEquals(expected, multiPolygonGeoJson);
// test #2: multipolygon; one polygon with one hole
// this test converting the multipolygon from a ShapeCollection type
// to a simple polygon (jtsGeom)
multiPolygonGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "MultiPolygon")
.startArray("coordinates")
.startArray()
.startArray()
.startArray().value(100.0).value(1.0).endArray()
.startArray().value(101.0).value(1.0).endArray()
.startArray().value(101.0).value(0.0).endArray()
.startArray().value(100.0).value(0.0).endArray()
.startArray().value(100.0).value(1.0).endArray()
.endArray()
.startArray()// hole
.startArray().value(100.2).value(0.8).endArray()
.startArray().value(100.2).value(0.2).endArray()
.startArray().value(100.8).value(0.2).endArray()
.startArray().value(100.8).value(0.8).endArray()
.startArray().value(100.2).value(0.8).endArray()
.endArray()
.endArray()
.endArray()
.endObject().string();
shellCoordinates = new ArrayList<>();
shellCoordinates.add(new Coordinate(100, 1));
shellCoordinates.add(new Coordinate(101, 1));
shellCoordinates.add(new Coordinate(101, 0));
shellCoordinates.add(new Coordinate(100, 0));
shellCoordinates.add(new Coordinate(100, 1));
holeCoordinates = new ArrayList<>();
holeCoordinates.add(new Coordinate(100.2, 0.8));
holeCoordinates.add(new Coordinate(100.2, 0.2));
holeCoordinates.add(new Coordinate(100.8, 0.2));
holeCoordinates.add(new Coordinate(100.8, 0.8));
holeCoordinates.add(new Coordinate(100.2, 0.8));
shell = GEOMETRY_FACTORY.createLinearRing(shellCoordinates.toArray(new Coordinate[shellCoordinates.size()]));
holes = new LinearRing[1];
holes[0] = GEOMETRY_FACTORY.createLinearRing(holeCoordinates.toArray(new Coordinate[holeCoordinates.size()]));
withHoles = GEOMETRY_FACTORY.createPolygon(shell, holes);
assertGeometryEquals(jtsGeom(withHoles), multiPolygonGeoJson);
}
@Test