From bed121efaf2be6e84ccc3687bc30796411af0a6a Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Mon, 23 Dec 2019 11:21:39 -0800 Subject: [PATCH] [7.x-backport] Centralize BoundingBox logic to a dedicated class (#50469) 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 --- .../common/geo/GeoBoundingBox.java | 229 ++++++++++++++++++ .../query/GeoBoundingBoxQueryBuilder.java | 125 ++-------- .../metrics/InternalGeoBounds.java | 84 ++----- .../aggregations/metrics/ParsedGeoBounds.java | 34 +-- .../common/geo/GeoBoundingBoxTests.java | 149 ++++++++++++ ...gBoxIT.java => GeoBoundingBoxQueryIT.java} | 2 +- 6 files changed, 434 insertions(+), 189 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java create mode 100644 server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java 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/GeoBoundingBox.java b/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java new file mode 100644 index 00000000000..78e81925275 --- /dev/null +++ b/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); + } + +} 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 699d4b79aa7..59025dc03e1 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 @@ 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 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); diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java new file mode 100644 index 00000000000..25c93cedde0 --- /dev/null +++ b/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.Arrays; + +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(Arrays.asList("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())); + } +} 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 4306ae680aa..8bfaa124821 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.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() {