Selaa lähdekoodia

Don't generate bounding box touching tiles in GeoTileGridAggregatorTests (#101137)

Ignacio Vera 2 vuotta sitten
vanhempi
commit
126412c8bf

+ 1 - 1
server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregatorTests.java

@@ -30,7 +30,7 @@ public class GeoHashGridAggregatorTests extends GeoGridAggregatorTestCase<Intern
     }
 
     @Override
-    protected GeoBoundingBox randomBBox() {
+    protected GeoBoundingBox randomBBox(int precision) {
         Rectangle rectangle = GeometryTestUtils.randomRectangle();
         return new GeoBoundingBox(
             new GeoPoint(rectangle.getMaxLat(), rectangle.getMinLon()),

+ 42 - 17
server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorTests.java

@@ -8,8 +8,6 @@
 
 package org.elasticsearch.search.aggregations.bucket.geogrid;
 
-import org.apache.lucene.geo.GeoEncodingUtils;
-import org.apache.lucene.tests.util.LuceneTestCase;
 import org.elasticsearch.common.geo.GeoBoundingBox;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
@@ -17,7 +15,11 @@ import org.elasticsearch.geo.GeometryTestUtils;
 import org.elasticsearch.geometry.Point;
 import org.elasticsearch.geometry.Rectangle;
 
-@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/101077")
+import static org.apache.lucene.geo.GeoEncodingUtils.decodeLatitude;
+import static org.apache.lucene.geo.GeoEncodingUtils.decodeLongitude;
+import static org.apache.lucene.geo.GeoEncodingUtils.encodeLatitude;
+import static org.apache.lucene.geo.GeoEncodingUtils.encodeLongitude;
+
 public class GeoTileGridAggregatorTests extends GeoGridAggregatorTestCase<InternalGeoTileGridBucket> {
 
     @Override
@@ -39,7 +41,7 @@ public class GeoTileGridAggregatorTests extends GeoGridAggregatorTestCase<Intern
     }
 
     @Override
-    protected GeoBoundingBox randomBBox() {
+    protected GeoBoundingBox randomBBox(int precision) {
         GeoBoundingBox bbox = randomValueOtherThanMany(
             (b) -> b.top() > GeoTileUtils.LATITUDE_MASK || b.bottom() < -GeoTileUtils.LATITUDE_MASK,
             () -> {
@@ -50,11 +52,42 @@ public class GeoTileGridAggregatorTests extends GeoGridAggregatorTestCase<Intern
                 );
             }
         );
-        // Avoid numerical errors for sub-atomic values
-        double left = GeoEncodingUtils.decodeLongitude(GeoEncodingUtils.encodeLongitude(bbox.left()));
-        double right = GeoEncodingUtils.decodeLongitude(GeoEncodingUtils.encodeLongitude(bbox.right()));
-        double top = GeoEncodingUtils.decodeLatitude(GeoEncodingUtils.encodeLatitude(bbox.top()));
-        double bottom = GeoEncodingUtils.decodeLatitude(GeoEncodingUtils.encodeLatitude(bbox.bottom()));
+        final int tiles = 1 << precision;
+
+        // Due to the way GeoTileBoundedPredicate works that adjust the given bounding box when it is touching the tiles, we need to
+        // adjust here in order not to generate bounding boxes touching tiles or the test will fail
+
+        // compute tile at the top left
+        final Rectangle minTile = GeoTileUtils.toBoundingBox(
+            GeoTileUtils.getXTile(bbox.left(), tiles),
+            GeoTileUtils.getYTile(bbox.top(), tiles),
+            precision
+        );
+        // adjust if it is touching the tile
+        final int encodedLeft = encodeLongitude(bbox.left());
+        final double left = encodeLongitude(minTile.getMaxX()) == encodedLeft
+            ? decodeLongitude(encodedLeft + 1)
+            : decodeLongitude(encodedLeft);
+        final int encodedTop = encodeLatitude(bbox.top());
+        final double bottom = encodeLatitude(minTile.getMinY()) == encodedTop
+            ? decodeLongitude(encodedTop + 1)
+            : decodeLatitude(encodedTop);
+        // compute tile at the bottom right
+        final Rectangle maxTile = GeoTileUtils.toBoundingBox(
+            GeoTileUtils.getXTile(bbox.right(), tiles),
+            GeoTileUtils.getYTile(bbox.bottom(), tiles),
+            precision
+        );
+        // adjust if it is touching the tile
+        final int encodedRight = encodeLongitude(bbox.right());
+        final double right = encodeLongitude(maxTile.getMinX()) == encodedRight
+            ? decodeLongitude(encodedRight)
+            : decodeLongitude(encodedRight + 1);
+        final int encodedBottom = encodeLatitude(bbox.bottom());
+        final double top = encodeLatitude(maxTile.getMaxY()) == encodedBottom
+            ? decodeLatitude(encodedBottom)
+            : decodeLatitude(encodedBottom + 1);
+
         bbox.topLeft().reset(top, left);
         bbox.bottomRight().reset(bottom, right);
         return bbox;
@@ -62,14 +95,6 @@ public class GeoTileGridAggregatorTests extends GeoGridAggregatorTestCase<Intern
 
     @Override
     protected Rectangle getTile(double lng, double lat, int precision) {
-        int tiles = 1 << precision;
-        int x = GeoTileUtils.getXTile(lng, tiles);
-        int y = GeoTileUtils.getYTile(lat, tiles);
-        Rectangle r1 = GeoTileUtils.toBoundingBox(x, y, precision);
-        Rectangle r2 = GeoTileUtils.toBoundingBox(GeoTileUtils.longEncode(lng, lat, precision));
-        if (r1.equals(r2) == false) {
-            int a = 0;
-        }
         return GeoTileUtils.toBoundingBox(GeoTileUtils.longEncode(lng, lat, precision));
     }
 

+ 15 - 13
test/framework/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregatorTestCase.java

@@ -74,7 +74,7 @@ public abstract class GeoGridAggregatorTestCase<T extends InternalGeoGridBucket>
     /**
      * Return a random {@link GeoBoundingBox} within the bounds of the tile grid.
      */
-    protected abstract GeoBoundingBox randomBBox();
+    protected abstract GeoBoundingBox randomBBox(int precision);
 
     /**
      * Return the bounding tile as a {@link Rectangle} for a given point
@@ -131,23 +131,24 @@ public abstract class GeoGridAggregatorTestCase<T extends InternalGeoGridBucket>
     }
 
     public void testSingletonDocs() throws IOException {
-        testWithSeveralDocs(() -> true, null);
+        testWithSeveralDocs(() -> true, null, randomPrecision());
     }
 
     public void testBoundedSingletonDocs() throws IOException {
-        testWithSeveralDocs(() -> true, randomBBox());
+        int precision = randomPrecision();
+        testWithSeveralDocs(() -> true, randomBBox(precision), precision);
     }
 
     public void testMultiValuedDocs() throws IOException {
-        testWithSeveralDocs(LuceneTestCase::rarely, null);
+        testWithSeveralDocs(LuceneTestCase::rarely, null, randomPrecision());
     }
 
     public void testBoundedMultiValuedDocs() throws IOException {
-        testWithSeveralDocs(LuceneTestCase::rarely, randomBBox());
+        int precision = randomPrecision();
+        testWithSeveralDocs(LuceneTestCase::rarely, randomBBox(precision), precision);
     }
 
-    private void testWithSeveralDocs(BooleanSupplier supplier, GeoBoundingBox bbox) throws IOException {
-        int precision = randomPrecision();
+    private void testWithSeveralDocs(BooleanSupplier supplier, GeoBoundingBox bbox, int precision) throws IOException {
         int numPoints = randomIntBetween(8, 128);
         Map<String, Integer> expectedCountPerGeoHash = new HashMap<>();
         testCase(new MatchAllDocsQuery(), FIELD_NAME, precision, bbox, geoHashGrid -> {
@@ -185,23 +186,24 @@ public abstract class GeoGridAggregatorTestCase<T extends InternalGeoGridBucket>
     }
 
     public void testSingletonDocsAsSubAgg() throws IOException {
-        testWithSeveralDocsAsSubAgg(() -> true, null);
+        testWithSeveralDocsAsSubAgg(() -> true, null, randomPrecision());
     }
 
     public void testBoundedSingletonDocsAsSubAgg() throws IOException {
-        testWithSeveralDocsAsSubAgg(() -> true, randomBBox());
+        int precision = randomPrecision();
+        testWithSeveralDocsAsSubAgg(() -> true, randomBBox(precision), precision);
     }
 
     public void testMultiValuedDocsAsSubAgg() throws IOException {
-        testWithSeveralDocsAsSubAgg(LuceneTestCase::rarely, null);
+        testWithSeveralDocsAsSubAgg(LuceneTestCase::rarely, null, randomPrecision());
     }
 
     public void testBoundedMultiValuedDocsAsSubAgg() throws IOException {
-        testWithSeveralDocsAsSubAgg(LuceneTestCase::rarely, randomBBox());
+        int precision = randomPrecision();
+        testWithSeveralDocsAsSubAgg(LuceneTestCase::rarely, randomBBox(precision), precision);
     }
 
-    private void testWithSeveralDocsAsSubAgg(BooleanSupplier supplier, GeoBoundingBox bbox) throws IOException {
-        int precision = randomPrecision();
+    private void testWithSeveralDocsAsSubAgg(BooleanSupplier supplier, GeoBoundingBox bbox, int precision) throws IOException {
         int numPoints = randomIntBetween(8, 128);
         Map<String, Map<String, Long>> expectedCountPerTPerGeoHash = new TreeMap<>();
         TermsAggregationBuilder aggregationBuilder = new TermsAggregationBuilder("t").field("t").size(numPoints);

+ 1 - 1
x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoHexAggregatorTests.java

@@ -72,7 +72,7 @@ public class GeoHexAggregatorTests extends GeoGridAggregatorTestCase<InternalGeo
     }
 
     @Override
-    protected GeoBoundingBox randomBBox() {
+    protected GeoBoundingBox randomBBox(int precision) {
         GeoBoundingBox bbox = randomValueOtherThanMany(
             (b) -> b.top() > GeoTileUtils.LATITUDE_MASK || b.bottom() < -GeoTileUtils.LATITUDE_MASK,
             () -> {