Centralize BoundingBox logic to a dedicated class (#50253)

Both geo_bounding_box query and geo_bounds aggregation have
a very similar definition of a "bounding box". A lot of this
logic (serialization, xcontent-parsing, etc) can be centralized
instead of having separated efforts to do the same things

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

@@ -0,0 +1,229 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.elasticsearch.common.geo;
+
+import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.ParseField;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.xcontent.ToXContentObject;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.geometry.Geometry;
+import org.elasticsearch.geometry.Rectangle;
+import org.elasticsearch.geometry.ShapeType;
+import org.elasticsearch.geometry.utils.StandardValidator;
+import org.elasticsearch.geometry.utils.WellKnownText;
+
+import java.io.IOException;
+import java.text.ParseException;
+import java.util.Objects;
+
+/**
+ * A class representing a Geo-Bounding-Box for use by Geo queries and aggregations
+ * that deal with extents/rectangles representing rectangular areas of interest.
+ */
+public class GeoBoundingBox implements ToXContentObject, Writeable {
+    private static final WellKnownText WKT_PARSER = new WellKnownText(true, new StandardValidator(true));
+    static final ParseField TOP_RIGHT_FIELD = new ParseField("top_right");
+    static final ParseField BOTTOM_LEFT_FIELD = new ParseField("bottom_left");
+    static final ParseField TOP_FIELD = new ParseField("top");
+    static final ParseField BOTTOM_FIELD = new ParseField("bottom");
+    static final ParseField LEFT_FIELD = new ParseField("left");
+    static final ParseField RIGHT_FIELD = new ParseField("right");
+    static final ParseField WKT_FIELD = new ParseField("wkt");
+    public static final ParseField BOUNDS_FIELD = new ParseField("bounds");
+    public static final ParseField LAT_FIELD = new ParseField("lat");
+    public static final ParseField LON_FIELD = new ParseField("lon");
+    public static final ParseField TOP_LEFT_FIELD = new ParseField("top_left");
+    public static final ParseField BOTTOM_RIGHT_FIELD = new ParseField("bottom_right");
+
+    private final GeoPoint topLeft;
+    private final GeoPoint bottomRight;
+
+    public GeoBoundingBox(GeoPoint topLeft, GeoPoint bottomRight) {
+        this.topLeft = topLeft;
+        this.bottomRight = bottomRight;
+    }
+
+    public GeoBoundingBox(StreamInput input) throws IOException {
+        this.topLeft = input.readGeoPoint();
+        this.bottomRight = input.readGeoPoint();
+    }
+
+    public GeoPoint topLeft() {
+        return topLeft;
+    }
+
+    public GeoPoint bottomRight() {
+        return bottomRight;
+    }
+
+    public double top() {
+        return topLeft.lat();
+    }
+
+    public double bottom() {
+        return bottomRight.lat();
+    }
+
+    public double left() {
+        return topLeft.lon();
+    }
+
+    public double right() {
+        return bottomRight.lon();
+    }
+
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject(BOUNDS_FIELD.getPreferredName());
+        toXContentFragment(builder, true);
+        builder.endObject();
+        return builder;
+    }
+
+    public XContentBuilder toXContentFragment(XContentBuilder builder, boolean buildLatLonFields) throws IOException {
+        if (buildLatLonFields) {
+            builder.startObject(TOP_LEFT_FIELD.getPreferredName());
+            builder.field(LAT_FIELD.getPreferredName(), topLeft.lat());
+            builder.field(LON_FIELD.getPreferredName(), topLeft.lon());
+            builder.endObject();
+        } else {
+            builder.array(TOP_LEFT_FIELD.getPreferredName(), topLeft.lon(), topLeft.lat());
+        }
+        if (buildLatLonFields) {
+            builder.startObject(BOTTOM_RIGHT_FIELD.getPreferredName());
+            builder.field(LAT_FIELD.getPreferredName(), bottomRight.lat());
+            builder.field(LON_FIELD.getPreferredName(), bottomRight.lon());
+            builder.endObject();
+        } else {
+            builder.array(BOTTOM_RIGHT_FIELD.getPreferredName(), bottomRight.lon(), bottomRight.lat());
+        }
+        return builder;
+    }
+
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        out.writeGeoPoint(topLeft);
+        out.writeGeoPoint(bottomRight);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        GeoBoundingBox that = (GeoBoundingBox) o;
+        return topLeft.equals(that.topLeft) &&
+            bottomRight.equals(that.bottomRight);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(topLeft, bottomRight);
+    }
+
+    @Override
+    public String toString() {
+        return "BBOX (" + topLeft.lon() + ", " + bottomRight.lon() + ", " + topLeft.lat() + ", " + bottomRight.lat() + ")";
+    }
+
+    /**
+     * Parses the bounding box and returns bottom, top, left, right coordinates
+     */
+    public static GeoBoundingBox parseBoundingBox(XContentParser parser) throws IOException, ElasticsearchParseException {
+        XContentParser.Token token = parser.currentToken();
+        if (token != XContentParser.Token.START_OBJECT) {
+            throw new ElasticsearchParseException("failed to parse bounding box. Expected start object but found [{}]", token);
+        }
+
+        double top = Double.NaN;
+        double bottom = Double.NaN;
+        double left = Double.NaN;
+        double right = Double.NaN;
+
+        String currentFieldName;
+        GeoPoint sparse = new GeoPoint();
+        Rectangle envelope = null;
+
+        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
+            if (token == XContentParser.Token.FIELD_NAME) {
+                currentFieldName = parser.currentName();
+                token = parser.nextToken();
+                if (WKT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
+                    try {
+                        Geometry geometry = WKT_PARSER.fromWKT(parser.text());
+                        if (ShapeType.ENVELOPE.equals(geometry.type()) == false) {
+                            throw new ElasticsearchParseException("failed to parse WKT bounding box. ["
+                                + geometry.type() + "] found. expected [" + ShapeType.ENVELOPE + "]");
+                        }
+                        envelope = (Rectangle) geometry;
+                    } catch (ParseException|IllegalArgumentException e) {
+                        throw new ElasticsearchParseException("failed to parse WKT bounding box", e);
+                    }
+                } else if (TOP_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
+                    top = parser.doubleValue();
+                } else if (BOTTOM_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
+                    bottom = parser.doubleValue();
+                } else if (LEFT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
+                    left = parser.doubleValue();
+                } else if (RIGHT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
+                    right = parser.doubleValue();
+                } else {
+                    if (TOP_LEFT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
+                        GeoUtils.parseGeoPoint(parser, sparse, false, GeoUtils.EffectivePoint.TOP_LEFT);
+                        top = sparse.getLat();
+                        left = sparse.getLon();
+                    } else if (BOTTOM_RIGHT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
+                        GeoUtils.parseGeoPoint(parser, sparse, false, GeoUtils.EffectivePoint.BOTTOM_RIGHT);
+                        bottom = sparse.getLat();
+                        right = sparse.getLon();
+                    } else if (TOP_RIGHT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
+                        GeoUtils.parseGeoPoint(parser, sparse, false, GeoUtils.EffectivePoint.TOP_RIGHT);
+                        top = sparse.getLat();
+                        right = sparse.getLon();
+                    } else if (BOTTOM_LEFT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
+                        GeoUtils.parseGeoPoint(parser, sparse, false, GeoUtils.EffectivePoint.BOTTOM_LEFT);
+                        bottom = sparse.getLat();
+                        left = sparse.getLon();
+                    } else {
+                        throw new ElasticsearchParseException("failed to parse bounding box. unexpected field [{}]", currentFieldName);
+                    }
+                }
+            } else {
+                throw new ElasticsearchParseException("failed to parse bounding box. field name expected but [{}] found", token);
+            }
+        }
+        if (envelope != null) {
+            if (Double.isNaN(top) == false || Double.isNaN(bottom) == false || Double.isNaN(left) == false ||
+                Double.isNaN(right) == false) {
+                throw new ElasticsearchParseException("failed to parse bounding box. Conflicting definition found "
+                    + "using well-known text and explicit corners.");
+            }
+            GeoPoint topLeft = new GeoPoint(envelope.getMaxLat(), envelope.getMinLon());
+            GeoPoint bottomRight = new GeoPoint(envelope.getMinLat(), envelope.getMaxLon());
+            return new GeoBoundingBox(topLeft, bottomRight);
+        }
+        GeoPoint topLeft = new GeoPoint(top, left);
+        GeoPoint bottomRight = new GeoPoint(bottom, right);
+        return new GeoBoundingBox(topLeft, bottomRight);
+    }
+
+}

