Fields API should return normalize geometries (#80649)

Fields API deliver now the normalize geometries instead of the plain geometries from source

Co-authored-by: James Rodewig <[email protected]

Author: iverase

docs/reference/migration/migrate_8_1.asciidoc

@@ -11,14 +11,13 @@ See also <<release-highlights>> and <<es-release-notes>>.
 
 coming[8.1.0]
 
-////
 [discrete]
 [[breaking-changes-8.1]]
 === Breaking changes
 
 The following changes in {es} 8.1 might affect your applications
 and prevent them from operating normally.
-Before upgrading to 8.0, review these changes and take the described steps
+Before upgrading to 8.1, review these changes and take the described steps
 to mitigate the impact.
 
 NOTE: Breaking changes introduced in minor versions are
@@ -32,9 +31,25 @@ enable <<deprecation-logging, deprecation logging>>.
 //Installation and Upgrade Guide
 
 //tag::notable-breaking-changes[]
+[discrete]
+[[breaking_81_rest_api_changes]]
+==== REST API changes
+
+[[fields_api_geoshape_normalize]]
+.The search API's `fields` parameter now normalizes geometry objects that cross the international dateline.
+[%collapsible]
+====
+*Details* +
+The search API's `fields` parameter now normalizes `geo_shape` objects that
+cross the international dateline (+/-180° longitude). For example, if a polygon
+crosses the dateline, the `fields` parameter returns it as two polygons. You can
+still retrieve original, unnormalized geometry objects from `_source`.
 
+*Impact* +
+If your application requires unnormalized geometry objects, retrieve them from
+`_source` rather than using the `fields` parameter.
+====
 //end::notable-breaking-changes[]
-////
 
 [discrete]
 [[deprecated-8.1]]
@@ -52,7 +67,6 @@ To find out if you are using any deprecated functionality,
 enable <<deprecation-logging, deprecation logging>>.
 
 //tag::notable-breaking-changes[]
-
 [discrete]
 [[breaking_8.1_cluster_node_setting_deprecations]]
 ==== Cluster and node setting deprecations

modules/legacy-geo/src/main/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapper.java

@@ -403,6 +403,11 @@ public void parse(
                 onMalformed.accept(e);
             }
         }
