ESQL: Use Point geometry in tests (#104120)

iverase · web-flow · commit eecac06b5ae6 · 2024-01-09T12:45:36.000+01:00
Replace SpatialPoint abstraction with Point geometries.
diff --git a/server/src/test/java/org/elasticsearch/common/geo/SpatialPointTests.java b/server/src/test/java/org/elasticsearch/common/geo/SpatialPointTests.java
@@ -8,6 +8,7 @@
 
 package org.elasticsearch.common.geo;
 
+import org.apache.lucene.tests.geo.GeoTestUtil;
 import org.elasticsearch.test.ESTestCase;
 
 import static org.hamcrest.Matchers.equalTo;
@@ -32,7 +33,7 @@ public void testEqualsAndHashcode() {
 
     public void testCompareTo() {
         for (int i = 0; i < 100; i++) {
-            SpatialPoint point = randomValueOtherThanMany(p -> p.getX() < -170 || p.getX() > 170, ESTestCase::randomGeoPoint);
+            SpatialPoint point = randomValueOtherThanMany(p -> p.getX() < -170 || p.getX() > 170, SpatialPointTests::randomGeoPoint);
             GeoPoint smaller = new GeoPoint(point.getY(), point.getX() - 1);
             GeoPoint bigger = new GeoPoint(point.getY(), point.getX() + 1);
             TestPoint testSmaller = new TestPoint(smaller);
@@ -58,6 +59,10 @@ private void assertNotEqualsAndHashcode(String message, SpatialPoint a, SpatialP
         assertThat("Compare: " + message, a.compareTo(b), not(equalTo(0)));
     }
 
+    private static GeoPoint randomGeoPoint() {
+        return new GeoPoint(GeoTestUtil.nextLatitude(), GeoTestUtil.nextLongitude());
+    }
+
     /**
      * This test class used to be trivial, when SpatialPoint was a concrete class.
      * If we ever revert back to a concrete class, we can simplify this test class.
diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java
@@ -51,8 +51,6 @@
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.bytes.CompositeBytesReference;
-import org.elasticsearch.common.geo.GeoPoint;
-import org.elasticsearch.common.geo.SpatialPoint;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.NamedWriteable;
 import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
@@ -1194,34 +1192,6 @@ public static String randomDateFormatterPattern() {
         return randomFrom(FormatNames.values()).getName();
     }
 
-    /**
-     * Generate a random valid point constrained to geographic ranges (lat, lon ranges).
-     */
-    public static SpatialPoint randomGeoPoint() {
-        double lat = randomDoubleBetween(-90, 90, true);
-        double lon = randomDoubleBetween(-180, 180, true);
-        return new GeoPoint(lat, lon);
-    }
-
-    /**
-     * Generate a random valid point constrained to cartesian ranges.
-     */
-    public static SpatialPoint randomCartesianPoint() {
-        double x = randomDoubleBetween(-Float.MAX_VALUE, Float.MAX_VALUE, true);
-        double y = randomDoubleBetween(-Float.MAX_VALUE, Float.MAX_VALUE, true);
-        return new SpatialPoint() {
-            @Override
-            public double getX() {
-                return x;
-            }
-
-            @Override
-            public double getY() {
-                return y;
-            }
-        };
-    }
-
     /**
      * helper to randomly perform on <code>consumer</code> with <code>value</code>
      */
diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BasicBlockTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BasicBlockTests.java
@@ -9,7 +9,6 @@
 
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.breaker.CircuitBreaker;
-import org.elasticsearch.common.geo.SpatialPoint;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.util.BitArray;
@@ -22,6 +21,9 @@
 import org.elasticsearch.core.RefCounted;
 import org.elasticsearch.core.Releasable;
 import org.elasticsearch.core.Releasables;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geo.ShapeTestUtils;
+import org.elasticsearch.geometry.Point;
 import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.test.ESTestCase;
 import org.junit.After;
@@ -445,11 +447,11 @@ public void testBytesRefBlock() {
     }
 
     public void testBytesRefBlockOnGeoPoints() {
-        testBytesRefBlock(() -> GEO.pointAsWKB(randomGeoPoint()), false, GEO::wkbAsString);
+        testBytesRefBlock(() -> GEO.pointAsWKB(GeometryTestUtils.randomPoint()), false, GEO::wkbAsString);
     }
 
     public void testBytesRefBlockOnCartesianPoints() {
-        testBytesRefBlock(() -> CARTESIAN.pointAsWKB(randomCartesianPoint()), false, CARTESIAN::wkbAsString);
+        testBytesRefBlock(() -> CARTESIAN.pointAsWKB(ShapeTestUtils.randomPoint()), false, CARTESIAN::wkbAsString);
     }
 
     public void testBytesRefBlockBuilderWithNulls() {
@@ -895,7 +897,7 @@ public static RandomBlock randomBlock(
         List<List<Object>> values = new ArrayList<>();
         try (var builder = elementType.newBlockBuilder(positionCount, blockFactory)) {
             boolean bytesRefFromPoints = randomBoolean();
-            Supplier<SpatialPoint> pointSupplier = randomBoolean() ? ESTestCase::randomGeoPoint : ESTestCase::randomCartesianPoint;
+            Supplier<Point> pointSupplier = randomBoolean() ? GeometryTestUtils::randomPoint : ShapeTestUtils::randomPoint;
             for (int p = 0; p < positionCount; p++) {
                 int valueCount = between(minValuesPerPosition, maxValuesPerPosition);
                 if (valueCount == 0 || nullAllowed && randomBoolean()) {
diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/topn/TopNEncoderTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/topn/TopNEncoderTests.java
@@ -11,8 +11,9 @@
 
 import org.apache.lucene.document.InetAddressPoint;
 import org.apache.lucene.util.BytesRef;
-import org.elasticsearch.common.geo.SpatialPoint;
 import org.elasticsearch.compute.operator.BreakingBytesRefBuilder;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geo.ShapeTestUtils;
 import org.elasticsearch.geometry.Point;
 import org.elasticsearch.geometry.utils.WellKnownBinary;
 import org.elasticsearch.test.ESTestCase;
@@ -139,8 +140,8 @@ static Version randomVersion() {
     }
 
     static BytesRef randomPointAsWKB() {
-        SpatialPoint point = randomBoolean() ? randomGeoPoint() : randomCartesianPoint();
-        byte[] wkb = WellKnownBinary.toWKB(new Point(point.getX(), point.getY()), ByteOrder.LITTLE_ENDIAN);
+        Point point = randomBoolean() ? GeometryTestUtils.randomPoint() : ShapeTestUtils.randomPoint();
+        byte[] wkb = WellKnownBinary.toWKB(point, ByteOrder.LITTLE_ENDIAN);
         return new BytesRef(wkb);
     }
 }
diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlSpecTestCase.java
@@ -12,7 +12,6 @@
 import org.elasticsearch.Version;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.ResponseException;
-import org.elasticsearch.common.geo.SpatialPoint;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.logging.LogManager;
 import org.elasticsearch.logging.Logger;
@@ -162,31 +161,7 @@ protected void assertResults(
         Logger logger
     ) {
         assertMetadata(expected, actualColumns, logger);
-        assertData(expected, actualValues, testCase.ignoreOrder, logger, EsqlSpecTestCase::valueToString);
-    }
-
-    /**
-     * Unfortunately the GeoPoint.toString method returns the old format, but cannot be changed due to BWC.
-     * So we need to custom format GeoPoint as well as wrap Lists to ensure this custom conversion applies to multi-value fields
-     */
-    private static String valueToString(Object value) {
-        if (value == null) {
-            return "null";
-        } else if (value instanceof List<?> list) {
-            StringBuilder sb = new StringBuilder("[");
-            for (Object field : list) {
-                if (sb.length() > 1) {
-                    sb.append(", ");
-                }
-                sb.append(valueToString(field));
-            }
-            return sb.append("]").toString();
-        } else if (value instanceof SpatialPoint point) {
-            // Alternatively we could just change GeoPoint.toString() to use WKT, but that has other side-effects
-            return point.toWKT();
-        } else {
-            return value.toString();
-        }
+        assertData(expected, actualValues, testCase.ignoreOrder, logger, value -> value == null ? "null" : value.toString());
     }
 
     private Throwable reworkException(Throwable th) {
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryResponseTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/action/EsqlQueryResponseTests.java
@@ -33,6 +33,8 @@
 import org.elasticsearch.compute.operator.DriverStatus;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.Releasables;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geo.ShapeTestUtils;
 import org.elasticsearch.test.AbstractChunkedSerializingTestCase;
 import org.elasticsearch.xcontent.InstantiatingObjectParser;
 import org.elasticsearch.xcontent.ObjectParser;
@@ -148,8 +150,10 @@ private Page randomPage(List<ColumnInfo> columns) {
                     new BytesRef(UnsupportedValueSource.UNSUPPORTED_OUTPUT)
                 );
                 case "version" -> ((BytesRefBlock.Builder) builder).appendBytesRef(new Version(randomIdentifier()).toBytesRef());
-                case "geo_point" -> ((BytesRefBlock.Builder) builder).appendBytesRef(GEO.pointAsWKB(randomGeoPoint()));
-                case "cartesian_point" -> ((BytesRefBlock.Builder) builder).appendBytesRef(CARTESIAN.pointAsWKB(randomCartesianPoint()));
+                case "geo_point" -> ((BytesRefBlock.Builder) builder).appendBytesRef(GEO.pointAsWKB(GeometryTestUtils.randomPoint()));
+                case "cartesian_point" -> ((BytesRefBlock.Builder) builder).appendBytesRef(
+                    CARTESIAN.pointAsWKB(ShapeTestUtils.randomPoint())
+                );
                 case "null" -> builder.appendNull();
                 case "_source" -> {
                     try {
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java
@@ -28,6 +28,8 @@
 import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
 import org.elasticsearch.core.PathUtils;
 import org.elasticsearch.core.Releasables;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geo.ShapeTestUtils;
 import org.elasticsearch.indices.CrankyCircuitBreakerService;
 import org.elasticsearch.logging.LogManager;
 import org.elasticsearch.test.ESTestCase;
@@ -125,8 +127,8 @@ public static Literal randomLiteral(DataType type) {
             case "time_duration" -> Duration.ofMillis(randomLongBetween(-604800000L, 604800000L)); // plus/minus 7 days
             case "text" -> new BytesRef(randomAlphaOfLength(50));
             case "version" -> randomVersion().toBytesRef();
-            case "geo_point" -> GEO.pointAsWKB(randomGeoPoint());
-            case "cartesian_point" -> CARTESIAN.pointAsWKB(randomCartesianPoint());
+            case "geo_point" -> GEO.pointAsWKB(GeometryTestUtils.randomPoint());
+            case "cartesian_point" -> CARTESIAN.pointAsWKB(ShapeTestUtils.randomPoint());
             case "null" -> null;
             case "_source" -> {
                 try {
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java
@@ -10,6 +10,8 @@
 import org.apache.lucene.document.InetAddressPoint;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.network.InetAddresses;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geo.ShapeTestUtils;
 import org.elasticsearch.logging.LogManager;
 import org.elasticsearch.logging.Logger;
 import org.elasticsearch.test.ESTestCase;
@@ -41,8 +43,6 @@
 import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
 
-import static org.elasticsearch.test.ESTestCase.randomCartesianPoint;
-import static org.elasticsearch.test.ESTestCase.randomGeoPoint;
 import static org.elasticsearch.xpack.ql.util.SpatialCoordinateTypes.CARTESIAN;
 import static org.elasticsearch.xpack.ql.util.SpatialCoordinateTypes.GEO;
 import static org.hamcrest.Matchers.equalTo;
@@ -913,12 +913,18 @@ public static List<TypedDataSupplier> timeDurationCases() {
     }
 
     private static List<TypedDataSupplier> geoPointCases() {
-        return List.of(new TypedDataSupplier("<geo_point>", () -> GEO.pointAsWKB(randomGeoPoint()), EsqlDataTypes.GEO_POINT));
+        return List.of(
+            new TypedDataSupplier("<geo_point>", () -> GEO.pointAsWKB(GeometryTestUtils.randomPoint()), EsqlDataTypes.GEO_POINT)
+        );
     }
 
     private static List<TypedDataSupplier> cartesianPointCases() {
         return List.of(
-            new TypedDataSupplier("<cartesian_point>", () -> CARTESIAN.pointAsWKB(randomCartesianPoint()), EsqlDataTypes.CARTESIAN_POINT)
+            new TypedDataSupplier(
+                "<cartesian_point>",
+                () -> CARTESIAN.pointAsWKB(ShapeTestUtils.randomPoint()),
+                EsqlDataTypes.CARTESIAN_POINT
+            )
         );
     }
 
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToCartesianPointTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToCartesianPointTests.java
@@ -11,6 +11,7 @@
 import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.geo.ShapeTestUtils;
 import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase;
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 import org.elasticsearch.xpack.esql.type.EsqlDataTypes;
@@ -76,7 +77,7 @@ public static Iterable<Object[]> parameters() {
             List.of(
                 new TestCaseSupplier.TypedDataSupplier(
                     "<cartesian point as string>",
-                    () -> new BytesRef(CARTESIAN.pointAsString(randomCartesianPoint())),
+                    () -> new BytesRef(CARTESIAN.pointAsString(ShapeTestUtils.randomPoint())),
                     DataTypes.KEYWORD
                 )
             ),
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToGeoPointTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToGeoPointTests.java
@@ -11,6 +11,7 @@
 import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.geo.GeometryTestUtils;
 import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase;
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 import org.elasticsearch.xpack.esql.type.EsqlDataTypes;
@@ -68,7 +69,7 @@ public static Iterable<Object[]> parameters() {
             List.of(
                 new TestCaseSupplier.TypedDataSupplier(
                     "<geo point as string>",
-                    () -> new BytesRef(GEO.pointAsString(randomGeoPoint())),
+                    () -> new BytesRef(GEO.pointAsString(GeometryTestUtils.randomPoint())),
                     DataTypes.KEYWORD
                 )
             ),
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/AbstractMultivalueFunctionTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/AbstractMultivalueFunctionTestCase.java
@@ -8,8 +8,10 @@
 package org.elasticsearch.xpack.esql.expression.function.scalar.multivalue;
 
 import org.apache.lucene.util.BytesRef;
-import org.elasticsearch.common.geo.SpatialPoint;
 import org.elasticsearch.compute.data.Block;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geo.ShapeTestUtils;
+import org.elasticsearch.geometry.Point;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 import org.elasticsearch.xpack.esql.expression.function.scalar.AbstractScalarFunctionTestCase;
@@ -413,7 +415,7 @@ protected static void geoPoints(
         DataType expectedDataType,
         BiFunction<Integer, Stream<BytesRef>, Matcher<Object>> matcher
     ) {
-        points(cases, name, evaluatorName, EsqlDataTypes.GEO_POINT, expectedDataType, GEO, ESTestCase::randomGeoPoint, matcher);
+        points(cases, name, evaluatorName, EsqlDataTypes.GEO_POINT, expectedDataType, GEO, GeometryTestUtils::randomPoint, matcher);
     }
 
     /**
@@ -448,7 +450,7 @@ protected static void cartesianPoints(
             EsqlDataTypes.CARTESIAN_POINT,
             expectedDataType,
             CARTESIAN,
-            ESTestCase::randomCartesianPoint,
+            ShapeTestUtils::randomPoint,
             matcher
         );
     }
@@ -463,12 +465,11 @@ protected static void points(
         DataType dataType,
         DataType expectedDataType,
         SpatialCoordinateTypes spatial,
-        Supplier<SpatialPoint> randomPoint,
+        Supplier<Point> randomPoint,
         BiFunction<Integer, Stream<BytesRef>, Matcher<Object>> matcher
     ) {
         cases.add(new TestCaseSupplier(name + "(" + dataType.typeName() + ")", List.of(dataType), () -> {
-            SpatialPoint point = randomPoint.get();
-            BytesRef wkb = spatial.pointAsWKB(point);
+            BytesRef wkb = spatial.pointAsWKB(randomPoint.get());
             return new TestCaseSupplier.TestCase(
                 List.of(new TestCaseSupplier.TypedData(List.of(wkb), dataType, "field")),
                 evaluatorName + "[field=Attribute[channel=0]]",
diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/util/SpatialCoordinateTypes.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/util/SpatialCoordinateTypes.java
diff --git a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/util/SpatialCoordinateTypesTests.java b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/util/SpatialCoordinateTypesTests.java

Original file line number	Diff line number	Diff line change
`@@ -11,8 +11,9 @@`
`11`	`11`
`12`	`12`	`import org.apache.lucene.document.InetAddressPoint;`
`13`	`13`	`import org.apache.lucene.util.BytesRef;`
`14`		`-import org.elasticsearch.common.geo.SpatialPoint;`
`15`	`14`	`import org.elasticsearch.compute.operator.BreakingBytesRefBuilder;`
	`15`	`+import org.elasticsearch.geo.GeometryTestUtils;`
	`16`	`+import org.elasticsearch.geo.ShapeTestUtils;`
`16`	`17`	`import org.elasticsearch.geometry.Point;`
`17`	`18`	`import org.elasticsearch.geometry.utils.WellKnownBinary;`
`18`	`19`	`import org.elasticsearch.test.ESTestCase;`
`@@ -139,8 +140,8 @@ static Version randomVersion() {`
`139`	`140`	`}`
`140`	`141`
`141`	`142`	`static BytesRef randomPointAsWKB() {`
`142`		`- SpatialPoint point = randomBoolean() ? randomGeoPoint() : randomCartesianPoint();`
`143`		`- byte[] wkb = WellKnownBinary.toWKB(new Point(point.getX(), point.getY()), ByteOrder.LITTLE_ENDIAN);`
	`143`	`+ Point point = randomBoolean() ? GeometryTestUtils.randomPoint() : ShapeTestUtils.randomPoint();`
	`144`	`+ byte[] wkb = WellKnownBinary.toWKB(point, ByteOrder.LITTLE_ENDIAN);`
`144`	`145`	`return new BytesRef(wkb);`
`145`	`146`	`}`
`146`	`147`	`}`