Fix flaky test, testIndexingMultiPolygon opensearch-project#294

Signed-off-by: Heemin Kim <[email protected]>
heemin32 · May 10, 2023 · d1d4c6c · d1d4c6c
1 parent 7ee2f13
commit d1d4c6c
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 2 deletions.
diff --git a/src/test/java/org/opensearch/geospatial/index/common/xyshape/ShapeObjectBuilder.java b/src/test/java/org/opensearch/geospatial/index/common/xyshape/ShapeObjectBuilder.java
@@ -7,6 +7,7 @@
 
 import static com.carrotsearch.randomizedtesting.RandomizedTest.randomDouble;
 import static com.carrotsearch.randomizedtesting.RandomizedTest.randomIntBetween;
+import static org.opensearch.test.OpenSearchTestCase.randomValueOtherThanMany;
 
 import java.io.IOException;
 import java.text.ParseException;
@@ -16,6 +17,7 @@
 import java.util.stream.IntStream;
 
 import org.apache.lucene.geo.XYPoint;
+import org.apache.lucene.tests.geo.ShapeTestUtil;
 import org.opensearch.common.Randomness;
 import org.opensearch.common.geo.ShapeRelation;
 import org.opensearch.geo.GeometryTestUtils;
@@ -31,6 +33,7 @@
 import org.opensearch.geometry.Polygon;
 import org.opensearch.geometry.Rectangle;
 import org.opensearch.geometry.utils.WellKnownText;
+import org.opensearch.geospatial.GeospatialTestHelper;
 import org.opensearch.test.OpenSearchTestCase;
 
 import com.carrotsearch.randomizedtesting.generators.RandomPicks;
@@ -120,6 +123,16 @@ public static Polygon randomPolygon() throws IOException, ParseException {
         );
     }
 
+    /**
+     * Generate multi polygon in double range
+     *
+     * If you try to index the multi polygon returned by this method, it might fail because
+     * polygon point will be cast to float type and lose its original value to form a correct polygon.
+     *
+     * @return Randomly generated multi polygon in double range
+     * @throws IOException
+     * @throws ParseException
+     */
     public static MultiPolygon randomMultiPolygon() throws IOException, ParseException {
         return (MultiPolygon) RandomPicks.randomFrom(
             Randomness.get(),
@@ -128,6 +141,89 @@ public static MultiPolygon randomMultiPolygon() throws IOException, ParseExcepti
         );
     }
 
