Ver código fonte

Fix GeoShapeWithDocValuesFieldTypeTests#testFetchVectorTile (#76063)

ignore invalid geometries the same way we are doing it in ArraySourceValueFetcher and
only simplify (multi)polygons if they are valid
Ignacio Vera 4 anos atrás
pai
commit
2985f7f6bb

+ 8 - 2
x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldTypeTests.java

@@ -79,7 +79,6 @@ public class GeoShapeWithDocValuesFieldTypeTests extends FieldTypeTestCase {
         assertEquals(List.of(wktLineString, wktPoint), fetchSourceValue(mapper, sourceValue, "wkt"));
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/76059")
     public void testFetchVectorTile() throws IOException {
         fetchVectorTile(GeometryTestUtils.randomPoint());
         fetchVectorTile(GeometryTestUtils.randomMultiPoint(false));
@@ -111,7 +110,14 @@ public class GeoShapeWithDocValuesFieldTypeTests extends FieldTypeTestCase {
         }
 
         final List<?> sourceValue = fetchSourceValue(mapper, WellKnownText.toWKT(geometry), mvtString);
-        final List<byte[]> features = featureFactory.getFeatures(geometry);
+        List<byte[]> features;
+        try {
+            features = featureFactory.getFeatures(geometry);
+        } catch (IllegalArgumentException iae) {
+            // if parsing fails means that we must be ignoring malformed values. In case of mvt might
+            // happen that the geometry is out of range (close to the poles).
+            features = List.of();
+        }
         assertThat(features.size(), Matchers.equalTo(sourceValue.size()));
         for (int i = 0; i < features.size(); i++) {
             assertThat(sourceValue.get(i), Matchers.equalTo(features.get(i)));

+ 14 - 11
x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java

@@ -163,12 +163,7 @@ public class FeatureFactory {
 
         @Override
         public org.locationtech.jts.geom.Geometry visit(Polygon polygon) throws RuntimeException {
-            final org.locationtech.jts.geom.Polygon jtsPolygon = buildPolygon(polygon);
-            if (jtsPolygon.contains(tile)) {
-                // shortcut, we return the tile
-                return tile;
-            }
-            return TopologyPreservingSimplifier.simplify(jtsPolygon, pixelPrecision);
+            return simplifyGeometry(buildPolygon(polygon));
         }
 
         @Override
@@ -176,13 +171,21 @@ public class FeatureFactory {
             final org.locationtech.jts.geom.Polygon[] polygons = new org.locationtech.jts.geom.Polygon[multiPolygon.size()];
             for (int i = 0; i < multiPolygon.size(); i++) {
                 final org.locationtech.jts.geom.Polygon jtsPolygon = buildPolygon(multiPolygon.get(i));
-                if (jtsPolygon.contains(tile)) {
-                    // shortcut, we return the tile
-                    return tile;
-                }
                 polygons[i] = jtsPolygon;
             }
-            return TopologyPreservingSimplifier.simplify(geomFactory.createMultiPolygon(polygons), pixelPrecision);
+            return simplifyGeometry(geomFactory.createMultiPolygon(polygons));
+        }
+
+        private org.locationtech.jts.geom.Geometry simplifyGeometry(org.locationtech.jts.geom.Geometry geometry) {
+            if (geometry.isValid() == false) {
+                // we only simplify the geometry if it is valid, otherwise algorithm might fail.
+                return geometry;
+            }
+            if (geometry.contains(tile)) {
+                // shortcut, we return the tile
+                return tile;
+            }
+            return TopologyPreservingSimplifier.simplify(geometry, pixelPrecision);
         }
 
         private org.locationtech.jts.geom.Polygon buildPolygon(Polygon polygon) {