From cceda1f1ec53abcf98edda7900ea37ba0e8da582 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Mon, 16 Dec 2019 14:22:18 -0800 Subject: [PATCH 1/5] Centralize BoundingBox logic to a dedicated class 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 --- .../elasticsearch/common/geo/BoundingBox.java | 217 ++++++++++++++++++ .../query/GeoBoundingBoxQueryBuilder.java | 125 ++-------- .../metrics/InternalGeoBounds.java | 56 +---- .../aggregations/metrics/ParsedGeoBounds.java | 34 +-- 4 files changed, 258 insertions(+), 174 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java diff --git a/server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java b/server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java new file mode 100644 index 0000000000000..c37d1e8bf1f83 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java @@ -0,0 +1,217 @@ +/* + * 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 java.io.IOException; +import java.text.ParseException; +import java.util.Objects; + +public class BoundingBox implements ToXContentObject, Writeable { + private static final GeometryParser GEOMETRY_PARSER = new GeometryParser(true, true, 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 BoundingBox(GeoPoint topLeft, GeoPoint bottomRight) { + this.topLeft = topLeft; + this.bottomRight = bottomRight; + } + + public BoundingBox(StreamInput input) throws IOException { + this.topLeft = input.readGeoPoint(); + this.bottomRight = input.readGeoPoint(); + } + + /** + * Parses the bounding box and returns bottom, top, left, right coordinates + */ + public static BoundingBox 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 = GEOMETRY_PARSER.parse(parser); + if (ShapeType.ENVELOPE.equals(geometry.type()) == false) { + throw new ElasticsearchParseException("failed to parse WKT bounding box. [" + + geometry.type() + "] found instead"); + } + envelope = (Rectangle) geometry; + } catch (ParseException 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 BoundingBox(topLeft, bottomRight); + } + GeoPoint topLeft = new GeoPoint(top, left); + GeoPoint bottomRight = new GeoPoint(bottom, right); + return new BoundingBox(topLeft, bottomRight); + } + + 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 null; + } + + 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; + BoundingBox that = (BoundingBox) o; + return topLeft.equals(that.topLeft) && + bottomRight.equals(that.bottomRight); + } + + @Override + public int hashCode() { + return Objects.hash(topLeft, bottomRight); + } +} diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java index 699d4b79aa72b..14fcc314b9d72 100644 --- a/server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java @@ -21,7 +21,6 @@ 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.common.Numbers; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.geo.BoundingBox; 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 path) { String bBoxSide = path.get(0); switch (bBoxSide) { case "top": - return boundingBox.topLeft.lat(); + return boundingBox.top(); case "left": - return boundingBox.topLeft.lon(); + return boundingBox.left(); case "bottom": - return boundingBox.bottomRight.lat(); + return boundingBox.bottom(); case "right": - return boundingBox.bottomRight.lon(); + return boundingBox.right(); default: throw new IllegalArgumentException("Found unknown path element [" + bBoxSide + "] in [" + getName() + "]"); } @@ -152,10 +144,10 @@ public Object getProperty(List path) { String cornerString = path.get(0); switch (cornerString) { case "top_left": - cornerPoint = boundingBox.topLeft; + cornerPoint = boundingBox.topLeft(); break; case "bottom_right": - cornerPoint = boundingBox.bottomRight; + cornerPoint = boundingBox.bottomRight(); break; default: throw new IllegalArgumentException("Found unknown path element [" + cornerString + "] in [" + getName() + "]"); @@ -176,41 +168,13 @@ public Object getProperty(List path) { @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(); + BoundingBox bbox = resolveBoundingBox(); + 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() { if (Double.isInfinite(top)) { return null; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java index 11d36d2ceeead..635b853d466b9 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java +++ b/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.BoundingBox; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ObjectParser; @@ -30,15 +31,14 @@ 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.BoundingBox.BOTTOM_RIGHT_FIELD; +import static org.elasticsearch.common.geo.BoundingBox.BOUNDS_FIELD; +import static org.elasticsearch.common.geo.BoundingBox.LAT_FIELD; +import static org.elasticsearch.common.geo.BoundingBox.LON_FIELD; +import static org.elasticsearch.common.geo.BoundingBox.TOP_LEFT_FIELD; public class ParsedGeoBounds extends ParsedAggregation implements GeoBounds { - private GeoPoint topLeft; - private GeoPoint bottomRight; + private BoundingBox boundingBox; @Override public String getType() { @@ -47,29 +47,20 @@ public String getType() { @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 (boundingBox != null) { + boundingBox.toXContent(builder, params); } return builder; } @Override public GeoPoint topLeft() { - return topLeft; + return boundingBox.topLeft(); } @Override public GeoPoint bottomRight() { - return bottomRight; + return boundingBox.bottomRight(); } private static final ObjectParser PARSER = new ObjectParser<>(ParsedGeoBounds.class.getSimpleName(), true, @@ -85,8 +76,7 @@ public GeoPoint bottomRight() { static { declareAggregationFields(PARSER); PARSER.declareObject((agg, bbox) -> { - agg.topLeft = bbox.v1(); - agg.bottomRight = bbox.v2(); + agg.boundingBox = new BoundingBox(bbox.v1(), bbox.v2()); }, BOUNDS_PARSER, BOUNDS_FIELD); BOUNDS_PARSER.declareObject(constructorArg(), GEO_POINT_PARSER, TOP_LEFT_FIELD); From 216348e1ae5274c2a33d94ab3bbf6f73385b36cc Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Tue, 17 Dec 2019 12:33:58 -0800 Subject: [PATCH 2/5] add tests --- .../elasticsearch/common/geo/BoundingBox.java | 162 ++++++++++-------- .../common/geo/BoundingBoxTests.java | 139 +++++++++++++++ 2 files changed, 226 insertions(+), 75 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java diff --git a/server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java b/server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java index c37d1e8bf1f83..729c27c14b843 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java +++ b/server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java @@ -29,13 +29,19 @@ 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 BoundingBox implements ToXContentObject, Writeable { - private static final GeometryParser GEOMETRY_PARSER = new GeometryParser(true, true, true); + 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"); @@ -62,6 +68,83 @@ public BoundingBox(StreamInput input) throws IOException { 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; + BoundingBox that = (BoundingBox) 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 */ @@ -86,13 +169,13 @@ public static BoundingBox parseBoundingBox(XContentParser parser) throws IOExcep token = parser.nextToken(); if (WKT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { try { - Geometry geometry = GEOMETRY_PARSER.parse(parser); + 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 instead"); + + geometry.type() + "] found. expected [" + ShapeType.ENVELOPE + "]"); } envelope = (Rectangle) geometry; - } catch (ParseException e) { + } catch (ParseException|IllegalArgumentException e) { throw new ElasticsearchParseException("failed to parse WKT bounding box", e); } } else if (TOP_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { @@ -143,75 +226,4 @@ public static BoundingBox parseBoundingBox(XContentParser parser) throws IOExcep return new BoundingBox(topLeft, bottomRight); } - 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 null; - } - - 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; - BoundingBox that = (BoundingBox) o; - return topLeft.equals(that.topLeft) && - bottomRight.equals(that.bottomRight); - } - - @Override - public int hashCode() { - return Objects.hash(topLeft, bottomRight); - } } diff --git a/server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java b/server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java new file mode 100644 index 0000000000000..fe30bb919ce76 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java @@ -0,0 +1,139 @@ +/* + * 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 {@code GeoJSONShapeParser} + */ +public class BoundingBoxTests 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, () -> BoundingBox.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, () -> BoundingBox.parseBoundingBox(parser)); + assertThat(e.getMessage(), equalTo("failed to parse WKT bounding box. [POINT] found. expected [ENVELOPE]")); + } + + public void testWKT() throws IOException { + BoundingBox boundingBox = randomBBox(); + assertBBox(boundingBox, + XContentFactory.jsonBuilder() + .startObject() + .field("wkt", boundingBox.toString()) + .endObject() + ); + } + + public void testTopBottomLeftRight() throws Exception { + BoundingBox boundingBox = randomBBox(); + assertBBox(boundingBox, + XContentFactory.jsonBuilder() + .startObject() + .field("top", boundingBox.top()) + .field("bottom", boundingBox.bottom()) + .field("left", boundingBox.left()) + .field("right", boundingBox.right()) + .endObject() + ); + } + + public void testTopLeftBottomRight() throws Exception { + BoundingBox boundingBox = randomBBox(); + assertBBox(boundingBox, + XContentFactory.jsonBuilder() + .startObject() + .field("top_left", boundingBox.topLeft()) + .field("bottom_right", boundingBox.bottomRight()) + .endObject() + ); + } + + public void testTopRightBottomLeft() throws Exception { + BoundingBox boundingBox = randomBBox(); + assertBBox(boundingBox, + XContentFactory.jsonBuilder() + .startObject() + .field("top_right", new GeoPoint(boundingBox.top(), boundingBox.right())) + .field("bottom_left", new GeoPoint(boundingBox.bottom(), boundingBox.left())) + .endObject() + ); + } + + // test that no exception is thrown. BBOX parsing is not validated + public void testNullTopBottomLeftRight() throws Exception { + BoundingBox boundingBox = randomBBox(); + XContentBuilder builder = XContentFactory.jsonBuilder().startObject(); + for (String field : randomSubsetOf(List.of("top", "bottom", "left", "right"))) { + switch (field) { + case "top": builder.field("top", boundingBox.top()); break; + case "bottom": builder.field("bottom", boundingBox.bottom()); break; + case "left": builder.field("left", boundingBox.left()); break; + case "right": builder.field("right", boundingBox.right()); break; + } + } + builder.endObject(); + try (XContentParser parser = createParser(builder)) { + parser.nextToken(); + BoundingBox.parseBoundingBox(parser); + } + } + + private void assertBBox(BoundingBox expected, XContentBuilder builder) throws IOException { + try (XContentParser parser = createParser(builder)) { + parser.nextToken(); + assertThat(BoundingBox.parseBoundingBox(parser), equalTo(expected)); + } + } + + private BoundingBox randomBBox() { + double topLat = GeometryTestUtils.randomLat(); + double bottomLat = randomDoubleBetween(GeoUtils.MIN_LAT, topLat, true); + return new BoundingBox(new GeoPoint(topLat, GeometryTestUtils.randomLon()), + new GeoPoint(bottomLat, GeometryTestUtils.randomLon())); + } +} From 6eab3fa148451cde95696c04492e9738b96b922d Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Tue, 17 Dec 2019 13:26:29 -0800 Subject: [PATCH 3/5] fix checkstyle --- .../common/geo/BoundingBoxTests.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java b/server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java index fe30bb919ce76..252b42f825d57 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java @@ -110,10 +110,20 @@ public void testNullTopBottomLeftRight() throws Exception { XContentBuilder builder = XContentFactory.jsonBuilder().startObject(); for (String field : randomSubsetOf(List.of("top", "bottom", "left", "right"))) { switch (field) { - case "top": builder.field("top", boundingBox.top()); break; - case "bottom": builder.field("bottom", boundingBox.bottom()); break; - case "left": builder.field("left", boundingBox.left()); break; - case "right": builder.field("right", boundingBox.right()); break; + case "top": + builder.field("top", boundingBox.top()); + break; + case "bottom": + builder.field("bottom", boundingBox.bottom()); + break; + case "left": + builder.field("left", boundingBox.left()); + break; + case "right": + builder.field("right", boundingBox.right()); + break; + default: + throw new IllegalStateException("unexpected branching"); } } builder.endObject(); From 4371bdfc6d1b9089be87c0de0cee3a7ecff7eee2 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Wed, 18 Dec 2019 09:45:31 -0800 Subject: [PATCH 4/5] rename BoundingBox -> GeoBoundingBox --- .../{BoundingBox.java => GeoBoundingBox.java} | 14 ++--- .../query/GeoBoundingBoxQueryBuilder.java | 34 +++++------ .../metrics/InternalGeoBounds.java | 44 +++++++------- .../aggregations/metrics/ParsedGeoBounds.java | 24 ++++---- ...BoxTests.java => GeoBoundingBoxTests.java} | 60 +++++++++---------- ...gBoxIT.java => GeoBoundingBoxQueryIT.java} | 2 +- 6 files changed, 89 insertions(+), 89 deletions(-) rename server/src/main/java/org/elasticsearch/common/geo/{BoundingBox.java => GeoBoundingBox.java} (95%) rename server/src/test/java/org/elasticsearch/common/geo/{BoundingBoxTests.java => GeoBoundingBoxTests.java} (69%) rename server/src/test/java/org/elasticsearch/search/geo/{GeoBoundingBoxIT.java => GeoBoundingBoxQueryIT.java} (99%) diff --git a/server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java b/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java similarity index 95% rename from server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java rename to server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java index 729c27c14b843..78e81925275d7 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/BoundingBox.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java @@ -40,7 +40,7 @@ * 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 BoundingBox implements ToXContentObject, Writeable { +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"); @@ -58,12 +58,12 @@ public class BoundingBox implements ToXContentObject, Writeable { private final GeoPoint topLeft; private final GeoPoint bottomRight; - public BoundingBox(GeoPoint topLeft, GeoPoint bottomRight) { + public GeoBoundingBox(GeoPoint topLeft, GeoPoint bottomRight) { this.topLeft = topLeft; this.bottomRight = bottomRight; } - public BoundingBox(StreamInput input) throws IOException { + public GeoBoundingBox(StreamInput input) throws IOException { this.topLeft = input.readGeoPoint(); this.bottomRight = input.readGeoPoint(); } @@ -130,7 +130,7 @@ public void writeTo(StreamOutput out) throws IOException { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - BoundingBox that = (BoundingBox) o; + GeoBoundingBox that = (GeoBoundingBox) o; return topLeft.equals(that.topLeft) && bottomRight.equals(that.bottomRight); } @@ -148,7 +148,7 @@ public String toString() { /** * Parses the bounding box and returns bottom, top, left, right coordinates */ - public static BoundingBox parseBoundingBox(XContentParser parser) throws IOException, ElasticsearchParseException { + 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); @@ -219,11 +219,11 @@ public static BoundingBox parseBoundingBox(XContentParser parser) throws IOExcep } GeoPoint topLeft = new GeoPoint(envelope.getMaxLat(), envelope.getMinLon()); GeoPoint bottomRight = new GeoPoint(envelope.getMinLat(), envelope.getMaxLon()); - return new BoundingBox(topLeft, bottomRight); + return new GeoBoundingBox(topLeft, bottomRight); } GeoPoint topLeft = new GeoPoint(top, left); GeoPoint bottomRight = new GeoPoint(bottom, right); - return new BoundingBox(topLeft, bottomRight); + return new GeoBoundingBox(topLeft, bottomRight); } } diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java index 14fcc314b9d72..59025dc03e161 100644 --- a/server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java @@ -28,7 +28,7 @@ import org.elasticsearch.common.Numbers; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; -import org.elasticsearch.common.geo.BoundingBox; +import org.elasticsearch.common.geo.GeoBoundingBox; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.io.stream.StreamInput; @@ -68,7 +68,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder path) { 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.top(); + return geoBoundingBox.top(); case "left": - return boundingBox.left(); + return geoBoundingBox.left(); case "bottom": - return boundingBox.bottom(); + return geoBoundingBox.bottom(); case "right": - return boundingBox.right(); + 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() + "]"); @@ -168,50 +168,50 @@ public Object getProperty(List path) { @Override public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { - BoundingBox bbox = resolveBoundingBox(); + GeoBoundingBox bbox = resolveGeoBoundingBox(); if (bbox != null) { bbox.toXContent(builder, params); } return builder; } - 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(); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java index 635b853d466b9..2b29ae15119d9 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java @@ -20,7 +20,7 @@ package org.elasticsearch.search.aggregations.metrics; import org.elasticsearch.common.collect.Tuple; -import org.elasticsearch.common.geo.BoundingBox; +import org.elasticsearch.common.geo.GeoBoundingBox; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ObjectParser; @@ -31,14 +31,14 @@ import java.io.IOException; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; -import static org.elasticsearch.common.geo.BoundingBox.BOTTOM_RIGHT_FIELD; -import static org.elasticsearch.common.geo.BoundingBox.BOUNDS_FIELD; -import static org.elasticsearch.common.geo.BoundingBox.LAT_FIELD; -import static org.elasticsearch.common.geo.BoundingBox.LON_FIELD; -import static org.elasticsearch.common.geo.BoundingBox.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 BoundingBox boundingBox; + private GeoBoundingBox geoBoundingBox; @Override public String getType() { @@ -47,20 +47,20 @@ public String getType() { @Override public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { - if (boundingBox != null) { - boundingBox.toXContent(builder, params); + if (geoBoundingBox != null) { + geoBoundingBox.toXContent(builder, params); } return builder; } @Override public GeoPoint topLeft() { - return boundingBox.topLeft(); + return geoBoundingBox.topLeft(); } @Override public GeoPoint bottomRight() { - return boundingBox.bottomRight(); + return geoBoundingBox.bottomRight(); } private static final ObjectParser PARSER = new ObjectParser<>(ParsedGeoBounds.class.getSimpleName(), true, @@ -76,7 +76,7 @@ public GeoPoint bottomRight() { static { declareAggregationFields(PARSER); PARSER.declareObject((agg, bbox) -> { - agg.boundingBox = new BoundingBox(bbox.v1(), bbox.v2()); + agg.geoBoundingBox = new GeoBoundingBox(bbox.v1(), bbox.v2()); }, BOUNDS_PARSER, BOUNDS_FIELD); BOUNDS_PARSER.declareObject(constructorArg(), GEO_POINT_PARSER, TOP_LEFT_FIELD); diff --git a/server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java similarity index 69% rename from server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java rename to server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java index 252b42f825d57..da7758a720022 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/BoundingBoxTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java @@ -35,7 +35,7 @@ /** * Tests for {@code GeoJSONShapeParser} */ -public class BoundingBoxTests extends ESTestCase { +public class GeoBoundingBoxTests extends ESTestCase { public void testInvalidParseInvalidWKT() throws IOException { XContentBuilder bboxBuilder = XContentFactory.jsonBuilder() @@ -44,7 +44,7 @@ public void testInvalidParseInvalidWKT() throws IOException { .endObject(); XContentParser parser = createParser(bboxBuilder); parser.nextToken(); - ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> BoundingBox.parseBoundingBox(parser)); + ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> GeoBoundingBox.parseBoundingBox(parser)); assertThat(e.getMessage(), equalTo("failed to parse WKT bounding box")); } @@ -55,72 +55,72 @@ public void testInvalidParsePoint() throws IOException { .endObject(); XContentParser parser = createParser(bboxBuilder); parser.nextToken(); - ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> BoundingBox.parseBoundingBox(parser)); + 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 { - BoundingBox boundingBox = randomBBox(); - assertBBox(boundingBox, + GeoBoundingBox geoBoundingBox = randomBBox(); + assertBBox(geoBoundingBox, XContentFactory.jsonBuilder() .startObject() - .field("wkt", boundingBox.toString()) + .field("wkt", geoBoundingBox.toString()) .endObject() ); } public void testTopBottomLeftRight() throws Exception { - BoundingBox boundingBox = randomBBox(); - assertBBox(boundingBox, + GeoBoundingBox geoBoundingBox = randomBBox(); + assertBBox(geoBoundingBox, XContentFactory.jsonBuilder() .startObject() - .field("top", boundingBox.top()) - .field("bottom", boundingBox.bottom()) - .field("left", boundingBox.left()) - .field("right", boundingBox.right()) + .field("top", geoBoundingBox.top()) + .field("bottom", geoBoundingBox.bottom()) + .field("left", geoBoundingBox.left()) + .field("right", geoBoundingBox.right()) .endObject() ); } public void testTopLeftBottomRight() throws Exception { - BoundingBox boundingBox = randomBBox(); - assertBBox(boundingBox, + GeoBoundingBox geoBoundingBox = randomBBox(); + assertBBox(geoBoundingBox, XContentFactory.jsonBuilder() .startObject() - .field("top_left", boundingBox.topLeft()) - .field("bottom_right", boundingBox.bottomRight()) + .field("top_left", geoBoundingBox.topLeft()) + .field("bottom_right", geoBoundingBox.bottomRight()) .endObject() ); } public void testTopRightBottomLeft() throws Exception { - BoundingBox boundingBox = randomBBox(); - assertBBox(boundingBox, + GeoBoundingBox geoBoundingBox = randomBBox(); + assertBBox(geoBoundingBox, XContentFactory.jsonBuilder() .startObject() - .field("top_right", new GeoPoint(boundingBox.top(), boundingBox.right())) - .field("bottom_left", new GeoPoint(boundingBox.bottom(), boundingBox.left())) + .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 { - BoundingBox boundingBox = randomBBox(); + 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", boundingBox.top()); + builder.field("top", geoBoundingBox.top()); break; case "bottom": - builder.field("bottom", boundingBox.bottom()); + builder.field("bottom", geoBoundingBox.bottom()); break; case "left": - builder.field("left", boundingBox.left()); + builder.field("left", geoBoundingBox.left()); break; case "right": - builder.field("right", boundingBox.right()); + builder.field("right", geoBoundingBox.right()); break; default: throw new IllegalStateException("unexpected branching"); @@ -129,21 +129,21 @@ public void testNullTopBottomLeftRight() throws Exception { builder.endObject(); try (XContentParser parser = createParser(builder)) { parser.nextToken(); - BoundingBox.parseBoundingBox(parser); + GeoBoundingBox.parseBoundingBox(parser); } } - private void assertBBox(BoundingBox expected, XContentBuilder builder) throws IOException { + private void assertBBox(GeoBoundingBox expected, XContentBuilder builder) throws IOException { try (XContentParser parser = createParser(builder)) { parser.nextToken(); - assertThat(BoundingBox.parseBoundingBox(parser), equalTo(expected)); + assertThat(GeoBoundingBox.parseBoundingBox(parser), equalTo(expected)); } } - private BoundingBox randomBBox() { + private GeoBoundingBox randomBBox() { double topLat = GeometryTestUtils.randomLat(); double bottomLat = randomDoubleBetween(GeoUtils.MIN_LAT, topLat, true); - return new BoundingBox(new GeoPoint(topLat, GeometryTestUtils.randomLon()), + return new GeoBoundingBox(new GeoPoint(topLat, GeometryTestUtils.randomLon()), new GeoPoint(bottomLat, GeometryTestUtils.randomLon())); } } diff --git a/server/src/test/java/org/elasticsearch/search/geo/GeoBoundingBoxIT.java b/server/src/test/java/org/elasticsearch/search/geo/GeoBoundingBoxQueryIT.java similarity index 99% rename from server/src/test/java/org/elasticsearch/search/geo/GeoBoundingBoxIT.java rename to server/src/test/java/org/elasticsearch/search/geo/GeoBoundingBoxQueryIT.java index 6e3f853e90eab..fe7800e137c9c 100644 --- a/server/src/test/java/org/elasticsearch/search/geo/GeoBoundingBoxIT.java +++ b/server/src/test/java/org/elasticsearch/search/geo/GeoBoundingBoxQueryIT.java @@ -39,7 +39,7 @@ 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() { From 1326d8c88e495d5e9bec5e36d81776230668d73e Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Mon, 23 Dec 2019 09:21:12 -0800 Subject: [PATCH 5/5] fix comment in GeoBoundingBoxTests --- .../java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java index da7758a720022..ef705fa27c42c 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java @@ -33,7 +33,7 @@ /** - * Tests for {@code GeoJSONShapeParser} + * Tests for {@link GeoBoundingBox} */ public class GeoBoundingBoxTests extends ESTestCase {