Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#6187, #6222] Fixing the GeoShapes GeoHash and GeoTile Aggregations Integration tests to make them more robust. #6242

Merged
merged 1 commit into from
Feb 10, 2023
Merged

[#6187, #6222] Fixing the GeoShapes GeoHash and GeoTile Aggregations Integration tests to make them more robust. #6242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 5 additions & 10 deletions .../org/opensearch/geo/search/aggregations/bucket/AbstractGeoBucketAggregationIntegTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
*/
public abstract class AbstractGeoBucketAggregationIntegTest extends GeoModulePluginIntegTestCase {

protected static final int MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING = 4;
protected static final int MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING = 2;

protected static final int MIN_PRECISION_WITHOUT_BB_AGGS = 2;

Expand Down Expand Up @@ -98,7 +98,7 @@ protected void prepareGeoShapeIndexForAggregations(final Random random) throws E
}

i++;
final Set<String> values = generateBucketsForGeometry(geometry, geometryDocValue, isShapeIntersectingBB);
final Set<String> 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);
Expand All @@ -112,16 +112,11 @@ protected void prepareGeoShapeIndexForAggregations(final Random random) throws E
* 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}
* @param intersectingWithBB boolean
* @param geometry {@link Geometry}
* @param geoShapeDocValue {@link GeoShapeDocValue}
* @return A {@link Set} of {@link String} which represents the buckets.
*/
protected abstract Set<String> generateBucketsForGeometry(
final Geometry geometry,
final GeoShapeDocValue geoShapeDocValue,
final boolean intersectingWithBB
);
protected abstract Set<String> generateBucketsForGeometry(final Geometry geometry, final GeoShapeDocValue geoShapeDocValue);

/**
* Prepares a GeoPoint index for testing the GeoPoint bucket aggregations. Different bucket aggregations can use
Expand Down
9 changes: 3 additions & 6 deletions ...internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoHashGridIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,18 +268,15 @@ public void testShardSizeIsZero() {
}

@Override
protected Set<String> generateBucketsForGeometry(
final Geometry geometry,
final GeoShapeDocValue geometryDocValue,
boolean intersectingWithBB
) {
protected Set<String> 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<String> geoHashes = new HashSet<>();
final boolean isIntersectingWithBoundingRectangle = geometryDocValue.isIntersectingRectangle(boundingRectangleForGeoShapesAgg);
for (int precision = MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision > 0; precision--) {
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && intersectingWithBB == false) {
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && isIntersectingWithBoundingRectangle == false) {
continue;
}
final GeoPoint topRight = new GeoPoint(topLeft.getLat(), bottomRight.getLon());
Expand Down
10 changes: 5 additions & 5 deletions ...internalClusterTest/java/org/opensearch/geo/search/aggregations/bucket/GeoTileGridIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,20 +134,20 @@ public void testMultivaluedGeoPointsAggregation() throws Exception {
* 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}
* @param intersectingWithBB
* @param geometry {@link Geometry}
* @param geoShapeDocValue {@link GeoShapeDocValue}
* @return A {@link Set} of {@link String} which represents the buckets.
*/
@Override
protected Set<String> generateBucketsForGeometry(Geometry geometry, GeoShapeDocValue geoShapeDocValue, boolean intersectingWithBB) {
protected Set<String> generateBucketsForGeometry(final Geometry geometry, final GeoShapeDocValue geoShapeDocValue) {
final GeoPoint topLeft = new GeoPoint();
final GeoPoint bottomRight = new GeoPoint();
assert geometry != null;
GeoBoundsHelper.updateBoundsForGeometry(geometry, topLeft, bottomRight);
final Set<String> geoTiles = new HashSet<>();
final boolean isIntersectingWithBoundingRectangle = geoShapeDocValue.isIntersectingRectangle(boundingRectangleForGeoShapesAgg);
for (int precision = MAX_PRECISION_FOR_GEO_SHAPES_AGG_TESTING; precision > 0; precision--) {
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && intersectingWithBB == false) {
if (precision > MIN_PRECISION_WITHOUT_BB_AGGS && isIntersectingWithBoundingRectangle == false) {
continue;
}
geoTiles.addAll(
Expand Down
25 changes: 8 additions & 17 deletions .../java/org/opensearch/geo/search/aggregations/bucket/geogrid/cells/GeoShapeCellValues.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.opensearch.common.geo.GeoBoundingBox;
import org.opensearch.common.geo.GeoShapeDocValue;
import org.opensearch.geometry.Rectangle;
import org.opensearch.index.fielddata.AbstractSortingNumericDocValues;
import org.opensearch.index.fielddata.GeoShapeValue;

Expand Down Expand Up @@ -57,8 +58,7 @@ public boolean advanceExact(int docId) throws IOException {
* @opensearch.internal
*/
static class BoundedCellValues extends GeoShapeCellValues {

private final GeoBoundingBox geoBoundingBox;
private final Rectangle geoBoundingBox;

public BoundedCellValues(
final GeoShapeValue geoShapeValue,
Expand All @@ -67,7 +67,7 @@ public BoundedCellValues(
final GeoBoundingBox boundingBox
) {
super(geoShapeValue, precision, encoder);
this.geoBoundingBox = boundingBox;
this.geoBoundingBox = new Rectangle(boundingBox.left(), boundingBox.right(), boundingBox.top(), boundingBox.bottom());
nknize marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -78,30 +78,21 @@ public BoundedCellValues(
*/
@Override
void relateShape(final GeoShapeDocValue geoShapeDocValue) {
if (intersect(geoShapeDocValue.getBoundingRectangle())) {
if (geoShapeDocValue.isIntersectingRectangle(geoBoundingBox)) {
// now we know the shape is in the bounding rectangle, we need add them in longValues
// generate all grid that this shape intersects
final List<Long> encodedValues = encoder.encode(geoShapeDocValue, precision);
resize(encodedValues.size());
for (int i = 0; i < encodedValues.size(); i++) {
values[i] = encodedValues.get(i);
}
} else {
// As the shape is not intersecting with GeoBounding box, we need to reset the GeoShapeCellValues
// calling this function resets the CellValues for the current shape.
resize(0);
}
}

/**
* Validate that shape is intersecting the bounding box provided as input.
*
* @param rectangle {@link GeoShapeDocValue.BoundingRectangle}
* @return true or false
*/
private boolean intersect(final GeoShapeDocValue.BoundingRectangle rectangle) {
return geoBoundingBox.pointInBounds(rectangle.getMaxLongitude(), rectangle.getMaxLatitude())
|| geoBoundingBox.pointInBounds(rectangle.getMaxLongitude(), rectangle.getMinLatitude())
|| geoBoundingBox.pointInBounds(rectangle.getMinLongitude(), rectangle.getMaxLatitude())
|| geoBoundingBox.pointInBounds(rectangle.getMinLongitude(), rectangle.getMinLatitude());
}

}

/**
Expand Down