diff --git a/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/AbstractBucketAggregationIntegTest.java b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/AbstractBucketAggregationIntegTest.java index b9d9c48aadf90..a538e4cd2f529 100644 --- a/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/AbstractBucketAggregationIntegTest.java +++ b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/AbstractBucketAggregationIntegTest.java @@ -8,17 +8,30 @@ package org.opensearch.geo.search.aggregations.bucket; +import com.carrotsearch.hppc.ObjectIntHashMap; +import com.carrotsearch.hppc.ObjectIntMap; import org.opensearch.Version; import org.opensearch.action.index.IndexRequestBuilder; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.geo.GeoPoint; +import org.opensearch.common.geo.GeoShapeDocValue; +import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentBuilder; import org.opensearch.geo.GeoModulePluginIntegTestCase; +import org.opensearch.geo.tests.common.RandomGeoGenerator; +import org.opensearch.geo.tests.common.RandomGeoGeometryGenerator; import org.opensearch.geometry.Geometry; import org.opensearch.geometry.Rectangle; import org.opensearch.test.VersionUtils; +import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Random; +import java.util.Set; import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; /** * This is the base class for all the Bucket Aggregation related integration tests. Use this class to add common @@ -33,11 +46,13 @@ public abstract class AbstractBucketAggregationIntegTest extends GeoModulePlugin protected static final String GEO_SHAPE_INDEX_NAME = "geoshape_index"; - // We don't want to generate this BB at random as it may lead to generation of very large or very small BB. - // Hence, we are making the parameters of this BB static for simplicity. - protected static final Rectangle BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG = new Rectangle(-4.1, 20.9, 21.9, -3.1); + protected static Rectangle boundingRectangleForGeoShapesAgg; - protected static final String AGG_NAME = "geohashgrid"; + protected static ObjectIntMap expectedDocsCountForGeoShapes; + + protected static ObjectIntMap expectedDocCountsForSingleGeoPoint; + + protected static ObjectIntMap multiValuedExpectedDocCountsGeoPoint; protected static final String GEO_SHAPE_FIELD_NAME = "location_geo_shape"; @@ -45,6 +60,8 @@ public abstract class AbstractBucketAggregationIntegTest extends GeoModulePlugin protected static final String KEYWORD_FIELD_NAME = "city"; + protected static String smallestGeoHash = null; + protected final Version version = VersionUtils.randomIndexCompatibleVersion(random()); @Override @@ -52,6 +69,130 @@ protected boolean forbidPrivateIndexSettings() { return false; } + /** + * Prepares a GeoShape index for testing the GeoShape bucket aggregations. Different bucket aggregations can use + * different techniques for creating buckets. Override the method + * {@link AbstractBucketAggregationIntegTest#generateBucketsForGeometry} in the test class for creating the + * buckets which will then be used for verifications. + * + * @param random {@link Random} + * @throws Exception thrown during index creation. + */ + protected void prepareGeoShapeIndexForAggregations(final Random random) throws Exception { + expectedDocsCountForGeoShapes = new ObjectIntHashMap<>(); + final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build(); + final List geoshapes = new ArrayList<>(); + assertAcked(prepareCreate(GEO_SHAPE_INDEX_NAME).setSettings(settings).setMapping(GEO_SHAPE_FIELD_NAME, "type" + "=geo_shape")); + boolean isShapeIntersectingBB = false; + for (int i = 0; i < NUM_DOCS;) { + final Geometry geometry = RandomGeoGeometryGenerator.randomGeometry(random); + final GeoShapeDocValue geometryDocValue = GeoShapeDocValue.createGeometryDocValue(geometry); + // make sure that there is 1 shape is intersecting with the bounding box + if (!isShapeIntersectingBB) { + isShapeIntersectingBB = geometryDocValue.isIntersectingRectangle(boundingRectangleForGeoShapesAgg); + if (!isShapeIntersectingBB && i == NUM_DOCS - 1) { + continue; + } + } + i++; + final Set values = generateBucketsForGeometry(geometry, geometryDocValue); + geoshapes.add(indexGeoShape(GEO_SHAPE_INDEX_NAME, geometry)); + for (final String hash : values) { + expectedDocsCountForGeoShapes.put(hash, expectedDocsCountForGeoShapes.getOrDefault(hash, 0) + 1); + } + } + indexRandom(true, geoshapes); + ensureGreen(GEO_SHAPE_INDEX_NAME); + } + + /** + * Returns a set of buckets for the shape at different precision level. Override this method for different bucket + * aggregations. + * + * @param geometry {@link Geometry} + * @param geoShapeDocValue {@link GeoShapeDocValue} + * @return A {@link Set} of {@link String} which represents the buckets. + */ + protected abstract Set generateBucketsForGeometry(final Geometry geometry, final GeoShapeDocValue geoShapeDocValue); + + /** + * Prepares a GeoPoint index for testing the GeoPoint bucket aggregations. Different bucket aggregations can use + * different techniques for creating buckets. Override the method + * {@link AbstractBucketAggregationIntegTest#generateBucketsForGeoPoint} in the test class for creating the + * buckets which will then be used for verifications. + * + * @param random {@link Random} + * @throws Exception thrown during index creation. + */ + protected void prepareSingleValueGeoPointIndex(final Random random) throws Exception { + expectedDocCountsForSingleGeoPoint = new ObjectIntHashMap<>(); + createIndex("idx_unmapped"); + final Settings settings = Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, version) + .put("index.number_of_shards", 4) + .put("index.number_of_replicas", 0) + .build(); + assertAcked( + prepareCreate("idx").setSettings(settings) + .setMapping(GEO_POINT_FIELD_NAME, "type=geo_point", KEYWORD_FIELD_NAME, "type=keyword") + ); + final List cities = new ArrayList<>(); + for (int i = 0; i < NUM_DOCS; i++) { + // generate random point + final GeoPoint geoPoint = RandomGeoGenerator.randomPoint(random); + cities.add(indexGeoPoint("idx", geoPoint.toString(), geoPoint.getLat() + ", " + geoPoint.getLon())); + final Set buckets = generateBucketsForGeoPoint(geoPoint); + for (final String bucket : buckets) { + expectedDocCountsForSingleGeoPoint.put(bucket, expectedDocCountsForSingleGeoPoint.getOrDefault(bucket, 0) + 1); + } + } + indexRandom(true, cities); + ensureGreen("idx_unmapped", "idx"); + } + + protected void prepareMultiValuedGeoPointIndex(final Random random) throws Exception { + multiValuedExpectedDocCountsGeoPoint = new ObjectIntHashMap<>(); + final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build(); + final List cities = new ArrayList<>(); + assertAcked( + prepareCreate("multi_valued_idx").setSettings(settings) + .setMapping(GEO_POINT_FIELD_NAME, "type=geo_point", KEYWORD_FIELD_NAME, "type=keyword") + ); + for (int i = 0; i < NUM_DOCS; i++) { + final int numPoints = random.nextInt(4); + final List points = new ArrayList<>(); + final Set buckets = new HashSet<>(); + for (int j = 0; j < numPoints; ++j) { + // generate random point + final GeoPoint geoPoint = RandomGeoGenerator.randomPoint(random); + points.add(geoPoint.getLat() + "," + geoPoint.getLon()); + buckets.addAll(generateBucketsForGeoPoint(geoPoint)); + } + cities.add(indexGeoPoints("multi_valued_idx", Integer.toString(i), points)); + for (final String bucket : buckets) { + multiValuedExpectedDocCountsGeoPoint.put(bucket, multiValuedExpectedDocCountsGeoPoint.getOrDefault(bucket, 0) + 1); + } + } + indexRandom(true, cities); + ensureGreen("multi_valued_idx"); + } + + /** + * Returns a set of buckets for the GeoPoint at different precision level. Override this method for different bucket + * aggregations. + * + * @param geoPoint {@link GeoPoint} + * @return A {@link Set} of {@link String} which represents the buckets. + */ + protected abstract Set generateBucketsForGeoPoint(final GeoPoint geoPoint); + + /** + * Indexes a GeoShape in the provided index. + * @param index {@link String} index name + * @param geometry {@link Geometry} the Geometry to be indexed + * @return {@link IndexRequestBuilder} + * @throws Exception thrown during creation of {@link IndexRequestBuilder} + */ protected IndexRequestBuilder indexGeoShape(final String index, final Geometry geometry) throws Exception { XContentBuilder source = jsonBuilder().startObject(); source = source.field(GEO_SHAPE_FIELD_NAME, WKT.toWKT(geometry)); @@ -59,7 +200,15 @@ protected IndexRequestBuilder indexGeoShape(final String index, final Geometry g return client().prepareIndex(index).setSource(source); } - protected IndexRequestBuilder indexCity(final String index, final String name, final List latLon) throws Exception { + /** + * Indexes a {@link List} of {@link GeoPoint}s in the provided Index name. + * @param index {@link String} index name + * @param name {@link String} value for the string field in index + * @param latLon {@link List} of {@link String} representing the String representation of GeoPoint + * @return {@link IndexRequestBuilder} + * @throws Exception thrown during indexing. + */ + protected IndexRequestBuilder indexGeoPoints(final String index, final String name, final List latLon) throws Exception { XContentBuilder source = jsonBuilder().startObject().field(KEYWORD_FIELD_NAME, name); if (latLon != null) { source = source.field(GEO_POINT_FIELD_NAME, latLon); @@ -68,8 +217,38 @@ protected IndexRequestBuilder indexCity(final String index, final String name, f return client().prepareIndex(index).setSource(source); } - protected IndexRequestBuilder indexCity(final String index, final String name, final String latLon) throws Exception { - return indexCity(index, name, List.of(latLon)); + /** + * Indexes a {@link GeoPoint} in the provided Index name. + * @param index {@link String} index name + * @param name {@link String} value for the string field in index + * @param latLon {@link String} representing the String representation of GeoPoint + * @return {@link IndexRequestBuilder} + * @throws Exception thrown during indexing. + */ + protected IndexRequestBuilder indexGeoPoint(final String index, final String name, final String latLon) throws Exception { + return indexGeoPoints(index, name, List.of(latLon)); + } + + /** + * Generates a Bounding Box of a fixed radius that can be used for shapes aggregations to reduce the size of + * aggregation results. + * @param random {@link Random} + * @return {@link Rectangle} + */ + protected Rectangle getGridAggregationBoundingBox(final Random random) { + final double radius = getRadiusOfBoundingBox(); + assertTrue("The radius of Bounding Box is less than or equal to 0", radius > 0); + return RandomGeoGeometryGenerator.randomRectangle(random, radius); + } + + /** + * Returns a radius for the Bounding box. Test classes can override this method to change the radius of BBox for + * the test cases. If we increase this value, it will lead to creation of a lot of buckets that can lead of + * IndexOutOfBoundsExceptions. + * @return double + */ + protected double getRadiusOfBoundingBox() { + return 5.0; } } diff --git a/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoHashGridIT.java b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoHashGridIT.java index cf95982b63585..595356297f6c3 100644 --- a/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoHashGridIT.java +++ b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoHashGridIT.java @@ -32,20 +32,15 @@ package org.opensearch.geo.search.aggregations.bucket; import com.carrotsearch.hppc.ObjectIntHashMap; -import com.carrotsearch.hppc.ObjectIntMap; import com.carrotsearch.hppc.cursors.ObjectIntCursor; -import org.opensearch.action.index.IndexRequestBuilder; import org.opensearch.action.search.SearchResponse; -import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.geo.GeoBoundingBox; import org.opensearch.common.geo.GeoPoint; import org.opensearch.common.geo.GeoShapeDocValue; -import org.opensearch.common.settings.Settings; import org.opensearch.geo.search.aggregations.bucket.geogrid.GeoGrid; import org.opensearch.geo.search.aggregations.bucket.geogrid.GeoGridAggregationBuilder; import org.opensearch.geo.search.aggregations.common.GeoBoundsHelper; import org.opensearch.geo.tests.common.AggregationBuilders; -import org.opensearch.geo.tests.common.RandomGeoGeometryGenerator; import org.opensearch.geometry.Geometry; import org.opensearch.geometry.Rectangle; import org.opensearch.geometry.utils.Geohash; @@ -54,7 +49,6 @@ import org.opensearch.search.aggregations.bucket.filter.Filter; import org.opensearch.test.OpenSearchIntegTestCase; -import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Random; @@ -64,29 +58,25 @@ import static org.hamcrest.Matchers.equalTo; import static org.opensearch.geometry.utils.Geohash.PRECISION; import static org.opensearch.geometry.utils.Geohash.stringEncode; -import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse; @OpenSearchIntegTestCase.SuiteScopeTestCase public class GeoHashGridIT extends AbstractBucketAggregationIntegTest { - private static ObjectIntMap expectedDocCountsForGeohash; - - private static ObjectIntMap multiValuedExpectedDocCountsForGeohash; - - private static ObjectIntMap expectedDocCountsForGeoshapeGeohash; - - private static String smallestGeoHash = null; + private static final String AGG_NAME = "geohashgrid"; @Override public void setupSuiteScopeCluster() throws Exception { Random random = random(); + // Creating a BB for limiting the number buckets generated during aggregation + boundingRectangleForGeoShapesAgg = getGridAggregationBoundingBox(random); + expectedDocCountsForSingleGeoPoint = new ObjectIntHashMap<>(); prepareSingleValueGeoPointIndex(random); prepareMultiValuedGeoPointIndex(random); - prepareGeoShapeIndex(random); + prepareGeoShapeIndexForAggregations(random); } - public void testSimple() throws Exception { + public void testSimple() { for (int precision = 1; precision <= PRECISION; precision++) { SearchResponse response = client().prepareSearch("idx") .addAggregation(AggregationBuilders.geohashGrid(AGG_NAME).field(GEO_POINT_FIELD_NAME).precision(precision)) @@ -103,7 +93,7 @@ public void testSimple() throws Exception { String geohash = cell.getKeyAsString(); long bucketCount = cell.getDocCount(); - int expectedBucketCount = expectedDocCountsForGeohash.get(geohash); + int expectedBucketCount = expectedDocCountsForSingleGeoPoint.get(geohash); assertNotSame(bucketCount, 0); assertEquals("Geohash " + geohash + " has wrong doc count ", expectedBucketCount, bucketCount); GeoPoint geoPoint = (GeoPoint) propertiesKeys[i]; @@ -115,8 +105,8 @@ public void testSimple() throws Exception { public void testGeoShapes() { final GeoBoundingBox boundingBox = new GeoBoundingBox( - new GeoPoint(BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG.getMaxLat(), BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG.getMinLon()), - new GeoPoint(BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG.getMinLat(), BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG.getMaxLon()) + new GeoPoint(boundingRectangleForGeoShapesAgg.getMaxLat(), boundingRectangleForGeoShapesAgg.getMinLon()), + new GeoPoint(boundingRectangleForGeoShapesAgg.getMinLat(), boundingRectangleForGeoShapesAgg.getMaxLon()) ); for (int precision = 1; precision <= MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision++) { GeoGridAggregationBuilder builder = AggregationBuilders.geohashGrid(AGG_NAME).field(GEO_SHAPE_FIELD_NAME).precision(precision); @@ -135,7 +125,7 @@ public void testGeoShapes() { final String geohash = cell.getKeyAsString(); final long bucketCount = cell.getDocCount(); - final int expectedBucketCount = expectedDocCountsForGeoshapeGeohash.get(geohash); + final int expectedBucketCount = expectedDocsCountForGeoShapes.get(geohash); assertNotSame(bucketCount, 0); assertEquals("Geohash " + geohash + " has wrong doc count ", expectedBucketCount, bucketCount); final GeoPoint geoPoint = (GeoPoint) propertiesKeys[i]; @@ -145,7 +135,7 @@ public void testGeoShapes() { } } - public void testMultivalued() throws Exception { + public void testMultivalued() { for (int precision = 1; precision <= PRECISION; precision++) { SearchResponse response = client().prepareSearch("multi_valued_idx") .addAggregation(AggregationBuilders.geohashGrid(AGG_NAME).field(GEO_POINT_FIELD_NAME).precision(precision)) @@ -156,14 +146,14 @@ public void testMultivalued() throws Exception { String geohash = cell.getKeyAsString(); long bucketCount = cell.getDocCount(); - int expectedBucketCount = multiValuedExpectedDocCountsForGeohash.get(geohash); + int expectedBucketCount = multiValuedExpectedDocCountsGeoPoint.get(geohash); assertNotSame(bucketCount, 0); assertEquals("Geohash " + geohash + " has wrong doc count ", expectedBucketCount, bucketCount); } } } - public void testFiltered() throws Exception { + public void testFiltered() { GeoBoundingBoxQueryBuilder bbox = new GeoBoundingBoxQueryBuilder(GEO_POINT_FIELD_NAME); bbox.setCorners(smallestGeoHash).queryName("bbox"); for (int precision = 1; precision <= PRECISION; precision++) { @@ -182,7 +172,7 @@ public void testFiltered() throws Exception { for (GeoGrid.Bucket cell : geoGrid.getBuckets()) { String geohash = cell.getKeyAsString(); long bucketCount = cell.getDocCount(); - int expectedBucketCount = expectedDocCountsForGeohash.get(geohash); + int expectedBucketCount = expectedDocCountsForSingleGeoPoint.get(geohash); assertNotSame(bucketCount, 0); assertTrue("Buckets must be filtered", geohash.startsWith(smallestGeoHash)); assertEquals("Geohash " + geohash + " has wrong doc count ", expectedBucketCount, bucketCount); @@ -191,7 +181,7 @@ public void testFiltered() throws Exception { } } - public void testUnmapped() throws Exception { + public void testUnmapped() { for (int precision = 1; precision <= PRECISION; precision++) { SearchResponse response = client().prepareSearch("idx_unmapped") .addAggregation(AggregationBuilders.geohashGrid(AGG_NAME).field(GEO_POINT_FIELD_NAME).precision(precision)) @@ -205,7 +195,7 @@ public void testUnmapped() throws Exception { } - public void testPartiallyUnmapped() throws Exception { + public void testPartiallyUnmapped() { for (int precision = 1; precision <= PRECISION; precision++) { SearchResponse response = client().prepareSearch("idx", "idx_unmapped") .addAggregation(AggregationBuilders.geohashGrid(AGG_NAME).field(GEO_POINT_FIELD_NAME).precision(precision)) @@ -218,14 +208,14 @@ public void testPartiallyUnmapped() throws Exception { String geohash = cell.getKeyAsString(); long bucketCount = cell.getDocCount(); - int expectedBucketCount = expectedDocCountsForGeohash.get(geohash); + int expectedBucketCount = expectedDocCountsForSingleGeoPoint.get(geohash); assertNotSame(bucketCount, 0); assertEquals("Geohash " + geohash + " has wrong doc count ", expectedBucketCount, bucketCount); } } } - public void testTopMatch() throws Exception { + public void testTopMatch() { for (int precision = 1; precision <= PRECISION; precision++) { SearchResponse response = client().prepareSearch("idx") .addAggregation( @@ -242,7 +232,7 @@ public void testTopMatch() throws Exception { String geohash = cell.getKeyAsString(); long bucketCount = cell.getDocCount(); int expectedBucketCount = 0; - for (ObjectIntCursor cursor : expectedDocCountsForGeohash) { + for (ObjectIntCursor cursor : expectedDocCountsForSingleGeoPoint) { if (cursor.key.length() == precision) { expectedBucketCount = Math.max(expectedBucketCount, cursor.value); } @@ -277,131 +267,56 @@ public void testShardSizeIsZero() { assertThat(exception.getMessage(), containsString("[shardSize] must be greater than 0. Found [0] in [" + AGG_NAME + "]")); } - private void prepareSingleValueGeoPointIndex(final Random random) throws Exception { - createIndex("idx_unmapped"); - final Settings settings = Settings.builder() - .put(IndexMetadata.SETTING_VERSION_CREATED, version) - .put("index.number_of_shards", 4) - .put("index.number_of_replicas", 0) - .build(); - assertAcked( - prepareCreate("idx").setSettings(settings) - .setMapping(GEO_POINT_FIELD_NAME, "type=geo_point", KEYWORD_FIELD_NAME, "type=keyword") - ); - final List cities = new ArrayList<>(); - expectedDocCountsForGeohash = new ObjectIntHashMap<>(NUM_DOCS * 2); - for (int i = 0; i < NUM_DOCS; i++) { - // generate random point - double lat = (180d * random.nextDouble()) - 90d; - double lng = (360d * random.nextDouble()) - 180d; - String randomGeoHash = stringEncode(lng, lat, PRECISION); - // Index at the highest resolution - cities.add(indexCity("idx", randomGeoHash, lat + ", " + lng)); - expectedDocCountsForGeohash.put(randomGeoHash, expectedDocCountsForGeohash.getOrDefault(randomGeoHash, 0) + 1); - // Update expected doc counts for all resolutions.. - for (int precision = PRECISION - 1; precision > 0; precision--) { - String hash = stringEncode(lng, lat, precision); - if ((smallestGeoHash == null) || (hash.length() < smallestGeoHash.length())) { - smallestGeoHash = hash; - } - expectedDocCountsForGeohash.put(hash, expectedDocCountsForGeohash.getOrDefault(hash, 0) + 1); - } - } - indexRandom(true, cities); - ensureGreen("idx_unmapped", "idx"); - } - - private void prepareMultiValuedGeoPointIndex(final Random random) throws Exception { - final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build(); - final List cities = new ArrayList<>(); - assertAcked( - prepareCreate("multi_valued_idx").setSettings(settings) - .setMapping(GEO_POINT_FIELD_NAME, "type=geo_point", KEYWORD_FIELD_NAME, "type=keyword") - ); - multiValuedExpectedDocCountsForGeohash = new ObjectIntHashMap<>(NUM_DOCS * 2); - for (int i = 0; i < NUM_DOCS; i++) { - final int numPoints = random.nextInt(4); - final List points = new ArrayList<>(); - final Set geoHashes = new HashSet<>(); - for (int j = 0; j < numPoints; ++j) { - final double lat = (180d * random.nextDouble()) - 90d; - final double lng = (360d * random.nextDouble()) - 180d; - points.add(lat + "," + lng); - // Update expected doc counts for all resolutions.. - for (int precision = PRECISION; precision > 0; precision--) { - final String geoHash = stringEncode(lng, lat, precision); - geoHashes.add(geoHash); - } - } - cities.add(indexCity("multi_valued_idx", Integer.toString(i), points)); - for (final String hash : geoHashes) { - multiValuedExpectedDocCountsForGeohash.put(hash, multiValuedExpectedDocCountsForGeohash.getOrDefault(hash, 0) + 1); + @Override + protected Set generateBucketsForGeometry(final Geometry geometry, final GeoShapeDocValue geometryDocValue) { + final GeoPoint topLeft = new GeoPoint(); + final GeoPoint bottomRight = new GeoPoint(); + assert geometry != null; + GeoBoundsHelper.updateBoundsForGeometry(geometry, topLeft, bottomRight); + final Set geoHashes = new HashSet<>(); + for (int precision = MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision > 0; precision--) { + if (precision > 2 && !geometryDocValue.isIntersectingRectangle(boundingRectangleForGeoShapesAgg)) { + continue; } - } - indexRandom(true, cities); - ensureGreen("multi_valued_idx"); - } - - private void prepareGeoShapeIndex(final Random random) throws Exception { - final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build(); - - final List geoshapes = new ArrayList<>(); - assertAcked(prepareCreate(GEO_SHAPE_INDEX_NAME).setSettings(settings).setMapping(GEO_SHAPE_FIELD_NAME, "type" + "=geo_shape")); - expectedDocCountsForGeoshapeGeohash = new ObjectIntHashMap<>(); - boolean isShapeIntersectingBB = false; - for (int i = 0; i < NUM_DOCS;) { - final Geometry geometry = RandomGeoGeometryGenerator.randomGeometry(random); - final GeoShapeDocValue geometryDocValue = GeoShapeDocValue.createGeometryDocValue(geometry); - // make sure that there is 1 shape is intersecting with the bounding box - if (!isShapeIntersectingBB) { - isShapeIntersectingBB = geometryDocValue.isIntersectingRectangle(BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG); - if (!isShapeIntersectingBB && i == NUM_DOCS - 1) { - continue; + final GeoPoint topRight = new GeoPoint(topLeft.getLat(), bottomRight.getLon()); + String currentGeoHash = Geohash.stringEncode(topLeft.getLon(), topLeft.getLat(), precision); + String startingRowGeoHash = currentGeoHash; + String endGeoHashForCurrentRow = Geohash.stringEncode(topRight.getLon(), topRight.getLat(), precision); + String terminatingGeoHash = Geohash.stringEncode(bottomRight.getLon(), bottomRight.getLat(), precision); + while (true) { + final Rectangle currentRectangle = Geohash.toBoundingBox(currentGeoHash); + if (geometryDocValue.isIntersectingRectangle(currentRectangle)) { + geoHashes.add(currentGeoHash); } - } - i++; - final GeoPoint topLeft = new GeoPoint(); - final GeoPoint bottomRight = new GeoPoint(); - assert geometry != null; - GeoBoundsHelper.updateBoundsForGeometry(geometry, topLeft, bottomRight); - final Set geoHashes = new HashSet<>(); - for (int precision = MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision > 0; precision--) { - if (precision > 2 && !geometryDocValue.isIntersectingRectangle(BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG)) { - continue; + assert currentGeoHash != null; + if (currentGeoHash.equals(terminatingGeoHash)) { + break; } - final GeoPoint topRight = new GeoPoint(topLeft.getLat(), bottomRight.getLon()); - String currentGeoHash = Geohash.stringEncode(topLeft.getLon(), topLeft.getLat(), precision); - String startingRowGeoHash = currentGeoHash; - String endGeoHashForCurrentRow = Geohash.stringEncode(topRight.getLon(), topRight.getLat(), precision); - String terminatingGeoHash = Geohash.stringEncode(bottomRight.getLon(), bottomRight.getLat(), precision); - while (true) { - final Rectangle currentRectangle = Geohash.toBoundingBox(currentGeoHash); - if (geometryDocValue.isIntersectingRectangle(currentRectangle)) { - geoHashes.add(currentGeoHash); - } - assert currentGeoHash != null; - if (currentGeoHash.equals(terminatingGeoHash)) { - break; - } - if (currentGeoHash.equals(endGeoHashForCurrentRow)) { - // move in south direction - currentGeoHash = Geohash.getNeighbor(startingRowGeoHash, precision, 0, -1); - startingRowGeoHash = currentGeoHash; - endGeoHashForCurrentRow = Geohash.getNeighbor(endGeoHashForCurrentRow, precision, 0, -1); - } else { - // move in East direction - currentGeoHash = Geohash.getNeighbor(currentGeoHash, precision, 1, 0); - } + if (currentGeoHash.equals(endGeoHashForCurrentRow)) { + // move in south direction + currentGeoHash = Geohash.getNeighbor(startingRowGeoHash, precision, 0, -1); + startingRowGeoHash = currentGeoHash; + endGeoHashForCurrentRow = Geohash.getNeighbor(endGeoHashForCurrentRow, precision, 0, -1); + } else { + // move in East direction + currentGeoHash = Geohash.getNeighbor(currentGeoHash, precision, 1, 0); } } + } + return geoHashes; + } - geoshapes.add(indexGeoShape(GEO_SHAPE_INDEX_NAME, geometry)); - for (final String hash : geoHashes) { - expectedDocCountsForGeoshapeGeohash.put(hash, expectedDocCountsForGeoshapeGeohash.getOrDefault(hash, 0) + 1); + @Override + protected Set generateBucketsForGeoPoint(final GeoPoint geoPoint) { + Set buckets = new HashSet<>(); + for (int precision = PRECISION; precision > 0; precision--) { + final String hash = Geohash.stringEncode(geoPoint.getLon(), geoPoint.getLat(), precision); + if ((smallestGeoHash == null) || (hash.length() < smallestGeoHash.length())) { + smallestGeoHash = hash; } + buckets.add(hash); } - indexRandom(true, geoshapes); - ensureGreen(GEO_SHAPE_INDEX_NAME); + return buckets; } } diff --git a/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoTileGridIT.java b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoTileGridIT.java index e5723876b74cc..47a3481b9f589 100644 --- a/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoTileGridIT.java +++ b/modules/geo/src/internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoTileGridIT.java @@ -8,31 +8,20 @@ package org.opensearch.geo.search.aggregations.bucket; -import com.carrotsearch.hppc.ObjectIntHashMap; -import com.carrotsearch.hppc.ObjectIntMap; import org.hamcrest.MatcherAssert; -import org.opensearch.Version; -import org.opensearch.action.index.IndexRequestBuilder; import org.opensearch.action.search.SearchResponse; -import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.geo.GeoBoundingBox; import org.opensearch.common.geo.GeoPoint; import org.opensearch.common.geo.GeoShapeDocValue; -import org.opensearch.common.settings.Settings; import org.opensearch.geo.search.aggregations.bucket.geogrid.GeoGrid; import org.opensearch.geo.search.aggregations.bucket.geogrid.GeoGridAggregationBuilder; import org.opensearch.geo.search.aggregations.common.GeoBoundsHelper; 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.Rectangle; import org.opensearch.search.aggregations.InternalAggregation; import org.opensearch.search.aggregations.bucket.GeoTileUtils; import org.opensearch.test.OpenSearchIntegTestCase; -import org.opensearch.test.VersionUtils; -import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Random; @@ -40,7 +29,6 @@ import java.util.stream.Collectors; import static org.hamcrest.Matchers.equalTo; -import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse; @OpenSearchIntegTestCase.SuiteScopeTestCase @@ -50,36 +38,26 @@ public class GeoTileGridIT extends AbstractBucketAggregationIntegTest { private static final String AGG_NAME = "geotilegrid"; - private static final String GEO_SHAPE_INDEX_NAME = "geoshape_index"; - - private static final String GEO_SHAPE_FIELD_NAME = "location_geo_shape"; - - private final Version version = VersionUtils.randomIndexCompatibleVersion(random()); - - private static final Rectangle BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG = new Rectangle(-4.1, 20.9, 21.9, -3.1); - - private static ObjectIntMap expectedDocCountsForGeoshapeGeoTiles; - - private static ObjectIntMap expectedDocCountsForGeoTiles; - - private static ObjectIntHashMap multiValuedExpectedDocCountsForGeoTiles; - @Override public void setupSuiteScopeCluster() throws Exception { final Random random = random(); + // Creating a BB for limiting the number buckets generated during aggregation + boundingRectangleForGeoShapesAgg = getGridAggregationBoundingBox(random); prepareSingleValueGeoPointIndex(random); prepareMultiValuedGeoPointIndex(random); - prepareGeoShapeIndex(random); + prepareGeoShapeIndexForAggregations(random); ensureSearchable(); } public void testGeoShapes() { final GeoBoundingBox boundingBox = new GeoBoundingBox( - new GeoPoint(BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG.getMaxLat(), BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG.getMinLon()), - new GeoPoint(BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG.getMinLat(), BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG.getMaxLon()) + new GeoPoint(boundingRectangleForGeoShapesAgg.getMaxLat(), boundingRectangleForGeoShapesAgg.getMinLon()), + new GeoPoint(boundingRectangleForGeoShapesAgg.getMinLat(), boundingRectangleForGeoShapesAgg.getMaxLon()) ); for (int precision = 1; precision <= MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision++) { - GeoGridAggregationBuilder builder = AggregationBuilders.geotileGrid(AGG_NAME).field(GEO_SHAPE_FIELD_NAME).precision(precision); + final GeoGridAggregationBuilder builder = AggregationBuilders.geotileGrid(AGG_NAME) + .field(GEO_SHAPE_FIELD_NAME) + .precision(precision); // This makes sure that for only higher precision we are providing the GeoBounding Box. This also ensures // that we are able to test both bounded and unbounded aggregations if (precision > 2) { @@ -95,7 +73,7 @@ public void testGeoShapes() { final String geoTile = cell.getKeyAsString(); final long bucketCount = cell.getDocCount(); - final int expectedBucketCount = expectedDocCountsForGeoshapeGeoTiles.get(geoTile); + final int expectedBucketCount = expectedDocsCountForGeoShapes.get(geoTile); assertNotSame(bucketCount, 0); assertEquals("Geotile " + geoTile + " has wrong doc count ", expectedBucketCount, bucketCount); final GeoPoint geoPoint = (GeoPoint) propertiesKeys[i]; @@ -122,7 +100,7 @@ public void testSimpleGeoPointsAggregation() { String geoTile = cell.getKeyAsString(); long bucketCount = cell.getDocCount(); - int expectedBucketCount = expectedDocCountsForGeoTiles.get(geoTile); + int expectedBucketCount = expectedDocCountsForSingleGeoPoint.get(geoTile); assertNotSame(bucketCount, 0); assertEquals("GeoTile " + geoTile + " has wrong doc count ", expectedBucketCount, bucketCount); GeoPoint geoPoint = (GeoPoint) propertiesKeys[i]; @@ -145,105 +123,42 @@ public void testMultivaluedGeoPointsAggregation() throws Exception { String geohash = cell.getKeyAsString(); long bucketCount = cell.getDocCount(); - int expectedBucketCount = multiValuedExpectedDocCountsForGeoTiles.get(geohash); + int expectedBucketCount = multiValuedExpectedDocCountsGeoPoint.get(geohash); assertNotSame(bucketCount, 0); assertEquals("Geohash " + geohash + " has wrong doc count ", expectedBucketCount, bucketCount); } } } - private void prepareGeoShapeIndex(final Random random) throws Exception { - final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build(); - final List geoshapes = new ArrayList<>(); - assertAcked(prepareCreate(GEO_SHAPE_INDEX_NAME).setSettings(settings).setMapping(GEO_SHAPE_FIELD_NAME, "type" + "=geo_shape")); - expectedDocCountsForGeoshapeGeoTiles = new ObjectIntHashMap<>(); - boolean isShapeIntersectingBB = false; - for (int i = 0; i < NUM_DOCS;) { - final Geometry geometry = RandomGeoGeometryGenerator.randomGeometry(random); - final GeoShapeDocValue geometryDocValue = GeoShapeDocValue.createGeometryDocValue(geometry); - // make sure that there is 1 shape is intersecting with the bounding box - if (!isShapeIntersectingBB) { - isShapeIntersectingBB = geometryDocValue.isIntersectingRectangle(BOUNDING_RECTANGLE_FOR_GEO_SHAPES_AGG); - if (!isShapeIntersectingBB && i == NUM_DOCS - 1) { - continue; - } - } - i++; - final GeoPoint topLeft = new GeoPoint(); - final GeoPoint bottomRight = new GeoPoint(); - assert geometry != null; - GeoBoundsHelper.updateBoundsForGeometry(geometry, topLeft, bottomRight); - final Set geoTiles = new HashSet<>(); - for (int precision = MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision > 0; precision--) { - geoTiles.addAll( - GeoTileUtils.encodeShape(geometryDocValue, precision) - .stream() - .map(GeoTileUtils::stringEncode) - .collect(Collectors.toSet()) - ); - } - - geoshapes.add(indexGeoShape(GEO_SHAPE_INDEX_NAME, geometry)); - for (final String hash : geoTiles) { - expectedDocCountsForGeoshapeGeoTiles.put(hash, expectedDocCountsForGeoshapeGeoTiles.getOrDefault(hash, 0) + 1); - } - } - indexRandom(true, geoshapes); - ensureGreen(GEO_SHAPE_INDEX_NAME); - } - - private void prepareSingleValueGeoPointIndex(final Random random) throws Exception { - final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build(); - assertAcked( - prepareCreate("idx").setSettings(settings) - .setMapping(GEO_POINT_FIELD_NAME, "type=geo_point", KEYWORD_FIELD_NAME, "type=keyword") - ); - final List cities = new ArrayList<>(); - expectedDocCountsForGeoTiles = new ObjectIntHashMap<>(); - for (int i = 0; i < NUM_DOCS; i++) { - // generate random point - final GeoPoint geoPoint = RandomGeoGenerator.randomPoint(random); - final String cityName = GeoTileUtils.stringEncode(geoPoint.getLon(), geoPoint.getLat(), GEOPOINT_MAX_PRECISION); - // Index at the highest resolution - cities.add(indexCity("idx", cityName, geoPoint.getLat() + ", " + geoPoint.getLon())); - // Update expected doc counts for all resolutions.. - for (int precision = GEOPOINT_MAX_PRECISION; precision > 0; precision--) { - String tile = GeoTileUtils.stringEncode(geoPoint.getLon(), geoPoint.getLat(), precision); - expectedDocCountsForGeoTiles.put(tile, expectedDocCountsForGeoTiles.getOrDefault(tile, 0) + 1); - } + /** + * Returns a set of buckets for the shape at different precision level. Override this method for different bucket + * aggregations. + * + * @param geometry {@link Geometry} + * @param geoShapeDocValue {@link GeoShapeDocValue} + * @return A {@link Set} of {@link String} which represents the buckets. + */ + @Override + protected Set generateBucketsForGeometry(Geometry geometry, GeoShapeDocValue geoShapeDocValue) { + final GeoPoint topLeft = new GeoPoint(); + final GeoPoint bottomRight = new GeoPoint(); + assert geometry != null; + GeoBoundsHelper.updateBoundsForGeometry(geometry, topLeft, bottomRight); + final Set geoTiles = new HashSet<>(); + for (int precision = MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision > 0; precision--) { + geoTiles.addAll( + GeoTileUtils.encodeShape(geoShapeDocValue, precision).stream().map(GeoTileUtils::stringEncode).collect(Collectors.toSet()) + ); } - indexRandom(true, cities); - ensureGreen("idx"); + return geoTiles; } - private void prepareMultiValuedGeoPointIndex(final Random random) throws Exception { - final Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, version).build(); - final List cities = new ArrayList<>(); - assertAcked( - prepareCreate("multi_valued_idx").setSettings(settings) - .setMapping(GEO_POINT_FIELD_NAME, "type=geo_point", KEYWORD_FIELD_NAME, "type=keyword") - ); - multiValuedExpectedDocCountsForGeoTiles = new ObjectIntHashMap<>(NUM_DOCS * 2); - for (int i = 0; i < NUM_DOCS; i++) { - final int numPoints = random.nextInt(4); - final List points = new ArrayList<>(); - final Set geoTiles = new HashSet<>(); - for (int j = 0; j < numPoints; ++j) { - // generate random point - final GeoPoint geoPoint = RandomGeoGenerator.randomPoint(random); - points.add(geoPoint.getLat() + "," + geoPoint.getLon()); - // Update expected doc counts for all resolutions.. - for (int precision = GEOPOINT_MAX_PRECISION; precision > 0; precision--) { - final String tile = GeoTileUtils.stringEncode(geoPoint.getLon(), geoPoint.getLat(), precision); - geoTiles.add(tile); - } - } - cities.add(indexCity("multi_valued_idx", Integer.toString(i), points)); - for (final String hash : geoTiles) { - multiValuedExpectedDocCountsForGeoTiles.put(hash, multiValuedExpectedDocCountsForGeoTiles.getOrDefault(hash, 0) + 1); - } + protected Set generateBucketsForGeoPoint(final GeoPoint geoPoint) { + Set buckets = new HashSet<>(); + for (int precision = GEOPOINT_MAX_PRECISION; precision > 0; precision--) { + final String tile = GeoTileUtils.stringEncode(geoPoint.getLon(), geoPoint.getLat(), precision); + buckets.add(tile); } - indexRandom(true, cities); - ensureGreen("multi_valued_idx"); + return buckets; } } 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 index 2fb403155e2bc..a3def686b282d 100644 --- 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 @@ -64,7 +64,7 @@ public static GeoPoint randomPointIn(Random r, final double minLon, final double } /** Puts latitude in range of -90 to 90. */ - private static double normalizeLatitude(double latitude) { + public static double normalizeLatitude(double latitude) { if (latitude >= -90 && latitude <= 90) { return latitude; // common case, and avoids slight double precision shifting } @@ -73,7 +73,7 @@ private static double normalizeLatitude(double latitude) { } /** Puts longitude in range of -180 to +180. */ - private static double normalizeLongitude(double longitude) { + public static double normalizeLongitude(double longitude) { if (longitude >= -180 && longitude <= 180) { return longitude; // common case, and avoids slight double precision shifting } diff --git a/modules/geo/src/test/java/org/opensearch/geo/tests/common/RandomGeoGeometryGenerator.java b/modules/geo/src/test/java/org/opensearch/geo/tests/common/RandomGeoGeometryGenerator.java index caf15507e08c5..c6f78e846955d 100644 --- a/modules/geo/src/test/java/org/opensearch/geo/tests/common/RandomGeoGeometryGenerator.java +++ b/modules/geo/src/test/java/org/opensearch/geo/tests/common/RandomGeoGeometryGenerator.java @@ -199,6 +199,26 @@ public static Rectangle randomRectangle(final Random r) { return new Rectangle(minX, maxX, maxY, minY); } + /** + * Generates a {@link Rectangle} of a specific radius. The generated rectangle can cross the international date line. + * + * @param r {@link Random} + * @param radius double + * @return {@link Rectangle} + */ + public static Rectangle randomRectangle(final Random r, double radius) { + final double[] centre = new double[2]; + RandomGeoGenerator.randomPointIn(r, -180, -(90 - radius), 180, 90 - radius, centre); + final double centreX = centre[0]; + final double centreY = centre[1]; + return new Rectangle( + RandomGeoGenerator.normalizeLongitude(centreX - radius), + RandomGeoGenerator.normalizeLongitude(centreX + radius), + centreY + radius, + centreY - radius + ); + } + /** * Returns a double array where pt[0] : longitude and pt[1] : latitude *