[7.15] Remove InvalidShapeException from GeoPolygonDecomposer (#77613) (#77740)

fixes a bug where InvalidShapeException are not properly handle so even if the mappings defined with
ignore_malformed set to true, the exception leaks and the setting is ignored

Author: iverase

server/src/main/java/org/elasticsearch/common/geo/GeoPolygonDecomposer.java

@@ -13,7 +13,6 @@
 import org.elasticsearch.geometry.MultiPolygon;
 import org.elasticsearch.geometry.Point;
 import org.elasticsearch.geometry.Polygon;
-import org.locationtech.spatial4j.exception.InvalidShapeException;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -192,7 +191,7 @@ private static Edge[] ring(int component, boolean direction, boolean handedness,
         }
         if (signedArea == 0) {
             // Points are collinear or self-intersection
-            throw new InvalidShapeException("Cannot determine orientation: signed area equal to 0." +
+            throw new IllegalArgumentException("Cannot determine orientation: signed area equal to 0." +
                 " Points are collinear or polygon self-intersects.");
         }
         boolean orientation = signedArea < 0;
@@ -339,7 +338,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 IllegalArgumentException("Provided shape has duplicate consecutive coordinates at: (" + nextPoint + ")");
             }
         }
 
@@ -464,7 +463,7 @@ private static void assign(Edge[] holes, Point[][] points, int numHoles, Edge[]
             if (intersections == 0) {
                 // There were no edges that intersect the line of longitude through
                 // holes[i].coordinate, so there's no way this hole is within the polygon.
-                throw new InvalidShapeException("Invalid shape: Hole is not within polygon");
+                throw new IllegalArgumentException("Invalid shape: Hole is not within polygon");
             }
 
             // Next we do a binary search to find the position of holes[i].coordinate in the array.
@@ -480,7 +479,7 @@ private static void assign(Edge[] holes, Point[][] points, int numHoles, Edge[]
                 // and it didn't match after all.
 
                 // TODO Can this actually happen? Needs a test to exercise it, or else needs to be removed.
-                throw new InvalidShapeException("Invalid shape: Hole is not within polygon");
+                throw new IllegalArgumentException("Invalid shape: Hole is not within polygon");
             }
 
             final int index;
@@ -546,7 +545,7 @@ private static int component(final Edge edge, final int id, final ArrayList<Edge
                     partitionPoint[1] = current.coordinate.getY();
                     partitionPoint[2] = current.coordinate.getZ();
                     if (connectedComponents > 0 && current.next != edge) {
-                        throw new InvalidShapeException("Shape contains more than one shared point");
+                        throw new IllegalArgumentException("Shape contains more than one shared point");
                     }
 
                     // a negative id flags the edge as visited for the edges(...) method.
@@ -594,10 +593,10 @@ private static Point[] coordinates(Edge component, Point[] coordinates, double[]
         // First and last coordinates must be equal
         if (coordinates[0].equals(coordinates[coordinates.length - 1]) == false) {
             if (Double.isNaN(partitionPoint[2])) {
-                throw new InvalidShapeException("Self-intersection at or near point ["
+                throw new IllegalArgumentException("Self-intersection at or near point ["
                     + partitionPoint[0] + "," + partitionPoint[1] + "]");
             } else {
-                throw new InvalidShapeException("Self-intersection at or near point ["
+                throw new IllegalArgumentException("Self-intersection at or near point ["
                     + partitionPoint[0] + "," + partitionPoint[1] + "," + partitionPoint[2] + "]");
             }
         }
@@ -728,7 +727,7 @@ void setNext(Edge next) {
             if (next != null) {
                 // self-loop throws an invalid shape
                 if (this.coordinate.equals(next.coordinate)) {
-                    throw new InvalidShapeException("Provided shape has duplicate consecutive coordinates at: " + this.coordinate);
+                    throw new IllegalArgumentException("Provided shape has duplicate consecutive coordinates at: " + this.coordinate);
                 }
                 this.next = next;
             }

server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java

@@ -27,7 +27,6 @@
 import org.elasticsearch.geometry.ShapeType;
 import org.elasticsearch.index.mapper.GeoShapeIndexer;
 import org.elasticsearch.test.ESTestCase;
-import org.locationtech.spatial4j.exception.InvalidShapeException;
 
 import java.io.IOException;
 import java.text.ParseException;
@@ -431,7 +430,7 @@ public void testInvalidSelfCrossingPolygon() {
         Polygon polygon = new Polygon(new LinearRing(
             new double[]{0, 0, 1, 0.5, 1.5, 1, 2, 2, 0}, new double[]{0, 2, 1.9, 1.8, 1.8, 1.9, 2, 0, 0}
         ));
-        Exception e = expectThrows(InvalidShapeException.class, () -> indexer.prepareForIndexing(polygon));
+        Exception e = expectThrows(IllegalArgumentException.class, () -> indexer.prepareForIndexing(polygon));
         assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
         assertThat(e.getMessage(), not(containsString("NaN")));
     }

server/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java

@@ -834,9 +834,6 @@ public void testInvalidSelfCrossingPolygon() {
         Exception e = expectThrows(InvalidShapeException.class, () -> builder.close().buildS4J());
         assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
         assertThat(e.getMessage(), not(containsString("NaN")));
-        e = expectThrows(InvalidShapeException.class, () -> buildGeometry(builder.close()));
-        assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
-        assertThat(e.getMessage(), not(containsString("NaN")));
     }
 
     public Object buildGeometry(ShapeBuilder<?, ?, ?> builder) {

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java

@@ -26,6 +26,7 @@
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.Mapper;
+import org.elasticsearch.index.mapper.MapperParsingException;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.MapperTestCase;
 import org.elasticsearch.index.mapper.ParsedDocument;
@@ -38,6 +39,7 @@
 import java.util.Collection;
 import java.util.Collections;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
@@ -225,6 +227,44 @@ public void testIgnoreMalformedParsing() throws IOException {
         assertThat(ignoreMalformed, equalTo(false));
     }
 
+    public void testIgnoreMalformedValues() throws IOException {
+
+        DocumentMapper ignoreMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("ignore_malformed", true);
+        }));
+        DocumentMapper failMapper = createDocumentMapper(fieldMapping(b -> {
+            b.field("type", "geo_shape");
+            b.field("ignore_malformed", false);
+        }));
+
+        {
+            BytesReference arrayedDoc = BytesReference.bytes(XContentFactory.jsonBuilder()
+                .startObject()
+                .field("field", "Bad shape")
+                .endObject()
+            );
+            SourceToParse sourceToParse = new SourceToParse("test", "_doc",  "1", arrayedDoc, XContentType.JSON);
+            ParsedDocument document = ignoreMapper.parse(sourceToParse);
+            assertThat(document.docs().get(0).getFields("field").length, equalTo(0));
+            MapperParsingException exception = expectThrows(MapperParsingException.class, () -> failMapper.parse(sourceToParse));
+            assertThat(exception.getCause().getMessage(), containsString("Unknown geometry type: bad"));
+        }
+        {
+            BytesReference arrayedDoc = BytesReference.bytes(XContentFactory.jsonBuilder()
+                .startObject()
+                .field("field", "POLYGON ((18.9401790919516 -33.9681188869036, 18.9401790919516 -33.9681188869037, 18.9401790919517 " +
+                    "-33.9681188869037, 18.9401790919517 -33.9681188869036, 18.9401790919516 -33.9681188869036))")
+                .endObject()
+            );
+            SourceToParse sourceToParse = new SourceToParse("test", "_doc",  "1", arrayedDoc, XContentType.JSON);
+            ParsedDocument document = ignoreMapper.parse(sourceToParse);
+            assertThat(document.docs().get(0).getFields("field").length, equalTo(0));
+            MapperParsingException exception = expectThrows(MapperParsingException.class, () -> failMapper.parse(sourceToParse));
+            assertThat(exception.getCause().getMessage(), containsString("Cannot determine orientation"));
+        }
+    }
+
     /**
      * Test that doc_values parameter correctly parses
      */