+    /**
+     * Generate multi polygon in float range
+     *
+     * You need to use this method to test indexing multi polygon as lucene XYPolygon support only float range.
+     *
+     * @return Randomly generated multi polygon in float range
+     * @throws IOException
+     * @throws ParseException
+     */
+    public static MultiPolygon randomMultiXYPolygon() throws IOException, ParseException {
+        return (MultiPolygon) RandomPicks.randomFrom(
+            Randomness.get(),
+            // TODO: Support z coordinates to be added to Multi polygon
+            List.of(getMultiPolygon(), randomMultiXYPolygon(false))
+        );
+    }
+
+    /**
+     * Copied from {@code org.opensearch.geo.GeometryTestUtils#randomMultiPolygon} with changes
+     * from calling {@code org.opensearch.geo.GeometryTestUtils#randomPolygon} to
+     * calling {@code org.opensearch.geospatial.index.common.xyshape.ShapeObjectBuilder#randomXyPolygon}
+     */
+    private static MultiPolygon randomMultiXYPolygon(final boolean hasAlt) {
+        int size = OpenSearchTestCase.randomIntBetween(3, 10);
+        List<Polygon> polygons = new ArrayList<>();
+        for (int i = 0; i < size; i++) {
+            polygons.add(randomXYPolygon(hasAlt));
+        }
+        return new MultiPolygon(polygons);
+    }
+
+    /**
+     * Copied from {@code org.opensearch.geo.GeometryTestUtils#randomPolygon} with changes
+     * from {@code org.apache.lucene.geo.Polygon} to {@code org.apache.lucene.geo.XYPolygon}
+     */
+    private static Polygon randomXYPolygon(boolean hasAlt) {
+        org.apache.lucene.geo.XYPolygon luceneXYPolygon = randomValueOtherThanMany(p -> area(p) == 0, ShapeTestUtil::nextPolygon);
+        if (luceneXYPolygon.numHoles() > 0) {
+            org.apache.lucene.geo.XYPolygon[] luceneHoles = luceneXYPolygon.getHoles();
+            List<LinearRing> holes = new ArrayList<>();
+            for (int i = 0; i < luceneXYPolygon.numHoles(); i++) {
+                org.apache.lucene.geo.XYPolygon xyPoly = luceneHoles[i];
+                holes.add(
+                    GeometryTestUtils.linearRing(
+                        GeospatialTestHelper.toDoubleArray(xyPoly.getPolyX()),
+                        GeospatialTestHelper.toDoubleArray(xyPoly.getPolyY()),
+                        hasAlt
+                    )
+                );
+            }
+            return new Polygon(
+                GeometryTestUtils.linearRing(
+                    GeospatialTestHelper.toDoubleArray(luceneXYPolygon.getPolyX()),
+                    GeospatialTestHelper.toDoubleArray(luceneXYPolygon.getPolyY()),
+                    hasAlt
+                ),
+                holes
+            );
+        }
+        return new Polygon(
+            GeometryTestUtils.linearRing(
+                GeospatialTestHelper.toDoubleArray(luceneXYPolygon.getPolyX()),
+                GeospatialTestHelper.toDoubleArray(luceneXYPolygon.getPolyY()),
+                hasAlt
+            )
+        );
+    }
+
+    /**
+     * Copied from {@code org.opensearch.geo.GeometryTestUtils#area} with changes
+     * from {@code org.apache.lucene.geo.Polygon} to {@code org.apache.lucene.geo.XYPolygon}
+     */
+    private static double area(org.apache.lucene.geo.XYPolygon luceneXYPolygon) {
+        double windingSum = 0;
+        final int numPts = luceneXYPolygon.numPoints() - 1;
+        for (int i = 0; i < numPts; i++) {
+            // compute signed area
+            windingSum += luceneXYPolygon.getPolyX(i) * luceneXYPolygon.getPolyY(i + 1) - luceneXYPolygon.getPolyY(i) * luceneXYPolygon
+                .getPolyX(i + 1);
+        }
+        return Math.abs(windingSum / 2);
+    }
+
     public static ShapeRelation randomShapeRelation() {
         return RandomPicks.randomFrom(
             Randomness.get(),

diff --git a/...va/org/opensearch/geospatial/index/mapper/xyshape/XYShapeIndexableFieldsVisitorTests.java b/...va/org/opensearch/geospatial/index/mapper/xyshape/XYShapeIndexableFieldsVisitorTests.java
@@ -11,7 +11,7 @@
 import static org.opensearch.geospatial.index.common.xyshape.ShapeObjectBuilder.randomLinearRing;
 import static org.opensearch.geospatial.index.common.xyshape.ShapeObjectBuilder.randomMultiLine;
 import static org.opensearch.geospatial.index.common.xyshape.ShapeObjectBuilder.randomMultiPoint;
-import static org.opensearch.geospatial.index.common.xyshape.ShapeObjectBuilder.randomMultiPolygon;
+import static org.opensearch.geospatial.index.common.xyshape.ShapeObjectBuilder.randomMultiXYPolygon;
 import static org.opensearch.geospatial.index.common.xyshape.ShapeObjectBuilder.randomPoint;
 import static org.opensearch.geospatial.index.common.xyshape.ShapeObjectBuilder.randomPolygon;
 import static org.opensearch.geospatial.index.common.xyshape.ShapeObjectBuilder.randomRectangle;
@@ -117,7 +117,7 @@ public void testIndexingPolygon() throws IOException, ParseException {
     }
 
     public void testIndexingMultiPolygon() throws IOException, ParseException {
-        MultiPolygon geometry = randomMultiPolygon();
+        MultiPolygon geometry = randomMultiXYPolygon();
         final IndexableField[] indexableFields = visitor.visit(geometry);
         assertNotNull("indexable field cannot be null", indexableFields);
         assertTrue("indexable field list cannot be empty", indexableFields.length >= geometry.size());