Skip to content

Commit

Permalink
Check for correct winding order of WKT polygons.
Browse files Browse the repository at this point in the history 
Resolves gbif/occurrence#340
Resolves gbif/portal-feedback#2222
  • Loading branch information
@MattBlissett
MattBlissett committed Feb 29, 2024
1 parent 6d1bc09 commit 97f9595
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 17 deletions.
43 changes: 40 additions & 3 deletions src/main/java/org/gbif/api/util/SearchTypeValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
import java.util.regex.Pattern;

import org.apache.commons.lang3.StringUtils;
import org.locationtech.jts.algorithm.Orientation;
import org.locationtech.jts.geom.Geometry;
import org.locationtech.jts.geom.MultiPolygon;
import org.locationtech.jts.geom.Polygon;
import org.locationtech.jts.operation.valid.IsValidOp;
import org.locationtech.spatial4j.context.jts.DatelineRule;
Expand Down Expand Up @@ -358,9 +360,17 @@ private static void validateGeometry(String wellKnownText) {
throw new IllegalArgumentException("Empty geometry: " + wellKnownText);
}

// Calculating the area > 0 ensures that polygons that are representing lines or points are invalidated
if (geometry instanceof Polygon && geometry.getArea() == 0.0) {
throw new IllegalArgumentException("Polygon with zero area: " + wellKnownText);
// Validate a polygon — check the winding order
if (geometry instanceof Polygon) {
validatePolygon((Polygon) geometry, wellKnownText);
}

// Validate polygons within a multipolygon
if (geometry instanceof MultiPolygon) {
MultiPolygon multiPolygon = (MultiPolygon) geometry;
for (int p = 0; p < multiPolygon.getNumGeometries(); p++) {
validatePolygon((Polygon) multiPolygon.getGeometryN(p), wellKnownText);
}
}

switch (geometry.getGeometryType().toUpperCase()) {
Expand All @@ -384,6 +394,33 @@ private static void validateGeometry(String wellKnownText) {
}
}

private static void validatePolygon(Polygon polygon, String wellKnownText) {
// Calculating the area > 0 ensures that polygons that are representing lines or points are invalidated
if (polygon.getArea() == 0.0) {
throw new IllegalArgumentException("Polygon with zero area: " + polygon.toText());
}

// Exterior ring must be anticlockwise
boolean isCCW = Orientation.isCCW(polygon.getExteriorRing().getCoordinates());
if (!isCCW) {
String reversedText = polygon.getExteriorRing().reverse().toText();
throw new IllegalArgumentException("Polygon with clockwise exterior ring: " + wellKnownText +
". Did you mean these coordinates? (Note this is only part of the polygon or multipolygon you provided.) " +
reversedText);
}

// Interior rings must be clockwise
for (int r = 0; r < polygon.getNumInteriorRing(); r++) {
isCCW = Orientation.isCCW(polygon.getInteriorRingN(r).getCoordinates());
if (isCCW) {
String reversedText = polygon.getInteriorRingN(r).reverse().toText();
throw new IllegalArgumentException("Polygon with anticlockwise interior ring: " + wellKnownText +
". Did you mean these coordinates? (Note this is only part of the polygon or multipolygon you provided.) " +
reversedText);
}
}
}

/**
* Validates if the value is a valid single integer or a range of integer values.
* If the value is a range each limit is validated and the wildcard character '*' is skipped.
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/gbif/api/model/predicate/WithinPredicateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void testNullConstructor() {
assertThrows(NullPointerException.class, () -> new WithinPredicate(null));
}

// API no longer throws an exeception here.
// API no longer throws an exception here.
public void testBadConstructor1() {
new WithinPredicate("POLYGON");
}
Expand Down
30 changes: 17 additions & 13 deletions src/test/java/org/gbif/api/util/SearchTypeValidatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,24 +81,28 @@ public static Stream<Arguments> data() {
Arguments.of(IS_EXTINCT, "no", false, false, false),
Arguments.of(SCIENTIFIC_NAME, "abies%", true, false, false),
// SearchParameter, value, isValid, isNumericRange, isDateRange
Arguments.of(GEOMETRY, "EMPTY", false, false, false), // Valid WKT but not accepted
Arguments.of(GEOMETRY, "POINT (30 10)", true, false, false),
Arguments.of(GEOMETRY, "POINT (30 10.12)", true, false, false),
Arguments.of(GEOMETRY, "POINT (30 10.12)", true, false, false),
Arguments.of(GEOMETRY, "POLYGON ((30 10, 10 20, 20 40, 40 40, 30 10))", true, false, false),
Arguments.of(GEOMETRY, "polygon ((30.12 10, 10 20, 20 40, 40 40, 30.12 10))", true, false, false),
Arguments.of(GEOMETRY, "Polygon ((30.12 10, 10 20, 20 40, 40 40, 30.1200 10.00))", true, false, false),
Arguments.of(GEOMETRY, "POLYGON ((30,12 10, 10 20, 20 40, 40 40, 30,12 10))", false, false, false),
Arguments.of(GEOMETRY, "POLYGON ((30 10, 10 20, 20 40, 40 40, 30 10,))", false, false, false),
Arguments.of(GEOMETRY, "POLYGON ((30 10, 10 20, 20 40, 40 40, 30 10),)", false, false, false),
Arguments.of(GEOMETRY, "POLYGON ((30.12 10, 10 20, 20 40, 40 40, 30.12 10.01))", false, false, false), // valid wkt, although not a closed polygon
Arguments.of(GEOMETRY, "POLYGON ( (30 10 , 10 20 , 20 40 , 40 40 , 30 20) )", false, false, false), // valid wkt, although not a closed polygon
Arguments.of(GEOMETRY, "POLYGON ((35 10, 10 20, 15 40, 45 45, 35 10),(20 30, 35 35, 30 20, 20 30))", true, false, false), // valid donut
Arguments.of(GEOMETRY, "POLYGON ((35 10, 10 20, 15 40, 45 45, 35 10),(20 30, 35 35, 30 20, 20 30))", true, false, false),
Arguments.of(GEOMETRY, "MULTIPOLYGON (((35 10, 10 20, 15 40, 45 45, 35 10)),((20 30, 35 35, 30 20, 20 30)))", false, false, false), // invalid, overlaps
Arguments.of(GEOMETRY, "MULTIPOLYGON (((35 10, 10 20, 15 40, 45 45, 35 10)),((120 30, 135 35, 130 20, 120 30)))", true, false, false),
Arguments.of(GEOMETRY, "POLYGON ((30 10, 40 40, 20 40, 10 20, 30 10))", true, false, false),
Arguments.of(GEOMETRY, "POLYGON ((30 10, 10 20, 20 40, 40 40, 30 10))", false, false, false), // Clockwise
Arguments.of(GEOMETRY, "POLYGON ((30.12 10, 40 40, 20 40, 10 20, 30.12 10))", true, false, false),
Arguments.of(GEOMETRY, "POLYGON ((30.12 10, 40 40, 20 40, 10 20, 30.1200 10.00))", true, false, false),
Arguments.of(GEOMETRY, "POLYGON ((30,12 10, 40 40, 20 40, 10 20, 30,12 10))", false, false, false), // Missing ordinate
Arguments.of(GEOMETRY, "POLYGON ((30 10, 40 40, 20 40, 10 20, 30 10,))", false, false, false), // Extra comma
Arguments.of(GEOMETRY, "POLYGON ((30 10, 40 40, 20 40, 10 20, 30 10),)", false, false, false), // Extra comma
Arguments.of(GEOMETRY, "POLYGON ((30.12 10, 10 20, 20 40, 40 40, 30.12 10.01))", false, false, false), // Valid WKT but not a closed polygon
Arguments.of(GEOMETRY, "POLYGON ( (30 10 , 10 20 , 20 40 , 40 40 , 30 20) )", false, false, false), // Valid WKT but not a closed polygon
Arguments.of(GEOMETRY, "POLYGON ((35 10, 45 45, 15 40, 10 20, 35 10),(20 30, 35 35, 30 20, 20 30))", true, false, false), // Valid donut
Arguments.of(GEOMETRY, "POLYGON ((35 10, 10 20, 15 40, 45 45, 35 10),(20 30, 30 20, 35 35, 20 30))", false, false, false), // Clockwise outer, anticlockwise inner
Arguments.of(GEOMETRY, "POLYGON ((35 10, 45 45, 15 40, 10 20, 35 10),(20 30, 30 20, 35 35, 20 30))", false, false, false), // Anticlockwise outer, anticlockwise inner
Arguments.of(GEOMETRY, "MULTIPOLYGON (((30 10, 40 40, 20 40, 10 20, 30 10)),((120 30, 130 20, 135 35, 120 30)))", true, false, false),
Arguments.of(GEOMETRY, "MULTIPOLYGON (((30 10, 10 20, 20 40, 40 40, 30 10)),((120 30, 130 20, 135 35, 120 30)))", false, false, false), // First polygon is clockwise
Arguments.of(GEOMETRY, "MULTIPOLYGON (((30 10, 40 40, 20 40, 10 20, 30 10)),((20 30, 30 20, 35 35, 20 30)))", false, false, false), // Overlaps
Arguments.of(GEOMETRY, "MULTIPOLYGON(((-125 38.4, -121.8 38.4, -121.8 40.9, -125 40.9, -125 38.4)),((-115 22.4, -111.8 22.4, -111.8 30.9, -115 30.9, -115 22.4)))", true, false, false),
Arguments.of(GEOMETRY, "LINESTRING (30 10, 10 30, 40 40)", true, false, false),
Arguments.of(GEOMETRY, "LINESTRING (30 10, 10 30, 40 40,)", false, false, false),
Arguments.of(GEOMETRY, "LINESTRING (30 10, 10 30, 40 40,)", false, false, false), // Extra comma
Arguments.of(YEAR, "1991", true, false, true),
Arguments.of(YEAR, "1991-01-31", false, false, false),
Arguments.of(YEAR, "860", true, false, false),
Expand Down

0 comments on commit 97f9595

Please sign in to comment.