Skip to content

Commit

Permalink
Refactored the src and test of GeoHashGrid and GeoTileGrid Aggregatio…
Browse files Browse the repository at this point in the history
…ns on GeoPoint from server folder to geo module.(#4071) (#4072) (#4180) (#4281)

The changes also includes:
   * Updated Search plugin to provide the interface so that plugins can also register the composite aggregations
   * Added YAML test for the geo_grid, geo_tile and composite aggregation

Signed-off-by: Navneet Verma <navneev@amazon.com>
  • Loading branch information
navneet1v authored Aug 30, 2022
1 parent 38372e1 commit da76f2a
Show file tree
Hide file tree
Showing 81 changed files with 1,395 additions and 763 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,6 @@
import org.opensearch.search.aggregations.bucket.filter.FiltersAggregationBuilder;
import org.opensearch.search.aggregations.bucket.filter.ParsedFilter;
import org.opensearch.search.aggregations.bucket.filter.ParsedFilters;
import org.opensearch.search.aggregations.bucket.geogrid.GeoHashGridAggregationBuilder;
import org.opensearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder;
import org.opensearch.search.aggregations.bucket.geogrid.ParsedGeoHashGrid;
import org.opensearch.search.aggregations.bucket.geogrid.ParsedGeoTileGrid;
import org.opensearch.search.aggregations.bucket.global.GlobalAggregationBuilder;
import org.opensearch.search.aggregations.bucket.global.ParsedGlobal;
import org.opensearch.search.aggregations.bucket.histogram.AutoDateHistogramAggregationBuilder;
Expand Down Expand Up @@ -2130,8 +2126,6 @@ static List<NamedXContentRegistry.Entry> getDefaultNamedXContents() {
map.put(GlobalAggregationBuilder.NAME, (p, c) -> ParsedGlobal.fromXContent(p, (String) c));
map.put(FilterAggregationBuilder.NAME, (p, c) -> ParsedFilter.fromXContent(p, (String) c));
map.put(InternalSampler.PARSER_NAME, (p, c) -> ParsedSampler.fromXContent(p, (String) c));
map.put(GeoHashGridAggregationBuilder.NAME, (p, c) -> ParsedGeoHashGrid.fromXContent(p, (String) c));
map.put(GeoTileGridAggregationBuilder.NAME, (p, c) -> ParsedGeoTileGrid.fromXContent(p, (String) c));
map.put(RangeAggregationBuilder.NAME, (p, c) -> ParsedRange.fromXContent(p, (String) c));
map.put(DateRangeAggregationBuilder.NAME, (p, c) -> ParsedDateRange.fromXContent(p, (String) c));
map.put(GeoDistanceAggregationBuilder.NAME, (p, c) -> ParsedGeoDistance.fromXContent(p, (String) c));
Expand Down
9 changes: 8 additions & 1 deletion modules/geo/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,16 @@ opensearchplugin {

restResources {
restApi {
includeCore '_common', 'indices', 'index', 'search'
includeCore '_common', 'indices', 'index', 'search', 'bulk'
}
}
artifacts {
restTests(project.file('src/yamlRestTest/resources/rest-api-spec/test'))
}
/**
* These compiler arguments needs to be removed, as there are raw types being used in the GeoGrid and GeoTile aggregations.
*/
tasks.withType(JavaCompile).configureEach {
options.compilerArgs -= '-Xlint:rawtypes'
options.compilerArgs -= '-Xlint:unchecked'
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
* GitHub history for details.
*/

package org.opensearch.search.aggregations.bucket;
package org.opensearch.geo.search.aggregations.bucket;

import com.carrotsearch.hppc.ObjectIntHashMap;
import com.carrotsearch.hppc.ObjectIntMap;
Expand All @@ -41,12 +41,12 @@
import org.opensearch.common.geo.GeoPoint;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.XContentBuilder;
import org.opensearch.geo.GeoModulePluginIntegTestCase;
import org.opensearch.geo.search.aggregations.bucket.geogrid.GeoGrid;
import org.opensearch.geo.tests.common.AggregationBuilders;
import org.opensearch.index.query.GeoBoundingBoxQueryBuilder;
import org.opensearch.search.aggregations.AggregationBuilders;
import org.opensearch.search.aggregations.InternalAggregation;
import org.opensearch.search.aggregations.bucket.filter.Filter;
import org.opensearch.search.aggregations.bucket.geogrid.GeoGrid;
import org.opensearch.search.aggregations.bucket.geogrid.GeoGrid.Bucket;
import org.opensearch.test.OpenSearchIntegTestCase;
import org.opensearch.test.VersionUtils;

Expand All @@ -57,17 +57,16 @@
import java.util.Random;
import java.util.Set;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.opensearch.geometry.utils.Geohash.PRECISION;
import static org.opensearch.geometry.utils.Geohash.stringEncode;
import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.opensearch.search.aggregations.AggregationBuilders.geohashGrid;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;

@OpenSearchIntegTestCase.SuiteScopeTestCase
public class GeoHashGridIT extends OpenSearchIntegTestCase {
public class GeoHashGridIT extends GeoModulePluginIntegTestCase {

@Override
protected boolean forbidPrivateIndexSettings() {
Expand Down Expand Up @@ -158,13 +157,13 @@ public void setupSuiteScopeCluster() throws Exception {
public void testSimple() throws Exception {
for (int precision = 1; precision <= PRECISION; precision++) {
SearchResponse response = client().prepareSearch("idx")
.addAggregation(geohashGrid("geohashgrid").field("location").precision(precision))
.addAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").precision(precision))
.get();

assertSearchResponse(response);

GeoGrid geoGrid = response.getAggregations().get("geohashgrid");
List<? extends Bucket> buckets = geoGrid.getBuckets();
List<? extends GeoGrid.Bucket> buckets = geoGrid.getBuckets();
Object[] propertiesKeys = (Object[]) ((InternalAggregation) geoGrid).getProperty("_key");
Object[] propertiesDocCounts = (Object[]) ((InternalAggregation) geoGrid).getProperty("_count");
for (int i = 0; i < buckets.size(); i++) {
Expand All @@ -185,7 +184,7 @@ public void testSimple() throws Exception {
public void testMultivalued() throws Exception {
for (int precision = 1; precision <= PRECISION; precision++) {
SearchResponse response = client().prepareSearch("multi_valued_idx")
.addAggregation(geohashGrid("geohashgrid").field("location").precision(precision))
.addAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").precision(precision))
.get();

assertSearchResponse(response);
Expand All @@ -208,8 +207,8 @@ public void testFiltered() throws Exception {
for (int precision = 1; precision <= PRECISION; precision++) {
SearchResponse response = client().prepareSearch("idx")
.addAggregation(
AggregationBuilders.filter("filtered", bbox)
.subAggregation(geohashGrid("geohashgrid").field("location").precision(precision))
org.opensearch.search.aggregations.AggregationBuilders.filter("filtered", bbox)
.subAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").precision(precision))
)
.get();

Expand All @@ -233,7 +232,7 @@ public void testFiltered() throws Exception {
public void testUnmapped() throws Exception {
for (int precision = 1; precision <= PRECISION; precision++) {
SearchResponse response = client().prepareSearch("idx_unmapped")
.addAggregation(geohashGrid("geohashgrid").field("location").precision(precision))
.addAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").precision(precision))
.get();

assertSearchResponse(response);
Expand All @@ -247,7 +246,7 @@ public void testUnmapped() throws Exception {
public void testPartiallyUnmapped() throws Exception {
for (int precision = 1; precision <= PRECISION; precision++) {
SearchResponse response = client().prepareSearch("idx", "idx_unmapped")
.addAggregation(geohashGrid("geohashgrid").field("location").precision(precision))
.addAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").precision(precision))
.get();

assertSearchResponse(response);
Expand All @@ -267,7 +266,9 @@ public void testPartiallyUnmapped() throws Exception {
public void testTopMatch() throws Exception {
for (int precision = 1; precision <= PRECISION; precision++) {
SearchResponse response = client().prepareSearch("idx")
.addAggregation(geohashGrid("geohashgrid").field("location").size(1).shardSize(100).precision(precision))
.addAggregation(
AggregationBuilders.geohashGrid("geohashgrid").field("location").size(1).shardSize(100).precision(precision)
)
.get();

assertSearchResponse(response);
Expand Down Expand Up @@ -296,7 +297,7 @@ public void testSizeIsZero() {
IllegalArgumentException exception = expectThrows(
IllegalArgumentException.class,
() -> client().prepareSearch("idx")
.addAggregation(geohashGrid("geohashgrid").field("location").size(size).shardSize(shardSize))
.addAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").size(size).shardSize(shardSize))
.get()
);
assertThat(exception.getMessage(), containsString("[size] must be greater than 0. Found [0] in [geohashgrid]"));
Expand All @@ -308,7 +309,7 @@ public void testShardSizeIsZero() {
IllegalArgumentException exception = expectThrows(
IllegalArgumentException.class,
() -> client().prepareSearch("idx")
.addAggregation(geohashGrid("geohashgrid").field("location").size(size).shardSize(shardSize))
.addAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").size(size).shardSize(shardSize))
.get()
);
assertThat(exception.getMessage(), containsString("[shardSize] must be greater than 0. Found [0] in [geohashgrid]"));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.geo.search.aggregations.bucket;

import org.opensearch.action.index.IndexRequestBuilder;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.geo.GeoModulePluginIntegTestCase;
import org.opensearch.geo.search.aggregations.bucket.geogrid.GeoGrid;
import org.opensearch.geo.tests.common.AggregationBuilders;
import org.opensearch.geometry.utils.Geohash;
import org.opensearch.index.query.QueryBuilders;
import org.opensearch.search.aggregations.bucket.histogram.DateHistogramInterval;
import org.opensearch.search.aggregations.bucket.histogram.Histogram;
import org.opensearch.test.OpenSearchIntegTestCase;

import static org.hamcrest.Matchers.equalTo;
import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.opensearch.search.aggregations.AggregationBuilders.dateHistogram;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse;

/**
* Tests making sure that the reduce is propagated to all aggregations in the hierarchy when executing on a single shard
* These tests are based on the date histogram in combination of min_doc_count=0. In order for the date histogram to
* compute empty buckets, its {@code reduce()} method must be called. So by adding the date histogram under other buckets,
* we can make sure that the reduce is properly propagated by checking that empty buckets were created.
*/
@OpenSearchIntegTestCase.SuiteScopeTestCase
public class ShardReduceIT extends GeoModulePluginIntegTestCase {

private IndexRequestBuilder indexDoc(String date, int value) throws Exception {
return client().prepareIndex("idx")
.setSource(
jsonBuilder().startObject()
.field("value", value)
.field("ip", "10.0.0." + value)
.field("location", Geohash.stringEncode(5, 52, Geohash.PRECISION))
.field("date", date)
.field("term-l", 1)
.field("term-d", 1.5)
.field("term-s", "term")
.startObject("nested")
.field("date", date)
.endObject()
.endObject()
);
}

@Override
public void setupSuiteScopeCluster() throws Exception {
assertAcked(
prepareCreate("idx").setMapping(
"nested",
"type=nested",
"ip",
"type=ip",
"location",
"type=geo_point",
"term-s",
"type=keyword"
)
);

indexRandom(true, indexDoc("2014-01-01", 1), indexDoc("2014-01-02", 2), indexDoc("2014-01-04", 3));
ensureSearchable();
}

public void testGeoHashGrid() throws Exception {
SearchResponse response = client().prepareSearch("idx")
.setQuery(QueryBuilders.matchAllQuery())
.addAggregation(
AggregationBuilders.geohashGrid("grid")
.field("location")
.subAggregation(dateHistogram("histo").field("date").fixedInterval(DateHistogramInterval.DAY).minDocCount(0))
)
.get();

assertSearchResponse(response);

GeoGrid grid = response.getAggregations().get("grid");
Histogram histo = grid.getBuckets().iterator().next().getAggregations().get("histo");
assertThat(histo.getBuckets().size(), equalTo(4));
}

public void testGeoTileGrid() throws Exception {
SearchResponse response = client().prepareSearch("idx")
.setQuery(QueryBuilders.matchAllQuery())
.addAggregation(
AggregationBuilders.geotileGrid("grid")
.field("location")
.subAggregation(dateHistogram("histo").field("date").fixedInterval(DateHistogramInterval.DAY).minDocCount(0))
)
.get();

assertSearchResponse(response);

GeoGrid grid = response.getAggregations().get("grid");
Histogram histo = grid.getBuckets().iterator().next().getAggregations().get("histo");
assertThat(histo.getBuckets().size(), equalTo(4));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
* to copy the code as we cannot depend on this class.
* <a href="https://github.com/opensearch-project/geospatial/issues/92">GitHub issue</a>
*/
public abstract class AbstractGeoAggregatorTestCaseModulePlugin extends GeoModulePluginIntegTestCase {
public abstract class AbstractGeoAggregatorModulePluginTestCase extends GeoModulePluginIntegTestCase {

protected static final String SINGLE_VALUED_FIELD_NAME = "geo_value";
protected static final String MULTI_VALUED_FIELD_NAME = "geo_values";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
import static org.opensearch.geo.tests.common.AggregationBuilders.geoBounds;

@OpenSearchIntegTestCase.SuiteScopeTestCase
public class GeoBoundsIT extends AbstractGeoAggregatorTestCaseModulePlugin {
public class GeoBoundsITTestCase extends AbstractGeoAggregatorModulePluginTestCase {
private static final String aggName = "geoBounds";

public void testSingleValuedField() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

/*
* 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.
*/

/*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.geo.search.aggregations.metrics;

import org.opensearch.action.search.SearchResponse;
import org.opensearch.common.geo.GeoPoint;
import org.opensearch.geo.search.aggregations.bucket.geogrid.GeoGrid;
import org.opensearch.geo.tests.common.AggregationBuilders;
import org.opensearch.search.aggregations.metrics.GeoCentroid;
import org.opensearch.test.OpenSearchIntegTestCase;

import java.util.List;

import static org.hamcrest.Matchers.closeTo;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.notNullValue;
import static org.opensearch.search.aggregations.AggregationBuilders.geoCentroid;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse;

@OpenSearchIntegTestCase.SuiteScopeTestCase
public class GeoCentroidITTestCase extends AbstractGeoAggregatorModulePluginTestCase {
private static final String aggName = "geoCentroid";

public void testSingleValueFieldAsSubAggToGeohashGrid() throws Exception {
SearchResponse response = client().prepareSearch(HIGH_CARD_IDX_NAME)
.addAggregation(
AggregationBuilders.geohashGrid("geoGrid")
.field(SINGLE_VALUED_FIELD_NAME)
.subAggregation(geoCentroid(aggName).field(SINGLE_VALUED_FIELD_NAME))
)
.get();
assertSearchResponse(response);

GeoGrid grid = response.getAggregations().get("geoGrid");
assertThat(grid, notNullValue());
assertThat(grid.getName(), equalTo("geoGrid"));
List<? extends GeoGrid.Bucket> buckets = grid.getBuckets();
for (GeoGrid.Bucket cell : buckets) {
String geohash = cell.getKeyAsString();
GeoPoint expectedCentroid = expectedCentroidsForGeoHash.get(geohash);
GeoCentroid centroidAgg = cell.getAggregations().get(aggName);
assertThat(
"Geohash " + geohash + " has wrong centroid latitude ",
expectedCentroid.lat(),
closeTo(centroidAgg.centroid().lat(), GEOHASH_TOLERANCE)
);
assertThat(
"Geohash " + geohash + " has wrong centroid longitude",
expectedCentroid.lon(),
closeTo(centroidAgg.centroid().lon(), GEOHASH_TOLERANCE)
);
}
}
}
Loading

0 comments on commit da76f2a

Please sign in to comment.