From 2dafb4de0a00230aa7c96b896ab93b760826d1b7 Mon Sep 17 00:00:00 2001 From: Navneet Verma Date: Tue, 9 Aug 2022 12:56:14 -0700 Subject: [PATCH] Refactors the GeoBoundsAggregation for geo_point types from the core server to the geo module. Signed-off-by: Navneet Verma --- .../client/RestHighLevelClient.java | 3 - modules/geo/build.gradle | 6 +- .../geo/GeoModulePluginIntegTestCase.java | 47 +++ .../opensearch/geo/search/MissingValueIT.java | 59 ++++ ...ractGeoAggregatorTestCaseModulePlugin.java | 295 ++++++++++++++++++ .../aggregations/metrics/GeoBoundsIT.java | 23 +- .../{GeoPlugin.java => GeoModulePlugin.java} | 20 +- .../metrics/AbstractGeoBoundsAggregator.java | 128 ++++++++ .../aggregations/metrics/GeoBounds.java | 2 +- .../metrics/GeoBoundsAggregationBuilder.java | 16 +- .../metrics/GeoBoundsAggregator.java | 92 +----- .../metrics/GeoBoundsAggregatorFactory.java | 2 +- .../metrics/InternalGeoBounds.java | 2 +- .../aggregations/metrics/ParsedGeoBounds.java | 2 +- .../GeoBoundsAggregationBuilderTests.java | 43 +++ .../metrics/GeoBoundsAggregatorTests.java | 26 +- .../metrics/InternalGeoBoundsTests.java | 33 +- .../geo/tests/common/AggregationBuilders.java | 21 ++ .../common/AggregationInspectionHelper.java | 18 ++ .../geo/tests/common/RandomGeoGenerator.java | 86 +++++ .../percolator/PercolatorQuerySearchIT.java | 4 +- .../search/aggregations/MissingValueIT.java | 24 -- .../org/opensearch/search/SearchModule.java | 8 - .../aggregations/AggregationBuilders.java | 9 - .../support/AggregationInspectionHelper.java | 5 - .../aggregations/AggregationsTests.java | 2 - .../aggregations/metrics/GeoBoundsTests.java | 53 ---- .../test/InternalAggregationTestCase.java | 3 - .../test/OpenSearchIntegTestCase.java | 1 + 29 files changed, 802 insertions(+), 231 deletions(-) create mode 100644 modules/geo/src/internalClusterTest/java/org/opensearch/geo/GeoModulePluginIntegTestCase.java create mode 100644 modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/MissingValueIT.java create mode 100644 modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoAggregatorTestCaseModulePlugin.java rename {server/src/internalClusterTest/java/org/opensearch => modules/geo/src/internalClusterTest/java/org/opensearch/geo}/search/aggregations/metrics/GeoBoundsIT.java (97%) rename modules/geo/src/main/java/org/opensearch/geo/{GeoPlugin.java => GeoModulePlugin.java} (63%) create mode 100644 modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoBoundsAggregator.java rename {server/src/main/java/org/opensearch => modules/geo/src/main/java/org/opensearch/geo}/search/aggregations/metrics/GeoBounds.java (96%) rename {server/src/main/java/org/opensearch => modules/geo/src/main/java/org/opensearch/geo}/search/aggregations/metrics/GeoBoundsAggregationBuilder.java (93%) rename {server/src/main/java/org/opensearch => modules/geo/src/main/java/org/opensearch/geo}/search/aggregations/metrics/GeoBoundsAggregator.java (51%) rename {server/src/main/java/org/opensearch => modules/geo/src/main/java/org/opensearch/geo}/search/aggregations/metrics/GeoBoundsAggregatorFactory.java (98%) rename {server/src/main/java/org/opensearch => modules/geo/src/main/java/org/opensearch/geo}/search/aggregations/metrics/InternalGeoBounds.java (99%) rename {server/src/main/java/org/opensearch => modules/geo/src/main/java/org/opensearch/geo}/search/aggregations/metrics/ParsedGeoBounds.java (98%) create mode 100644 modules/geo/src/test/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregationBuilderTests.java rename {server/src/test/java/org/opensearch => modules/geo/src/test/java/org/opensearch/geo}/search/aggregations/metrics/GeoBoundsAggregatorTests.java (88%) rename {server/src/test/java/org/opensearch => modules/geo/src/test/java/org/opensearch/geo}/search/aggregations/metrics/InternalGeoBoundsTests.java (81%) create mode 100644 modules/geo/src/test/java/org/opensearch/geo/tests/common/AggregationBuilders.java create mode 100644 modules/geo/src/test/java/org/opensearch/geo/tests/common/AggregationInspectionHelper.java create mode 100644 modules/geo/src/test/java/org/opensearch/geo/tests/common/RandomGeoGenerator.java delete mode 100644 server/src/test/java/org/opensearch/search/aggregations/metrics/GeoBoundsTests.java diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java index d293b979debb5..7ae8f8826c5a4 100644 --- a/client/rest-high-level/src/main/java/org/opensearch/client/RestHighLevelClient.java +++ b/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 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)); diff --git a/modules/geo/build.gradle b/modules/geo/build.gradle index d78e83ec7c4c6..0b8e623c24ac6 100644 --- a/modules/geo/build.gradle +++ b/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 diff --git a/modules/geo/src/internalClusterTest/java/org/opensearch/geo/GeoModulePluginIntegTestCase.java b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/GeoModulePluginIntegTestCase.java new file mode 100644 index 0000000000000..7dc6f2c1b89b7 --- /dev/null +++ b/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> 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; + } +} 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 new file mode 100644 index 0000000000000..2ac73728b2dab --- /dev/null +++ b/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)); + } +} diff --git a/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoAggregatorTestCaseModulePlugin.java b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoAggregatorTestCaseModulePlugin.java new file mode 100644 index 0000000000000..0065cca7d6101 --- /dev/null +++ b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoAggregatorTestCaseModulePlugin.java @@ -0,0 +1,295 @@ +/* + * 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.metrics; + +import com.carrotsearch.hppc.ObjectIntHashMap; +import com.carrotsearch.hppc.ObjectIntMap; +import com.carrotsearch.hppc.ObjectObjectHashMap; +import com.carrotsearch.hppc.ObjectObjectMap; +import org.opensearch.action.index.IndexRequestBuilder; +import org.opensearch.action.search.SearchResponse; +import org.opensearch.common.Strings; +import org.opensearch.common.document.DocumentField; +import org.opensearch.common.geo.GeoPoint; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.ToXContent; +import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.geo.GeoModulePluginIntegTestCase; +import org.opensearch.geo.tests.common.RandomGeoGenerator; +import org.opensearch.geometry.utils.Geohash; +import org.opensearch.search.SearchHit; +import org.opensearch.search.sort.SortBuilders; +import org.opensearch.search.sort.SortOrder; + +import java.util.ArrayList; +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse; + +/** + * This is base class for all Geo Aggregations Integration Tests. This class is similar to what we have in the server + * folder of the OpenSearch repo. As part of moving the Geo based aggregation into separate module and plugin we need + * to copy the code as we cannot depend on this class. + * GitHub issue + */ +public abstract class AbstractGeoAggregatorTestCaseModulePlugin extends GeoModulePluginIntegTestCase { + + protected static final String SINGLE_VALUED_FIELD_NAME = "geo_value"; + protected static final String MULTI_VALUED_FIELD_NAME = "geo_values"; + protected static final String NUMBER_FIELD_NAME = "l_values"; + protected static final String UNMAPPED_IDX_NAME = "idx_unmapped"; + protected static final String IDX_NAME = "idx"; + protected static final String EMPTY_IDX_NAME = "empty_idx"; + protected static final String DATELINE_IDX_NAME = "dateline_idx"; + protected static final String HIGH_CARD_IDX_NAME = "high_card_idx"; + protected static final String IDX_ZERO_NAME = "idx_zero"; + + protected static int numDocs; + protected static int numUniqueGeoPoints; + protected static GeoPoint[] singleValues, multiValues; + protected static GeoPoint singleTopLeft, singleBottomRight, multiTopLeft, multiBottomRight, singleCentroid, multiCentroid, + unmappedCentroid; + protected static ObjectIntMap expectedDocCountsForGeoHash = null; + protected static ObjectObjectMap expectedCentroidsForGeoHash = null; + protected static final double GEOHASH_TOLERANCE = 1E-5D; + + @Override + public void setupSuiteScopeCluster() throws Exception { + createIndex(UNMAPPED_IDX_NAME); + assertAcked( + prepareCreate(IDX_NAME).setMapping( + SINGLE_VALUED_FIELD_NAME, + "type=geo_point", + MULTI_VALUED_FIELD_NAME, + "type=geo_point", + NUMBER_FIELD_NAME, + "type=long", + "tag", + "type=keyword" + ) + ); + + singleTopLeft = new GeoPoint(Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY); + singleBottomRight = new GeoPoint(Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY); + multiTopLeft = new GeoPoint(Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY); + multiBottomRight = new GeoPoint(Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY); + singleCentroid = new GeoPoint(0, 0); + multiCentroid = new GeoPoint(0, 0); + unmappedCentroid = new GeoPoint(0, 0); + + numDocs = randomIntBetween(6, 20); + numUniqueGeoPoints = randomIntBetween(1, numDocs); + expectedDocCountsForGeoHash = new ObjectIntHashMap<>(numDocs * 2); + expectedCentroidsForGeoHash = new ObjectObjectHashMap<>(numDocs * 2); + + singleValues = new GeoPoint[numUniqueGeoPoints]; + for (int i = 0; i < singleValues.length; i++) { + singleValues[i] = RandomGeoGenerator.randomPoint(random()); + updateBoundsTopLeft(singleValues[i], singleTopLeft); + updateBoundsBottomRight(singleValues[i], singleBottomRight); + } + + multiValues = new GeoPoint[numUniqueGeoPoints]; + for (int i = 0; i < multiValues.length; i++) { + multiValues[i] = RandomGeoGenerator.randomPoint(random()); + updateBoundsTopLeft(multiValues[i], multiTopLeft); + updateBoundsBottomRight(multiValues[i], multiBottomRight); + } + + List builders = new ArrayList<>(); + + GeoPoint singleVal; + final GeoPoint[] multiVal = new GeoPoint[2]; + double newMVLat, newMVLon; + for (int i = 0; i < numDocs; i++) { + singleVal = singleValues[i % numUniqueGeoPoints]; + multiVal[0] = multiValues[i % numUniqueGeoPoints]; + multiVal[1] = multiValues[(i + 1) % numUniqueGeoPoints]; + builders.add( + client().prepareIndex(IDX_NAME) + .setSource( + jsonBuilder().startObject() + .array(SINGLE_VALUED_FIELD_NAME, singleVal.lon(), singleVal.lat()) + .startArray(MULTI_VALUED_FIELD_NAME) + .startArray() + .value(multiVal[0].lon()) + .value(multiVal[0].lat()) + .endArray() + .startArray() + .value(multiVal[1].lon()) + .value(multiVal[1].lat()) + .endArray() + .endArray() + .field(NUMBER_FIELD_NAME, i) + .field("tag", "tag" + i) + .endObject() + ) + ); + singleCentroid = singleCentroid.reset( + singleCentroid.lat() + (singleVal.lat() - singleCentroid.lat()) / (i + 1), + singleCentroid.lon() + (singleVal.lon() - singleCentroid.lon()) / (i + 1) + ); + newMVLat = (multiVal[0].lat() + multiVal[1].lat()) / 2d; + newMVLon = (multiVal[0].lon() + multiVal[1].lon()) / 2d; + multiCentroid = multiCentroid.reset( + multiCentroid.lat() + (newMVLat - multiCentroid.lat()) / (i + 1), + multiCentroid.lon() + (newMVLon - multiCentroid.lon()) / (i + 1) + ); + } + + assertAcked(prepareCreate(EMPTY_IDX_NAME).setMapping(SINGLE_VALUED_FIELD_NAME, "type=geo_point")); + + assertAcked( + prepareCreate(DATELINE_IDX_NAME).setMapping( + SINGLE_VALUED_FIELD_NAME, + "type=geo_point", + MULTI_VALUED_FIELD_NAME, + "type=geo_point", + NUMBER_FIELD_NAME, + "type=long", + "tag", + "type=keyword" + ) + ); + + GeoPoint[] geoValues = new GeoPoint[5]; + geoValues[0] = new GeoPoint(38, 178); + geoValues[1] = new GeoPoint(12, -179); + geoValues[2] = new GeoPoint(-24, 170); + geoValues[3] = new GeoPoint(32, -175); + geoValues[4] = new GeoPoint(-11, 178); + + for (int i = 0; i < 5; i++) { + builders.add( + client().prepareIndex(DATELINE_IDX_NAME) + .setSource( + jsonBuilder().startObject() + .array(SINGLE_VALUED_FIELD_NAME, geoValues[i].lon(), geoValues[i].lat()) + .field(NUMBER_FIELD_NAME, i) + .field("tag", "tag" + i) + .endObject() + ) + ); + } + assertAcked( + prepareCreate(HIGH_CARD_IDX_NAME).setSettings(Settings.builder().put("number_of_shards", 2)) + .setMapping( + SINGLE_VALUED_FIELD_NAME, + "type=geo_point", + MULTI_VALUED_FIELD_NAME, + "type=geo_point", + NUMBER_FIELD_NAME, + "type=long,store=true", + "tag", + "type=keyword" + ) + ); + + for (int i = 0; i < 2000; i++) { + singleVal = singleValues[i % numUniqueGeoPoints]; + builders.add( + client().prepareIndex(HIGH_CARD_IDX_NAME) + .setSource( + jsonBuilder().startObject() + .array(SINGLE_VALUED_FIELD_NAME, singleVal.lon(), singleVal.lat()) + .startArray(MULTI_VALUED_FIELD_NAME) + .startArray() + .value(multiValues[i % numUniqueGeoPoints].lon()) + .value(multiValues[i % numUniqueGeoPoints].lat()) + .endArray() + .startArray() + .value(multiValues[(i + 1) % numUniqueGeoPoints].lon()) + .value(multiValues[(i + 1) % numUniqueGeoPoints].lat()) + .endArray() + .endArray() + .field(NUMBER_FIELD_NAME, i) + .field("tag", "tag" + i) + .endObject() + ) + ); + updateGeohashBucketsCentroid(singleVal); + } + + builders.add( + client().prepareIndex(IDX_ZERO_NAME) + .setSource(jsonBuilder().startObject().array(SINGLE_VALUED_FIELD_NAME, 0.0, 1.0).endObject()) + ); + assertAcked(prepareCreate(IDX_ZERO_NAME).setMapping(SINGLE_VALUED_FIELD_NAME, "type=geo_point")); + + indexRandom(true, builders); + ensureSearchable(); + + // Added to debug a test failure where the terms aggregation seems to be reporting two documents with the same + // value for NUMBER_FIELD_NAME. This will check that after random indexing each document only has 1 value for + // NUMBER_FIELD_NAME and it is the correct value. Following this initial change its seems that this call was getting + // more that 2000 hits (actual value was 2059) so now it will also check to ensure all hits have the correct index and type. + SearchResponse response = client().prepareSearch(HIGH_CARD_IDX_NAME) + .addStoredField(NUMBER_FIELD_NAME) + .addSort(SortBuilders.fieldSort(NUMBER_FIELD_NAME).order(SortOrder.ASC)) + .setSize(5000) + .get(); + assertSearchResponse(response); + long totalHits = response.getHits().getTotalHits().value; + XContentBuilder builder = XContentFactory.jsonBuilder(); + response.toXContent(builder, ToXContent.EMPTY_PARAMS); + logger.info("Full high_card_idx Response Content:\n{ {} }", Strings.toString(builder)); + for (int i = 0; i < totalHits; i++) { + SearchHit searchHit = response.getHits().getAt(i); + assertThat("Hit " + i + " with id: " + searchHit.getId(), searchHit.getIndex(), equalTo("high_card_idx")); + DocumentField hitField = searchHit.field(NUMBER_FIELD_NAME); + + assertThat("Hit " + i + " has wrong number of values", hitField.getValues().size(), equalTo(1)); + Long value = hitField.getValue(); + assertThat("Hit " + i + " has wrong value", value.intValue(), equalTo(i)); + } + assertThat(totalHits, equalTo(2000L)); + } + + private void updateGeohashBucketsCentroid(final GeoPoint location) { + String hash = Geohash.stringEncode(location.lon(), location.lat(), Geohash.PRECISION); + for (int precision = Geohash.PRECISION; precision > 0; --precision) { + final String h = hash.substring(0, precision); + expectedDocCountsForGeoHash.put(h, expectedDocCountsForGeoHash.getOrDefault(h, 0) + 1); + expectedCentroidsForGeoHash.put(h, updateHashCentroid(h, location)); + } + } + + private GeoPoint updateHashCentroid(String hash, final GeoPoint location) { + GeoPoint centroid = expectedCentroidsForGeoHash.getOrDefault(hash, null); + if (centroid == null) { + return new GeoPoint(location.lat(), location.lon()); + } + final int docCount = expectedDocCountsForGeoHash.get(hash); + final double newLon = centroid.lon() + (location.lon() - centroid.lon()) / docCount; + final double newLat = centroid.lat() + (location.lat() - centroid.lat()) / docCount; + return centroid.reset(newLat, newLon); + } + + private void updateBoundsBottomRight(GeoPoint geoPoint, GeoPoint currentBound) { + if (geoPoint.lat() < currentBound.lat()) { + currentBound.resetLat(geoPoint.lat()); + } + if (geoPoint.lon() > currentBound.lon()) { + currentBound.resetLon(geoPoint.lon()); + } + } + + private void updateBoundsTopLeft(GeoPoint geoPoint, GeoPoint currentBound) { + if (geoPoint.lat() > currentBound.lat()) { + currentBound.resetLat(geoPoint.lat()); + } + if (geoPoint.lon() < currentBound.lon()) { + currentBound.resetLon(geoPoint.lon()); + } + } +} diff --git a/server/src/internalClusterTest/java/org/opensearch/search/aggregations/metrics/GeoBoundsIT.java b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsIT.java similarity index 97% rename from server/src/internalClusterTest/java/org/opensearch/search/aggregations/metrics/GeoBoundsIT.java rename to modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsIT.java index 3af3b9e5212f8..5cbd98a4936e4 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/aggregations/metrics/GeoBoundsIT.java +++ b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsIT.java @@ -30,7 +30,7 @@ * GitHub history for details. */ -package org.opensearch.search.aggregations.metrics; +package org.opensearch.geo.search.aggregations.metrics; import org.opensearch.action.search.SearchResponse; import org.opensearch.common.geo.GeoPoint; @@ -43,21 +43,21 @@ import java.util.List; -import static org.opensearch.index.query.QueryBuilders.matchAllQuery; -import static org.opensearch.search.aggregations.AggregationBuilders.geoBounds; -import static org.opensearch.search.aggregations.AggregationBuilders.global; -import static org.opensearch.search.aggregations.AggregationBuilders.terms; -import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse; +import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.closeTo; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.lessThanOrEqualTo; -import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.sameInstance; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.closeTo; +import static org.opensearch.index.query.QueryBuilders.matchAllQuery; +import static org.opensearch.search.aggregations.AggregationBuilders.global; +import static org.opensearch.search.aggregations.AggregationBuilders.terms; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse; +import static org.opensearch.geo.tests.common.AggregationBuilders.geoBounds; @OpenSearchIntegTestCase.SuiteScopeTestCase -public class GeoBoundsIT extends AbstractGeoTestCase { +public class GeoBoundsIT extends AbstractGeoAggregatorTestCaseModulePlugin { private static final String aggName = "geoBounds"; public void testSingleValuedField() throws Exception { @@ -226,7 +226,8 @@ public void testSingleValuedFieldNearDateLineWrapLongitude() throws Exception { } /** - * This test forces the {@link GeoBoundsAggregator} to resize the {@link BigArray}s it uses to ensure they are resized correctly + * This test forces the {@link GeoBoundsAggregator} to resize the {@link BigArray}s it uses to ensure they are + * resized correctly */ public void testSingleValuedFieldAsSubAggToHighCardTermsAgg() { SearchResponse response = client().prepareSearch(HIGH_CARD_IDX_NAME) diff --git a/modules/geo/src/main/java/org/opensearch/geo/GeoPlugin.java b/modules/geo/src/main/java/org/opensearch/geo/GeoModulePlugin.java similarity index 63% rename from modules/geo/src/main/java/org/opensearch/geo/GeoPlugin.java rename to modules/geo/src/main/java/org/opensearch/geo/GeoModulePlugin.java index 9b898da33bb12..64aac66b7eef3 100644 --- a/modules/geo/src/main/java/org/opensearch/geo/GeoPlugin.java +++ b/modules/geo/src/main/java/org/opensearch/geo/GeoModulePlugin.java @@ -32,18 +32,36 @@ package org.opensearch.geo; +import org.opensearch.geo.search.aggregations.metrics.GeoBounds; +import org.opensearch.geo.search.aggregations.metrics.GeoBoundsAggregationBuilder; +import org.opensearch.geo.search.aggregations.metrics.InternalGeoBounds; import org.opensearch.index.mapper.GeoShapeFieldMapper; import org.opensearch.index.mapper.Mapper; import org.opensearch.plugins.MapperPlugin; import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.SearchPlugin; import java.util.Collections; +import java.util.List; import java.util.Map; -public class GeoPlugin extends Plugin implements MapperPlugin { +public class GeoModulePlugin extends Plugin implements MapperPlugin, SearchPlugin { @Override public Map getMappers() { return Collections.singletonMap(GeoShapeFieldMapper.CONTENT_TYPE, new GeoShapeFieldMapper.TypeParser()); } + + /** + * Registering {@link GeoBounds} aggregation on GeoPoint field. + */ + @Override + public List getAggregations() { + final AggregationSpec spec = new AggregationSpec( + GeoBoundsAggregationBuilder.NAME, + GeoBoundsAggregationBuilder::new, + GeoBoundsAggregationBuilder.PARSER + ).addResultReader(InternalGeoBounds::new).setAggregatorRegistrar(GeoBoundsAggregationBuilder::registerAggregators); + return Collections.singletonList(spec); + } } diff --git a/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoBoundsAggregator.java b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoBoundsAggregator.java new file mode 100644 index 0000000000000..4a39fa1da04eb --- /dev/null +++ b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/AbstractGeoBoundsAggregator.java @@ -0,0 +1,128 @@ +/* + * 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.metrics; + +import org.opensearch.common.lease.Releasables; +import org.opensearch.common.util.BigArrays; +import org.opensearch.common.util.DoubleArray; +import org.opensearch.search.aggregations.Aggregator; +import org.opensearch.search.aggregations.InternalAggregation; +import org.opensearch.search.aggregations.metrics.MetricsAggregator; +import org.opensearch.search.aggregations.support.ValuesSource; +import org.opensearch.search.aggregations.support.ValuesSourceConfig; +import org.opensearch.search.internal.SearchContext; + +import java.io.IOException; +import java.util.Map; + +/** + * Abstract class for doing the {@link GeoBounds} Aggregation over fields of type geo_shape and geo_point. + * + * @param Class extending the {@link ValuesSource} which will provide the data on which aggregation will happen. + * @opensearch.internal + */ +public abstract class AbstractGeoBoundsAggregator extends MetricsAggregator { + + protected final T valuesSource; + protected final boolean wrapLongitude; + protected DoubleArray tops; + protected DoubleArray bottoms; + protected DoubleArray posLefts; + protected DoubleArray posRights; + protected DoubleArray negLefts; + protected DoubleArray negRights; + + @SuppressWarnings("unchecked") + protected AbstractGeoBoundsAggregator( + String name, + SearchContext searchContext, + Aggregator aggregator, + ValuesSourceConfig valuesSourceConfig, + boolean wrapLongitude, + Map metaData + ) throws IOException { + super(name, searchContext, aggregator, metaData); + this.wrapLongitude = wrapLongitude; + valuesSource = valuesSourceConfig.hasValues() ? (T) valuesSourceConfig.getValuesSource() : null; + + if (valuesSource != null) { + final BigArrays bigArrays = context.bigArrays(); + tops = bigArrays.newDoubleArray(1, false); + tops.fill(0, tops.size(), Double.NEGATIVE_INFINITY); + bottoms = bigArrays.newDoubleArray(1, false); + bottoms.fill(0, bottoms.size(), Double.POSITIVE_INFINITY); + posLefts = bigArrays.newDoubleArray(1, false); + posLefts.fill(0, posLefts.size(), Double.POSITIVE_INFINITY); + posRights = bigArrays.newDoubleArray(1, false); + posRights.fill(0, posRights.size(), Double.NEGATIVE_INFINITY); + negLefts = bigArrays.newDoubleArray(1, false); + negLefts.fill(0, negLefts.size(), Double.POSITIVE_INFINITY); + negRights = bigArrays.newDoubleArray(1, false); + negRights.fill(0, negRights.size(), Double.NEGATIVE_INFINITY); + } + } + + /** + * Build an empty aggregation. + */ + @Override + public InternalAggregation buildEmptyAggregation() { + return new InternalGeoBounds( + name, + Double.NEGATIVE_INFINITY, + Double.POSITIVE_INFINITY, + Double.POSITIVE_INFINITY, + Double.NEGATIVE_INFINITY, + Double.POSITIVE_INFINITY, + Double.NEGATIVE_INFINITY, + wrapLongitude, + metadata() + ); + } + + /** + * Build an aggregation for data that has been collected into owningBucketOrd. + */ + @Override + public InternalAggregation buildAggregation(long owningBucketOrdinal) throws IOException { + if (valuesSource == null) { + return buildEmptyAggregation(); + } + double top = tops.get(owningBucketOrdinal); + double bottom = bottoms.get(owningBucketOrdinal); + double posLeft = posLefts.get(owningBucketOrdinal); + double posRight = posRights.get(owningBucketOrdinal); + double negLeft = negLefts.get(owningBucketOrdinal); + double negRight = negRights.get(owningBucketOrdinal); + return new InternalGeoBounds(name, top, bottom, posLeft, posRight, negLeft, negRight, wrapLongitude, metadata()); + } + + @Override + public void doClose() { + Releasables.close(tops, bottoms, posLefts, posRights, negLefts, negRights); + } + + protected void setBucketSize(final long bucket, final BigArrays bigArrays) { + if (bucket >= tops.size()) { + long from = tops.size(); + tops = bigArrays.grow(tops, bucket + 1); + tops.fill(from, tops.size(), Double.NEGATIVE_INFINITY); + bottoms = bigArrays.resize(bottoms, tops.size()); + bottoms.fill(from, bottoms.size(), Double.POSITIVE_INFINITY); + posLefts = bigArrays.resize(posLefts, tops.size()); + posLefts.fill(from, posLefts.size(), Double.POSITIVE_INFINITY); + posRights = bigArrays.resize(posRights, tops.size()); + posRights.fill(from, posRights.size(), Double.NEGATIVE_INFINITY); + negLefts = bigArrays.resize(negLefts, tops.size()); + negLefts.fill(from, negLefts.size(), Double.POSITIVE_INFINITY); + negRights = bigArrays.resize(negRights, tops.size()); + negRights.fill(from, negRights.size(), Double.NEGATIVE_INFINITY); + } + } +} diff --git a/server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBounds.java b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBounds.java similarity index 96% rename from server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBounds.java rename to modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBounds.java index 380fbce85ada7..81ef502dda130 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBounds.java +++ b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBounds.java @@ -30,7 +30,7 @@ * GitHub history for details. */ -package org.opensearch.search.aggregations.metrics; +package org.opensearch.geo.search.aggregations.metrics; import org.opensearch.common.geo.GeoPoint; import org.opensearch.search.aggregations.Aggregation; diff --git a/server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregationBuilder.java b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregationBuilder.java similarity index 93% rename from server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregationBuilder.java rename to modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregationBuilder.java index 64e27fa7e13d1..b2c441f9a951c 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregationBuilder.java +++ b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregationBuilder.java @@ -30,8 +30,9 @@ * GitHub history for details. */ -package org.opensearch.search.aggregations.metrics; +package org.opensearch.geo.search.aggregations.metrics; +import org.opensearch.common.ParseField; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.xcontent.ObjectParser; @@ -40,6 +41,7 @@ import org.opensearch.search.aggregations.AggregationBuilder; import org.opensearch.search.aggregations.AggregatorFactories; import org.opensearch.search.aggregations.AggregatorFactory; +import org.opensearch.search.aggregations.metrics.GeoBoundsAggregatorSupplier; import org.opensearch.search.aggregations.support.CoreValuesSourceType; import org.opensearch.search.aggregations.support.ValuesSourceAggregationBuilder; import org.opensearch.search.aggregations.support.ValuesSourceConfig; @@ -57,6 +59,7 @@ */ public class GeoBoundsAggregationBuilder extends ValuesSourceAggregationBuilder { public static final String NAME = "geo_bounds"; + private static final ParseField WRAP_LONGITUDE_FIELD = new ParseField("wrap_longitude"); public static final ValuesSourceRegistry.RegistryKey REGISTRY_KEY = new ValuesSourceRegistry.RegistryKey<>( NAME, GeoBoundsAggregatorSupplier.class @@ -68,7 +71,7 @@ public class GeoBoundsAggregationBuilder extends ValuesSourceAggregationBuilder< ); static { ValuesSourceAggregationBuilder.declareFields(PARSER, false, false, false); - PARSER.declareBoolean(GeoBoundsAggregationBuilder::wrapLongitude, GeoBoundsAggregator.WRAP_LONGITUDE_FIELD); + PARSER.declareBoolean(GeoBoundsAggregationBuilder::wrapLongitude, WRAP_LONGITUDE_FIELD); } public static void registerAggregators(ValuesSourceRegistry.Builder builder) { @@ -121,13 +124,6 @@ public GeoBoundsAggregationBuilder wrapLongitude(boolean wrapLongitude) { return this; } - /** - * Get whether to wrap longitudes. - */ - public boolean wrapLongitude() { - return wrapLongitude; - } - @Override public BucketCardinality bucketCardinality() { return BucketCardinality.NONE; @@ -145,7 +141,7 @@ protected GeoBoundsAggregatorFactory innerBuild( @Override public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { - builder.field(GeoBoundsAggregator.WRAP_LONGITUDE_FIELD.getPreferredName(), wrapLongitude); + builder.field(WRAP_LONGITUDE_FIELD.getPreferredName(), wrapLongitude); return builder; } diff --git a/server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregator.java b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregator.java similarity index 51% rename from server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregator.java rename to modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregator.java index 054e8d4cb1c6c..a6518ea702be6 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregator.java +++ b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregator.java @@ -30,17 +30,13 @@ * GitHub history for details. */ -package org.opensearch.search.aggregations.metrics; +package org.opensearch.geo.search.aggregations.metrics; import org.apache.lucene.index.LeafReaderContext; -import org.opensearch.common.ParseField; import org.opensearch.common.geo.GeoPoint; -import org.opensearch.common.lease.Releasables; import org.opensearch.common.util.BigArrays; -import org.opensearch.common.util.DoubleArray; import org.opensearch.index.fielddata.MultiGeoPointValues; import org.opensearch.search.aggregations.Aggregator; -import org.opensearch.search.aggregations.InternalAggregation; import org.opensearch.search.aggregations.LeafBucketCollector; import org.opensearch.search.aggregations.LeafBucketCollectorBase; import org.opensearch.search.aggregations.support.ValuesSource; @@ -51,22 +47,11 @@ import java.util.Map; /** - * Aggregate all docs into a geographic bounds + * Aggregate all docs into a geographic bounds for field GeoPoint. * * @opensearch.internal */ -final class GeoBoundsAggregator extends MetricsAggregator { - - static final ParseField WRAP_LONGITUDE_FIELD = new ParseField("wrap_longitude"); - - private final ValuesSource.GeoPoint valuesSource; - private final boolean wrapLongitude; - DoubleArray tops; - DoubleArray bottoms; - DoubleArray posLefts; - DoubleArray posRights; - DoubleArray negLefts; - DoubleArray negRights; +final class GeoBoundsAggregator extends AbstractGeoBoundsAggregator { GeoBoundsAggregator( String name, @@ -76,25 +61,7 @@ final class GeoBoundsAggregator extends MetricsAggregator { boolean wrapLongitude, Map metadata ) throws IOException { - super(name, aggregationContext, parent, metadata); - // TODO: stop expecting nulls here - this.valuesSource = valuesSourceConfig.hasValues() ? (ValuesSource.GeoPoint) valuesSourceConfig.getValuesSource() : null; - this.wrapLongitude = wrapLongitude; - if (valuesSource != null) { - final BigArrays bigArrays = context.bigArrays(); - tops = bigArrays.newDoubleArray(1, false); - tops.fill(0, tops.size(), Double.NEGATIVE_INFINITY); - bottoms = bigArrays.newDoubleArray(1, false); - bottoms.fill(0, bottoms.size(), Double.POSITIVE_INFINITY); - posLefts = bigArrays.newDoubleArray(1, false); - posLefts.fill(0, posLefts.size(), Double.POSITIVE_INFINITY); - posRights = bigArrays.newDoubleArray(1, false); - posRights.fill(0, posRights.size(), Double.NEGATIVE_INFINITY); - negLefts = bigArrays.newDoubleArray(1, false); - negLefts.fill(0, negLefts.size(), Double.POSITIVE_INFINITY); - negRights = bigArrays.newDoubleArray(1, false); - negRights.fill(0, negRights.size(), Double.NEGATIVE_INFINITY); - } + super(name, aggregationContext, parent, valuesSourceConfig, wrapLongitude, metadata); } @Override @@ -107,25 +74,10 @@ public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, LeafBucketCol return new LeafBucketCollectorBase(sub, values) { @Override public void collect(int doc, long bucket) throws IOException { - if (bucket >= tops.size()) { - long from = tops.size(); - tops = bigArrays.grow(tops, bucket + 1); - tops.fill(from, tops.size(), Double.NEGATIVE_INFINITY); - bottoms = bigArrays.resize(bottoms, tops.size()); - bottoms.fill(from, bottoms.size(), Double.POSITIVE_INFINITY); - posLefts = bigArrays.resize(posLefts, tops.size()); - posLefts.fill(from, posLefts.size(), Double.POSITIVE_INFINITY); - posRights = bigArrays.resize(posRights, tops.size()); - posRights.fill(from, posRights.size(), Double.NEGATIVE_INFINITY); - negLefts = bigArrays.resize(negLefts, tops.size()); - negLefts.fill(from, negLefts.size(), Double.POSITIVE_INFINITY); - negRights = bigArrays.resize(negRights, tops.size()); - negRights.fill(from, negRights.size(), Double.NEGATIVE_INFINITY); - } + setBucketSize(bucket, bigArrays); if (values.advanceExact(doc)) { final int valuesCount = values.docValueCount(); - for (int i = 0; i < valuesCount; ++i) { GeoPoint value = values.nextValue(); double top = tops.get(bucket); @@ -163,38 +115,4 @@ public void collect(int doc, long bucket) throws IOException { } }; } - - @Override - public InternalAggregation buildAggregation(long owningBucketOrdinal) { - if (valuesSource == null) { - return buildEmptyAggregation(); - } - double top = tops.get(owningBucketOrdinal); - double bottom = bottoms.get(owningBucketOrdinal); - double posLeft = posLefts.get(owningBucketOrdinal); - double posRight = posRights.get(owningBucketOrdinal); - double negLeft = negLefts.get(owningBucketOrdinal); - double negRight = negRights.get(owningBucketOrdinal); - return new InternalGeoBounds(name, top, bottom, posLeft, posRight, negLeft, negRight, wrapLongitude, metadata()); - } - - @Override - public InternalAggregation buildEmptyAggregation() { - return new InternalGeoBounds( - name, - Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY, - Double.POSITIVE_INFINITY, - Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY, - Double.NEGATIVE_INFINITY, - wrapLongitude, - metadata() - ); - } - - @Override - public void doClose() { - Releasables.close(tops, bottoms, posLefts, posRights, negLefts, negRights); - } } diff --git a/server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.java b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregatorFactory.java similarity index 98% rename from server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.java rename to modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregatorFactory.java index 2c6b75842b6f5..149e052b4db7d 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregatorFactory.java +++ b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregatorFactory.java @@ -30,7 +30,7 @@ * GitHub history for details. */ -package org.opensearch.search.aggregations.metrics; +package org.opensearch.geo.search.aggregations.metrics; import org.opensearch.index.query.QueryShardContext; import org.opensearch.search.aggregations.Aggregator; diff --git a/server/src/main/java/org/opensearch/search/aggregations/metrics/InternalGeoBounds.java b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/InternalGeoBounds.java similarity index 99% rename from server/src/main/java/org/opensearch/search/aggregations/metrics/InternalGeoBounds.java rename to modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/InternalGeoBounds.java index 87018242ee8df..7c708de88a49c 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/metrics/InternalGeoBounds.java +++ b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/InternalGeoBounds.java @@ -30,7 +30,7 @@ * GitHub history for details. */ -package org.opensearch.search.aggregations.metrics; +package org.opensearch.geo.search.aggregations.metrics; import org.opensearch.common.geo.GeoBoundingBox; import org.opensearch.common.geo.GeoPoint; diff --git a/server/src/main/java/org/opensearch/search/aggregations/metrics/ParsedGeoBounds.java b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/ParsedGeoBounds.java similarity index 98% rename from server/src/main/java/org/opensearch/search/aggregations/metrics/ParsedGeoBounds.java rename to modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/ParsedGeoBounds.java index a482fcfdf08dd..7643ac9d9a010 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/metrics/ParsedGeoBounds.java +++ b/modules/geo/src/main/java/org/opensearch/geo/search/aggregations/metrics/ParsedGeoBounds.java @@ -30,7 +30,7 @@ * GitHub history for details. */ -package org.opensearch.search.aggregations.metrics; +package org.opensearch.geo.search.aggregations.metrics; import org.opensearch.common.Nullable; import org.opensearch.common.collect.Tuple; diff --git a/modules/geo/src/test/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregationBuilderTests.java b/modules/geo/src/test/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregationBuilderTests.java new file mode 100644 index 0000000000000..49b455bbf389e --- /dev/null +++ b/modules/geo/src/test/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregationBuilderTests.java @@ -0,0 +1,43 @@ +/* + * 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.metrics; + +import org.opensearch.geo.GeoModulePlugin; +import org.opensearch.plugins.Plugin; +import org.opensearch.search.aggregations.BaseAggregationTestCase; + +import java.util.Collection; +import java.util.Collections; + +public class GeoBoundsAggregationBuilderTests extends BaseAggregationTestCase { + + /** + * This registers the GeoShape mapper with the Tests so that it can be used for testing the aggregation builders + * + * @return A Collection containing {@link GeoModulePlugin} + */ + protected Collection> getPlugins() { + return Collections.singletonList(GeoModulePlugin.class); + } + + @Override + protected GeoBoundsAggregationBuilder createTestAggregatorBuilder() { + GeoBoundsAggregationBuilder factory = new GeoBoundsAggregationBuilder(randomAlphaOfLengthBetween(1, 20)); + String field = randomAlphaOfLengthBetween(3, 20); + factory.field(field); + if (randomBoolean()) { + factory.wrapLongitude(randomBoolean()); + } + if (randomBoolean()) { + factory.missing("0,0"); + } + return factory; + } + +} diff --git a/server/src/test/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregatorTests.java b/modules/geo/src/test/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregatorTests.java similarity index 88% rename from server/src/test/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregatorTests.java rename to modules/geo/src/test/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregatorTests.java index 6440c62e58e18..ee7a3c7e3faa2 100644 --- a/server/src/test/java/org/opensearch/search/aggregations/metrics/GeoBoundsAggregatorTests.java +++ b/modules/geo/src/test/java/org/opensearch/geo/search/aggregations/metrics/GeoBoundsAggregatorTests.java @@ -30,26 +30,42 @@ * GitHub history for details. */ -package org.opensearch.search.aggregations.metrics; +package org.opensearch.geo.search.aggregations.metrics; import org.apache.lucene.document.Document; import org.apache.lucene.document.LatLonDocValuesField; import org.apache.lucene.index.IndexReader; -import org.apache.lucene.tests.index.RandomIndexWriter; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.index.RandomIndexWriter; import org.opensearch.common.geo.GeoPoint; +import org.opensearch.geo.GeoModulePlugin; +import org.opensearch.geo.tests.common.AggregationInspectionHelper; +import org.opensearch.geo.tests.common.RandomGeoGenerator; import org.opensearch.index.mapper.GeoPointFieldMapper; import org.opensearch.index.mapper.MappedFieldType; +import org.opensearch.plugins.SearchPlugin; import org.opensearch.search.aggregations.AggregatorTestCase; -import org.opensearch.search.aggregations.support.AggregationInspectionHelper; -import org.opensearch.test.geo.RandomGeoGenerator; -import static org.opensearch.search.aggregations.metrics.InternalGeoBoundsTests.GEOHASH_TOLERANCE; +import java.util.Collections; +import java.util.List; + import static org.hamcrest.Matchers.closeTo; public class GeoBoundsAggregatorTests extends AggregatorTestCase { + public static final double GEOHASH_TOLERANCE = 1E-5D; + + /** + * Overriding the Search Plugins list with {@link GeoModulePlugin} so that the testcase will know that this plugin is + * to be loaded during the tests. + * @return List of {@link SearchPlugin} + */ + @Override + protected List getSearchPlugins() { + return Collections.singletonList(new GeoModulePlugin()); + } + public void testEmpty() throws Exception { try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { GeoBoundsAggregationBuilder aggBuilder = new GeoBoundsAggregationBuilder("my_agg").field("field").wrapLongitude(false); diff --git a/server/src/test/java/org/opensearch/search/aggregations/metrics/InternalGeoBoundsTests.java b/modules/geo/src/test/java/org/opensearch/geo/search/aggregations/metrics/InternalGeoBoundsTests.java similarity index 81% rename from server/src/test/java/org/opensearch/search/aggregations/metrics/InternalGeoBoundsTests.java rename to modules/geo/src/test/java/org/opensearch/geo/search/aggregations/metrics/InternalGeoBoundsTests.java index e3857efff5d4d..22915212ff415 100644 --- a/server/src/test/java/org/opensearch/search/aggregations/metrics/InternalGeoBoundsTests.java +++ b/modules/geo/src/test/java/org/opensearch/geo/search/aggregations/metrics/InternalGeoBoundsTests.java @@ -30,11 +30,18 @@ * GitHub history for details. */ -package org.opensearch.search.aggregations.metrics; +package org.opensearch.geo.search.aggregations.metrics; +import org.opensearch.common.ParseField; +import org.opensearch.common.xcontent.ContextParser; +import org.opensearch.common.xcontent.NamedXContentRegistry; +import org.opensearch.geo.GeoModulePlugin; +import org.opensearch.plugins.SearchPlugin; +import org.opensearch.search.aggregations.Aggregation; import org.opensearch.search.aggregations.ParsedAggregation; import org.opensearch.test.InternalAggregationTestCase; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -44,6 +51,30 @@ public class InternalGeoBoundsTests extends InternalAggregationTestCase { static final double GEOHASH_TOLERANCE = 1E-5D; + /** + * Overriding the method so that tests can get the aggregation specs for namedWriteable. + * + * @return GeoPlugin + */ + @Override + protected SearchPlugin registerPlugin() { + return new GeoModulePlugin(); + } + + /** + * Overriding with the {@link ParsedGeoBounds} so that it can be parsed. We need to do this as {@link GeoModulePlugin} + * is registering this Aggregation. + * + * @return a List of {@link NamedXContentRegistry.Entry} + */ + @Override + protected List getNamedXContents() { + final List namedXContents = new ArrayList<>(getDefaultNamedXContents()); + final ContextParser parser = (p, c) -> ParsedGeoBounds.fromXContent(p, (String) c); + namedXContents.add(new NamedXContentRegistry.Entry(Aggregation.class, new ParseField(GeoBoundsAggregationBuilder.NAME), parser)); + return namedXContents; + } + @Override protected InternalGeoBounds createTestInstance(String name, Map metadata) { // we occasionally want to test top = Double.NEGATIVE_INFINITY since this triggers empty xContent object diff --git a/modules/geo/src/test/java/org/opensearch/geo/tests/common/AggregationBuilders.java b/modules/geo/src/test/java/org/opensearch/geo/tests/common/AggregationBuilders.java new file mode 100644 index 0000000000000..c1f27b71c326d --- /dev/null +++ b/modules/geo/src/test/java/org/opensearch/geo/tests/common/AggregationBuilders.java @@ -0,0 +1,21 @@ +/* + * 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.tests.common; + +import org.opensearch.geo.search.aggregations.metrics.GeoBounds; +import org.opensearch.geo.search.aggregations.metrics.GeoBoundsAggregationBuilder; + +public class AggregationBuilders { + /** + * Create a new {@link GeoBounds} aggregation with the given name. + */ + public static GeoBoundsAggregationBuilder geoBounds(String name) { + return new GeoBoundsAggregationBuilder(name); + } +} diff --git a/modules/geo/src/test/java/org/opensearch/geo/tests/common/AggregationInspectionHelper.java b/modules/geo/src/test/java/org/opensearch/geo/tests/common/AggregationInspectionHelper.java new file mode 100644 index 0000000000000..208187bf34a5c --- /dev/null +++ b/modules/geo/src/test/java/org/opensearch/geo/tests/common/AggregationInspectionHelper.java @@ -0,0 +1,18 @@ +/* + * 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.tests.common; + +import org.opensearch.geo.search.aggregations.metrics.InternalGeoBounds; + +public class AggregationInspectionHelper { + + public static boolean hasValue(InternalGeoBounds agg) { + return (agg.topLeft() == null && agg.bottomRight() == null) == false; + } +} diff --git a/modules/geo/src/test/java/org/opensearch/geo/tests/common/RandomGeoGenerator.java b/modules/geo/src/test/java/org/opensearch/geo/tests/common/RandomGeoGenerator.java new file mode 100644 index 0000000000000..2cf32c36b97ec --- /dev/null +++ b/modules/geo/src/test/java/org/opensearch/geo/tests/common/RandomGeoGenerator.java @@ -0,0 +1,86 @@ +/* + * 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.tests.common; + +import org.opensearch.common.geo.GeoPoint; + +import java.util.Random; + +/** + * Random geo generation utilities for randomized {@code geo_point} type testing + * does not depend on jts or spatial4j. Use RandomShapeGenerator to create random OGC compliant shapes. + * This is a copy of the file present in the server folder. We need to keep both as there are tests which are + * dependent on that file. + */ +public class RandomGeoGenerator { + + public static void randomPoint(Random r, double[] pt) { + final double[] min = { -180, -90 }; + final double[] max = { 180, 90 }; + randomPointIn(r, min[0], min[1], max[0], max[1], pt); + } + + public static void randomPointIn( + Random r, + final double minLon, + final double minLat, + final double maxLon, + final double maxLat, + double[] pt + ) { + assert pt != null && pt.length == 2; + + // normalize min and max + double[] min = { normalizeLongitude(minLon), normalizeLatitude(minLat) }; + double[] max = { normalizeLongitude(maxLon), normalizeLatitude(maxLat) }; + final double[] tMin = new double[2]; + final double[] tMax = new double[2]; + tMin[0] = Math.min(min[0], max[0]); + tMax[0] = Math.max(min[0], max[0]); + tMin[1] = Math.min(min[1], max[1]); + tMax[1] = Math.max(min[1], max[1]); + + pt[0] = tMin[0] + r.nextDouble() * (tMax[0] - tMin[0]); + pt[1] = tMin[1] + r.nextDouble() * (tMax[1] - tMin[1]); + } + + public static GeoPoint randomPoint(Random r) { + return randomPointIn(r, -180, -90, 180, 90); + } + + public static GeoPoint randomPointIn(Random r, final double minLon, final double minLat, final double maxLon, final double maxLat) { + double[] pt = new double[2]; + randomPointIn(r, minLon, minLat, maxLon, maxLat, pt); + return new GeoPoint(pt[1], pt[0]); + } + + /** Puts latitude in range of -90 to 90. */ + private static double normalizeLatitude(double latitude) { + if (latitude >= -90 && latitude <= 90) { + return latitude; // common case, and avoids slight double precision shifting + } + double off = Math.abs((latitude + 90) % 360); + return (off <= 180 ? off : 360 - off) - 90; + } + + /** Puts longitude in range of -180 to +180. */ + private static double normalizeLongitude(double longitude) { + if (longitude >= -180 && longitude <= 180) { + return longitude; // common case, and avoids slight double precision shifting + } + double off = (longitude + 180) % 360; + if (off < 0) { + return 180 + off; + } else if (off == 0 && longitude > 0) { + return 180; + } else { + return -180 + off; + } + } +} diff --git a/modules/percolator/src/internalClusterTest/java/org/opensearch/percolator/PercolatorQuerySearchIT.java b/modules/percolator/src/internalClusterTest/java/org/opensearch/percolator/PercolatorQuerySearchIT.java index 8d3c37bc9b039..b318a504ab7e7 100644 --- a/modules/percolator/src/internalClusterTest/java/org/opensearch/percolator/PercolatorQuerySearchIT.java +++ b/modules/percolator/src/internalClusterTest/java/org/opensearch/percolator/PercolatorQuerySearchIT.java @@ -44,7 +44,7 @@ import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentType; -import org.opensearch.geo.GeoPlugin; +import org.opensearch.geo.GeoModulePlugin; import org.opensearch.index.mapper.MapperParsingException; import org.opensearch.index.query.MatchPhraseQueryBuilder; import org.opensearch.index.query.MultiMatchQueryBuilder; @@ -93,7 +93,7 @@ protected boolean addMockGeoShapeFieldMapper() { @Override protected Collection> nodePlugins() { - return Arrays.asList(PercolatorPlugin.class, GeoPlugin.class); + return Arrays.asList(PercolatorPlugin.class, GeoModulePlugin.class); } public void testPercolatorQuery() throws Exception { diff --git a/server/src/internalClusterTest/java/org/opensearch/search/aggregations/MissingValueIT.java b/server/src/internalClusterTest/java/org/opensearch/search/aggregations/MissingValueIT.java index 7d3f06760882d..26bfe59618275 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/aggregations/MissingValueIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/aggregations/MissingValueIT.java @@ -39,7 +39,6 @@ import org.opensearch.search.aggregations.bucket.terms.Terms; import org.opensearch.search.aggregations.bucket.terms.TermsAggregatorFactory.ExecutionMode; import org.opensearch.search.aggregations.metrics.Cardinality; -import org.opensearch.search.aggregations.metrics.GeoBounds; import org.opensearch.search.aggregations.metrics.GeoCentroid; import org.opensearch.search.aggregations.metrics.Percentiles; import org.opensearch.search.aggregations.metrics.Stats; @@ -47,7 +46,6 @@ import static org.opensearch.search.aggregations.AggregationBuilders.cardinality; import static org.opensearch.search.aggregations.AggregationBuilders.dateHistogram; -import static org.opensearch.search.aggregations.AggregationBuilders.geoBounds; import static org.opensearch.search.aggregations.AggregationBuilders.geoCentroid; import static org.opensearch.search.aggregations.AggregationBuilders.histogram; import static org.opensearch.search.aggregations.AggregationBuilders.percentiles; @@ -213,28 +211,6 @@ public void testStats() { assertEquals(4, stats.getAvg(), 0); } - public void testUnmappedGeoBounds() { - SearchResponse response = client().prepareSearch("idx") - .addAggregation(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(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)); - } - public void testGeoCentroid() { SearchResponse response = client().prepareSearch("idx") .addAggregation(geoCentroid("centroid").field("location").missing("2,1")) diff --git a/server/src/main/java/org/opensearch/search/SearchModule.java b/server/src/main/java/org/opensearch/search/SearchModule.java index 0995497eba50e..80e025a3651a8 100644 --- a/server/src/main/java/org/opensearch/search/SearchModule.java +++ b/server/src/main/java/org/opensearch/search/SearchModule.java @@ -185,12 +185,10 @@ 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.InternalAvg; import org.opensearch.search.aggregations.metrics.InternalCardinality; import org.opensearch.search.aggregations.metrics.InternalExtendedStats; -import org.opensearch.search.aggregations.metrics.InternalGeoBounds; import org.opensearch.search.aggregations.metrics.InternalGeoCentroid; import org.opensearch.search.aggregations.metrics.InternalHDRPercentileRanks; import org.opensearch.search.aggregations.metrics.InternalHDRPercentiles; @@ -664,12 +662,6 @@ private ValuesSourceRegistry registerAggregations(List plugins) { .addResultReader(InternalTopHits::new), builder ); - registerAggregation( - new AggregationSpec(GeoBoundsAggregationBuilder.NAME, GeoBoundsAggregationBuilder::new, GeoBoundsAggregationBuilder.PARSER) - .addResultReader(InternalGeoBounds::new) - .setAggregatorRegistrar(GeoBoundsAggregationBuilder::registerAggregators), - builder - ); registerAggregation( new AggregationSpec( GeoCentroidAggregationBuilder.NAME, diff --git a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilders.java b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilders.java index 01e0f95b0d750..382455093309d 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilders.java +++ b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilders.java @@ -78,8 +78,6 @@ import org.opensearch.search.aggregations.metrics.CardinalityAggregationBuilder; import org.opensearch.search.aggregations.metrics.ExtendedStats; import org.opensearch.search.aggregations.metrics.ExtendedStatsAggregationBuilder; -import org.opensearch.search.aggregations.metrics.GeoBounds; -import org.opensearch.search.aggregations.metrics.GeoBoundsAggregationBuilder; import org.opensearch.search.aggregations.metrics.GeoCentroid; import org.opensearch.search.aggregations.metrics.GeoCentroidAggregationBuilder; import org.opensearch.search.aggregations.metrics.Max; @@ -364,13 +362,6 @@ public static TopHitsAggregationBuilder topHits(String name) { return new TopHitsAggregationBuilder(name); } - /** - * Create a new {@link GeoBounds} aggregation with the given name. - */ - public static GeoBoundsAggregationBuilder geoBounds(String name) { - return new GeoBoundsAggregationBuilder(name); - } - /** * Create a new {@link GeoCentroid} aggregation with the given name. */ diff --git a/server/src/main/java/org/opensearch/search/aggregations/support/AggregationInspectionHelper.java b/server/src/main/java/org/opensearch/search/aggregations/support/AggregationInspectionHelper.java index 8d036503d1330..f36c4620d5b33 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/support/AggregationInspectionHelper.java +++ b/server/src/main/java/org/opensearch/search/aggregations/support/AggregationInspectionHelper.java @@ -54,7 +54,6 @@ import org.opensearch.search.aggregations.metrics.InternalAvg; import org.opensearch.search.aggregations.metrics.InternalCardinality; import org.opensearch.search.aggregations.metrics.InternalExtendedStats; -import org.opensearch.search.aggregations.metrics.InternalGeoBounds; import org.opensearch.search.aggregations.metrics.InternalGeoCentroid; import org.opensearch.search.aggregations.metrics.InternalHDRPercentileRanks; import org.opensearch.search.aggregations.metrics.InternalHDRPercentiles; @@ -191,10 +190,6 @@ public static boolean hasValue(InternalExtendedStats agg) { return agg.getCount() > 0; } - public static boolean hasValue(InternalGeoBounds agg) { - return (agg.topLeft() == null && agg.bottomRight() == null) == false; - } - public static boolean hasValue(InternalGeoCentroid agg) { return agg.centroid() != null && agg.count() > 0; } diff --git a/server/src/test/java/org/opensearch/search/aggregations/AggregationsTests.java b/server/src/test/java/org/opensearch/search/aggregations/AggregationsTests.java index 421865013a28c..111ce23f8a0cb 100644 --- a/server/src/test/java/org/opensearch/search/aggregations/AggregationsTests.java +++ b/server/src/test/java/org/opensearch/search/aggregations/AggregationsTests.java @@ -80,7 +80,6 @@ import org.opensearch.search.aggregations.metrics.InternalSumTests; import org.opensearch.search.aggregations.metrics.InternalAvgTests; import org.opensearch.search.aggregations.metrics.InternalCardinalityTests; -import org.opensearch.search.aggregations.metrics.InternalGeoBoundsTests; import org.opensearch.search.aggregations.metrics.InternalGeoCentroidTests; import org.opensearch.search.aggregations.metrics.InternalHDRPercentilesRanksTests; import org.opensearch.search.aggregations.metrics.InternalHDRPercentilesTests; @@ -142,7 +141,6 @@ private static List> getAggsTests() { aggsTests.add(new InternalStatsBucketTests()); aggsTests.add(new InternalExtendedStatsTests()); aggsTests.add(new InternalExtendedStatsBucketTests()); - aggsTests.add(new InternalGeoBoundsTests()); aggsTests.add(new InternalGeoCentroidTests()); aggsTests.add(new InternalHistogramTests()); aggsTests.add(new InternalDateHistogramTests()); diff --git a/server/src/test/java/org/opensearch/search/aggregations/metrics/GeoBoundsTests.java b/server/src/test/java/org/opensearch/search/aggregations/metrics/GeoBoundsTests.java deleted file mode 100644 index e132426680fc8..0000000000000 --- a/server/src/test/java/org/opensearch/search/aggregations/metrics/GeoBoundsTests.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * 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.search.aggregations.metrics; - -import org.opensearch.search.aggregations.BaseAggregationTestCase; - -public class GeoBoundsTests extends BaseAggregationTestCase { - - @Override - protected GeoBoundsAggregationBuilder createTestAggregatorBuilder() { - GeoBoundsAggregationBuilder factory = new GeoBoundsAggregationBuilder(randomAlphaOfLengthBetween(1, 20)); - String field = randomAlphaOfLengthBetween(3, 20); - factory.field(field); - if (randomBoolean()) { - factory.wrapLongitude(randomBoolean()); - } - if (randomBoolean()) { - factory.missing("0,0"); - } - return factory; - } - -} diff --git a/test/framework/src/main/java/org/opensearch/test/InternalAggregationTestCase.java b/test/framework/src/main/java/org/opensearch/test/InternalAggregationTestCase.java index f138de152a488..a4099d66de28e 100644 --- a/test/framework/src/main/java/org/opensearch/test/InternalAggregationTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/InternalAggregationTestCase.java @@ -117,7 +117,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; @@ -129,7 +128,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; @@ -261,7 +259,6 @@ public ReduceContext forFinalReduction() { 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)); diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index fef69cde3cea0..c634d7b14e99a 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -2103,6 +2103,7 @@ protected Collection> getMockPlugins() { if (addMockGeoShapeFieldMapper()) { mocks.add(TestGeoShapeFieldMapperPlugin.class); } + return Collections.unmodifiableList(mocks); }