From af29e5e197266195b8d2c90f36b08d1426432196 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Mon, 9 Dec 2019 15:47:07 -0800 Subject: [PATCH] Adds support for geo-bounds filtering in geogrid aggregations It is fairly common to filter the geo point candidates in geohash_grid and geotile_grid aggregations according to some viewable bounding box. This change introduces the option of specifying this filter directly in the tiling aggregation. --- .../bucket/geohashgrid-aggregation.asciidoc | 58 +++++++++++ .../bucket/geotilegrid-aggregation.asciidoc | 58 +++++++++++ .../query-dsl/geo-bounding-box-query.asciidoc | 1 + .../common/geo/GeoBoundingBox.java | 25 +++++ .../elasticsearch/common/geo/GeoUtils.java | 1 + .../GeoTileGridValuesSourceBuilder.java | 27 ++++- .../bucket/geogrid/CellIdSource.java | 22 +++-- .../geogrid/GeoGridAggregationBuilder.java | 40 +++++++- .../bucket/geogrid/GeoGridAggregator.java | 4 +- .../GeoHashGridAggregationBuilder.java | 12 ++- .../bucket/geogrid/GeoHashGridAggregator.java | 8 +- .../geogrid/GeoHashGridAggregatorFactory.java | 19 ++-- .../GeoTileGridAggregationBuilder.java | 12 +-- .../bucket/geogrid/GeoTileGridAggregator.java | 8 +- .../geogrid/GeoTileGridAggregatorFactory.java | 19 ++-- .../bucket/geogrid/GeoTileUtils.java | 2 +- .../common/geo/GeoBoundingBoxTests.java | 31 +++++- .../aggregations/bucket/GeoHashGridTests.java | 4 + .../aggregations/bucket/GeoTileGridTests.java | 49 +++++++++ .../CompositeAggregationBuilderTests.java | 9 +- .../geogrid/GeoGridAggregatorTestCase.java | 99 ++++++++++++++++--- .../geogrid/GeoHashGridParserTests.java | 17 ++++ .../geogrid/GeoTileGridAggregatorTests.java | 1 - .../geogrid/GeoTileGridParserTests.java | 17 ++++ 24 files changed, 476 insertions(+), 67 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoTileGridTests.java diff --git a/docs/reference/aggregations/bucket/geohashgrid-aggregation.asciidoc b/docs/reference/aggregations/bucket/geohashgrid-aggregation.asciidoc index 59033843078b1..8d4652aebe2c5 100644 --- a/docs/reference/aggregations/bucket/geohashgrid-aggregation.asciidoc +++ b/docs/reference/aggregations/bucket/geohashgrid-aggregation.asciidoc @@ -191,6 +191,62 @@ var bbox = geohash.decode_bbox('u17'); -------------------------------------------------- // NOTCONSOLE +==== Requests with additional bounding box filtering + +The `geohash_grid` aggregation supports an optional `bounds` parameter +that restricts the points considered to those that fall within the +bounds provided. The `bounds` parameter accepts the bounding box in +all the same <> of the +bounds specified in the Geo Bounding Box Query. This bounding box can be used with or +without an additional `geo_bounding_box` query filtering the points prior to aggregating. +It is an independent bounding box that can intersect with, be equal to, or be disjoint +to any additional `geo_bounding_box` queries defined in the context of the aggregation. + +[source,console,id=geohashgrid-aggregation-with-bounds] +-------------------------------------------------- +POST /museums/_search?size=0 +{ + "aggregations" : { + "tiles-in-bounds" : { + "geohash_grid" : { + "field" : "location", + "precision" : 8, + "bounds": { + "top_left" : "53.4375, 4.21875", + "bottom_right" : "52.03125, 5.625" + } + } + } + } +} +-------------------------------------------------- +// TEST[continued] + +[source,console-result] +-------------------------------------------------- +{ + ... + "aggregations" : { + "tiles-in-bounds" : { + "buckets" : [ + { + "key" : "u173zy3j", + "doc_count" : 1 + }, + { + "key" : "u173zvfz", + "doc_count" : 1 + }, + { + "key" : "u173zt90", + "doc_count" : 1 + } + ] + } + } +} +-------------------------------------------------- +// TESTRESPONSE[s/\.\.\./"took": $body.took,"_shards": $body._shards,"hits":$body.hits,"timed_out":false,/] ==== Cell dimensions at the equator The table below shows the metric dimensions for cells covered by various string lengths of geohash. @@ -230,6 +286,8 @@ precision:: Optional. The string length of the geohashes used to define to precision levels higher than the supported 12 levels, (e.g. for distances <5.6cm) the value is rejected. +bounds: Optional. The bounding box to filter the points in the bucket. + size:: Optional. The maximum number of geohash buckets to return (defaults to 10,000). When results are trimmed, buckets are prioritised based on the volumes of documents they contain. diff --git a/docs/reference/aggregations/bucket/geotilegrid-aggregation.asciidoc b/docs/reference/aggregations/bucket/geotilegrid-aggregation.asciidoc index c60b14b0f2687..25cc1a977c86b 100644 --- a/docs/reference/aggregations/bucket/geotilegrid-aggregation.asciidoc +++ b/docs/reference/aggregations/bucket/geotilegrid-aggregation.asciidoc @@ -162,6 +162,62 @@ POST /museums/_search?size=0 -------------------------------------------------- // TESTRESPONSE[s/\.\.\./"took": $body.took,"_shards": $body._shards,"hits":$body.hits,"timed_out":false,/] +==== Requests with additional bounding box filtering + +The `geotile_grid` aggregation supports an optional `bounds` parameter +that restricts the points considered to those that fall within the +bounds provided. The `bounds` parameter accepts the bounding box in +all the same <> of the +bounds specified in the Geo Bounding Box Query. This bounding box can be used with or +without an additional `geo_bounding_box` query filtering the points prior to aggregating. +It is an independent bounding box that can intersect with, be equal to, or be disjoint +to any additional `geo_bounding_box` queries defined in the context of the aggregation. + +[source,console,id=geotilegrid-aggregation-with-bounds] +-------------------------------------------------- +POST /museums/_search?size=0 +{ + "aggregations" : { + "tiles-in-bounds" : { + "geotile_grid" : { + "field" : "location", + "precision" : 22, + "bounds": { + "top_left" : "52.4, 4.9", + "bottom_right" : "52.3, 5.0" + } + } + } + } +} +-------------------------------------------------- +// TEST[continued] + +[source,console-result] +-------------------------------------------------- +{ + ... + "aggregations" : { + "tiles-in-bounds" : { + "buckets" : [ + { + "key" : "22/2154412/1378379", + "doc_count" : 1 + }, + { + "key" : "22/2154385/1378332", + "doc_count" : 1 + }, + { + "key" : "22/2154259/1378425", + "doc_count" : 1 + } + ] + } + } +} +-------------------------------------------------- +// TESTRESPONSE[s/\.\.\./"took": $body.took,"_shards": $body._shards,"hits":$body.hits,"timed_out":false,/] ==== Options @@ -172,6 +228,8 @@ precision:: Optional. The integer zoom of the key used to define cells/buckets in the results. Defaults to 7. Values outside of [0,29] will be rejected. +bounds: Optional. The bounding box to filter the points in the bucket. + size:: Optional. The maximum number of geohash buckets to return (defaults to 10,000). When results are trimmed, buckets are prioritised based on the volumes of documents they contain. diff --git a/docs/reference/query-dsl/geo-bounding-box-query.asciidoc b/docs/reference/query-dsl/geo-bounding-box-query.asciidoc index a51283ceeb341..27f37423fa28f 100644 --- a/docs/reference/query-dsl/geo-bounding-box-query.asciidoc +++ b/docs/reference/query-dsl/geo-bounding-box-query.asciidoc @@ -84,6 +84,7 @@ be executed in memory or indexed. See <> below for further d Default is `memory`. |======================================================================= +[[query-dsl-geo-bounding-box-query-accepted-formats]] [float] ==== Accepted Formats diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java b/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java index 78e81925275d7..01315d3e9848c 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java @@ -68,6 +68,11 @@ public GeoBoundingBox(StreamInput input) throws IOException { this.bottomRight = input.readGeoPoint(); } + public boolean isUnbounded() { + return Double.isNaN(topLeft.lon()) || Double.isNaN(topLeft.lat()) + || Double.isNaN(bottomRight.lon()) || Double.isNaN(bottomRight.lat()); + } + public GeoPoint topLeft() { return topLeft; } @@ -120,6 +125,26 @@ public XContentBuilder toXContentFragment(XContentBuilder builder, boolean build return builder; } + /** + * If the bounding box crosses the date-line (left greater-than right) then the + * longitude of the point need only to be higher than the left or lower + * than the right. Otherwise, it must be both. + * + * @param lon the longitude of the point + * @param lat the latitude of the point + * @return whether the point (lon, lat) is in the specified bounding box + */ + public boolean pointInBounds(double lon, double lat) { + if (lat >= bottom() && lat <= top()) { + if (left() <= right()) { + return lon >= left() && lon <= right(); + } else { + return lon >= left() || lon <= right(); + } + } + return false; + } + @Override public void writeTo(StreamOutput out) throws IOException { out.writeGeoPoint(topLeft); diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java index 359b7781b895f..d33f90043b1d5 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java @@ -93,6 +93,7 @@ public static boolean isValidLongitude(double longitude) { return true; } + /** * Calculate the width (in meters) of geohash cells at a specific level * @param level geohash level must be greater or equal to zero diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java index b6f2b2788cd25..c8aef28f871ef 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java @@ -19,7 +19,10 @@ package org.elasticsearch.search.aggregations.bucket.composite; +import org.elasticsearch.Version; 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; import org.elasticsearch.common.xcontent.ObjectParser; @@ -45,6 +48,8 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder static { PARSER = new ObjectParser<>(GeoTileGridValuesSourceBuilder.TYPE); PARSER.declareInt(GeoTileGridValuesSourceBuilder::precision, new ParseField("precision")); + PARSER.declareField(((p, builder, context) -> builder.geoBoundingBox(GeoBoundingBox.parseBoundingBox(p))), + GeoBoundingBox.BOUNDS_FIELD, ObjectParser.ValueType.OBJECT); CompositeValuesSourceParserHelper.declareValuesSourceFields(PARSER, ValueType.NUMERIC); } @@ -53,6 +58,7 @@ static GeoTileGridValuesSourceBuilder parse(String name, XContentParser parser) } private int precision = GeoTileGridAggregationBuilder.DEFAULT_PRECISION; + private GeoBoundingBox geoBoundingBox = new GeoBoundingBox(new GeoPoint(Double.NaN, Double.NaN), new GeoPoint(Double.NaN, Double.NaN)); GeoTileGridValuesSourceBuilder(String name) { super(name); @@ -61,6 +67,9 @@ static GeoTileGridValuesSourceBuilder parse(String name, XContentParser parser) GeoTileGridValuesSourceBuilder(StreamInput in) throws IOException { super(in); this.precision = in.readInt(); + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + this.geoBoundingBox = new GeoBoundingBox(in); + } } public GeoTileGridValuesSourceBuilder precision(int precision) { @@ -68,6 +77,11 @@ public GeoTileGridValuesSourceBuilder precision(int precision) { return this; } + public GeoTileGridValuesSourceBuilder geoBoundingBox(GeoBoundingBox geoBoundingBox) { + this.geoBoundingBox = geoBoundingBox; + return this; + } + @Override public GeoTileGridValuesSourceBuilder format(String format) { throw new IllegalArgumentException("[format] is not supported for [" + TYPE + "]"); @@ -76,11 +90,17 @@ public GeoTileGridValuesSourceBuilder format(String format) { @Override protected void innerWriteTo(StreamOutput out) throws IOException { out.writeInt(precision); + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + geoBoundingBox.writeTo(out); + } } @Override protected void doXContentBody(XContentBuilder builder, Params params) throws IOException { builder.field("precision", precision); + if (geoBoundingBox.isUnbounded() == false) { + geoBoundingBox.toXContent(builder, params); + } } @Override @@ -90,7 +110,7 @@ String type() { @Override public int hashCode() { - return Objects.hash(super.hashCode(), precision); + return Objects.hash(super.hashCode(), precision, geoBoundingBox); } @Override @@ -99,7 +119,8 @@ public boolean equals(Object obj) { if (obj == null || getClass() != obj.getClass()) return false; if (super.equals(obj) == false) return false; GeoTileGridValuesSourceBuilder other = (GeoTileGridValuesSourceBuilder) obj; - return precision == other.precision; + return Objects.equals(precision,other.precision) + && Objects.equals(geoBoundingBox, other.geoBoundingBox); } @Override @@ -112,7 +133,7 @@ protected CompositeValuesSourceConfig innerBuild(QueryShardContext queryShardCon ValuesSource.GeoPoint geoPoint = (ValuesSource.GeoPoint) orig; // is specified in the builder. final MappedFieldType fieldType = config.fieldContext() != null ? config.fieldContext().fieldType() : null; - CellIdSource cellIdSource = new CellIdSource(geoPoint, precision, GeoTileUtils::longEncode); + CellIdSource cellIdSource = new CellIdSource(geoPoint, precision, geoBoundingBox, GeoTileUtils::longEncode); return new CompositeValuesSourceConfig(name, fieldType, cellIdSource, DocValueFormat.GEOTILE, order(), missingBucket(), script() != null); } else { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java index 4ebb689c7c44f..3ef91e2d61120 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java @@ -20,6 +20,7 @@ import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.SortedNumericDocValues; +import org.elasticsearch.common.geo.GeoBoundingBox; import org.elasticsearch.index.fielddata.AbstractSortingNumericDocValues; import org.elasticsearch.index.fielddata.MultiGeoPointValues; import org.elasticsearch.index.fielddata.SortedBinaryDocValues; @@ -36,11 +37,13 @@ public class CellIdSource extends ValuesSource.Numeric { private final ValuesSource.GeoPoint valuesSource; private final int precision; private final GeoPointLongEncoder encoder; + private final GeoBoundingBox geoBoundingBox; - public CellIdSource(GeoPoint valuesSource, int precision, GeoPointLongEncoder encoder) { + public CellIdSource(GeoPoint valuesSource,int precision, GeoBoundingBox geoBoundingBox, GeoPointLongEncoder encoder) { this.valuesSource = valuesSource; //different GeoPoints could map to the same or different hashing cells. this.precision = precision; + this.geoBoundingBox = geoBoundingBox; this.encoder = encoder; } @@ -55,7 +58,7 @@ public boolean isFloatingPoint() { @Override public SortedNumericDocValues longValues(LeafReaderContext ctx) { - return new CellValues(valuesSource.geoPointValues(ctx), precision, encoder); + return new CellValues(valuesSource.geoPointValues(ctx), precision, geoBoundingBox, encoder); } @Override @@ -81,21 +84,28 @@ private static class CellValues extends AbstractSortingNumericDocValues { private MultiGeoPointValues geoValues; private int precision; private GeoPointLongEncoder encoder; + private GeoBoundingBox geoBoundingBox; - protected CellValues(MultiGeoPointValues geoValues, int precision, GeoPointLongEncoder encoder) { + protected CellValues(MultiGeoPointValues geoValues, int precision, GeoBoundingBox geoBoundingBox, GeoPointLongEncoder encoder) { this.geoValues = geoValues; this.precision = precision; this.encoder = encoder; + this.geoBoundingBox = geoBoundingBox; } @Override public boolean advanceExact(int docId) throws IOException { if (geoValues.advanceExact(docId)) { - resize(geoValues.docValueCount()); - for (int i = 0; i < docValueCount(); ++i) { + int docValueCount = geoValues.docValueCount(); + resize(docValueCount); + int j = 0; + for (int i = 0; i < docValueCount; i++) { org.elasticsearch.common.geo.GeoPoint target = geoValues.nextValue(); - values[i] = encoder.encode(target.getLon(), target.getLat(), precision); + if (geoBoundingBox.isUnbounded() || geoBoundingBox.pointInBounds(target.getLon(), target.getLat())) { + values[j++] = encoder.encode(target.getLon(), target.getLat(), precision); + } } + resize(j); sort(); return true; } else { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java index e1c4221139202..233fd4c6885e8 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java @@ -20,7 +20,10 @@ package org.elasticsearch.search.aggregations.bucket.geogrid; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.Version; 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; import org.elasticsearch.common.xcontent.ObjectParser; @@ -53,6 +56,8 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB protected int precision; protected int requiredSize; protected int shardSize; + private GeoBoundingBox geoBoundingBox = new GeoBoundingBox(new GeoPoint(Double.NaN, Double.NaN), new GeoPoint(Double.NaN, Double.NaN)); + @FunctionalInterface protected interface PrecisionParser { @@ -66,6 +71,10 @@ public static ObjectParser createParser(String org.elasticsearch.common.xcontent.ObjectParser.ValueType.INT); parser.declareInt(GeoGridAggregationBuilder::size, FIELD_SIZE); parser.declareInt(GeoGridAggregationBuilder::shardSize, FIELD_SHARD_SIZE); + parser.declareField((p, builder, context) -> { + builder.setGeoBoundingBox(GeoBoundingBox.parseBoundingBox(p)); + }, + GeoBoundingBox.BOUNDS_FIELD, org.elasticsearch.common.xcontent.ObjectParser.ValueType.OBJECT); return parser; } @@ -78,7 +87,7 @@ protected GeoGridAggregationBuilder(GeoGridAggregationBuilder clone, Builder fac this.precision = clone.precision; this.requiredSize = clone.requiredSize; this.shardSize = clone.shardSize; - + this.geoBoundingBox = clone.geoBoundingBox; } /** @@ -89,6 +98,9 @@ public GeoGridAggregationBuilder(StreamInput in) throws IOException { precision = in.readVInt(); requiredSize = in.readVInt(); shardSize = in.readVInt(); + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + geoBoundingBox = new GeoBoundingBox(in); + } } @Override @@ -96,6 +108,9 @@ protected void innerWriteTo(StreamOutput out) throws IOException { out.writeVInt(precision); out.writeVInt(requiredSize); out.writeVInt(shardSize); + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + geoBoundingBox.writeTo(out); + } } /** @@ -110,7 +125,8 @@ protected void innerWriteTo(StreamOutput out) throws IOException { */ protected abstract ValuesSourceAggregatorFactory createFactory( String name, ValuesSourceConfig config, int precision, int requiredSize, int shardSize, - QueryShardContext queryShardContext, AggregatorFactory parent, Builder subFactoriesBuilder, Map metaData + GeoBoundingBox geoBoundingBox, QueryShardContext queryShardContext, AggregatorFactory parent, + Builder subFactoriesBuilder, Map metaData ) throws IOException; public int precision() { @@ -143,6 +159,16 @@ public int shardSize() { return shardSize; } + public GeoGridAggregationBuilder setGeoBoundingBox(GeoBoundingBox geoBoundingBox) { + this.geoBoundingBox = geoBoundingBox; + // no validation done here, similar to geo_bounding_box query behavior. + return this; + } + + public GeoBoundingBox geoBoundingBox() { + return geoBoundingBox; + } + @Override protected ValuesSourceAggregatorFactory innerBuild(QueryShardContext queryShardContext, ValuesSourceConfig config, @@ -166,7 +192,7 @@ protected ValuesSourceAggregatorFactory innerBuild(QueryS if (shardSize < requiredSize) { shardSize = requiredSize; } - return createFactory(name, config, precision, requiredSize, shardSize, queryShardContext, parent, + return createFactory(name, config, precision, requiredSize, shardSize, geoBoundingBox, queryShardContext, parent, subFactoriesBuilder, metaData); } @@ -177,6 +203,9 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) if (shardSize > -1) { builder.field(FIELD_SHARD_SIZE.getPreferredName(), shardSize); } + if (geoBoundingBox.isUnbounded() == false) { + geoBoundingBox.toXContent(builder, params); + } return builder; } @@ -188,11 +217,12 @@ public boolean equals(Object obj) { GeoGridAggregationBuilder other = (GeoGridAggregationBuilder) obj; return precision == other.precision && requiredSize == other.requiredSize - && shardSize == other.shardSize; + && shardSize == other.shardSize + && Objects.equals(geoBoundingBox, other.geoBoundingBox); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), precision, requiredSize, shardSize); + return Objects.hash(super.hashCode(), precision, requiredSize, shardSize, geoBoundingBox); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregator.java index c91f763b603c0..c77578f2570c7 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregator.java @@ -48,8 +48,8 @@ public abstract class GeoGridAggregator extends Bucke protected final LongHash bucketOrds; GeoGridAggregator(String name, AggregatorFactories factories, CellIdSource valuesSource, - int requiredSize, int shardSize, SearchContext aggregationContext, Aggregator parent, - List pipelineAggregators, Map metaData) throws IOException { + int requiredSize, int shardSize, SearchContext aggregationContext, + Aggregator parent, List pipelineAggregators, Map metaData) throws IOException { super(name, factories, aggregationContext, parent, pipelineAggregators, metaData); this.valuesSource = valuesSource; this.requiredSize = requiredSize; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregationBuilder.java index d58beeb781c25..acc9cde113164 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregationBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.bucket.geogrid; +import org.elasticsearch.common.geo.GeoBoundingBox; import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.xcontent.ObjectParser; @@ -60,11 +61,12 @@ public GeoGridAggregationBuilder precision(int precision) { @Override protected ValuesSourceAggregatorFactory createFactory( - String name, ValuesSourceConfig config, int precision, int requiredSize, int shardSize, - QueryShardContext queryShardContext, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, - Map metaData) throws IOException { - return new GeoHashGridAggregatorFactory(name, config, precision, requiredSize, shardSize, queryShardContext, parent, - subFactoriesBuilder, metaData); + String name, ValuesSourceConfig config, int precision, int requiredSize, int shardSize, + GeoBoundingBox geoBoundingBox, QueryShardContext queryShardContext, + AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, + Map metaData) throws IOException { + return new GeoHashGridAggregatorFactory(name, config, precision, requiredSize, shardSize, geoBoundingBox, + queryShardContext, parent, subFactoriesBuilder, metaData); } private GeoHashGridAggregationBuilder(GeoHashGridAggregationBuilder clone, AggregatorFactories.Builder factoriesBuilder, diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java index 54d1e2e940649..1ad59ccc45100 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java @@ -34,9 +34,11 @@ public class GeoHashGridAggregator extends GeoGridAggregator { GeoHashGridAggregator(String name, AggregatorFactories factories, CellIdSource valuesSource, - int requiredSize, int shardSize, SearchContext aggregationContext, Aggregator parent, - List pipelineAggregators, Map metaData) throws IOException { - super(name, factories, valuesSource, requiredSize, shardSize, aggregationContext, parent, pipelineAggregators, metaData); + int requiredSize, int shardSize, SearchContext aggregationContext, + Aggregator parent, List pipelineAggregators, + Map metaData) throws IOException { + super(name, factories, valuesSource, requiredSize, shardSize, aggregationContext, parent, + pipelineAggregators, metaData); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregatorFactory.java index a049a07f13dbe..2d7087a693bfa 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregatorFactory.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.bucket.geogrid; +import org.elasticsearch.common.geo.GeoBoundingBox; import org.elasticsearch.geometry.utils.Geohash; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.aggregations.Aggregator; @@ -28,7 +29,6 @@ import org.elasticsearch.search.aggregations.NonCollectingAggregator; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import org.elasticsearch.search.aggregations.support.ValuesSource; -import org.elasticsearch.search.aggregations.support.ValuesSource.GeoPoint; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import org.elasticsearch.search.internal.SearchContext; @@ -43,14 +43,17 @@ public class GeoHashGridAggregatorFactory extends ValuesSourceAggregatorFactory< private final int precision; private final int requiredSize; private final int shardSize; + private final GeoBoundingBox geoBoundingBox; - GeoHashGridAggregatorFactory(String name, ValuesSourceConfig config, int precision, int requiredSize, - int shardSize, QueryShardContext queryShardContext, AggregatorFactory parent, - AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { + GeoHashGridAggregatorFactory(String name, ValuesSourceConfig config, int precision, int requiredSize, + int shardSize, GeoBoundingBox geoBoundingBox, QueryShardContext queryShardContext, + AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, + Map metaData) throws IOException { super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData); this.precision = precision; this.requiredSize = requiredSize; this.shardSize = shardSize; + this.geoBoundingBox = geoBoundingBox; } @Override @@ -69,7 +72,7 @@ public InternalAggregation buildEmptyAggregation() { } @Override - protected Aggregator doCreateInternal(final GeoPoint valuesSource, + protected Aggregator doCreateInternal(final ValuesSource.GeoPoint valuesSource, SearchContext searchContext, Aggregator parent, boolean collectsFromSingleBucket, @@ -78,8 +81,8 @@ protected Aggregator doCreateInternal(final GeoPoint valuesSource, if (collectsFromSingleBucket == false) { return asMultiBucketAggregator(this, searchContext, parent); } - CellIdSource cellIdSource = new CellIdSource(valuesSource, precision, Geohash::longEncode); - return new GeoHashGridAggregator(name, factories, cellIdSource, requiredSize, shardSize, searchContext, parent, - pipelineAggregators, metaData); + CellIdSource cellIdSource = new CellIdSource(valuesSource, precision, geoBoundingBox, Geohash::longEncode); + return new GeoHashGridAggregator(name, factories, cellIdSource, requiredSize, shardSize, + searchContext, parent, pipelineAggregators, metaData); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregationBuilder.java index b3d9888781362..595c6cab6e718 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregationBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.bucket.geogrid; +import org.elasticsearch.common.geo.GeoBoundingBox; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentParser; @@ -59,12 +60,11 @@ public GeoGridAggregationBuilder precision(int precision) { @Override protected ValuesSourceAggregatorFactory createFactory( - String name, ValuesSourceConfig config, int precision, int requiredSize, int shardSize, - QueryShardContext queryShardContext, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, - Map metaData - ) throws IOException { - return new GeoTileGridAggregatorFactory(name, config, precision, requiredSize, shardSize, queryShardContext, parent, - subFactoriesBuilder, metaData); + String name, ValuesSourceConfig config, int precision, int requiredSize, int shardSize, + GeoBoundingBox geoBoundingBox, QueryShardContext queryShardContext, AggregatorFactory parent, + AggregatorFactories.Builder subFactoriesBuilder, Map metaData ) throws IOException { + return new GeoTileGridAggregatorFactory(name, config, precision, requiredSize, shardSize, geoBoundingBox, + queryShardContext, parent, subFactoriesBuilder, metaData); } private GeoTileGridAggregationBuilder(GeoTileGridAggregationBuilder clone, AggregatorFactories.Builder factoriesBuilder, diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregator.java index d2ff5ed82513c..350761aa84050 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregator.java @@ -35,9 +35,11 @@ public class GeoTileGridAggregator extends GeoGridAggregator { GeoTileGridAggregator(String name, AggregatorFactories factories, CellIdSource valuesSource, - int requiredSize, int shardSize, SearchContext aggregationContext, Aggregator parent, - List pipelineAggregators, Map metaData) throws IOException { - super(name, factories, valuesSource, requiredSize, shardSize, aggregationContext, parent, pipelineAggregators, metaData); + int requiredSize, int shardSize, SearchContext aggregationContext, + Aggregator parent, List pipelineAggregators, + Map metaData) throws IOException { + super(name, factories, valuesSource, requiredSize, shardSize, aggregationContext, parent, + pipelineAggregators, metaData); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorFactory.java index 8380a4172c9c5..0f59c9a71ea40 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorFactory.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.bucket.geogrid; +import org.elasticsearch.common.geo.GeoBoundingBox; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorFactories; @@ -27,7 +28,6 @@ import org.elasticsearch.search.aggregations.NonCollectingAggregator; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import org.elasticsearch.search.aggregations.support.ValuesSource; -import org.elasticsearch.search.aggregations.support.ValuesSource.GeoPoint; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import org.elasticsearch.search.internal.SearchContext; @@ -42,14 +42,17 @@ public class GeoTileGridAggregatorFactory extends ValuesSourceAggregatorFactory< private final int precision; private final int requiredSize; private final int shardSize; + private final GeoBoundingBox geoBoundingBox; - GeoTileGridAggregatorFactory(String name, ValuesSourceConfig config, int precision, int requiredSize, - int shardSize, QueryShardContext queryShardContext, AggregatorFactory parent, - AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { + GeoTileGridAggregatorFactory(String name, ValuesSourceConfig config, int precision, int requiredSize, + int shardSize, GeoBoundingBox geoBoundingBox, QueryShardContext queryShardContext, + AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, + Map metaData) throws IOException { super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData); this.precision = precision; this.requiredSize = requiredSize; this.shardSize = shardSize; + this.geoBoundingBox = geoBoundingBox; } @Override @@ -68,7 +71,7 @@ public InternalAggregation buildEmptyAggregation() { } @Override - protected Aggregator doCreateInternal(final GeoPoint valuesSource, + protected Aggregator doCreateInternal(final ValuesSource.GeoPoint valuesSource, SearchContext searchContext, Aggregator parent, boolean collectsFromSingleBucket, @@ -77,8 +80,8 @@ protected Aggregator doCreateInternal(final GeoPoint valuesSource, if (collectsFromSingleBucket == false) { return asMultiBucketAggregator(this, searchContext, parent); } - CellIdSource cellIdSource = new CellIdSource(valuesSource, precision, GeoTileUtils::longEncode); - return new GeoTileGridAggregator(name, factories, cellIdSource, requiredSize, shardSize, searchContext, parent, - pipelineAggregators, metaData); + CellIdSource cellIdSource = new CellIdSource(valuesSource, precision, geoBoundingBox, GeoTileUtils::longEncode); + return new GeoTileGridAggregator(name, factories, cellIdSource, requiredSize, shardSize, + searchContext, parent, pipelineAggregators, metaData); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java index 19381e87fedb6..03f821296f2a6 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java @@ -51,7 +51,7 @@ private GeoTileUtils() {} * Another consideration is that index optimizes lat/lng storage, loosing some precision. * E.g. hash lng=140.74779717298918D lat=45.61884022447444D == "18/233561/93659", but shown as "18/233561/93658" */ - static final int MAX_ZOOM = 29; + public static final int MAX_ZOOM = 29; /** * Bit position of the zoom value within hash - zoom is stored in the most significant 6 bits of a long number. 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..7105ef6d1e060 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.geo.GeometryTestUtils; +import org.elasticsearch.geometry.Rectangle; import org.elasticsearch.test.ESTestCase; import java.io.IOException; @@ -133,6 +134,34 @@ public void testNullTopBottomLeftRight() throws Exception { } } + + public void testPointInBounds() { + for (int iter = 0; iter < 100; iter++) { + Rectangle rectangle = GeometryTestUtils.randomRectangle(); + GeoBoundingBox geoBoundingBox = new GeoBoundingBox(new GeoPoint(rectangle.getMaxLat(), rectangle.getMinLon()), + new GeoPoint(rectangle.getMinLat(), rectangle.getMaxLon())); + if (rectangle.getMinX() > rectangle.getMaxX()) { + double lonWithin = randomBoolean() ? + randomDoubleBetween(rectangle.getMinX(), 180.0, true) + : randomDoubleBetween(-180.0, rectangle.getMaxX(), true); + double latWithin = randomDoubleBetween(rectangle.getMinY(), rectangle.getMaxY(), true); + double lonOutside = (rectangle.getMinX() + rectangle.getMaxX()) / 2; + double latOutside = rectangle.getMinX() - randomIntBetween(1, 10); + + assertTrue(geoBoundingBox.pointInBounds(lonWithin, latWithin)); + assertFalse(geoBoundingBox.pointInBounds(lonOutside, latOutside)); + } else { + double lonWithin = randomDoubleBetween(rectangle.getMinX(), rectangle.getMaxX(), true); + double latWithin = randomDoubleBetween(rectangle.getMinY(), rectangle.getMaxY(), true); + double lonOutside = randomDoubleBetween(rectangle.getMaxX(), 180.0, false); + double latOutside = randomDoubleBetween(rectangle.getMaxY(), 90.0, false); + + assertTrue(geoBoundingBox.pointInBounds(lonWithin, latWithin)); + assertFalse(geoBoundingBox.pointInBounds(lonOutside, latOutside)); + } + } + } + private void assertBBox(GeoBoundingBox expected, XContentBuilder builder) throws IOException { try (XContentParser parser = createParser(builder)) { parser.nextToken(); @@ -140,7 +169,7 @@ private void assertBBox(GeoBoundingBox expected, XContentBuilder builder) throws } } - private GeoBoundingBox randomBBox() { + public static GeoBoundingBox randomBBox() { double topLat = GeometryTestUtils.randomLat(); double bottomLat = randomDoubleBetween(GeoUtils.MIN_LAT, topLat, true); return new GeoBoundingBox(new GeoPoint(topLat, GeometryTestUtils.randomLon()), diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java index e414b86403a09..629ccfbce08f1 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.bucket; +import org.elasticsearch.common.geo.GeoBoundingBoxTests; import org.elasticsearch.search.aggregations.BaseAggregationTestCase; import org.elasticsearch.search.aggregations.bucket.geogrid.GeoGridAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregationBuilder; @@ -39,6 +40,9 @@ protected GeoHashGridAggregationBuilder createTestAggregatorBuilder() { if (randomBoolean()) { factory.shardSize(randomIntBetween(1, Integer.MAX_VALUE)); } + if (randomBoolean()) { + factory.setGeoBoundingBox(GeoBoundingBoxTests.randomBBox()); + } return factory; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoTileGridTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoTileGridTests.java new file mode 100644 index 0000000000000..825c201139442 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoTileGridTests.java @@ -0,0 +1,49 @@ +/* + * 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.search.aggregations.bucket; + +import org.elasticsearch.common.geo.GeoBoundingBoxTests; +import org.elasticsearch.search.aggregations.BaseAggregationTestCase; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoGridAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils; + +public class GeoTileGridTests extends BaseAggregationTestCase { + + @Override + protected GeoTileGridAggregationBuilder createTestAggregatorBuilder() { + String name = randomAlphaOfLengthBetween(3, 20); + GeoTileGridAggregationBuilder factory = new GeoTileGridAggregationBuilder(name); + if (randomBoolean()) { + factory.precision(randomIntBetween(0, GeoTileUtils.MAX_ZOOM)); + } + if (randomBoolean()) { + factory.size(randomIntBetween(1, Integer.MAX_VALUE)); + } + if (randomBoolean()) { + factory.shardSize(randomIntBetween(1, Integer.MAX_VALUE)); + } + if (randomBoolean()) { + factory.setGeoBoundingBox(GeoBoundingBoxTests.randomBBox()); + } + return factory; + } + +} diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java index c78a55b253297..c36ff50b7a36d 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java @@ -19,8 +19,10 @@ package org.elasticsearch.search.aggregations.bucket.composite; +import org.elasticsearch.common.geo.GeoBoundingBoxTests; import org.elasticsearch.script.Script; import org.elasticsearch.search.aggregations.BaseAggregationTestCase; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.search.sort.SortOrder; @@ -54,7 +56,10 @@ private DateHistogramValuesSourceBuilder randomDateHistogramSourceBuilder() { private GeoTileGridValuesSourceBuilder randomGeoTileGridValuesSourceBuilder() { GeoTileGridValuesSourceBuilder geoTile = new GeoTileGridValuesSourceBuilder(randomAlphaOfLengthBetween(5, 10)); if (randomBoolean()) { - geoTile.precision(randomIntBetween(1, 12)); + geoTile.precision(randomIntBetween(0, GeoTileUtils.MAX_ZOOM)); + } + if (randomBoolean()) { + geoTile.geoBoundingBox(GeoBoundingBoxTests.randomBBox()); } return geoTile; } @@ -90,9 +95,11 @@ private HistogramValuesSourceBuilder randomHistogramSourceBuilder() { @Override protected CompositeAggregationBuilder createTestAggregatorBuilder() { int numSources = randomIntBetween(1, 10); + numSources = 1; List> sources = new ArrayList<>(); for (int i = 0; i < numSources; i++) { int type = randomIntBetween(0, 3); + type = 3; switch (type) { case 0: sources.add(randomTermsSourceBuilder()); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregatorTestCase.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregatorTestCase.java index 047903bc86100..ebd7a2b8ddc2a 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregatorTestCase.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregatorTestCase.java @@ -28,6 +28,10 @@ import org.apache.lucene.search.Query; import org.apache.lucene.store.Directory; import org.elasticsearch.common.CheckedConsumer; +import org.elasticsearch.common.geo.GeoBoundingBox; +import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.geo.GeometryTestUtils; +import org.elasticsearch.geometry.Rectangle; import org.elasticsearch.index.mapper.GeoPointFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.search.aggregations.Aggregator; @@ -44,6 +48,8 @@ import java.util.Set; import java.util.function.Consumer; +import static org.hamcrest.Matchers.equalTo; + public abstract class GeoGridAggregatorTestCase extends AggregatorTestCase { private static final String FIELD_NAME = "location"; @@ -64,18 +70,18 @@ public abstract class GeoGridAggregatorTestCase protected abstract GeoGridAggregationBuilder createBuilder(String name); public void testNoDocs() throws IOException { - testCase(new MatchAllDocsQuery(), FIELD_NAME, randomPrecision(), iw -> { - // Intentionally not writing any docs - }, geoGrid -> { + testCase(new MatchAllDocsQuery(), FIELD_NAME, randomPrecision(), null, geoGrid -> { assertEquals(0, geoGrid.getBuckets().size()); + }, iw -> { + // Intentionally not writing any docs }); } public void testFieldMissing() throws IOException { - testCase(new MatchAllDocsQuery(), "wrong_field", randomPrecision(), iw -> { - iw.addDocument(Collections.singleton(new LatLonDocValuesField(FIELD_NAME, 10D, 10D))); - }, geoGrid -> { + testCase(new MatchAllDocsQuery(), "wrong_field", randomPrecision(), null, geoGrid -> { assertEquals(0, geoGrid.getBuckets().size()); + }, iw -> { + iw.addDocument(Collections.singleton(new LatLonDocValuesField(FIELD_NAME, 10D, 10D))); }); } @@ -83,7 +89,13 @@ public void testWithSeveralDocs() throws IOException { int precision = randomPrecision(); int numPoints = randomIntBetween(8, 128); Map expectedCountPerGeoHash = new HashMap<>(); - testCase(new MatchAllDocsQuery(), FIELD_NAME, precision, iw -> { + testCase(new MatchAllDocsQuery(), FIELD_NAME, precision, null, geoHashGrid -> { + assertEquals(expectedCountPerGeoHash.size(), geoHashGrid.getBuckets().size()); + for (GeoGrid.Bucket bucket : geoHashGrid.getBuckets()) { + assertEquals((long) expectedCountPerGeoHash.get(bucket.getKeyAsString()), bucket.getDocCount()); + } + assertTrue(AggregationInspectionHelper.hasValue(geoHashGrid)); + }, iw -> { List points = new ArrayList<>(); Set distinctHashesPerDoc = new HashSet<>(); for (int pointId = 0; pointId < numPoints; pointId++) { @@ -112,17 +124,69 @@ public void testWithSeveralDocs() throws IOException { if (points.size() != 0) { iw.addDocument(points); } - }, geoHashGrid -> { - assertEquals(expectedCountPerGeoHash.size(), geoHashGrid.getBuckets().size()); - for (GeoGrid.Bucket bucket : geoHashGrid.getBuckets()) { - assertEquals((long) expectedCountPerGeoHash.get(bucket.getKeyAsString()), bucket.getDocCount()); + }); + } + + public void testBounds() throws IOException { + final int numDocs = randomIntBetween(64, 256); + final GeoGridAggregationBuilder builder = createBuilder("_name"); + + expectThrows(IllegalArgumentException.class, () -> builder.precision(-1)); + expectThrows(IllegalArgumentException.class, () -> builder.precision(30)); + + Rectangle rectangle = GeometryTestUtils.randomRectangle(); + + int in = 0, out = 0; + List docs = new ArrayList<>(); + while (in + out < numDocs) { + if (rectangle.getMinX() > rectangle.getMaxX()) { + if (randomBoolean()) { + double lonWithin = randomBoolean() ? + randomDoubleBetween(rectangle.getMinX(), 180.0, true) + : randomDoubleBetween(-180.0, rectangle.getMaxX(), true); + double latWithin = randomDoubleBetween(rectangle.getMinY(), rectangle.getMaxY(), true); + in++; + docs.add(new LatLonDocValuesField(FIELD_NAME, latWithin, lonWithin)); + } else { + double lonOutside = (rectangle.getMinX() + rectangle.getMaxX()) / 2; + double latOutside = rectangle.getMinX() - randomIntBetween(1, 10); + out++; + docs.add(new LatLonDocValuesField(FIELD_NAME, latOutside, lonOutside)); + } + + } else { + if (randomBoolean()) { + double lonWithin = randomDoubleBetween(rectangle.getMinX(), rectangle.getMaxX(), true); + double latWithin = randomDoubleBetween(rectangle.getMinY(), rectangle.getMaxY(), true); + in++; + docs.add(new LatLonDocValuesField(FIELD_NAME, latWithin, lonWithin)); + } else { + double lonOutside = randomDoubleBetween(rectangle.getMaxX(), 180.0, false); + double latOutside = randomDoubleBetween(rectangle.getMaxY(), 90.0, false); + out++; + docs.add(new LatLonDocValuesField(FIELD_NAME, latOutside, lonOutside)); + } + } + + } + + final long numDocsInBucket = in; + + testCase(new MatchAllDocsQuery(), FIELD_NAME, 0, rectangle, geoGrid -> { + assertThat(geoGrid.getBuckets().size(), equalTo(1)); + long docCount = geoGrid.getBuckets().get(0).getDocCount(); + assertThat(docCount, equalTo(numDocsInBucket)); + assertTrue(AggregationInspectionHelper.hasValue(geoGrid)); + }, iw -> { + for (LatLonDocValuesField docField : docs) { + iw.addDocument(Collections.singletonList(docField)); } - assertTrue(AggregationInspectionHelper.hasValue(geoHashGrid)); }); } - private void testCase(Query query, String field, int precision, CheckedConsumer buildIndex, - Consumer> verify) throws IOException { + private void testCase(Query query, String field, int precision, Rectangle boundingBox, + Consumer> verify, + CheckedConsumer buildIndex) throws IOException { Directory directory = newDirectory(); RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory); buildIndex.accept(indexWriter); @@ -133,6 +197,13 @@ private void testCase(Query query, String field, int precision, CheckedConsumer< GeoGridAggregationBuilder aggregationBuilder = createBuilder("_name").field(field); aggregationBuilder.precision(precision); + if (boundingBox != null) { + GeoBoundingBox geoBoundingBox = new GeoBoundingBox(new GeoPoint(boundingBox.getMaxLat(), boundingBox.getMinLon()), + new GeoPoint(boundingBox.getMinLat(), boundingBox.getMaxLon())); + aggregationBuilder.setGeoBoundingBox(geoBoundingBox); + assertThat(aggregationBuilder.geoBoundingBox(), equalTo(geoBoundingBox)); + } + MappedFieldType fieldType = new GeoPointFieldMapper.GeoPointFieldType(); fieldType.setHasDocValues(true); fieldType.setName(FIELD_NAME); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java index 824a069203856..5075ed9f02aa2 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java @@ -22,6 +22,8 @@ import org.elasticsearch.common.xcontent.XContentParseException; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.geo.GeometryTestUtils; +import org.elasticsearch.geometry.Rectangle; import org.elasticsearch.test.ESTestCase; import static org.hamcrest.Matchers.containsString; @@ -113,4 +115,19 @@ public void testParseErrorOnPrecisionOutOfRange() throws Exception { assertEquals("Invalid geohash aggregation precision of 13. Must be between 1 and 12.", ex.getCause().getMessage()); } } + + public void testParseValidBounds() throws Exception { + Rectangle bbox = GeometryTestUtils.randomRectangle(); + XContentParser stParser = createParser(JsonXContent.jsonXContent, + "{\"field\":\"my_loc\", \"precision\": 5, \"size\": 500, \"shard_size\": 550," + "\"bounds\": { " + + "\"top\": " + bbox.getMaxY() + "," + + "\"bottom\": " + bbox.getMinY() + "," + + "\"left\": " + bbox.getMinX() + "," + + "\"right\": " + bbox.getMaxX() + "}" + + "}"); + XContentParser.Token token = stParser.nextToken(); + assertSame(XContentParser.Token.START_OBJECT, token); + // can create a factory + assertNotNull(GeoHashGridAggregationBuilder.parse("geohash_grid", stParser)); + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorTests.java index 6544344543e34..85b2306403230 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorTests.java @@ -46,5 +46,4 @@ public void testPrecision() { builder.precision(precision); assertEquals(precision, builder.precision()); } - } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridParserTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridParserTests.java index 6f15263a53b11..aa5253564fb49 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridParserTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridParserTests.java @@ -22,6 +22,8 @@ import org.elasticsearch.common.xcontent.XContentParseException; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.geo.GeometryTestUtils; +import org.elasticsearch.geometry.Rectangle; import org.elasticsearch.test.ESTestCase; import static org.hamcrest.Matchers.containsString; @@ -70,4 +72,19 @@ public void testParseErrorOnPrecisionOutOfRange() throws Exception { assertEquals("Invalid geotile_grid precision of 30. Must be between 0 and 29.", ex.getCause().getMessage()); } } + + public void testParseValidBounds() throws Exception { + Rectangle bbox = GeometryTestUtils.randomRectangle(); + XContentParser stParser = createParser(JsonXContent.jsonXContent, + "{\"field\":\"my_loc\", \"precision\": 5, \"size\": 500, \"shard_size\": 550," + "\"bounds\": { " + + "\"top\": " + bbox.getMaxY() + "," + + "\"bottom\": " + bbox.getMinY() + "," + + "\"left\": " + bbox.getMinX() + "," + + "\"right\": " + bbox.getMaxX() + "}" + + "}"); + XContentParser.Token token = stParser.nextToken(); + assertSame(XContentParser.Token.START_OBJECT, token); + // can create a factory + assertNotNull(GeoTileGridAggregationBuilder.parse("geotile_grid", stParser)); + } }