server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java

@@ -21,7 +21,6 @@ package org.elasticsearch.index.query;
 
 import org.apache.lucene.document.LatLonDocValuesField;
 import org.apache.lucene.document.LatLonPoint;
-//import org.apache.lucene.geo.Rectangle;
 import org.apache.lucene.search.IndexOrDocValuesQuery;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Query;
@@ -29,11 +28,9 @@ import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.Numbers;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.ParsingException;
+import org.elasticsearch.common.geo.GeoBoundingBox;
 import org.elasticsearch.common.geo.GeoPoint;
-import org.elasticsearch.common.geo.GeoShapeType;
 import org.elasticsearch.common.geo.GeoUtils;
-import org.elasticsearch.common.geo.builders.EnvelopeBuilder;
-import org.elasticsearch.common.geo.parsers.GeoWKTParser;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -66,24 +63,12 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
 
     private static final ParseField TYPE_FIELD = new ParseField("type");
     private static final ParseField VALIDATION_METHOD_FIELD = new ParseField("validation_method");
-    private static final ParseField TOP_FIELD = new ParseField("top");
-    private static final ParseField BOTTOM_FIELD = new ParseField("bottom");
-    private static final ParseField LEFT_FIELD = new ParseField("left");
-    private static final ParseField RIGHT_FIELD = new ParseField("right");
-    private static final ParseField TOP_LEFT_FIELD = new ParseField("top_left");
-    private static final ParseField BOTTOM_RIGHT_FIELD = new ParseField("bottom_right");
-    private static final ParseField TOP_RIGHT_FIELD = new ParseField("top_right");
-    private static final ParseField BOTTOM_LEFT_FIELD = new ParseField("bottom_left");
     private static final ParseField IGNORE_UNMAPPED_FIELD = new ParseField("ignore_unmapped");
