Geo: Don't flip longitude of envelopes crossing dateline (#34535)

When a envelope that crosses the dateline is specified as a part of
geo_shape query is parsed it shouldn't have its left and right points
flipped.

Fixes #34418

Author: imotov

docs/reference/mapping/types/geo-shape.asciidoc

@@ -567,7 +567,7 @@ POST /example/doc
 
 Elasticsearch supports an `envelope` type, which consists of coordinates
 for upper left and lower right points of the shape to represent a
-bounding rectangle:
+bounding rectangle in the format [[minLon, maxLat],[maxLon, minLat]]:
 
 [source,js]
 --------------------------------------------------

docs/reference/migration/migrate_6_0/search.asciidoc

@@ -51,6 +51,10 @@ been removed. Also the deprecated `min_word_len` (a synonym for `min_word_length
 
 * The `optimize_bbox` parameter has been removed from `geo_distance` queries.
 
+* An `envelope` crossing the dateline in a `geo_shape `query is now processed
+  correctly when specified using REST API instead of having its left and
+  right corners flipped.
+
 * The `ignore_malformed` and `coerce` parameters have been removed from
   `geo_bounding_box`, `geo_polygon`, and `geo_distance` queries.
 

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

@@ -223,11 +223,6 @@ public ShapeBuilder getBuilder(CoordinateNode coordinates, DistanceUnit.Distance
             // verify coordinate bounds, correct if necessary
             Coordinate uL = coordinates.children.get(0).coordinate;
             Coordinate lR = coordinates.children.get(1).coordinate;
-            if (((lR.x < uL.x) || (uL.y < lR.y))) {
-                Coordinate uLtmp = uL;
-                uL = new Coordinate(Math.min(uL.x, lR.x), Math.max(uL.y, lR.y));
-                lR = new Coordinate(Math.max(uLtmp.x, lR.x), Math.min(uLtmp.y, lR.y));
-            }
             return new EnvelopeBuilder(uL, lR);
         }
 

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

@@ -176,15 +176,15 @@ public void testParseEnvelope() throws IOException {
         Rectangle expected = SPATIAL_CONTEXT.makeRectangle(-50, 50, -30, 30);
         assertGeometryEquals(expected, multilinesGeoJson);
 
-        // test #2: envelope with agnostic coordinate order (TopRight, BottomLeft)
+        // test #2: envelope that spans dateline
         multilinesGeoJson = XContentFactory.jsonBuilder().startObject().field("type", "envelope")
                 .startArray("coordinates")
                 .startArray().value(50).value(30).endArray()
                 .startArray().value(-50).value(-30).endArray()
                 .endArray()
                 .endObject();
 
-        expected = SPATIAL_CONTEXT.makeRectangle(-50, 50, -30, 30);
+        expected = SPATIAL_CONTEXT.makeRectangle(50, -50, -30, 30);
         assertGeometryEquals(expected, multilinesGeoJson);
 
         // test #3: "envelope" (actually a triangle) with invalid number of coordinates (TopRight, BottomLeft, BottomRight)

server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java

@@ -20,7 +20,9 @@
 package org.elasticsearch.search.geo;
 
 import org.elasticsearch.action.get.GetResponse;
+import org.elasticsearch.action.index.IndexRequest;
 import org.elasticsearch.action.search.SearchResponse;
+import org.elasticsearch.common.CheckedSupplier;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.geo.builders.CoordinatesBuilder;
@@ -32,6 +34,7 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.mapper.MapperParsingException;
 import org.elasticsearch.index.query.GeoShapeQueryBuilder;
@@ -531,4 +534,73 @@ public void testFieldAlias() throws IOException {
             .execute().actionGet();
         assertEquals(1, response.getHits().getTotalHits());
     }
+
+    // Test for issue #34418
+    public void testEnvelopeSpanningDateline() throws IOException {
+        XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
+            .startObject("doc")
+                .startObject("properties")
+                    .startObject("geo").field("type", "geo_shape").endObject()
+                .endObject()
+            .endObject()
+        .endObject();
+
+        createIndex("test", Settings.builder().put("index.number_of_shards", 1).build(), "doc", mapping);
+
+        String doc1 = "{\"geo\": {\r\n" + "\"coordinates\": [\r\n" + "-33.918711,\r\n" + "18.847685\r\n" + "],\r\n" +
+                "\"type\": \"Point\"\r\n" + "}}";
+        client().index(new IndexRequest("test", "doc", "1").source(doc1, XContentType.JSON).setRefreshPolicy(IMMEDIATE)).actionGet();
+
+        String doc2 = "{\"geo\": {\r\n" + "\"coordinates\": [\r\n" + "-49.0,\r\n" + "18.847685\r\n" + "],\r\n" +
+            "\"type\": \"Point\"\r\n" + "}}";
+        client().index(new IndexRequest("test", "doc", "2").source(doc2, XContentType.JSON).setRefreshPolicy(IMMEDIATE)).actionGet();
+
+        String doc3 = "{\"geo\": {\r\n" + "\"coordinates\": [\r\n" + "49.0,\r\n" + "18.847685\r\n" + "],\r\n" +
+            "\"type\": \"Point\"\r\n" + "}}";
+        client().index(new IndexRequest("test", "doc", "3").source(doc3, XContentType.JSON).setRefreshPolicy(IMMEDIATE)).actionGet();
+
+        @SuppressWarnings("unchecked") CheckedSupplier<GeoShapeQueryBuilder, IOException> querySupplier = randomFrom(
+            () -> QueryBuilders.geoShapeQuery(
+                "geo",
+                new EnvelopeBuilder(new Coordinate(-21, 44), new Coordinate(-39, 9))
+            ).relation(ShapeRelation.WITHIN),
+            () -> {
+                XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
+                    .startObject("geo")
+                    .startObject("shape")
+                    .field("type", "envelope")
+                    .startArray("coordinates")
+                    .startArray().value(-21).value(44).endArray()
+                    .startArray().value(-39).value(9).endArray()
+                    .endArray()
+                    .endObject()
+                    .field("relation", "within")
+                    .endObject()
+                    .endObject();
+                try (XContentParser parser = createParser(builder)){
+                    parser.nextToken();
+                    return GeoShapeQueryBuilder.fromXContent(parser);
+                }
+            },
+            () -> {
+                XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
+                    .startObject("geo")
+                    .field("shape", "BBOX (-21, -39, 44, 9)")
+                    .field("relation", "within")
+                    .endObject()
+                    .endObject();
+                try (XContentParser parser = createParser(builder)){
+                    parser.nextToken();
+                    return GeoShapeQueryBuilder.fromXContent(parser);
+                }
+            }
+        );
+
+        SearchResponse response = client().prepareSearch("test")
+            .setQuery(querySupplier.get())
+            .execute().actionGet();
+        assertEquals(2, response.getHits().getTotalHits());
+        assertNotEquals("1", response.getHits().getAt(0).getId());
+        assertNotEquals("1", response.getHits().getAt(1).getId());
+    }
 }