Skip to content

Commit

Permalink
[GEO] Throw helpful exception for Polygons with holes outside the shell
Browse files Browse the repository at this point in the history 
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 #9071
  • Loading branch information
@nknize
nknize committed Jan 2, 2015
1 parent 93dddcd commit b21024b
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 2 deletions.
7 changes: 5 additions & 2 deletions src/main/java/org/elasticsearch/common/geo/builders/BasePolygonBuilder.java
View file
Expand Up @@ -361,8 +361,11 @@ private static void assign(Edge[] holes, Coordinate[][] points, int numHoles, Ed
// 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);
Expand Down
82 changes: 82 additions & 0 deletions src/test/java/org/elasticsearch/common/geo/GeoJSONShapeParserTests.java
View file
Expand Up @@ -243,6 +243,42 @@ public void testParse_invalidMultipoint() throws IOException {
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
Expand Down Expand Up @@ -595,6 +631,7 @@ public void testParse_multiPoint() throws IOException {

@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)
Expand Down Expand Up @@ -658,6 +695,51 @@ public void testParse_multiPolygon() throws IOException {
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
Expand Down

0 comments on commit b21024b

Please sign in to comment.