Refactored the src and test of GeoHashGrid and GeoTileGrid Aggregations on GeoPoint from server folder to geo module.(#4071) (#4072)

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

@@ -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;
@@ -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));

modules/geo/build.gradle

@@ -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'
+}

server/src/internalClusterTest/java/org/opensearch/search/aggregations/bucket/GeoHashGridIT.java → modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoHashGridIT.java

@@ -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;
@@ -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;
 
@@ -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() {
@@ -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++) {
@@ -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);
@@ -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();
 
@@ -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);
@@ -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);
@@ -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);
@@ -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]"));
@@ -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]"));

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

@@ -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));
+    }
+}

modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoAggregatorTestCaseModulePlugin.java → modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoAggregatorModulePluginTestCase.java

@@ -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";

modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsIT.java → modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsITTestCase.java

@@ -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 {

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

@@ -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)
+            );
+        }
+    }
+}