[Backport 2.x] Refactors the GeoBoundsAggregation for geo_point types from the core server to the geo module (#3992)

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

@@ -157,7 +157,6 @@
 import org.opensearch.search.aggregations.metrics.AvgAggregationBuilder;
 import org.opensearch.search.aggregations.metrics.CardinalityAggregationBuilder;
 import org.opensearch.search.aggregations.metrics.ExtendedStatsAggregationBuilder;
-import org.opensearch.search.aggregations.metrics.GeoBoundsAggregationBuilder;
 import org.opensearch.search.aggregations.metrics.GeoCentroidAggregationBuilder;
 import org.opensearch.search.aggregations.metrics.InternalHDRPercentileRanks;
 import org.opensearch.search.aggregations.metrics.InternalHDRPercentiles;
@@ -169,7 +168,6 @@
 import org.opensearch.search.aggregations.metrics.ParsedAvg;
 import org.opensearch.search.aggregations.metrics.ParsedCardinality;
 import org.opensearch.search.aggregations.metrics.ParsedExtendedStats;
-import org.opensearch.search.aggregations.metrics.ParsedGeoBounds;
 import org.opensearch.search.aggregations.metrics.ParsedGeoCentroid;
 import org.opensearch.search.aggregations.metrics.ParsedHDRPercentileRanks;
 import org.opensearch.search.aggregations.metrics.ParsedHDRPercentiles;
@@ -2116,7 +2114,6 @@ static List<NamedXContentRegistry.Entry> getDefaultNamedXContents() {
         map.put(StatsBucketPipelineAggregationBuilder.NAME, (p, c) -> ParsedStatsBucket.fromXContent(p, (String) c));
         map.put(ExtendedStatsAggregationBuilder.NAME, (p, c) -> ParsedExtendedStats.fromXContent(p, (String) c));
         map.put(ExtendedStatsBucketPipelineAggregationBuilder.NAME, (p, c) -> ParsedExtendedStatsBucket.fromXContent(p, (String) c));
-        map.put(GeoBoundsAggregationBuilder.NAME, (p, c) -> ParsedGeoBounds.fromXContent(p, (String) c));
         map.put(GeoCentroidAggregationBuilder.NAME, (p, c) -> ParsedGeoCentroid.fromXContent(p, (String) c));
         map.put(HistogramAggregationBuilder.NAME, (p, c) -> ParsedHistogram.fromXContent(p, (String) c));
         map.put(DateHistogramAggregationBuilder.NAME, (p, c) -> ParsedDateHistogram.fromXContent(p, (String) c));

modules/geo/build.gradle

@@ -28,10 +28,11 @@
  * under the License.
  */
 apply plugin: 'opensearch.yaml-rest-test'
+apply plugin: 'opensearch.internal-cluster-test'
 
 opensearchplugin {
-  description 'Placeholder plugin for geospatial features in OpenSearch. only registers geo_shape field mapper for now'
-  classname 'org.opensearch.geo.GeoPlugin'
+  description 'Plugin for geospatial features in OpenSearch. Registering the geo_shape and aggregations GeoBounds on Geo_Shape and Geo_Point'
+  classname 'org.opensearch.geo.GeoModulePlugin'
 }
 
 restResources {
@@ -42,4 +43,3 @@ restResources {
 artifacts {
   restTests(project.file('src/yamlRestTest/resources/rest-api-spec/test'))
 }
-test.enabled = false

modules/geo/src/internalClusterTest/java/org/opensearch/geo/GeoModulePluginIntegTestCase.java

@@ -0,0 +1,47 @@
+/*
+ * 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;
+
+import org.opensearch.index.mapper.GeoShapeFieldMapper;
+import org.opensearch.plugins.Plugin;
+import org.opensearch.test.OpenSearchIntegTestCase;
+import org.opensearch.test.TestGeoShapeFieldMapperPlugin;
+
+import java.util.Collection;
+import java.util.Collections;
+
+/**
+ * This is the base class for all the Geo related integration tests. Use this class to add the features and settings
+ * for the test cluster on which integration tests are running.
+ */
+public abstract class GeoModulePluginIntegTestCase extends OpenSearchIntegTestCase {
+    /**
+     * Returns a collection of plugins that should be loaded on each node for doing the integration tests. As this
+     * geo plugin is not getting packaged in a zip, we need to load it before the tests run.
+     *
+     * @return List of {@link Plugin}
+     */
+    @Override
+    protected Collection<Class<? extends Plugin>> nodePlugins() {
+        return Collections.singletonList(GeoModulePlugin.class);
+    }
+
+    /**
+     * This was added as a backdoor to Mock the implementation of {@link GeoShapeFieldMapper} which was coming from
+     * {@link GeoModulePlugin}. Mock implementation is {@link TestGeoShapeFieldMapperPlugin}. Now we are using the
+     * {@link GeoModulePlugin} in our integration tests we need to override this functionality to avoid multiple mapper
+     * error.
+     *
+     * @return boolean
+     */
+    @Override
+    protected boolean addMockGeoShapeFieldMapper() {
+        return false;
+    }
+}

modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/MissingValueIT.java

@@ -0,0 +1,59 @@
+/*
+ * 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;
+
+import org.opensearch.action.search.SearchResponse;
+import org.opensearch.geo.GeoModulePluginIntegTestCase;
+import org.opensearch.geo.search.aggregations.metrics.GeoBounds;
+import org.opensearch.geo.tests.common.AggregationBuilders;
+import org.opensearch.test.OpenSearchIntegTestCase;
+
+import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
+import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse;
+import static org.hamcrest.Matchers.closeTo;
+
+@OpenSearchIntegTestCase.SuiteScopeTestCase
+public class MissingValueIT extends GeoModulePluginIntegTestCase {
+
+    @Override
+    protected void setupSuiteScopeCluster() throws Exception {
+        assertAcked(prepareCreate("idx").setMapping("date", "type=date", "location", "type=geo_point", "str", "type=keyword").get());
+        indexRandom(
+            true,
+            client().prepareIndex("idx").setId("1").setSource(),
+            client().prepareIndex("idx")
+                .setId("2")
+                .setSource("str", "foo", "long", 3L, "double", 5.5, "date", "2015-05-07", "location", "1,2")
+        );
+    }
+
+    public void testUnmappedGeoBounds() {
+        SearchResponse response = client().prepareSearch("idx")
+            .addAggregation(AggregationBuilders.geoBounds("bounds").field("non_existing_field").missing("2,1"))
+            .get();
+        assertSearchResponse(response);
+        GeoBounds bounds = response.getAggregations().get("bounds");
+        assertThat(bounds.bottomRight().lat(), closeTo(2.0, 1E-5));
+        assertThat(bounds.bottomRight().lon(), closeTo(1.0, 1E-5));
+        assertThat(bounds.topLeft().lat(), closeTo(2.0, 1E-5));
+        assertThat(bounds.topLeft().lon(), closeTo(1.0, 1E-5));
+    }
+
+    public void testGeoBounds() {
+        SearchResponse response = client().prepareSearch("idx")
+            .addAggregation(AggregationBuilders.geoBounds("bounds").field("location").missing("2,1"))
+            .get();
+        assertSearchResponse(response);
+        GeoBounds bounds = response.getAggregations().get("bounds");
+        assertThat(bounds.bottomRight().lat(), closeTo(1.0, 1E-5));
+        assertThat(bounds.bottomRight().lon(), closeTo(2.0, 1E-5));
+        assertThat(bounds.topLeft().lat(), closeTo(2.0, 1E-5));
+        assertThat(bounds.topLeft().lon(), closeTo(1.0, 1E-5));
+    }
+}