Skip to content

Commit da7dbea

Browse files
navneet1vnavneet1v
committed
Refactored the src and test of GeoHashGrid and GeoTileGrid Aggregations on GeoPoint from server folder to geo module.(opensearch-project#4071) (opensearch-project#4072)
The changes also include: * Updated Search plugin to provide the interface so that plugins can also register the compositie aggregations * Added YAML test for the geo_grid, geo_tile and composite aggregation Signed-off-by: Navneet Verma <navneev@amazon.com>
1 parent 4643620 commit da7dbea

File tree

81 files changed

+1395
-761
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

81 files changed

+1395
-761
lines changed

‎client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,6 @@
108108
import org.opensearch.search.aggregations.bucket.filter.FiltersAggregationBuilder;
109109
import org.opensearch.search.aggregations.bucket.filter.ParsedFilter;
110110
import org.opensearch.search.aggregations.bucket.filter.ParsedFilters;
111-
import org.opensearch.search.aggregations.bucket.geogrid.GeoHashGridAggregationBuilder;
112-
import org.opensearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder;
113-
import org.opensearch.search.aggregations.bucket.geogrid.ParsedGeoHashGrid;
114-
import org.opensearch.search.aggregations.bucket.geogrid.ParsedGeoTileGrid;
115111
import org.opensearch.search.aggregations.bucket.global.GlobalAggregationBuilder;
116112
import org.opensearch.search.aggregations.bucket.global.ParsedGlobal;
117113
import org.opensearch.search.aggregations.bucket.histogram.AutoDateHistogramAggregationBuilder;
@@ -2130,8 +2126,6 @@ static List<NamedXContentRegistry.Entry> getDefaultNamedXContents() {
21302126
map.put(GlobalAggregationBuilder.NAME, (p, c) -> ParsedGlobal.fromXContent(p, (String) c));
21312127
map.put(FilterAggregationBuilder.NAME, (p, c) -> ParsedFilter.fromXContent(p, (String) c));
21322128
map.put(InternalSampler.PARSER_NAME, (p, c) -> ParsedSampler.fromXContent(p, (String) c));
2133-
map.put(GeoHashGridAggregationBuilder.NAME, (p, c) -> ParsedGeoHashGrid.fromXContent(p, (String) c));
2134-
map.put(GeoTileGridAggregationBuilder.NAME, (p, c) -> ParsedGeoTileGrid.fromXContent(p, (String) c));
21352129
map.put(RangeAggregationBuilder.NAME, (p, c) -> ParsedRange.fromXContent(p, (String) c));
21362130
map.put(DateRangeAggregationBuilder.NAME, (p, c) -> ParsedDateRange.fromXContent(p, (String) c));
21372131
map.put(GeoDistanceAggregationBuilder.NAME, (p, c) -> ParsedGeoDistance.fromXContent(p, (String) c));

‎modules/geo/build.gradle

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,16 @@ opensearchplugin {
3737

3838
restResources {
3939
restApi {
40-
includeCore '_common', 'indices', 'index', 'search'
40+
includeCore '_common', 'indices', 'index', 'search', 'bulk'
4141
}
4242
}
4343
artifacts {
4444
restTests(project.file('src/yamlRestTest/resources/rest-api-spec/test'))
4545
}
46+
/**
47+
* These compiler arguments needs to be removed, as there are raw types being used in the GeoGrid and GeoTile aggregations.
48+
*/
49+
tasks.withType(JavaCompile).configureEach {
50+
options.compilerArgs -= '-Xlint:rawtypes'
51+
options.compilerArgs -= '-Xlint:unchecked'
52+
}

‎server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/GeoHashGridIT.java renamed to ‎modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoHashGridIT.java

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
* GitHub history for details.
3030
*/
3131

32-
package org.opensearch.search.aggregations.bucket;
32+
package org.opensearch.geo.search.aggregations.bucket;
3333

3434
import com.carrotsearch.hppc.ObjectIntHashMap;
3535
import com.carrotsearch.hppc.ObjectIntMap;
@@ -41,12 +41,12 @@
4141
import org.opensearch.common.geo.GeoPoint;
4242
import org.opensearch.common.settings.Settings;
4343
import org.opensearch.common.xcontent.XContentBuilder;
44+
import org.opensearch.geo.GeoModulePluginIntegTestCase;
45+
import org.opensearch.geo.search.aggregations.bucket.geogrid.GeoGrid;
46+
import org.opensearch.geo.tests.common.AggregationBuilders;
4447
import org.opensearch.index.query.GeoBoundingBoxQueryBuilder;
45-
import org.opensearch.search.aggregations.AggregationBuilders;
4648
import org.opensearch.search.aggregations.InternalAggregation;
4749
import org.opensearch.search.aggregations.bucket.filter.Filter;
48-
import org.opensearch.search.aggregations.bucket.geogrid.GeoGrid;
49-
import org.opensearch.search.aggregations.bucket.geogrid.GeoGrid.Bucket;
5050
import org.opensearch.test.OpenSearchIntegTestCase;
5151
import org.opensearch.test.VersionUtils;
5252

@@ -57,17 +57,16 @@
5757
import java.util.Random;
5858
import java.util.Set;
5959

60+
import static org.hamcrest.Matchers.containsString;
61+
import static org.hamcrest.Matchers.equalTo;
62+
import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
6063
import static org.opensearch.geometry.utils.Geohash.PRECISION;
6164
import static org.opensearch.geometry.utils.Geohash.stringEncode;
62-
import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
63-
import static org.opensearch.search.aggregations.AggregationBuilders.geohashGrid;
6465
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
6566
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse;
66-
import static org.hamcrest.Matchers.containsString;
67-
import static org.hamcrest.Matchers.equalTo;
6867

6968
@OpenSearchIntegTestCase.SuiteScopeTestCase
70-
public class GeoHashGridIT extends OpenSearchIntegTestCase {
69+
public class GeoHashGridIT extends GeoModulePluginIntegTestCase {
7170

7271
@Override
7372
protected boolean forbidPrivateIndexSettings() {
@@ -158,13 +157,13 @@ public void setupSuiteScopeCluster() throws Exception {
158157
public void testSimple() throws Exception {
159158
for (int precision = 1; precision <= PRECISION; precision++) {
160159
SearchResponse response = client().prepareSearch("idx")
161-
.addAggregation(geohashGrid("geohashgrid").field("location").precision(precision))
160+
.addAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").precision(precision))
162161
.get();
163162

164163
assertSearchResponse(response);
165164

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

191190
assertSearchResponse(response);
@@ -208,8 +207,8 @@ public void testFiltered() throws Exception {
208207
for (int precision = 1; precision <= PRECISION; precision++) {
209208
SearchResponse response = client().prepareSearch("idx")
210209
.addAggregation(
211-
AggregationBuilders.filter("filtered", bbox)
212-
.subAggregation(geohashGrid("geohashgrid").field("location").precision(precision))
210+
org.opensearch.search.aggregations.AggregationBuilders.filter("filtered", bbox)
211+
.subAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").precision(precision))
213212
)
214213
.get();
215214

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

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

253252
assertSearchResponse(response);
@@ -267,7 +266,9 @@ public void testPartiallyUnmapped() throws Exception {
267266
public void testTopMatch() throws Exception {
268267
for (int precision = 1; precision <= PRECISION; precision++) {
269268
SearchResponse response = client().prepareSearch("idx")
270-
.addAggregation(geohashGrid("geohashgrid").field("location").size(1).shardSize(100).precision(precision))
269+
.addAggregation(
270+
AggregationBuilders.geohashGrid("geohashgrid").field("location").size(1).shardSize(100).precision(precision)
271+
)
271272
.get();
272273

273274
assertSearchResponse(response);
@@ -296,7 +297,7 @@ public void testSizeIsZero() {
296297
IllegalArgumentException exception = expectThrows(
297298
IllegalArgumentException.class,
298299
() -> client().prepareSearch("idx")
299-
.addAggregation(geohashGrid("geohashgrid").field("location").size(size).shardSize(shardSize))
300+
.addAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").size(size).shardSize(shardSize))
300301
.get()
301302
);
302303
assertThat(exception.getMessage(), containsString("[size] must be greater than 0. Found [0] in [geohashgrid]"));
@@ -308,7 +309,7 @@ public void testShardSizeIsZero() {
308309
IllegalArgumentException exception = expectThrows(
309310
IllegalArgumentException.class,
310311
() -> client().prepareSearch("idx")
311-
.addAggregation(geohashGrid("geohashgrid").field("location").size(size).shardSize(shardSize))
312+
.addAggregation(AggregationBuilders.geohashGrid("geohashgrid").field("location").size(size).shardSize(shardSize))
312313
.get()
313314
);
314315
assertThat(exception.getMessage(), containsString("[shardSize] must be greater than 0. Found [0] in [geohashgrid]"));

‎modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/ShardReduceIT.java

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.geo.search.aggregations.bucket;
10+
11+
import org.opensearch.action.index.IndexRequestBuilder;
12+
import org.opensearch.action.search.SearchResponse;
13+
import org.opensearch.geo.GeoModulePluginIntegTestCase;
14+
import org.opensearch.geo.search.aggregations.bucket.geogrid.GeoGrid;
15+
import org.opensearch.geo.tests.common.AggregationBuilders;
16+
import org.opensearch.geometry.utils.Geohash;
17+
import org.opensearch.index.query.QueryBuilders;
18+
import org.opensearch.search.aggregations.bucket.histogram.DateHistogramInterval;
19+
import org.opensearch.search.aggregations.bucket.histogram.Histogram;
20+
import org.opensearch.test.OpenSearchIntegTestCase;
21+
22+
import static org.hamcrest.Matchers.equalTo;
23+
import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
24+
import static org.opensearch.search.aggregations.AggregationBuilders.dateHistogram;
25+
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
26+
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse;
27+
28+
/**
29+
* Tests making sure that the reduce is propagated to all aggregations in the hierarchy when executing on a single shard
30+
* These tests are based on the date histogram in combination of min_doc_count=0. In order for the date histogram to
31+
* compute empty buckets, its {@code reduce()} method must be called. So by adding the date histogram under other buckets,
32+
* we can make sure that the reduce is properly propagated by checking that empty buckets were created.
33+
*/
34+
@OpenSearchIntegTestCase.SuiteScopeTestCase
35+
public class ShardReduceIT extends GeoModulePluginIntegTestCase {
36+
37+
private IndexRequestBuilder indexDoc(String date, int value) throws Exception {
38+
return client().prepareIndex("idx")
39+
.setSource(
40+
jsonBuilder().startObject()
41+
.field("value", value)
42+
.field("ip", "10.0.0." + value)
43+
.field("location", Geohash.stringEncode(5, 52, Geohash.PRECISION))
44+
.field("date", date)
45+
.field("term-l", 1)
46+
.field("term-d", 1.5)
47+
.field("term-s", "term")
48+
.startObject("nested")
49+
.field("date", date)
50+
.endObject()
51+
.endObject()
52+
);
53+
}
54+
55+
@Override
56+
public void setupSuiteScopeCluster() throws Exception {
57+
assertAcked(
58+
prepareCreate("idx").setMapping(
59+
"nested",
60+
"type=nested",
61+
"ip",
62+
"type=ip",
63+
"location",
64+
"type=geo_point",
65+
"term-s",
66+
"type=keyword"
67+
)
68+
);
69+
70+
indexRandom(true, indexDoc("2014-01-01", 1), indexDoc("2014-01-02", 2), indexDoc("2014-01-04", 3));
71+
ensureSearchable();
72+
}
73+
74+
public void testGeoHashGrid() throws Exception {
75+
SearchResponse response = client().prepareSearch("idx")
76+
.setQuery(QueryBuilders.matchAllQuery())
77+
.addAggregation(
78+
AggregationBuilders.geohashGrid("grid")
79+
.field("location")
80+
.subAggregation(dateHistogram("histo").field("date").fixedInterval(DateHistogramInterval.DAY).minDocCount(0))
81+
)
82+
.get();
83+
84+
assertSearchResponse(response);
85+
86+
GeoGrid grid = response.getAggregations().get("grid");
87+
Histogram histo = grid.getBuckets().iterator().next().getAggregations().get("histo");
88+
assertThat(histo.getBuckets().size(), equalTo(4));
89+
}
90+
91+
public void testGeoTileGrid() throws Exception {
92+
SearchResponse response = client().prepareSearch("idx")
93+
.setQuery(QueryBuilders.matchAllQuery())
94+
.addAggregation(
95+
AggregationBuilders.geotileGrid("grid")
96+
.field("location")
97+
.subAggregation(dateHistogram("histo").field("date").fixedInterval(DateHistogramInterval.DAY).minDocCount(0))
98+
)
99+
.get();
100+
101+
assertSearchResponse(response);
102+
103+
GeoGrid grid = response.getAggregations().get("grid");
104+
Histogram histo = grid.getBuckets().iterator().next().getAggregations().get("histo");
105+
assertThat(histo.getBuckets().size(), equalTo(4));
106+
}
107+
}

‎modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoAggregatorTestCaseModulePlugin.java renamed to ‎modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoAggregatorModulePluginTestCase.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
* to copy the code as we cannot depend on this class.
4343
* <a href="https://github.com/opensearch-project/geospatial/issues/92">GitHub issue</a>
4444
*/
45-
public abstract class AbstractGeoAggregatorTestCaseModulePlugin extends GeoModulePluginIntegTestCase {
45+
public abstract class AbstractGeoAggregatorModulePluginTestCase extends GeoModulePluginIntegTestCase {
4646

4747
protected static final String SINGLE_VALUED_FIELD_NAME = "geo_value";
4848
protected static final String MULTI_VALUED_FIELD_NAME = "geo_values";

‎modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsIT.java renamed to ‎modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsITTestCase.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
import static org.opensearch.geo.tests.common.AggregationBuilders.geoBounds;
5858

5959
@OpenSearchIntegTestCase.SuiteScopeTestCase
60-
public class GeoBoundsIT extends AbstractGeoAggregatorTestCaseModulePlugin {
60+
public class GeoBoundsITTestCase extends AbstractGeoAggregatorModulePluginTestCase {
6161
private static final String aggName = "geoBounds";
6262

6363
public void testSingleValuedField() throws Exception {

‎modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/GeoCentroidITTestCase.java

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
/*
10+
* Licensed to Elasticsearch under one or more contributor
11+
* license agreements. See the NOTICE file distributed with
12+
* this work for additional information regarding copyright
13+
* ownership. Elasticsearch licenses this file to you under
14+
* the Apache License, Version 2.0 (the "License"); you may
15+
* not use this file except in compliance with the License.
16+
* You may obtain a copy of the License at
17+
*
18+
* http://www.apache.org/licenses/LICENSE-2.0
19+
*
20+
* Unless required by applicable law or agreed to in writing,
21+
* software distributed under the License is distributed on an
22+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
23+
* KIND, either express or implied. See the License for the
24+
* specific language governing permissions and limitations
25+
* under the License.
26+
*/
27+
28+
/*
29+
* Modifications Copyright OpenSearch Contributors. See
30+
* GitHub history for details.
31+
*/
32+
33+
package org.opensearch.geo.search.aggregations.metrics;
34+
35+
import org.opensearch.action.search.SearchResponse;
36+
import org.opensearch.common.geo.GeoPoint;
37+
import org.opensearch.geo.search.aggregations.bucket.geogrid.GeoGrid;
38+
import org.opensearch.geo.tests.common.AggregationBuilders;
39+
import org.opensearch.search.aggregations.metrics.GeoCentroid;
40+
import org.opensearch.test.OpenSearchIntegTestCase;
41+
42+
import java.util.List;
43+
44+
import static org.hamcrest.Matchers.closeTo;
45+
import static org.hamcrest.Matchers.equalTo;
46+
import static org.hamcrest.Matchers.notNullValue;
47+
import static org.opensearch.search.aggregations.AggregationBuilders.geoCentroid;
48+
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse;
49+
50+
@OpenSearchIntegTestCase.SuiteScopeTestCase
51+
public class GeoCentroidITTestCase extends AbstractGeoAggregatorModulePluginTestCase {
52+
private static final String aggName = "geoCentroid";
53+
54+
public void testSingleValueFieldAsSubAggToGeohashGrid() throws Exception {
55+
SearchResponse response = client().prepareSearch(HIGH_CARD_IDX_NAME)
56+
.addAggregation(
57+
AggregationBuilders.geohashGrid("geoGrid")
58+
.field(SINGLE_VALUED_FIELD_NAME)
59+
.subAggregation(geoCentroid(aggName).field(SINGLE_VALUED_FIELD_NAME))
60+
)
61+
.get();
62+
assertSearchResponse(response);
63+
64+
GeoGrid grid = response.getAggregations().get("geoGrid");
65+
assertThat(grid, notNullValue());
66+
assertThat(grid.getName(), equalTo("geoGrid"));
67+
List<? extends GeoGrid.Bucket> buckets = grid.getBuckets();
68+
for (GeoGrid.Bucket cell : buckets) {
69+
String geohash = cell.getKeyAsString();
70+
GeoPoint expectedCentroid = expectedCentroidsForGeoHash.get(geohash);
71+
GeoCentroid centroidAgg = cell.getAggregations().get(aggName);
72+
assertThat(
73+
"Geohash " + geohash + " has wrong centroid latitude ",
74+
expectedCentroid.lat(),
75+
closeTo(centroidAgg.centroid().lat(), GEOHASH_TOLERANCE)
76+
);
77+
assertThat(
78+
"Geohash " + geohash + " has wrong centroid longitude",
79+
expectedCentroid.lon(),
80+
closeTo(centroidAgg.centroid().lon(), GEOHASH_TOLERANCE)
81+
);
82+
}
83+
}
84+
}

0 commit comments

Comments
 (0)