From 40a2f149b41d79feb2a54bbde288f6cf74045b38 Mon Sep 17 00:00:00 2001 From: Heemin Kim Date: Wed, 10 May 2023 15:10:52 -0700 Subject: [PATCH] Fix flaky test, testIndexingMultiPolygon #294 Signed-off-by: Heemin Kim --- CHANGELOG.md | 1 + .../opensearch/geospatial/ShapeTestUtil.java | 302 ++++++++++++++++++ .../common/xyshape/ShapeObjectBuilder.java | 152 ++++++++- .../XYShapeIndexableFieldsVisitorTests.java | 8 +- 4 files changed, 458 insertions(+), 5 deletions(-) create mode 100644 src/test/java/org/opensearch/geospatial/ShapeTestUtil.java diff --git a/CHANGELOG.md b/CHANGELOG.md index cc43d7dc..4526e876 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Features ### Enhancements ### Bug Fixes +* Fix flaky test, testIndexingMultiPolygon ([#483](https://github.com/opensearch-project/geospatial/pull/483)) ### Infrastructure ### Documentation ### Maintenance diff --git a/src/test/java/org/opensearch/geospatial/ShapeTestUtil.java b/src/test/java/org/opensearch/geospatial/ShapeTestUtil.java new file mode 100644 index 00000000..29519c0b --- /dev/null +++ b/src/test/java/org/opensearch/geospatial/ShapeTestUtil.java @@ -0,0 +1,302 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.geospatial; + +import java.util.ArrayList; +import java.util.Random; + +import org.apache.lucene.geo.GeoUtils; +import org.apache.lucene.geo.XYCircle; +import org.apache.lucene.geo.XYEncodingUtils; +import org.apache.lucene.geo.XYLine; +import org.apache.lucene.geo.XYPoint; +import org.apache.lucene.geo.XYPolygon; +import org.apache.lucene.geo.XYRectangle; +import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.lucene.tests.util.TestUtil; + +import com.carrotsearch.randomizedtesting.RandomizedContext; +import com.carrotsearch.randomizedtesting.generators.BiasedNumbers; + +/** + * This class is copied version of {@link org.apache.lucene.tests.geo.ShapeTestUtil} + * See https://github.com/apache/lucene/pull/12287, https://github.com/apache/lucene/issues/12596 + */ +public class ShapeTestUtil { + + /** returns next pseudorandom polygon */ + public static XYPolygon nextPolygon() { + Random random = random(); + if (random.nextBoolean()) { + return surpriseMePolygon(random); + } else if (LuceneTestCase.TEST_NIGHTLY && random.nextInt(10) == 1) { + // this poly is slow to create ... only do it 10% of the time: + while (true) { + int gons = TestUtil.nextInt(random, 4, 500); + // So the poly can cover at most 50% of the earth's surface: + double radius = random.nextDouble() * 0.5 * Float.MAX_VALUE + 1.0; + try { + return createRegularPolygon(nextFloat(random), nextFloat(random), radius, gons); + } catch (@SuppressWarnings("unused") IllegalArgumentException iae) { + // something went wrong, try again + } + } + } + + XYRectangle box = nextBox(random); + if (random.nextBoolean()) { + // box + return boxPolygon(box); + } else { + // triangle + return trianglePolygon(box); + } + } + + public static XYPoint nextPoint() { + Random random = random(); + float x = nextFloat(random); + float y = nextFloat(random); + return new XYPoint(x, y); + } + + public static XYLine nextLine() { + XYPolygon poly = org.apache.lucene.tests.geo.ShapeTestUtil.nextPolygon(); + float[] x = new float[poly.numPoints() - 1]; + float[] y = new float[x.length]; + for (int i = 0; i < x.length; ++i) { + x[i] = poly.getPolyX(i); + y[i] = poly.getPolyY(i); + } + return new XYLine(x, y); + } + + public static XYCircle nextCircle() { + Random random = random(); + float x = nextFloat(random); + float y = nextFloat(random); + float radius = 0; + while (radius == 0) { + radius = random().nextFloat() * Float.MAX_VALUE / 2; + } + assert radius != 0; + return new XYCircle(x, y, radius); + } + + private static XYPolygon trianglePolygon(XYRectangle box) { + final float[] polyX = new float[4]; + final float[] polyY = new float[4]; + polyX[0] = box.minX; + polyY[0] = box.minY; + polyX[1] = box.maxX; + polyY[1] = box.minY; + polyX[2] = box.maxX; + polyY[2] = box.maxY; + polyX[3] = box.minX; + polyY[3] = box.minY; + return new XYPolygon(polyX, polyY); + } + + public static XYRectangle nextBox(Random random) { + // prevent lines instead of boxes + float x0 = nextFloat(random); + float x1 = nextFloat(random); + while (x0 == x1) { + x1 = nextFloat(random); + } + // prevent lines instead of boxes + float y0 = nextFloat(random); + float y1 = nextFloat(random); + while (y0 == y1) { + y1 = nextFloat(random); + } + + if (x1 < x0) { + float x = x0; + x0 = x1; + x1 = x; + } + + if (y1 < y0) { + float y = y0; + y0 = y1; + y1 = y; + } + + return new XYRectangle(x0, x1, y0, y1); + } + + private static XYPolygon boxPolygon(XYRectangle box) { + final float[] polyX = new float[5]; + final float[] polyY = new float[5]; + polyX[0] = box.minX; + polyY[0] = box.minY; + polyX[1] = box.maxX; + polyY[1] = box.minY; + polyX[2] = box.maxX; + polyY[2] = box.maxY; + polyX[3] = box.minX; + polyY[3] = box.maxY; + polyX[4] = box.minX; + polyY[4] = box.minY; + + return new XYPolygon(polyX, polyY); + } + + private static XYPolygon surpriseMePolygon(Random random) { + while (true) { + float centerX = nextFloat(random); + float centerY = nextFloat(random); + double radius = 0.1 + 20 * random.nextDouble(); + double radiusDelta = random.nextDouble(); + + ArrayList xList = new ArrayList<>(); + ArrayList yList = new ArrayList<>(); + double angle = 0.0; + while (true) { + angle += random.nextDouble() * 40.0; + if (angle > 360) { + break; + } + double len = radius * (1.0 - radiusDelta + radiusDelta * random.nextDouble()); + float maxX = StrictMath.min(StrictMath.abs(Float.MAX_VALUE - centerX), StrictMath.abs(-Float.MAX_VALUE - centerX)); + float maxY = StrictMath.min(StrictMath.abs(Float.MAX_VALUE - centerY), StrictMath.abs(-Float.MAX_VALUE - centerY)); + + len = StrictMath.min(len, StrictMath.min(maxX, maxY)); + + float x = (float) (centerX + len * Math.cos(Math.toRadians(angle))); + float y = (float) (centerY + len * Math.sin(Math.toRadians(angle))); + + xList.add(x); + yList.add(y); + } + + // close it + xList.add(xList.get(0)); + yList.add(yList.get(0)); + + float[] xArray = new float[xList.size()]; + float[] yArray = new float[yList.size()]; + for (int i = 0; i < xList.size(); i++) { + xArray[i] = xList.get(i); + yArray[i] = yList.get(i); + } + return new XYPolygon(xArray, yArray); + } + } + + /** + * Makes an n-gon, centered at the provided x/y, and each vertex approximately distanceMeters away + * from the center. + * + *