-    private static final ParseField WKT_FIELD = new ParseField("wkt");
 
 
     /** Name of field holding geo coordinates to compute the bounding box on.*/
     private final String fieldName;
-    /** Top left corner coordinates of bounding box. */
-    private GeoPoint topLeft = new GeoPoint(Double.NaN, Double.NaN);
-    /** Bottom right corner coordinates of bounding box.*/
-    private GeoPoint bottomRight = new GeoPoint(Double.NaN, Double.NaN);
+    private GeoBoundingBox geoBoundingBox = new GeoBoundingBox(new GeoPoint(Double.NaN, Double.NaN), new GeoPoint(Double.NaN, Double.NaN));
     /** How to deal with incorrect coordinates.*/
     private GeoValidationMethod validationMethod = GeoValidationMethod.DEFAULT;
     /** How the query should be run. */
@@ -108,8 +93,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
     public GeoBoundingBoxQueryBuilder(StreamInput in) throws IOException {
         super(in);
         fieldName = in.readString();
-        topLeft = in.readGeoPoint();
-        bottomRight = in.readGeoPoint();
+        geoBoundingBox = new GeoBoundingBox(in);
         type = GeoExecType.readFromStream(in);
         validationMethod = GeoValidationMethod.readFromStream(in);
         ignoreUnmapped = in.readBoolean();
@@ -118,8 +102,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
     @Override
     protected void doWriteTo(StreamOutput out) throws IOException {
         out.writeString(fieldName);
-        out.writeGeoPoint(topLeft);
-        out.writeGeoPoint(bottomRight);
+        geoBoundingBox.writeTo(out);
         type.writeTo(out);
         validationMethod.writeTo(out);
         out.writeBoolean(ignoreUnmapped);
@@ -162,8 +145,8 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
                 // we do not check longitudes as the query generation code can deal with flipped left/right values
         }
 
-        topLeft.reset(top, left);
-        bottomRight.reset(bottom, right);
+        geoBoundingBox.topLeft().reset(top, left);
+        geoBoundingBox.bottomRight().reset(bottom, right);
         return this;
     }
 
