Skip to content

Commit 0c011cf

Browse files
committed
[opensearch-project#6187, opensearch-project#6222] Fixing the GeoShapes GeoHash and GeoTile Aggregations Integration tests.
Changes done: * Fixed the ArrayIndexOutOfBoundsException. * Reduced the precision for GeoShapes Aggregation IT testing. Signed-off-by: Navneet Verma <navneev@amazon.com>
1 parent 1f4cdd2 commit 0c011cf

File tree

4 files changed

+21
-38
lines changed

4 files changed

+21
-38
lines changed

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

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
*/
4141
public abstract class AbstractGeoBucketAggregationIntegTest extends GeoModulePluginIntegTestCase {
4242

43-
protected static final int MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING = 4;
43+
protected static final int MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING = 2;
4444

4545
protected static final int MIN_PRECISION_WITHOUT_BB_AGGS = 2;
4646

@@ -98,7 +98,7 @@ protected void prepareGeoShapeIndexForAggregations(final Random random) throws E
9898
}
9999

100100
i++;
101-
final Set<String> values = generateBucketsForGeometry(geometry, geometryDocValue, isShapeIntersectingBB);
101+
final Set<String> values = generateBucketsForGeometry(geometry, geometryDocValue);
102102
geoshapes.add(indexGeoShape(GEO_SHAPE_INDEX_NAME, geometry));
103103
for (final String hash : values) {
104104
expectedDocsCountForGeoShapes.put(hash, expectedDocsCountForGeoShapes.getOrDefault(hash, 0) + 1);
@@ -112,16 +112,11 @@ protected void prepareGeoShapeIndexForAggregations(final Random random) throws E
112112
* Returns a set of buckets for the shape at different precision level. Override this method for different bucket
113113
* aggregations.
114114
*
115-
* @param geometry {@link Geometry}
116-
* @param geoShapeDocValue {@link GeoShapeDocValue}
117-
* @param intersectingWithBB boolean
115+
* @param geometry {@link Geometry}
116+
* @param geoShapeDocValue {@link GeoShapeDocValue}
118117
* @return A {@link Set} of {@link String} which represents the buckets.
119118
*/
120-
protected abstract Set<String> generateBucketsForGeometry(
121-
final Geometry geometry,
122-
final GeoShapeDocValue geoShapeDocValue,
123-
final boolean intersectingWithBB
124-
);
119+
protected abstract Set<String> generateBucketsForGeometry(final Geometry geometry, final GeoShapeDocValue geoShapeDocValue);
125120

126121
/**
127122
* Prepares a GeoPoint index for testing the GeoPoint bucket aggregations. Different bucket aggregations can use

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -268,18 +268,15 @@ public void testShardSizeIsZero() {
268268
}
269269

270270
@Override
271-
protected Set<String> generateBucketsForGeometry(
272-
final Geometry geometry,
273-
final GeoShapeDocValue geometryDocValue,
274-
boolean intersectingWithBB
275-
) {
271+
protected Set<String> generateBucketsForGeometry(final Geometry geometry, final GeoShapeDocValue geometryDocValue) {
276272
final GeoPoint topLeft = new GeoPoint();
277273
final GeoPoint bottomRight = new GeoPoint();
278274
assert geometry != null;
279275
GeoBoundsHelper.updateBoundsForGeometry(geometry, topLeft, bottomRight);
280276
final Set<String> geoHashes = new HashSet<>();
277+
final boolean isIntersectingWithBoundingRectangle = geometryDocValue.isIntersectingRectangle(boundingRectangleForGeoShapesAgg);
281278
for (int precision = MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision > 0; precision--) {
282-
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && intersectingWithBB == false) {
279+
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && isIntersectingWithBoundingRectangle == false) {
283280
continue;
284281
}
285282
final GeoPoint topRight = new GeoPoint(topLeft.getLat(), bottomRight.getLon());

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,20 +134,20 @@ public void testMultivaluedGeoPointsAggregation() throws Exception {
134134
* Returns a set of buckets for the shape at different precision level. Override this method for different bucket
135135
* aggregations.
136136
*
137-
* @param geometry {@link Geometry}
138-
* @param geoShapeDocValue {@link GeoShapeDocValue}
139-
* @param intersectingWithBB
137+
* @param geometry {@link Geometry}
138+
* @param geoShapeDocValue {@link GeoShapeDocValue}
140139
* @return A {@link Set} of {@link String} which represents the buckets.
141140
*/
142141
@Override
143-
protected Set<String> generateBucketsForGeometry(Geometry geometry, GeoShapeDocValue geoShapeDocValue, boolean intersectingWithBB) {
142+
protected Set<String> generateBucketsForGeometry(final Geometry geometry, final GeoShapeDocValue geoShapeDocValue) {
144143
final GeoPoint topLeft = new GeoPoint();
145144
final GeoPoint bottomRight = new GeoPoint();
146145
assert geometry != null;
147146
GeoBoundsHelper.updateBoundsForGeometry(geometry, topLeft, bottomRight);
148147
final Set<String> geoTiles = new HashSet<>();
148+
final boolean isIntersectingWithBoundingRectangle = geoShapeDocValue.isIntersectingRectangle(boundingRectangleForGeoShapesAgg);
149149
for (int precision = MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision > 0; precision--) {
150-
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && intersectingWithBB == false) {
150+
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && isIntersectingWithBoundingRectangle == false) {
151151
continue;
152152
}
153153
geoTiles.addAll(

modules/geo/src/main/java/org/opensearch/geo/search/aggregations/bucket/geogrid/cells/GeoShapeCellValues.java

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import org.opensearch.common.geo.GeoBoundingBox;
1212
import org.opensearch.common.geo.GeoShapeDocValue;
13+
import org.opensearch.geometry.Rectangle;
1314
import org.opensearch.index.fielddata.AbstractSortingNumericDocValues;
1415
import org.opensearch.index.fielddata.GeoShapeValue;
1516

@@ -57,8 +58,7 @@ public boolean advanceExact(int docId) throws IOException {
5758
* @opensearch.internal
5859
*/
5960
static class BoundedCellValues extends GeoShapeCellValues {
60-
61-
private final GeoBoundingBox geoBoundingBox;
61+
private final Rectangle geoBoundingBox;
6262

6363
public BoundedCellValues(
6464
final GeoShapeValue geoShapeValue,
@@ -67,7 +67,7 @@ public BoundedCellValues(
6767
final GeoBoundingBox boundingBox
6868
) {
6969
super(geoShapeValue, precision, encoder);
70-
this.geoBoundingBox = boundingBox;
70+
this.geoBoundingBox = new Rectangle(boundingBox.left(), boundingBox.right(), boundingBox.top(), boundingBox.bottom());
7171
}
7272

7373
/**
@@ -78,30 +78,21 @@ public BoundedCellValues(
7878
*/
7979
@Override
8080
void relateShape(final GeoShapeDocValue geoShapeDocValue) {
81-
if (intersect(geoShapeDocValue.getBoundingRectangle())) {
81+
if (geoShapeDocValue.isIntersectingRectangle(geoBoundingBox)) {
8282
// now we know the shape is in the bounding rectangle, we need add them in longValues
8383
// generate all grid that this shape intersects
8484
final List<Long> encodedValues = encoder.encode(geoShapeDocValue, precision);
8585
resize(encodedValues.size());
8686
for (int i = 0; i < encodedValues.size(); i++) {
8787
values[i] = encodedValues.get(i);
8888
}
89+
} else {
90+
// As the shape is not intersecting with GeoBounding box, we need to reset the GeoShapeCellValues
91+
// calling this function resets the CellValues for the current shape.
92+
resize(0);
8993
}
9094
}
9195

92-
/**
93-
* Validate that shape is intersecting the bounding box provided as input.
94-
*
95-
* @param rectangle {@link GeoShapeDocValue.BoundingRectangle}
96-
* @return true or false
97-
*/
98-
private boolean intersect(final GeoShapeDocValue.BoundingRectangle rectangle) {
99-
return geoBoundingBox.pointInBounds(rectangle.getMaxLongitude(), rectangle.getMaxLatitude())
100-
|| geoBoundingBox.pointInBounds(rectangle.getMaxLongitude(), rectangle.getMinLatitude())
101-
|| geoBoundingBox.pointInBounds(rectangle.getMinLongitude(), rectangle.getMaxLatitude())
102-
|| geoBoundingBox.pointInBounds(rectangle.getMinLongitude(), rectangle.getMinLatitude());
103-
}
104-
10596
}
10697

10798
/**

0 commit comments

Comments
 (0)