+
+        @Override
+        public ShapeBuilder<?, ?, ?> normalizeFromSource(ShapeBuilder<?, ?, ?> geometry) {
+            return geometry;
+        }
     }
 
     public static final class GeoShapeFieldType extends AbstractShapeGeometryFieldType<ShapeBuilder<?, ?, ?>> implements GeoShapeQueryable {

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

@@ -27,6 +27,21 @@ private GeoLineDecomposer() {
         // no instances
     }
 
+    /**
+     * Checks if the provided line needs to be split by the dateline.
+     */
+    public static boolean needsDecomposing(Line line) {
+        for (int i = 0; i < line.length(); i++) {
+            if (GeoUtils.needsNormalizeLat(line.getLat(i)) || GeoUtils.needsNormalizeLon(line.getLon(i))) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Splits the specified lines in the Multiline by datelines and adds them to the supplied lines array
+     */
     public static void decomposeMultiLine(MultiLine multiLine, List<Line> collector) {
         for (Line line : multiLine) {
             decomposeLine(line, collector);

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

@@ -39,6 +39,24 @@ private GeoPolygonDecomposer() {
         // no instances
     }
 
+    public static boolean needsDecomposing(Polygon polygon) {
+        double minX = Double.POSITIVE_INFINITY;
+        double maxX = Double.NEGATIVE_INFINITY;
+        LinearRing linearRing = polygon.getPolygon();
+        for (int i = 0; i < linearRing.length(); i++) {
+            if (GeoUtils.needsNormalizeLat(linearRing.getLat(i)) || GeoUtils.needsNormalizeLon(linearRing.getLon(i))) {
+                return true;
+            }
+            minX = Math.min(minX, linearRing.getLon(i));
+            maxX = Math.max(maxX, linearRing.getLon(i));
+        }
+        // calculate range
+        final double rng = maxX - minX;
+        // we need to decompose tif the range is greater than a hemisphere (180 degrees)
+        // but not spanning 2 hemispheres (translation would result in a collapsed poly)
+        return rng > DATELINE && rng != 2 * DATELINE;
+    }
+
     public static void decomposeMultiPolygon(MultiPolygon multiPolygon, boolean orientation, List<Polygon> collector) {
         for (Polygon polygon : multiPolygon) {
             decomposePolygon(polygon, orientation, collector);

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

@@ -293,11 +293,19 @@ public static void normalizePoint(double[] lonLat) {
         normalizePoint(lonLat, true, true);
     }
 
+    public static boolean needsNormalizeLat(double lat) {
+        return lat > 90 || lat < -90;
+    }
+
+    public static boolean needsNormalizeLon(double lon) {
+        return lon > 180 || lon < -180;
+    }
+
     public static void normalizePoint(double[] lonLat, boolean normLon, boolean normLat) {
         assert lonLat != null && lonLat.length == 2;
 
-        normLat = normLat && (lonLat[1] > 90 || lonLat[1] < -90);
-        normLon = normLon && (lonLat[0] > 180 || lonLat[0] < -180 || normLat);
+        normLat = normLat && needsNormalizeLat(lonLat[1]);
+        normLon = normLon && (needsNormalizeLon(lonLat[0]) || normLat);
 
         if (normLat) {
             lonLat[1] = centeredModulus(lonLat[1], 360);

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

@@ -24,6 +24,8 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import static org.elasticsearch.common.geo.GeoUtils.needsNormalizeLat;
+import static org.elasticsearch.common.geo.GeoUtils.needsNormalizeLon;
 import static org.elasticsearch.common.geo.GeoUtils.normalizePoint;
 
 /**
@@ -160,4 +162,90 @@ public Geometry visit(Rectangle rectangle) {
             }
         });
     }
+
+    /**
+     * Return false if the provided {@link Geometry} is already Lucene friendly,
+     * else return false.
+     */
+    public static boolean needsNormalize(Orientation orientation, Geometry geometry) {
+        if (geometry == null) {
+            return false;
+        }
+
+        return geometry.visit(new GeometryVisitor<>() {
+            @Override
+            public Boolean visit(Circle circle) {
+                if (circle.isEmpty()) {
+                    return Boolean.FALSE;
+                }
+                return needsNormalizeLat(circle.getLat()) || needsNormalizeLon(circle.getLon());
+            }
+
+            @Override
+            public Boolean visit(GeometryCollection<?> collection) {
+                for (Geometry shape : collection) {
+                    if (shape.visit(this)) {
+                        return Boolean.TRUE;
+                    }
+                }
+                return Boolean.FALSE;
+            }
+
+            @Override
+            public Boolean visit(Line line) {
+                return GeoLineDecomposer.needsDecomposing(line);
+            }
+
+            @Override
+            public Boolean visit(LinearRing ring) {
+                throw new IllegalArgumentException("invalid shape type found [LinearRing]");
+            }
+
+            @Override
+            public Boolean visit(MultiLine multiLine) {
+                for (Line line : multiLine) {
+                    if (visit(line)) {
+                        return Boolean.TRUE;
+                    }
+                }
+                return Boolean.FALSE;
+            }
+
+            @Override
+            public Boolean visit(MultiPoint multiPoint) {
+                for (Point point : multiPoint) {
+                    if (visit(point)) {
+                        return Boolean.TRUE;
+                    }
+                }
+                return Boolean.FALSE;
+            }
+
+            @Override
+            public Boolean visit(MultiPolygon multiPolygon) {
+                for (Polygon polygon : multiPolygon) {
+                    if (visit(polygon)) {
+                        return Boolean.TRUE;
+                    }
+                }
+                return Boolean.FALSE;
+            }
+
+            @Override
+            public Boolean visit(Point point) {
+                return needsNormalizeLat(point.getLat()) || needsNormalizeLon(point.getLon());
+            }
+
+            @Override
+            public Boolean visit(Polygon polygon) {
+                return GeoPolygonDecomposer.needsDecomposing(polygon);
+            }
+
+            @Override
+            public Boolean visit(Rectangle rectangle) {
+                // TODO: what happen with rectangles over the dateline
+                return Boolean.FALSE;
+            }
+        });
+    }
 }

server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java

@@ -54,12 +54,19 @@ public abstract void parse(XContentParser parser, CheckedConsumer<T, IOException
 
         private void fetchFromSource(Object sourceMap, Consumer<T> consumer) {
             try (XContentParser parser = MapXContentParser.wrapObject(sourceMap)) {
-                parse(parser, v -> consumer.accept(v), e -> {}); /* ignore malformed */
+                parse(parser, v -> consumer.accept(normalizeFromSource(v)), e -> {}); /* ignore malformed */
             } catch (IOException e) {
                 throw new UncheckedIOException(e);
             }
         }
 
+        /**
+         * Normalize a geometry when reading from source. When reading from source we can skip
+         * some expensive steps as the geometry has already been indexed.
+         */
+        // TODO: move geometry normalization to the geometry parser.
+        public abstract T normalizeFromSource(T geometry);
+
     }
 
     public abstract static class AbstractGeometryFieldType<T> extends MappedFieldType {

server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java

@@ -368,5 +368,11 @@ private boolean isNormalizable(double coord) {
         protected void reset(GeoPoint in, double x, double y) {
             in.reset(y, x);
         }
+
+        @Override
+        public GeoPoint normalizeFromSource(GeoPoint point) {
+            // normalize during parsing
+            return point;
+        }
     }
 }

server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java

@@ -101,7 +101,7 @@ public GeoShapeFieldMapper build(MapperBuilderContext context) {
                 coerce.get().value(),
                 ignoreZValue.get().value()
             );
-            GeoShapeParser geoShapeParser = new GeoShapeParser(geometryParser);
+            GeoShapeParser geoShapeParser = new GeoShapeParser(geometryParser, orientation.get().value());
             GeoShapeFieldType ft = new GeoShapeFieldType(
                 context.buildFullName(name),
                 indexed.get(),

server/src/main/java/org/elasticsearch/index/mapper/GeoShapeParser.java

@@ -9,7 +9,9 @@
 package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.geo.GeometryNormalizer;
 import org.elasticsearch.common.geo.GeometryParser;
+import org.elasticsearch.common.geo.Orientation;
 import org.elasticsearch.core.CheckedConsumer;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.xcontent.XContentParser;
@@ -20,9 +22,11 @@
 
 public class GeoShapeParser extends AbstractGeometryFieldMapper.Parser<Geometry> {
     private final GeometryParser geometryParser;
+    private final Orientation orientation;
 
-    public GeoShapeParser(GeometryParser geometryParser) {
+    public GeoShapeParser(GeometryParser geometryParser, Orientation orientation) {
         this.geometryParser = geometryParser;
+        this.orientation = orientation;
     }
 
     @Override
@@ -40,4 +44,17 @@ public void parse(XContentParser parser, CheckedConsumer<Geometry, IOException>
             onMalformed.accept(e);
         }
     }
+
+    @Override
+    public Geometry normalizeFromSource(Geometry geometry) {
+        // GeometryNormalizer contains logic for validating the input geometry,
+        // so it needs to be run always at indexing time. When run over source we can skip
+        // the validation, and we run normalization (which is expensive) only when we need
+        // to split geometries around the dateline.
+        if (GeometryNormalizer.needsNormalize(orientation, geometry)) {
+            return GeometryNormalizer.apply(orientation, geometry);
+        } else {
+            return geometry;
+        }
+    }
 }