Browse Source

Make FeatureFactory tests more resilient (#75405)

making sure the spatial relationships between the tile and the generated points is honour.
Ignacio Vera 4 years ago
parent
commit
9032b6f765

+ 3 - 3
x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/SimpleFeatureFactory.java

@@ -67,8 +67,7 @@ public class SimpleFeatureFactory {
         multiPoint.sort(Comparator.comparingDouble(Point::getLon).thenComparingDouble(Point::getLat));
         final int[] commands = new int[2 * multiPoint.size() + 1];
         int pos = 1, prevLon = 0, prevLat = 0, numPoints = 0;
-        for (int i = 0; i < multiPoint.size(); i++) {
-            final Point point = multiPoint.get(i);
+        for (Point point : multiPoint) {
             final int posLon = lon(point.getLon());
             if (posLon > extent || posLon < 0) {
                 continue;
@@ -77,7 +76,8 @@ public class SimpleFeatureFactory {
             if (posLat > extent || posLat < 0) {
                 continue;
             }
-            if (i == 0 || posLon != prevLon || posLat != prevLat) {
+            // filter out repeated points
+            if (numPoints == 0 || posLon != prevLon || posLat != prevLat) {
                 commands[pos++] = BitUtil.zigZagEncode(posLon - prevLon);
                 commands[pos++] = BitUtil.zigZagEncode(posLat - prevLat);
                 prevLon = posLon;

+ 22 - 12
x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java

@@ -31,33 +31,40 @@ import java.util.List;
 
 public class FeatureFactoryTests extends ESTestCase {
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/75325")
     public void testPoint() {
-        int z = randomIntBetween(1, 10);
+        int z = randomIntBetween(3, 10);
         int x = randomIntBetween(0, (1 << z) - 1);
         int y = randomIntBetween(0, (1 << z) - 1);
         int extent = randomIntBetween(1 << 8, 1 << 14);
         FeatureFactory builder = new FeatureFactory(z, x, y, extent);
         Rectangle rectangle = GeoTileUtils.toBoundingBox(x, y, z);
         {
-            double lat = randomValueOtherThanMany((l) -> rectangle.getMinY() > l || rectangle.getMaxY() < l, GeoTestUtil::nextLatitude);
-            double lon = randomValueOtherThanMany((l) -> rectangle.getMinX() > l || rectangle.getMaxX() < l, GeoTestUtil::nextLongitude);
+            double lat = randomValueOtherThanMany((l) -> rectangle.getMinY() >= l || rectangle.getMaxY() <= l, GeoTestUtil::nextLatitude);
+            double lon = randomValueOtherThanMany((l) -> rectangle.getMinX() >= l || rectangle.getMaxX() <= l, GeoTestUtil::nextLongitude);
             List<VectorTile.Tile.Feature> features = builder.getFeatures(new Point(lon, lat), new UserDataIgnoreConverter());
             assertThat(features.size(), Matchers.equalTo(1));
             VectorTile.Tile.Feature feature = features.get(0);
             assertThat(feature.getType(), Matchers.equalTo(VectorTile.Tile.GeomType.POINT));
         }
         {
-            double lat = randomValueOtherThanMany((l) -> rectangle.getMinY() <= l && rectangle.getMaxY() >= l, GeoTestUtil::nextLatitude);
-            double lon = randomValueOtherThanMany((l) -> rectangle.getMinX() <= l && rectangle.getMaxX() >= l, GeoTestUtil::nextLongitude);
+            int xNew = randomValueOtherThanMany(v -> Math.abs(v - x) < 2, () -> randomIntBetween(0, (1 << z) - 1));
+            int yNew = randomValueOtherThanMany(v -> Math.abs(v - y) < 2, () -> randomIntBetween(0, (1 << z) - 1));
+            Rectangle rectangleNew = GeoTileUtils.toBoundingBox(xNew, yNew, z);
+            double lat = randomValueOtherThanMany(
+                (l) -> rectangleNew.getMinY() >= l || rectangleNew.getMaxY() <= l,
+                GeoTestUtil::nextLatitude
+            );
+            double lon = randomValueOtherThanMany(
+                (l) -> rectangleNew.getMinX() >= l || rectangleNew.getMaxX() <= l,
+                GeoTestUtil::nextLongitude
+            );
             List<VectorTile.Tile.Feature> features = builder.getFeatures(new Point(lon, lat), new UserDataIgnoreConverter());
             assertThat(features.size(), Matchers.equalTo(0));
         }
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/75325")
     public void testMultiPoint() {
-        int z = randomIntBetween(1, 10);
+        int z = randomIntBetween(3, 10);
         int x = randomIntBetween(0, (1 << z) - 1);
         int y = randomIntBetween(0, (1 << z) - 1);
         int extent = randomIntBetween(1 << 8, 1 << 14);
@@ -66,8 +73,8 @@ public class FeatureFactoryTests extends ESTestCase {
         int numPoints = randomIntBetween(2, 10);
         {
             List<Point> points = new ArrayList<>();
-            double lat = randomValueOtherThanMany((l) -> rectangle.getMinY() > l || rectangle.getMaxY() < l, GeoTestUtil::nextLatitude);
-            double lon = randomValueOtherThanMany((l) -> rectangle.getMinX() > l || rectangle.getMaxX() < l, GeoTestUtil::nextLongitude);
+            double lat = randomValueOtherThanMany((l) -> rectangle.getMinY() >= l || rectangle.getMaxY() <= l, GeoTestUtil::nextLatitude);
+            double lon = randomValueOtherThanMany((l) -> rectangle.getMinX() >= l || rectangle.getMaxX() <= l, GeoTestUtil::nextLongitude);
             points.add(new Point(lon, lat));
             for (int i = 0; i < numPoints - 1; i++) {
                 points.add(new Point(GeoTestUtil.nextLongitude(), GeoTestUtil.nextLatitude()));
@@ -78,14 +85,17 @@ public class FeatureFactoryTests extends ESTestCase {
             assertThat(feature.getType(), Matchers.equalTo(VectorTile.Tile.GeomType.POINT));
         }
         {
+            int xNew = randomValueOtherThanMany(v -> Math.abs(v - x) < 2, () -> randomIntBetween(0, (1 << z) - 1));
+            int yNew = randomValueOtherThanMany(v -> Math.abs(v - y) < 2, () -> randomIntBetween(0, (1 << z) - 1));
+            Rectangle rectangleNew = GeoTileUtils.toBoundingBox(xNew, yNew, z);
             List<Point> points = new ArrayList<>();
             for (int i = 0; i < numPoints; i++) {
                 double lat = randomValueOtherThanMany(
-                    (l) -> rectangle.getMinY() <= l && rectangle.getMaxY() >= l,
+                    (l) -> rectangleNew.getMinY() >= l || rectangleNew.getMaxY() <= l,
                     GeoTestUtil::nextLatitude
                 );
                 double lon = randomValueOtherThanMany(
-                    (l) -> rectangle.getMinX() <= l && rectangle.getMaxX() >= l,
+                    (l) -> rectangleNew.getMinX() >= l || rectangleNew.getMaxX() <= l,
                     GeoTestUtil::nextLongitude
                 );
                 points.add(new Point(lon, lat));

+ 22 - 12
x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/SimpleFeatureFactoryTests.java

@@ -20,29 +20,36 @@ import java.util.List;
 
 public class SimpleFeatureFactoryTests extends ESTestCase {
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/75358")
     public void testPoint() throws IOException {
-        int z = randomIntBetween(1, 10);
+        int z = randomIntBetween(3, 10);
         int x = randomIntBetween(0, (1 << z) - 1);
         int y = randomIntBetween(0, (1 << z) - 1);
         int extent = randomIntBetween(1 << 8, 1 << 14);
         SimpleFeatureFactory builder = new SimpleFeatureFactory(z, x, y, extent);
         Rectangle rectangle = GeoTileUtils.toBoundingBox(x, y, z);
         {
-            double lat = randomValueOtherThanMany((l) -> rectangle.getMinY() > l || rectangle.getMaxY() < l, GeoTestUtil::nextLatitude);
-            double lon = randomValueOtherThanMany((l) -> rectangle.getMinX() > l || rectangle.getMaxX() < l, GeoTestUtil::nextLongitude);
+            double lat = randomValueOtherThanMany(l -> rectangle.getMinY() >= l || rectangle.getMaxY() <= l, GeoTestUtil::nextLatitude);
+            double lon = randomValueOtherThanMany(l -> rectangle.getMinX() >= l || rectangle.getMaxX() <= l, GeoTestUtil::nextLongitude);
             assertThat(builder.point(lon, lat).length, Matchers.greaterThan(0));
         }
         {
-            double lat = randomValueOtherThanMany((l) -> rectangle.getMinY() <= l && rectangle.getMaxY() >= l, GeoTestUtil::nextLatitude);
-            double lon = randomValueOtherThanMany((l) -> rectangle.getMinX() <= l && rectangle.getMaxX() >= l, GeoTestUtil::nextLongitude);
+            int xNew = randomValueOtherThanMany(v -> Math.abs(v - x) < 2, () -> randomIntBetween(0, (1 << z) - 1));
+            int yNew = randomValueOtherThanMany(v -> Math.abs(v - y) < 2, () -> randomIntBetween(0, (1 << z) - 1));
+            Rectangle rectangleNew = GeoTileUtils.toBoundingBox(xNew, yNew, z);
+            double lat = randomValueOtherThanMany(
+                l -> rectangleNew.getMinY() >= l || rectangleNew.getMaxY() <= l,
+                GeoTestUtil::nextLatitude
+            );
+            double lon = randomValueOtherThanMany(
+                (l) -> rectangleNew.getMinX() >= l || rectangleNew.getMaxX() <= l,
+                GeoTestUtil::nextLongitude
+            );
             assertThat(builder.point(lon, lat).length, Matchers.equalTo(0));
         }
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/75358")
     public void testMultiPoint() throws IOException {
-        int z = randomIntBetween(1, 10);
+        int z = randomIntBetween(3, 10);
         int x = randomIntBetween(0, (1 << z) - 1);
         int y = randomIntBetween(0, (1 << z) - 1);
         int extent = randomIntBetween(1 << 8, 1 << 14);
@@ -51,8 +58,8 @@ public class SimpleFeatureFactoryTests extends ESTestCase {
         int numPoints = randomIntBetween(2, 10);
         {
             List<Point> points = new ArrayList<>();
-            double lat = randomValueOtherThanMany((l) -> rectangle.getMinY() > l || rectangle.getMaxY() < l, GeoTestUtil::nextLatitude);
-            double lon = randomValueOtherThanMany((l) -> rectangle.getMinX() > l || rectangle.getMaxX() < l, GeoTestUtil::nextLongitude);
+            double lat = randomValueOtherThanMany((l) -> rectangle.getMinY() >= l || rectangle.getMaxY() <= l, GeoTestUtil::nextLatitude);
+            double lon = randomValueOtherThanMany((l) -> rectangle.getMinX() >= l || rectangle.getMaxX() <= l, GeoTestUtil::nextLongitude);
             points.add(new Point(lon, lat));
             for (int i = 0; i < numPoints - 1; i++) {
                 points.add(new Point(GeoTestUtil.nextLongitude(), GeoTestUtil.nextLatitude()));
@@ -60,14 +67,17 @@ public class SimpleFeatureFactoryTests extends ESTestCase {
             assertThat(builder.points(points).length, Matchers.greaterThan(0));
         }
         {
+            int xNew = randomValueOtherThanMany(v -> Math.abs(v - x) < 2, () -> randomIntBetween(0, (1 << z) - 1));
+            int yNew = randomValueOtherThanMany(v -> Math.abs(v - y) < 2, () -> randomIntBetween(0, (1 << z) - 1));
+            Rectangle rectangleNew = GeoTileUtils.toBoundingBox(xNew, yNew, z);
             List<Point> points = new ArrayList<>();
             for (int i = 0; i < numPoints; i++) {
                 double lat = randomValueOtherThanMany(
-                    (l) -> rectangle.getMinY() <= l && rectangle.getMaxY() >= l,
+                    (l) -> rectangleNew.getMinY() >= l || rectangleNew.getMaxY() <= l,
                     GeoTestUtil::nextLatitude
                 );
                 double lon = randomValueOtherThanMany(
-                    (l) -> rectangle.getMinX() <= l && rectangle.getMaxX() >= l,
+                    (l) -> rectangleNew.getMinX() >= l || rectangleNew.getMaxX() <= l,
                     GeoTestUtil::nextLongitude
                 );
                 points.add(new Point(lon, lat));