Do not invoke me across the dateline or a pole!! + */ + public static XYPolygon createRegularPolygon(double centerX, double centerY, double radius, int gons) { + + double maxX = StrictMath.min(StrictMath.abs(Float.MAX_VALUE - centerX), StrictMath.abs(-Float.MAX_VALUE - centerX)); + double maxY = StrictMath.min(StrictMath.abs(Float.MAX_VALUE - centerY), StrictMath.abs(-Float.MAX_VALUE - centerY)); + + radius = StrictMath.min(radius, StrictMath.min(maxX, maxY)); + + float[][] result = new float[2][]; + result[0] = new float[gons + 1]; + result[1] = new float[gons + 1]; + // System.out.println("make gon=" + gons); + for (int i = 0; i < gons; i++) { + double angle = 360.0 - i * (360.0 / gons); + // System.out.println(" angle " + angle); + double x = Math.cos(StrictMath.toRadians(angle)); + double y = Math.sin(StrictMath.toRadians(angle)); + result[0][i] = (float) (centerY + y * radius); + result[1][i] = (float) (centerX + x * radius); + } + + // close poly + result[0][gons] = result[0][0]; + result[1][gons] = result[1][0]; + + return new XYPolygon(result[0], result[1]); + } + + public static float nextFloat(Random random) { + return BiasedNumbers.randomFloatBetween(random, -Float.MAX_VALUE, Float.MAX_VALUE); + } + + /** Keep it simple, we don't need to take arbitrary Random for geo tests */ + private static Random random() { + return RandomizedContext.current().getRandom(); + } + + /** Simple slow point in polygon check (for testing) */ + // direct port of PNPOLY C code (https://www.ecse.rpi.edu/~wrf/Research/Short_Notes/pnpoly.html) + // this allows us to improve the code yet still ensure we have its properties + // it is under the BSD license + // (https://www.ecse.rpi.edu/~wrf/Research/Short_Notes/pnpoly.html#License%20to%20Use) + // + // Copyright (c) 1970-2003, Wm. Randolph Franklin + // + // Permission is hereby granted, free of charge, to any person obtaining a copy of this software + // and associated + // documentation files (the "Software"), to deal in the Software without restriction, including + // without limitation + // the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + // the Software, and + // to permit persons to whom the Software is furnished to do so, subject to the following + // conditions: + // + // 1. Redistributions of source code must retain the above copyright + // notice, this list of conditions and the following disclaimers. + // 2. Redistributions in binary form must reproduce the above copyright + // notice in the documentation and/or other materials provided with + // the distribution. + // 3. The name of W. Randolph Franklin may not be used to endorse or + // promote products derived from this Software without specific + // prior written permission. + // + // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING + // BUT NOT LIMITED + // TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN + // NO EVENT SHALL + // THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + // IN AN ACTION OF + // CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE + // OR OTHER DEALINGS + // IN THE SOFTWARE. + public static boolean containsSlowly(XYPolygon polygon, double x, double y) { + if (polygon.getHoles().length > 0) { + throw new UnsupportedOperationException("this testing method does not support holes"); + } + double[] polyXs = XYEncodingUtils.floatArrayToDoubleArray(polygon.getPolyX()); + double[] polyYs = XYEncodingUtils.floatArrayToDoubleArray(polygon.getPolyY()); + // bounding box check required due to rounding errors (we don't solve that problem) + if (x < polygon.minX || x > polygon.maxX || y < polygon.minY || y > polygon.maxY) { + return false; + } + + boolean c = false; + int i, j; + int nvert = polyYs.length; + double[] verty = polyYs; + double[] vertx = polyXs; + double testy = y; + double testx = x; + for (i = 0, j = 1; j < nvert; ++i, ++j) { + if (testy == verty[j] && testy == verty[i] + || ((testy <= verty[j] && testy >= verty[i]) != (testy >= verty[j] && testy <= verty[i]))) { + if ((testx == vertx[j] && testx == vertx[i]) + || ((testx <= vertx[j] && testx >= vertx[i]) != (testx >= vertx[j] && testx <= vertx[i]) + && GeoUtils.orient(vertx[i], verty[i], vertx[j], verty[j], testx, testy) == 0)) { + // return true if point is on boundary + return true; + } else if (((verty[i] > testy) != (verty[j] > testy)) + && (testx < (vertx[j] - vertx[i]) * (testy - verty[i]) / (verty[j] - verty[i]) + vertx[i])) { + c = !c; + } + } + } + return c; + } +} 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 index b40b91df..ad52179d 100644 --- 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; @@ -15,7 +16,11 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; +import lombok.SneakyThrows; + +import org.apache.lucene.geo.Tessellator; import org.apache.lucene.geo.XYPoint; +import org.apache.lucene.geo.XYPolygon; import org.opensearch.common.Randomness; import org.opensearch.common.geo.ShapeRelation; import org.opensearch.geo.GeometryTestUtils; @@ -31,6 +36,8 @@ import org.opensearch.geometry.Polygon; import org.opensearch.geometry.Rectangle; import org.opensearch.geometry.utils.WellKnownText; +import org.opensearch.geospatial.GeospatialTestHelper; +import org.opensearch.geospatial.ShapeTestUtil; import org.opensearch.test.OpenSearchTestCase; import com.carrotsearch.randomizedtesting.generators.RandomPicks; @@ -111,7 +118,18 @@ public static MultiLine randomMultiLine(int size, int maximumLines, boolean hasZ return new MultiLine(lines); } - // Generating truly random polygon is complicated, hence, will be randomly selecting from with holes, without holes and geo-polygon + /** + * Generate polygon in double range + * + * Generating truly random polygon is complicated, hence, will be randomly selecting from with holes, without holes and geo-polygon + * If you try to index the 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. + * Use {@link #randomXYPolygon()} if you need to index a polygon. + * + * @return Randomly generated polygon in double range + * @throws IOException + * @throws ParseException + */ public static Polygon randomPolygon() throws IOException, ParseException { return (Polygon) RandomPicks.randomFrom( Randomness.get(), @@ -120,6 +138,36 @@ public static Polygon randomPolygon() throws IOException, ParseException { ); } + /** + * Generate polygon in float range + * + * Generating truly random polygon is complicated, hence, will be randomly selecting from with holes, without holes and geo-polygon + * You need to use this method to test indexing polygon as lucene XYPolygon support only float range. + * + * @return Randomly generated polygon in float range + * @throws IOException + * @throws ParseException + */ + public static Polygon randomXYPolygon() throws IOException, ParseException { + return (Polygon) RandomPicks.randomFrom( + Randomness.get(), + // TODO: Support z coordinates to be added to polygon + List.of(getPolygonWithHoles(), getPolygonWithoutHoles(), validRandomXYPolygon(false)) + ); + } + + /** + * 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. + * + * Use {@link #randomMultiXYPolygon()} if you need to index a multi 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 +176,108 @@ 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 polygons = new ArrayList<>(); + for (int i = 0; i < size; i++) { + polygons.add(validRandomXYPolygon(hasAlt)); + } + return new MultiPolygon(polygons); + } + + /** + * Temporary method to avoid invalid polygon returned by {@link #randomXYPolygon} + * @param hasAlt + * @return + */ + // TODO: Implement randomXYPolygon to return only valid polygon and remove this method + @SneakyThrows + private static Polygon validRandomXYPolygon(boolean hasAlt) { + Polygon polygon = randomXYPolygon(hasAlt); + try { + XYPolygon xyPolygon = XYShapeConverter.toXYPolygon(polygon); + // Test validity of polygon + Tessellator.tessellate(xyPolygon, true); + return polygon; + } catch (Exception e) { + return (Polygon) RandomPicks.randomFrom(Randomness.get(), List.of(getPolygonWithHoles(), getPolygonWithoutHoles())); + } + } + + /** + * 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 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/src/test/java/org/opensearch/geospatial/index/mapper/xyshape/XYShapeIndexableFieldsVisitorTests.java b/src/test/java/org/opensearch/geospatial/index/mapper/xyshape/XYShapeIndexableFieldsVisitorTests.java index ce2eadc0..35c1b6cd 100644 --- a/src/test/java/org/opensearch/geospatial/index/mapper/xyshape/XYShapeIndexableFieldsVisitorTests.java +++ b/src/test/java/org/opensearch/geospatial/index/mapper/xyshape/XYShapeIndexableFieldsVisitorTests.java @@ -11,10 +11,10 @@ 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; +import static org.opensearch.geospatial.index.common.xyshape.ShapeObjectBuilder.randomXYPolygon; import java.io.IOException; import java.text.ParseException; @@ -110,14 +110,14 @@ public void testIndexingMultiPoint() { } public void testIndexingPolygon() throws IOException, ParseException { - Polygon geometry = randomPolygon(); + Polygon geometry = randomXYPolygon(); final IndexableField[] indexableFields = visitor.visit(geometry); assertNotNull("indexable field cannot be null", indexableFields); assertTrue("indexable field list cannot be empty", indexableFields.length > 0); } 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());