Add GeoBounds aggregation on GeoShape field type.(opensearch-project#…

…3980) (opensearch-project#4266) Enables geo_bounds aggregation to work with geo_shape field types. This enhancement includes: * Addition of Doc values on the GeoShape Field. * Addition of GeoShape ValueSource level code interfaces for accessing the DocValues. * Addition of Missing Value feature in the GeoShape Aggregations. Signed-off-by: Navneet Verma <[email protected]> Signed-off-by: Vishal Sarda <[email protected]>
Vishalks · Sep 28, 2022 · 4829d7f · 4829d7f
1 parent 76b1fe8
commit 4829d7f
Show file tree

Hide file tree

Showing 31 changed files with 2,369 additions and 64 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -80,6 +80,10 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Github workflow for changelog verification ([#4085](https://github.com/opensearch-project/OpenSearch/pull/4085))
 - Label configuration for dependabot PRs ([#4348](https://github.com/opensearch-project/OpenSearch/pull/4348))
 - Added RestLayer Changes for PIT stats ([#4217](https://github.com/opensearch-project/OpenSearch/pull/4217))
+- Added GeoBounds aggregation on GeoShape field type.([#4266](https://github.com/opensearch-project/OpenSearch/pull/4266))
+  - Addition of Doc values on the GeoShape Field
+  - Addition of GeoShape ValueSource level code interfaces for accessing the DocValues.
+  - Addition of Missing Value feature in the GeoShape Aggregations.
 
 ### Changed
 

diff --git a/libs/geo/src/main/java/org/opensearch/geometry/GeometryCollection.java b/libs/geo/src/main/java/org/opensearch/geometry/GeometryCollection.java
@@ -88,6 +88,15 @@ public G get(int i) {
         return shapes.get(i);
     }
 
+    /**
+     * Returns a {@link List} of All {@link Geometry} present in this collection.
+     *
+     * @return a {@link List} of All {@link Geometry}
+     */
+    public List<G> getAll() {
+        return shapes;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;

diff --git a/modules/geo/build.gradle b/modules/geo/build.gradle
@@ -40,6 +40,7 @@ restResources {
     includeCore '_common', 'indices', 'index', 'search', 'bulk'
   }
 }
+
 artifacts {
   restTests(project.file('src/yamlRestTest/resources/rest-api-spec/test'))
 }

diff --git a/...les/geo/src/internalClusterTest/java/org/opensearch/geo/GeoModulePluginIntegTestCase.java b/...les/geo/src/internalClusterTest/java/org/opensearch/geo/GeoModulePluginIntegTestCase.java
@@ -21,6 +21,9 @@
  * for the test cluster on which integration tests are running.
  */
 public abstract class GeoModulePluginIntegTestCase extends OpenSearchIntegTestCase {
+
+    protected static final double GEOHASH_TOLERANCE = 1E-5D;
+
     /**
      * 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.

diff --git a/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/MissingValueIT.java b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/MissingValueIT.java
@@ -8,52 +8,149 @@
 
 package org.opensearch.geo.search;
 
+import org.hamcrest.MatcherAssert;
+import org.junit.Before;
 import org.opensearch.action.search.SearchResponse;
+import org.opensearch.common.geo.GeoPoint;
 import org.opensearch.geo.GeoModulePluginIntegTestCase;
+import org.opensearch.geo.search.aggregations.common.GeoBoundsHelper;
 import org.opensearch.geo.search.aggregations.metrics.GeoBounds;
 import org.opensearch.geo.tests.common.AggregationBuilders;
+import org.opensearch.geo.tests.common.RandomGeoGenerator;
+import org.opensearch.geo.tests.common.RandomGeoGeometryGenerator;
+import org.opensearch.geometry.Geometry;
+import org.opensearch.geometry.utils.WellKnownText;
 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;
 
+/**
+ * Tests to validate if user specified a missingValue in the input while doing the aggregation
+ */
 @OpenSearchIntegTestCase.SuiteScopeTestCase
 public class MissingValueIT extends GeoModulePluginIntegTestCase {
 
+    private static final String INDEX_NAME = "idx";
+    private static final String GEO_SHAPE_FIELD_NAME = "myshape";
+    private static final String GEO_SHAPE_FIELD_TYPE = "type=geo_shape";
+    private static final String AGGREGATION_NAME = "bounds";
+    private static final String NON_EXISTENT_FIELD = "non_existing_field";
+    private static final WellKnownText WKT = WellKnownText.INSTANCE;
+    private static Geometry indexedGeometry;
+    private static GeoPoint indexedGeoPoint;
+    private GeoPoint bottomRight;
+    private GeoPoint topLeft;
+
     @Override
     protected void setupSuiteScopeCluster() throws Exception {
-        assertAcked(prepareCreate("idx").setMapping("date", "type=date", "location", "type=geo_point", "str", "type=keyword").get());
+        assertAcked(
+            prepareCreate(INDEX_NAME).setMapping(
+                "date",
+                "type=date",
+                "location",
+                "type=geo_point",
+                "str",
+                "type=keyword",
+                GEO_SHAPE_FIELD_NAME,
+                GEO_SHAPE_FIELD_TYPE
+            ).get()
+        );
+        indexedGeometry = RandomGeoGeometryGenerator.randomGeometry(random());
+        indexedGeoPoint = RandomGeoGenerator.randomPoint(random());
+        assert indexedGeometry != null;
         indexRandom(
             true,
-            client().prepareIndex("idx").setId("1").setSource(),
-            client().prepareIndex("idx")
+            client().prepareIndex(INDEX_NAME).setId("1").setSource(),
+            client().prepareIndex(INDEX_NAME)
                 .setId("2")
-                .setSource("str", "foo", "long", 3L, "double", 5.5, "date", "2015-05-07", "location", "1,2")
+                .setSource(
+                    "str",
+                    "foo",
+                    "long",
+                    3L,
+                    "double",
+                    5.5,
+                    "date",
+                    "2015-05-07",
+                    "location",
+                    indexedGeoPoint.toString(),
+                    GEO_SHAPE_FIELD_NAME,
+                    WKT.toWKT(indexedGeometry)
+                )
         );
     }
 
+    @Before
+    public void runBeforeEachTest() {
+        bottomRight = new GeoPoint(Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY);
+        topLeft = new GeoPoint(Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY);
+    }
+
     public void testUnmappedGeoBounds() {
-        SearchResponse response = client().prepareSearch("idx")
-            .addAggregation(AggregationBuilders.geoBounds("bounds").field("non_existing_field").missing("2,1"))
+        final GeoPoint missingGeoPoint = RandomGeoGenerator.randomPoint(random());
+        GeoBoundsHelper.updateBoundsBottomRight(missingGeoPoint, bottomRight);
+        GeoBoundsHelper.updateBoundsTopLeft(missingGeoPoint, topLeft);
+        SearchResponse response = client().prepareSearch(INDEX_NAME)
+            .addAggregation(
+                AggregationBuilders.geoBounds(AGGREGATION_NAME)
+                    .field(NON_EXISTENT_FIELD)
+                    .wrapLongitude(false)
+                    .missing(missingGeoPoint.toString())
+            )
             .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));
+        validateResult(response.getAggregations().get(AGGREGATION_NAME));
     }
 
     public void testGeoBounds() {
-        SearchResponse response = client().prepareSearch("idx")
-            .addAggregation(AggregationBuilders.geoBounds("bounds").field("location").missing("2,1"))
+        GeoBoundsHelper.updateBoundsForGeoPoint(indexedGeoPoint, topLeft, bottomRight);
+        final GeoPoint missingGeoPoint = RandomGeoGenerator.randomPoint(random());
+        GeoBoundsHelper.updateBoundsForGeoPoint(missingGeoPoint, topLeft, bottomRight);
+        SearchResponse response = client().prepareSearch(INDEX_NAME)
+            .addAggregation(
+                AggregationBuilders.geoBounds(AGGREGATION_NAME).field("location").wrapLongitude(false).missing(missingGeoPoint.toString())
+            )
             .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));
+        validateResult(response.getAggregations().get(AGGREGATION_NAME));
+    }
+
+    public void testGeoBoundsWithMissingShape() {
+        // create GeoBounds for the indexed Field
+        GeoBoundsHelper.updateBoundsForGeometry(indexedGeometry, topLeft, bottomRight);
+        final Geometry missingGeometry = RandomGeoGeometryGenerator.randomGeometry(random());
+        assert missingGeometry != null;
+        GeoBoundsHelper.updateBoundsForGeometry(missingGeometry, topLeft, bottomRight);
+        final SearchResponse response = client().prepareSearch(INDEX_NAME)
+            .addAggregation(
+                AggregationBuilders.geoBounds(AGGREGATION_NAME)
+                    .wrapLongitude(false)
+                    .field(GEO_SHAPE_FIELD_NAME)
+                    .missing(WKT.toWKT(missingGeometry))
+            )
+            .get();
+        assertSearchResponse(response);
+        validateResult(response.getAggregations().get(AGGREGATION_NAME));
+    }
+
+    public void testUnmappedGeoBoundsOnGeoShape() {
+        // We cannot useGeometry other than Point as for GeoBoundsAggregation as the Default Value for the
+        // CoreValueSourceType is GeoPoint hence we need to use Point here.
+        final Geometry missingGeometry = RandomGeoGeometryGenerator.randomPoint(random());
+        final SearchResponse response = client().prepareSearch(INDEX_NAME)
+            .addAggregation(AggregationBuilders.geoBounds(AGGREGATION_NAME).field(NON_EXISTENT_FIELD).missing(WKT.toWKT(missingGeometry)))
+            .get();
+        GeoBoundsHelper.updateBoundsForGeometry(missingGeometry, topLeft, bottomRight);
+        assertSearchResponse(response);
+        validateResult(response.getAggregations().get(AGGREGATION_NAME));
+    }
+
+    private void validateResult(final GeoBounds bounds) {
+        MatcherAssert.assertThat(bounds.bottomRight().lat(), closeTo(bottomRight.lat(), GEOHASH_TOLERANCE));
+        MatcherAssert.assertThat(bounds.bottomRight().lon(), closeTo(bottomRight.lon(), GEOHASH_TOLERANCE));
+        MatcherAssert.assertThat(bounds.topLeft().lat(), closeTo(topLeft.lat(), GEOHASH_TOLERANCE));
+        MatcherAssert.assertThat(bounds.topLeft().lon(), closeTo(topLeft.lon(), GEOHASH_TOLERANCE));
     }
 }