@@ -197,12 +180,12 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
 
     /** Returns the top left corner of the bounding box. */
     public GeoPoint topLeft() {
-        return topLeft;
+        return geoBoundingBox.topLeft();
     }
 
     /** Returns the bottom right corner of the bounding box. */
     public GeoPoint bottomRight() {
-        return bottomRight;
+        return geoBoundingBox.bottomRight();
     }
 
     /**
@@ -295,6 +278,9 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
             return null;
         }
 
+        GeoPoint topLeft = geoBoundingBox.topLeft();
+        GeoPoint bottomRight = geoBoundingBox.bottomRight();
+
         QueryValidationException validationException = null;
         // For everything post 2.0 validate latitude and longitude unless validation was explicitly turned off
         if (GeoUtils.isValidLatitude(topLeft.getLat()) == false) {
@@ -335,8 +321,8 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
             throw new QueryShardException(context, "couldn't validate latitude/ longitude values", exception);
         }
 
-        GeoPoint luceneTopLeft = new GeoPoint(topLeft);
-        GeoPoint luceneBottomRight = new GeoPoint(bottomRight);
+        GeoPoint luceneTopLeft = new GeoPoint(geoBoundingBox.topLeft());
+        GeoPoint luceneBottomRight = new GeoPoint(geoBoundingBox.bottomRight());
         if (GeoValidationMethod.isCoerce(validationMethod)) {
             // Special case: if the difference between the left and right is 360 and the right is greater than the left, we are asking for
             // the complete longitude range so need to set longitude to the complete longitude range
@@ -368,8 +354,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
         builder.startObject(NAME);
 
         builder.startObject(fieldName);
-        builder.array(TOP_LEFT_FIELD.getPreferredName(), topLeft.getLon(), topLeft.getLat());
-        builder.array(BOTTOM_RIGHT_FIELD.getPreferredName(), bottomRight.getLon(), bottomRight.getLat());
+        geoBoundingBox.toXContentFragment(builder, false);
         builder.endObject();
         builder.field(VALIDATION_METHOD_FIELD.getPreferredName(), validationMethod);
         builder.field(TYPE_FIELD.getPreferredName(), type);
@@ -391,7 +376,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
         boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED;
 
         // bottom (minLat), top (maxLat), left (minLon), right (maxLon)
-        double[] bbox = null;
+        GeoBoundingBox bbox = null;
         String type = "memory";
 
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
@@ -399,7 +384,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
                 currentFieldName = parser.currentName();
             } else if (token == XContentParser.Token.START_OBJECT) {
                 try {
-                    bbox = parseBoundingBox(parser);
+                    bbox = GeoBoundingBox.parseBoundingBox(parser);
                     fieldName = currentFieldName;
                 } catch (Exception e) {
                     throw new ElasticsearchParseException("failed to parse [{}] query. [{}]", NAME, e.getMessage());
@@ -426,11 +411,8 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
             throw new ElasticsearchParseException("failed to parse [{}] query. bounding box not provided", NAME);
         }
 
-        final GeoPoint topLeft = new GeoPoint(bbox[1], bbox[2]);
-        final GeoPoint bottomRight = new GeoPoint(bbox[0], bbox[3]);
-
         GeoBoundingBoxQueryBuilder builder = new GeoBoundingBoxQueryBuilder(fieldName);
-        builder.setCorners(topLeft, bottomRight);
+        builder.setCorners(bbox.topLeft(), bbox.bottomRight());
         builder.queryName(queryName);
         builder.boost(boost);
         builder.type(GeoExecType.fromString(type));
@@ -444,8 +426,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
 
     @Override
     protected boolean doEquals(GeoBoundingBoxQueryBuilder other) {
-        return Objects.equals(topLeft, other.topLeft) &&
-                Objects.equals(bottomRight, other.bottomRight) &&
+        return Objects.equals(geoBoundingBox, other.geoBoundingBox) &&
                 Objects.equals(type, other.type) &&
                 Objects.equals(validationMethod, other.validationMethod) &&
                 Objects.equals(fieldName, other.fieldName) &&
@@ -454,7 +435,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
 
     @Override
     protected int doHashCode() {
-        return Objects.hash(topLeft, bottomRight, type, validationMethod, fieldName, ignoreUnmapped);
+        return Objects.hash(geoBoundingBox, type, validationMethod, fieldName, ignoreUnmapped);
     }
 
     @Override
@@ -462,72 +443,4 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
         return NAME;
     }
 
-    /**
-     * Parses the bounding box and returns bottom, top, left, right coordinates
-     */
-    public static double[] parseBoundingBox(XContentParser parser) throws IOException, ElasticsearchParseException {
-        XContentParser.Token token = parser.currentToken();
-        if (token != XContentParser.Token.START_OBJECT) {
-            throw new ElasticsearchParseException("failed to parse bounding box. Expected start object but found [{}]", token);
-        }
-
-        double top = Double.NaN;
-        double bottom = Double.NaN;
-        double left = Double.NaN;
-        double right = Double.NaN;
-
-        String currentFieldName;
-        GeoPoint sparse = new GeoPoint();
-        EnvelopeBuilder envelope = null;
-
-        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
-            if (token == XContentParser.Token.FIELD_NAME) {
-                currentFieldName = parser.currentName();
-                token = parser.nextToken();
-                if (WKT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    envelope = (EnvelopeBuilder)(GeoWKTParser.parseExpectedType(parser, GeoShapeType.ENVELOPE));
-                } else if (TOP_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    top = parser.doubleValue();
-                } else if (BOTTOM_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    bottom = parser.doubleValue();
-                } else if (LEFT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    left = parser.doubleValue();
-                } else if (RIGHT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    right = parser.doubleValue();
-                } else {
-                    if (TOP_LEFT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                        GeoUtils.parseGeoPoint(parser, sparse, false, GeoUtils.EffectivePoint.TOP_LEFT);
-                        top = sparse.getLat();
-                        left = sparse.getLon();
-                    } else if (BOTTOM_RIGHT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                        GeoUtils.parseGeoPoint(parser, sparse, false, GeoUtils.EffectivePoint.BOTTOM_RIGHT);
-                        bottom = sparse.getLat();
-                        right = sparse.getLon();
-                    } else if (TOP_RIGHT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                        GeoUtils.parseGeoPoint(parser, sparse, false, GeoUtils.EffectivePoint.TOP_RIGHT);
-                        top = sparse.getLat();
-                        right = sparse.getLon();
-                    } else if (BOTTOM_LEFT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                        GeoUtils.parseGeoPoint(parser, sparse, false, GeoUtils.EffectivePoint.BOTTOM_LEFT);
-                        bottom = sparse.getLat();
-                        left = sparse.getLon();
-                    } else {
-                        throw new ElasticsearchParseException("failed to parse bounding box. unexpected field [{}]", currentFieldName);
-                    }
-                }
-            } else {
-                throw new ElasticsearchParseException("failed to parse bounding box. field name expected but [{}] found", token);
-            }
-        }
-        if (envelope != null) {
-            if (Double.isNaN(top) == false || Double.isNaN(bottom) == false || Double.isNaN(left) == false ||
-                Double.isNaN(right) == false) {
-                throw new ElasticsearchParseException("failed to parse bounding box. Conflicting definition found "
-                    + "using well-known text and explicit corners.");
-            }
-            org.locationtech.spatial4j.shape.Rectangle r = envelope.buildS4J();
-            return new double[]{r.getMinY(), r.getMaxY(), r.getMinX(), r.getMaxX()};
-        }
-        return new double[]{bottom, top, left, right};
-    }
 }

server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java

@@ -19,7 +19,7 @@
 
 package org.elasticsearch.search.aggregations.metrics;
 
-import org.elasticsearch.common.ParseField;
+import org.elasticsearch.common.geo.GeoBoundingBox;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -33,14 +33,6 @@ import java.util.Map;
 import java.util.Objects;
 
 public class InternalGeoBounds extends InternalAggregation implements GeoBounds {
-
-    static final ParseField BOUNDS_FIELD = new ParseField("bounds");
-    static final ParseField TOP_LEFT_FIELD = new ParseField("top_left");
-    static final ParseField BOTTOM_RIGHT_FIELD = new ParseField("bottom_right");
-    static final ParseField LAT_FIELD = new ParseField("lat");
-    static final ParseField LON_FIELD = new ParseField("lon");
-
-
     final double top;
     final double bottom;
     final double posLeft;
@@ -132,30 +124,30 @@ public class InternalGeoBounds extends InternalAggregation implements GeoBounds
         if (path.isEmpty()) {
             return this;
         } else if (path.size() == 1) {
-            BoundingBox boundingBox = resolveBoundingBox();
+            GeoBoundingBox geoBoundingBox = resolveGeoBoundingBox();
             String bBoxSide = path.get(0);
             switch (bBoxSide) {
             case "top":
-                return boundingBox.topLeft.lat();
+                return geoBoundingBox.top();
             case "left":
-                return boundingBox.topLeft.lon();
+                return geoBoundingBox.left();
             case "bottom":
-                return boundingBox.bottomRight.lat();
+                return geoBoundingBox.bottom();
             case "right":
-                return boundingBox.bottomRight.lon();
+                return geoBoundingBox.right();
             default:
                 throw new IllegalArgumentException("Found unknown path element [" + bBoxSide + "] in [" + getName() + "]");
             }
         } else if (path.size() == 2) {
-            BoundingBox boundingBox = resolveBoundingBox();
+            GeoBoundingBox geoBoundingBox = resolveGeoBoundingBox();
             GeoPoint cornerPoint = null;
             String cornerString = path.get(0);
             switch (cornerString) {
             case "top_left":
-                cornerPoint = boundingBox.topLeft;
+                cornerPoint = geoBoundingBox.topLeft();
                 break;
             case "bottom_right":
-                cornerPoint = boundingBox.bottomRight;
+                cornerPoint = geoBoundingBox.bottomRight();
                 break;
             default:
                 throw new IllegalArgumentException("Found unknown path element [" + cornerString + "] in [" + getName() + "]");
@@ -176,78 +168,50 @@ public class InternalGeoBounds extends InternalAggregation implements GeoBounds
 
     @Override
     public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
-        GeoPoint topLeft = topLeft();
-        GeoPoint bottomRight = bottomRight();
-        if (topLeft != null) {
-            builder.startObject(BOUNDS_FIELD.getPreferredName());
-            builder.startObject(TOP_LEFT_FIELD.getPreferredName());
-            builder.field(LAT_FIELD.getPreferredName(), topLeft.lat());
-            builder.field(LON_FIELD.getPreferredName(), topLeft.lon());
-            builder.endObject();
-            builder.startObject(BOTTOM_RIGHT_FIELD.getPreferredName());
-            builder.field(LAT_FIELD.getPreferredName(), bottomRight.lat());
-            builder.field(LON_FIELD.getPreferredName(), bottomRight.lon());
-            builder.endObject();
-            builder.endObject();
+        GeoBoundingBox bbox = resolveGeoBoundingBox();
+        if (bbox != null) {
+            bbox.toXContent(builder, params);
         }
         return builder;
     }
 
-    private static class BoundingBox {
-        private final GeoPoint topLeft;
-        private final GeoPoint bottomRight;
-
-        BoundingBox(GeoPoint topLeft, GeoPoint bottomRight) {
-            this.topLeft = topLeft;
-            this.bottomRight = bottomRight;
-        }
-
-        public GeoPoint topLeft() {
-            return topLeft;
-        }
-
-        public GeoPoint bottomRight() {
-            return bottomRight;
-        }
-    }
-
-    private BoundingBox resolveBoundingBox() {
+    private GeoBoundingBox resolveGeoBoundingBox() {
         if (Double.isInfinite(top)) {
             return null;
         } else if (Double.isInfinite(posLeft)) {
-            return new BoundingBox(new GeoPoint(top, negLeft), new GeoPoint(bottom, negRight));
+            return new GeoBoundingBox(new GeoPoint(top, negLeft), new GeoPoint(bottom, negRight));
         } else if (Double.isInfinite(negLeft)) {
-            return new BoundingBox(new GeoPoint(top, posLeft), new GeoPoint(bottom, posRight));
+            return new GeoBoundingBox(new GeoPoint(top, posLeft), new GeoPoint(bottom, posRight));
         } else if (wrapLongitude) {
             double unwrappedWidth = posRight - negLeft;
             double wrappedWidth = (180 - posLeft) - (-180 - negRight);
             if (unwrappedWidth <= wrappedWidth) {
-                return new BoundingBox(new GeoPoint(top, negLeft), new GeoPoint(bottom, posRight));
+                return new GeoBoundingBox(new GeoPoint(top, negLeft), new GeoPoint(bottom, posRight));
             } else {
-                return new BoundingBox(new GeoPoint(top, posLeft), new GeoPoint(bottom, negRight));
+                return new GeoBoundingBox(new GeoPoint(top, posLeft), new GeoPoint(bottom, negRight));
             }
         } else {
-            return new BoundingBox(new GeoPoint(top, negLeft), new GeoPoint(bottom, posRight));
+            return new GeoBoundingBox(new GeoPoint(top, negLeft), new GeoPoint(bottom, posRight));
         }
     }
 
     @Override
     public GeoPoint topLeft() {
-        BoundingBox boundingBox = resolveBoundingBox();
-        if (boundingBox == null) {
+        GeoBoundingBox geoBoundingBox = resolveGeoBoundingBox();
+        if (geoBoundingBox == null) {
             return null;
         } else {
-            return boundingBox.topLeft();
+            return geoBoundingBox.topLeft();
         }
     }
 
     @Override
     public GeoPoint bottomRight() {
-        BoundingBox boundingBox = resolveBoundingBox();
-        if (boundingBox == null) {
+        GeoBoundingBox geoBoundingBox = resolveGeoBoundingBox();
+        if (geoBoundingBox == null) {
             return null;
         } else {
-            return boundingBox.bottomRight();
+            return geoBoundingBox.bottomRight();
         }
     }
 

server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.search.aggregations.metrics;
 
 import org.elasticsearch.common.collect.Tuple;
+import org.elasticsearch.common.geo.GeoBoundingBox;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.ObjectParser;
@@ -30,15 +31,14 @@ import org.elasticsearch.search.aggregations.ParsedAggregation;
 import java.io.IOException;
 
 import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
-import static org.elasticsearch.search.aggregations.metrics.InternalGeoBounds.BOTTOM_RIGHT_FIELD;
-import static org.elasticsearch.search.aggregations.metrics.InternalGeoBounds.BOUNDS_FIELD;
-import static org.elasticsearch.search.aggregations.metrics.InternalGeoBounds.LAT_FIELD;
-import static org.elasticsearch.search.aggregations.metrics.InternalGeoBounds.LON_FIELD;
-import static org.elasticsearch.search.aggregations.metrics.InternalGeoBounds.TOP_LEFT_FIELD;
+import static org.elasticsearch.common.geo.GeoBoundingBox.BOTTOM_RIGHT_FIELD;
+import static org.elasticsearch.common.geo.GeoBoundingBox.BOUNDS_FIELD;
+import static org.elasticsearch.common.geo.GeoBoundingBox.LAT_FIELD;
+import static org.elasticsearch.common.geo.GeoBoundingBox.LON_FIELD;
+import static org.elasticsearch.common.geo.GeoBoundingBox.TOP_LEFT_FIELD;
 
 public class ParsedGeoBounds extends ParsedAggregation implements GeoBounds {
-    private GeoPoint topLeft;
-    private GeoPoint bottomRight;
+    private GeoBoundingBox geoBoundingBox;
 
     @Override
     public String getType() {
@@ -47,29 +47,20 @@ public class ParsedGeoBounds extends ParsedAggregation implements GeoBounds {
 
     @Override
     public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
-        if (topLeft != null) {
-            builder.startObject(BOUNDS_FIELD.getPreferredName());
-            builder.startObject(TOP_LEFT_FIELD.getPreferredName());
-            builder.field(LAT_FIELD.getPreferredName(), topLeft.getLat());
-            builder.field(LON_FIELD.getPreferredName(), topLeft.getLon());
-            builder.endObject();
-            builder.startObject(BOTTOM_RIGHT_FIELD.getPreferredName());
-            builder.field(LAT_FIELD.getPreferredName(), bottomRight.getLat());
-            builder.field(LON_FIELD.getPreferredName(), bottomRight.getLon());
-            builder.endObject();
-            builder.endObject();
+        if (geoBoundingBox != null) {
+            geoBoundingBox.toXContent(builder, params);
         }
         return builder;
     }
 
     @Override
     public GeoPoint topLeft() {
-        return topLeft;
+        return geoBoundingBox.topLeft();
     }
 
     @Override
     public GeoPoint bottomRight() {
-        return bottomRight;
+        return geoBoundingBox.bottomRight();
     }
 
     private static final ObjectParser<ParsedGeoBounds, Void> PARSER = new ObjectParser<>(ParsedGeoBounds.class.getSimpleName(), true,
@@ -85,8 +76,7 @@ public class ParsedGeoBounds extends ParsedAggregation implements GeoBounds {
     static {
         declareAggregationFields(PARSER);
         PARSER.declareObject((agg, bbox) -> {
-            agg.topLeft = bbox.v1();
-            agg.bottomRight = bbox.v2();
+            agg.geoBoundingBox = new GeoBoundingBox(bbox.v1(), bbox.v2());
         }, BOUNDS_PARSER, BOUNDS_FIELD);
 
         BOUNDS_PARSER.declareObject(constructorArg(), GEO_POINT_PARSER, TOP_LEFT_FIELD);

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

@@ -0,0 +1,149 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.common.geo;
+
+import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+import java.util.List;
+
+import static org.hamcrest.Matchers.equalTo;
+
+
+/**
+ * Tests for {@link GeoBoundingBox}
+ */
+public class GeoBoundingBoxTests extends ESTestCase {
+
+    public void testInvalidParseInvalidWKT() throws IOException {
+        XContentBuilder bboxBuilder = XContentFactory.jsonBuilder()
+            .startObject()
+            .field("wkt", "invalid")
+            .endObject();
+        XContentParser parser = createParser(bboxBuilder);
+        parser.nextToken();
+        ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> GeoBoundingBox.parseBoundingBox(parser));
+        assertThat(e.getMessage(), equalTo("failed to parse WKT bounding box"));
+    }
+
+    public void testInvalidParsePoint() throws IOException {
+        XContentBuilder bboxBuilder = XContentFactory.jsonBuilder()
+            .startObject()
+                .field("wkt", "POINT (100.0 100.0)")
+            .endObject();
+        XContentParser parser = createParser(bboxBuilder);
+        parser.nextToken();
+        ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> GeoBoundingBox.parseBoundingBox(parser));
+        assertThat(e.getMessage(), equalTo("failed to parse WKT bounding box. [POINT] found. expected [ENVELOPE]"));
+    }
+
+    public void testWKT() throws IOException {
+        GeoBoundingBox geoBoundingBox = randomBBox();
+        assertBBox(geoBoundingBox,
+            XContentFactory.jsonBuilder()
+                .startObject()
+                .field("wkt", geoBoundingBox.toString())
+                .endObject()
+        );
+    }
+
+    public void testTopBottomLeftRight() throws Exception {
+        GeoBoundingBox geoBoundingBox = randomBBox();
+        assertBBox(geoBoundingBox,
+            XContentFactory.jsonBuilder()
+                .startObject()
+                .field("top", geoBoundingBox.top())
+                .field("bottom", geoBoundingBox.bottom())
+                .field("left", geoBoundingBox.left())
+                .field("right", geoBoundingBox.right())
+                .endObject()
+        );
+    }
+
+    public void testTopLeftBottomRight() throws Exception {
+        GeoBoundingBox geoBoundingBox = randomBBox();
+        assertBBox(geoBoundingBox,
+            XContentFactory.jsonBuilder()
+                .startObject()
+                .field("top_left", geoBoundingBox.topLeft())
+                .field("bottom_right", geoBoundingBox.bottomRight())
+                .endObject()
+        );
+    }
+
+    public void testTopRightBottomLeft() throws Exception {
+        GeoBoundingBox geoBoundingBox = randomBBox();
+        assertBBox(geoBoundingBox,
+            XContentFactory.jsonBuilder()
+                .startObject()
+                .field("top_right", new GeoPoint(geoBoundingBox.top(), geoBoundingBox.right()))
+                .field("bottom_left", new GeoPoint(geoBoundingBox.bottom(), geoBoundingBox.left()))
+                .endObject()
+        );
+    }
+
+    // test that no exception is thrown. BBOX parsing is not validated
+    public void testNullTopBottomLeftRight() throws Exception {
+        GeoBoundingBox geoBoundingBox = randomBBox();
+        XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
+        for (String field : randomSubsetOf(List.of("top", "bottom", "left", "right"))) {
+            switch (field) {
+                case "top":
+                    builder.field("top", geoBoundingBox.top());
+                    break;
+                case "bottom":
+                    builder.field("bottom", geoBoundingBox.bottom());
+                    break;
+                case "left":
+                    builder.field("left", geoBoundingBox.left());
+                    break;
+                case "right":
+                    builder.field("right", geoBoundingBox.right());
+                    break;
+                default:
+                    throw new IllegalStateException("unexpected branching");
+            }
+        }
+        builder.endObject();
+        try (XContentParser parser = createParser(builder)) {
+            parser.nextToken();
+            GeoBoundingBox.parseBoundingBox(parser);
+        }
+    }
+
+    private void assertBBox(GeoBoundingBox expected, XContentBuilder builder) throws IOException {
+        try (XContentParser parser = createParser(builder)) {
+            parser.nextToken();
+            assertThat(GeoBoundingBox.parseBoundingBox(parser), equalTo(expected));
+        }
+    }
+
+    private GeoBoundingBox randomBBox() {
+        double topLat = GeometryTestUtils.randomLat();
+        double bottomLat = randomDoubleBetween(GeoUtils.MIN_LAT, topLat, true);
+        return new GeoBoundingBox(new GeoPoint(topLat, GeometryTestUtils.randomLon()),
+            new GeoPoint(bottomLat, GeometryTestUtils.randomLon()));
+    }
+}

server/src/test/java/org/elasticsearch/search/geo/GeoBoundingBoxIT.java → server/src/test/java/org/elasticsearch/search/geo/GeoBoundingBoxQueryIT.java

@@ -39,7 +39,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcke
 import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.equalTo;
 
-public class GeoBoundingBoxIT extends ESIntegTestCase {
+public class GeoBoundingBoxQueryIT extends ESIntegTestCase {
 
     @Override
     protected boolean forbidPrivateIndexSettings() {