From 1dd747e8bb680537d8b5f45154fa96bf73046c80 Mon Sep 17 00:00:00 2001 From: Omric Date: Fri, 28 Mar 2025 23:57:48 +0300 Subject: [PATCH 1/3] Support explicit Z/M coordinate values in WKT parsing for geometry (#12311) --- .../geometry/utils/WellKnownText.java | 24 +++++++- .../geometry/BaseGeometryTestCase.java | 3 + .../geometry/GeometryCollectionTests.java | 30 ++++++++++ .../org/elasticsearch/geometry/LineTests.java | 20 +++++++ .../geometry/MultiLineTests.java | 48 ++++++++++++++++ .../geometry/MultiPointTests.java | 21 +++++++ .../geometry/MultiPolygonTests.java | 42 ++++++++++++++ .../elasticsearch/geometry/PointTests.java | 17 ++++++ .../elasticsearch/geometry/PolygonTests.java | 56 +++++++++++++++++++ 9 files changed, 260 insertions(+), 1 deletion(-) diff --git a/libs/geo/src/main/java/org/elasticsearch/geometry/utils/WellKnownText.java b/libs/geo/src/main/java/org/elasticsearch/geometry/utils/WellKnownText.java index 9e657013a80dd..b77d9e6e24d2b 100644 --- a/libs/geo/src/main/java/org/elasticsearch/geometry/utils/WellKnownText.java +++ b/libs/geo/src/main/java/org/elasticsearch/geometry/utils/WellKnownText.java @@ -44,6 +44,8 @@ public class WellKnownText { public static final String RPAREN = ")"; public static final String COMMA = ","; public static final String NAN = "NaN"; + public static final String Z = "Z"; + public static final String M = "M"; public static final int MAX_NESTED_DEPTH = 1000; private static final String NUMBER = ""; @@ -440,7 +442,8 @@ public static Geometry fromWKT(GeometryValidator validator, boolean coerce, Stri */ private static Geometry parseGeometry(StreamTokenizer stream, boolean coerce, int depth) throws IOException, ParseException { final String type = nextWord(stream).toLowerCase(Locale.ROOT); - return switch (type) { + final boolean isExplicitlySpecifiesZorM = isZOrMNext(stream); + Geometry geometry = switch (type) { case "point" -> parsePoint(stream); case "multipoint" -> parseMultiPoint(stream); case "linestring" -> parseLine(stream); @@ -453,6 +456,16 @@ private static Geometry parseGeometry(StreamTokenizer stream, boolean coerce, in parseCircle(stream); default -> throw new IllegalArgumentException("Unknown geometry type: " + type); }; + checkZorMAttribute(isExplicitlySpecifiesZorM, geometry.hasZ()); + return geometry; + } + + private static void checkZorMAttribute(boolean isExplicitlySpecifiesZorM, boolean hasZ) { + if (isExplicitlySpecifiesZorM && hasZ == false) { + throw new IllegalArgumentException( + "When specifying 'Z' or 'M', coordinates must include three values. Only two coordinates were provided" + ); + } } private static GeometryCollection parseGeometryCollection(StreamTokenizer stream, boolean coerce, int depth) @@ -710,6 +723,15 @@ private static boolean isNumberNext(StreamTokenizer stream) throws IOException { return type == StreamTokenizer.TT_WORD; } + private static boolean isZOrMNext(StreamTokenizer stream) throws ParseException, IOException { + String token = nextWord(stream); + if (token.equals(Z) || token.equals(M)) { + return true; + } + stream.pushBack(); + return false; + } + private static String nextEmptyOrOpen(StreamTokenizer stream) throws IOException, ParseException { final String next = nextWord(stream); if (next.equals(EMPTY) || next.equals(LPAREN)) { diff --git a/libs/geo/src/test/java/org/elasticsearch/geometry/BaseGeometryTestCase.java b/libs/geo/src/test/java/org/elasticsearch/geometry/BaseGeometryTestCase.java index eb66f8b148c00..40ff3fc9fccf6 100644 --- a/libs/geo/src/test/java/org/elasticsearch/geometry/BaseGeometryTestCase.java +++ b/libs/geo/src/test/java/org/elasticsearch/geometry/BaseGeometryTestCase.java @@ -21,6 +21,9 @@ abstract class BaseGeometryTestCase extends AbstractWireTestCase { + public static final String ZorMMustIncludeThreeValuesMsg = + "When specifying 'Z' or 'M', coordinates must include three values. Only two coordinates were provided"; + @Override protected final T createTestInstance() { boolean hasAlt = randomBoolean(); diff --git a/libs/geo/src/test/java/org/elasticsearch/geometry/GeometryCollectionTests.java b/libs/geo/src/test/java/org/elasticsearch/geometry/GeometryCollectionTests.java index 3d5f3e904ddb4..b4d772ef7b9b5 100644 --- a/libs/geo/src/test/java/org/elasticsearch/geometry/GeometryCollectionTests.java +++ b/libs/geo/src/test/java/org/elasticsearch/geometry/GeometryCollectionTests.java @@ -19,6 +19,7 @@ import java.text.ParseException; import java.util.Arrays; import java.util.Collections; +import java.util.List; import static org.hamcrest.Matchers.containsString; @@ -93,6 +94,35 @@ private int countNestedGeometryCollections(GeometryCollection geometry) { return count; } + public void testParseGeometryCollectionZorMWithThreeCoordinates() throws IOException, ParseException { + GeometryValidator validator = GeographyValidator.instance(true); + + GeometryCollection expected = new GeometryCollection<>( + Arrays.asList( + new Point(20.0, 10.0, 100.0), + new Line(new double[] { 10.0, 20.0 }, new double[] { 5.0, 15.0 }, new double[] { 50.0, 150.0 }) + ) + ); + + String point = "(POINT Z (20.0 10.0 100.0)"; + String lineString = "LINESTRING M (10.0 5.0 50.0, 20.0 15.0 150.0)"; + assertEquals(expected, WellKnownText.fromWKT(validator, true, "GEOMETRYCOLLECTION Z " + point + ", " + lineString + ")")); + + assertEquals(expected, WellKnownText.fromWKT(validator, true, "GEOMETRYCOLLECTION M " + point + ", " + lineString + ")")); + } + + public void testParseGeometryCollectionZorMWithTwoCoordinatesThrowsException() { + GeometryValidator validator = GeographyValidator.instance(true); + List gcWkt = List.of( + "GEOMETRYCOLLECTION Z (POINT (20.0 10.0), LINESTRING (10.0 5.0, 20.0 15.0))", + "GEOMETRYCOLLECTION M (POINT (20.0 10.0), LINESTRING (10.0 5.0, 20.0 15.0))" + ); + for (String gc : gcWkt) { + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> WellKnownText.fromWKT(validator, true, gc)); + assertEquals(ZorMMustIncludeThreeValuesMsg, ex.getMessage()); + } + } + @Override protected GeometryCollection mutateInstance(GeometryCollection instance) { return null;// TODO implement https://github.com/elastic/elasticsearch/issues/25929 diff --git a/libs/geo/src/test/java/org/elasticsearch/geometry/LineTests.java b/libs/geo/src/test/java/org/elasticsearch/geometry/LineTests.java index 48edb1be95e2f..4082508d128b3 100644 --- a/libs/geo/src/test/java/org/elasticsearch/geometry/LineTests.java +++ b/libs/geo/src/test/java/org/elasticsearch/geometry/LineTests.java @@ -17,6 +17,7 @@ import java.io.IOException; import java.text.ParseException; +import java.util.List; public class LineTests extends BaseGeometryTestCase { @Override @@ -82,6 +83,25 @@ public void testWKTValidation() { assertEquals("found Z value [6.0] but [ignore_z_value] parameter is [false]", ex.getMessage()); } + public void testParseLineZorMWithThreeCoordinates() throws IOException, ParseException { + GeometryValidator validator = GeographyValidator.instance(true); + + Line expectedZ = new Line(new double[] { 20.0, 30.0 }, new double[] { 10.0, 15.0 }, new double[] { 100.0, 200.0 }); + assertEquals(expectedZ, WellKnownText.fromWKT(validator, true, "LINESTRING Z (20.0 10.0 100.0, 30.0 15.0 200.0)")); + + Line expectedM = new Line(new double[] { 20.0, 30.0 }, new double[] { 10.0, 15.0 }, new double[] { 100.0, 200.0 }); + assertEquals(expectedM, WellKnownText.fromWKT(validator, true, "LINESTRING M (20.0 10.0 100.0, 30.0 15.0 200.0)")); + } + + public void testParseLineZorMWithTwoCoordinatesThrowsException() { + GeometryValidator validator = GeographyValidator.instance(true); + List linesWkt = List.of("LINESTRING Z (20.0 10.0, 30.0 15.0)", "LINESTRING M (20.0 10.0, 30.0 15.0)"); + for (String line : linesWkt) { + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> WellKnownText.fromWKT(validator, true, line)); + assertEquals(ZorMMustIncludeThreeValuesMsg, ex.getMessage()); + } + } + @Override protected Line mutateInstance(Line instance) { return null;// TODO implement https://github.com/elastic/elasticsearch/issues/25929 diff --git a/libs/geo/src/test/java/org/elasticsearch/geometry/MultiLineTests.java b/libs/geo/src/test/java/org/elasticsearch/geometry/MultiLineTests.java index b09bd2f893650..337b9a265a79c 100644 --- a/libs/geo/src/test/java/org/elasticsearch/geometry/MultiLineTests.java +++ b/libs/geo/src/test/java/org/elasticsearch/geometry/MultiLineTests.java @@ -64,6 +64,54 @@ public void testValidation() { ); } + public void testParseMultiLineZorMWithThreeCoordinates() throws IOException, ParseException { + GeometryValidator validator = GeographyValidator.instance(true); + MultiLine expectedZ = new MultiLine( + List.of( + new Line(new double[] { 20.0, 30.0 }, new double[] { 10.0, 15.0 }, new double[] { 100.0, 200.0 }), + new Line(new double[] { 40.0, 50.0 }, new double[] { 20.0, 25.0 }, new double[] { 300.0, 400.0 }) + ) + ); + assertEquals( + expectedZ, + WellKnownText.fromWKT( + validator, + true, + "MULTILINESTRING Z ((20.0 10.0 100.0, 30.0 15.0 200.0), (40.0 20.0 300.0, 50.0 25.0 400.0))" + ) + ); + + MultiLine expectedM = new MultiLine( + List.of( + new Line(new double[] { 20.0, 30.0 }, new double[] { 10.0, 15.0 }, new double[] { 100.0, 200.0 }), + new Line(new double[] { 40.0, 50.0 }, new double[] { 20.0, 25.0 }, new double[] { 300.0, 400.0 }) + ) + ); + assertEquals( + expectedM, + WellKnownText.fromWKT( + validator, + true, + "MULTILINESTRING M ((20.0 10.0 100.0, 30.0 15.0 200.0), (40.0 20.0 300.0, 50.0 25.0 400.0))" + ) + ); + } + + public void testParseMultiLineZorMWithTwoCoordinatesThrowsException() { + GeometryValidator validator = GeographyValidator.instance(true); + List multiLinesWkt = List.of( + "MULTILINESTRING Z ((20.0 10.0, 30.0 15.0), (40.0 20.0, 50.0 25.0))", + "MULTILINESTRING M ((20.0 10.0, 30.0 15.0), (40.0 20.0, 50.0 25.0))" + ); + for (String multiLine : multiLinesWkt) { + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> WellKnownText.fromWKT(validator, true, multiLine) + ); + assertEquals(ZorMMustIncludeThreeValuesMsg, ex.getMessage()); + } + } + @Override protected MultiLine mutateInstance(MultiLine instance) { return null;// TODO implement https://github.com/elastic/elasticsearch/issues/25929 diff --git a/libs/geo/src/test/java/org/elasticsearch/geometry/MultiPointTests.java b/libs/geo/src/test/java/org/elasticsearch/geometry/MultiPointTests.java index d5bc9664f8f54..9d559dcbdc54f 100644 --- a/libs/geo/src/test/java/org/elasticsearch/geometry/MultiPointTests.java +++ b/libs/geo/src/test/java/org/elasticsearch/geometry/MultiPointTests.java @@ -71,6 +71,27 @@ public void testValidation() { StandardValidator.instance(true).validate(new MultiPoint(Collections.singletonList(new Point(2, 1, 3)))); } + public void testParseMultiPointWithThreeCoordinates() throws IOException, ParseException { + GeometryValidator validator = GeographyValidator.instance(true); + MultiPoint expectedZ = new MultiPoint(Arrays.asList(new Point(10, 20, 30), new Point(40, 50, 60))); + MultiPoint expectedM = new MultiPoint(Arrays.asList(new Point(10, 20, 30), new Point(40, 50, 60))); + + assertEquals(expectedZ, WellKnownText.fromWKT(validator, true, "MULTIPOINT Z (10 20 30, 40 50 60)")); + assertEquals(expectedM, WellKnownText.fromWKT(validator, true, "MULTIPOINT M (10 20 30, 40 50 60)")); + } + + public void testParseMultiPointWithTwoCoordinatesThrowsException() { + GeometryValidator validator = GeographyValidator.instance(true); + List multiPointsWkt = List.of("MULTIPOINT Z (10 20, 40 50)", "MULTIPOINT M (10 20, 40 50)"); + for (String multiPoint : multiPointsWkt) { + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> WellKnownText.fromWKT(validator, true, multiPoint) + ); + assertEquals(ZorMMustIncludeThreeValuesMsg, ex.getMessage()); + } + } + @Override protected MultiPoint mutateInstance(MultiPoint instance) { return null;// TODO implement https://github.com/elastic/elasticsearch/issues/25929 diff --git a/libs/geo/src/test/java/org/elasticsearch/geometry/MultiPolygonTests.java b/libs/geo/src/test/java/org/elasticsearch/geometry/MultiPolygonTests.java index 0a9024652b55f..509a5789ca028 100644 --- a/libs/geo/src/test/java/org/elasticsearch/geometry/MultiPolygonTests.java +++ b/libs/geo/src/test/java/org/elasticsearch/geometry/MultiPolygonTests.java @@ -80,6 +80,48 @@ public void testValidation() { ); } + public void testParseMultiPolygonZorMWithThreeCoordinates() throws IOException, ParseException { + GeometryValidator validator = GeographyValidator.instance(true); + + MultiPolygon expected = new MultiPolygon( + List.of( + new Polygon( + new LinearRing( + new double[] { 20.0, 30.0, 40.0, 20.0 }, + new double[] { 10.0, 15.0, 10.0, 10.0 }, + new double[] { 100.0, 200.0, 300.0, 100.0 } + ) + ), + new Polygon( + new LinearRing( + new double[] { 0.0, 10.0, 10.0, 0.0 }, + new double[] { 0.0, 0.0, 10.0, 0.0 }, + new double[] { 10.0, 20.0, 30.0, 10.0 } + ) + ) + ) + ); + String polygonA = "(20.0 10.0 100.0, 30.0 15.0 200.0, 40.0 10.0 300.0, 20.0 10.0 100.0)"; + String polygonB = "(0.0 0.0 10.0, 10.0 0.0 20.0, 10.0 10.0 30.0, 0.0 0.0 10.0)"; + assertEquals(expected, WellKnownText.fromWKT(validator, true, "MULTIPOLYGON Z ((" + polygonA + "), (" + polygonB + "))")); + assertEquals(expected, WellKnownText.fromWKT(validator, true, "MULTIPOLYGON M ((" + polygonA + "), (" + polygonB + "))")); + } + + public void testParseMultiPolygonZorMWithTwoCoordinatesThrowsException() { + GeometryValidator validator = GeographyValidator.instance(true); + List multiPolygonsWkt = List.of( + "MULTIPOLYGON Z (((20.0 10.0, 30.0 15.0, 40.0 10.0, 20.0 10.0)), ((0.0 0.0, 10.0 0.0, 10.0 10.0, 0.0 0.0)))", + "MULTIPOLYGON M (((20.0 10.0, 30.0 15.0, 40.0 10.0, 20.0 10.0)), ((0.0 0.0, 10.0 0.0, 10.0 10.0, 0.0 0.0)))" + ); + for (String multiPolygon : multiPolygonsWkt) { + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> WellKnownText.fromWKT(validator, true, multiPolygon) + ); + assertEquals(ZorMMustIncludeThreeValuesMsg, ex.getMessage()); + } + } + @Override protected MultiPolygon mutateInstance(MultiPolygon instance) { return null;// TODO implement https://github.com/elastic/elasticsearch/issues/25929 diff --git a/libs/geo/src/test/java/org/elasticsearch/geometry/PointTests.java b/libs/geo/src/test/java/org/elasticsearch/geometry/PointTests.java index b61c36579bfca..91b4e7ba347d3 100644 --- a/libs/geo/src/test/java/org/elasticsearch/geometry/PointTests.java +++ b/libs/geo/src/test/java/org/elasticsearch/geometry/PointTests.java @@ -17,8 +17,10 @@ import java.io.IOException; import java.text.ParseException; +import java.util.List; public class PointTests extends BaseGeometryTestCase { + @Override protected Point createTestInstance(boolean hasAlt) { return GeometryTestUtils.randomPoint(hasAlt); @@ -58,6 +60,21 @@ public void testWKTValidation() { assertEquals("found Z value [100.0] but [ignore_z_value] parameter is [false]", ex.getMessage()); } + public void testParsePointZorMWithThreeCoordinates() throws IOException, ParseException { + GeometryValidator validator = GeographyValidator.instance(true); + assertEquals(new Point(20, 10, 100), WellKnownText.fromWKT(validator, true, "POINT Z (20.0 10.0 100.0)")); + assertEquals(new Point(20, 10, 100), WellKnownText.fromWKT(validator, true, "POINT M (20.0 10.0 100.0)")); + } + + public void testParsePointZorMWithTwoCoordinatesThrowsException() { + GeometryValidator validator = GeographyValidator.instance(true); + List pointsWkt = List.of("POINT Z (20.0 10.0)", "POINT M (20.0 10.0)"); + for (String point : pointsWkt) { + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> WellKnownText.fromWKT(validator, true, point)); + assertEquals(ZorMMustIncludeThreeValuesMsg, ex.getMessage()); + } + } + @Override protected Point mutateInstance(Point instance) { return null;// TODO implement https://github.com/elastic/elasticsearch/issues/25929 diff --git a/libs/geo/src/test/java/org/elasticsearch/geometry/PolygonTests.java b/libs/geo/src/test/java/org/elasticsearch/geometry/PolygonTests.java index 27cab467135c8..2662cf5d6301f 100644 --- a/libs/geo/src/test/java/org/elasticsearch/geometry/PolygonTests.java +++ b/libs/geo/src/test/java/org/elasticsearch/geometry/PolygonTests.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.text.ParseException; import java.util.Collections; +import java.util.List; public class PolygonTests extends BaseGeometryTestCase { @Override @@ -134,6 +135,61 @@ public void testWKTValidation() { ); } + public void testParsePolygonZorMWithThreeCoordinates() throws IOException, ParseException { + GeometryValidator validator = GeographyValidator.instance(true); + + Polygon expectedZ = new Polygon( + new LinearRing( + new double[] { 20.0, 30.0, 40.0, 20.0 }, + new double[] { 10.0, 15.0, 10.0, 10.0 }, + new double[] { 100.0, 200.0, 300.0, 100.0 } + ) + ); + assertEquals( + expectedZ, + WellKnownText.fromWKT(validator, true, "POLYGON Z ((20.0 10.0 100.0, 30.0 15.0 200.0, 40.0 10.0 300.0, 20.0 10.0 100.0))") + ); + + Polygon expectedM = new Polygon( + new LinearRing( + new double[] { 20.0, 30.0, 40.0, 20.0 }, + new double[] { 10.0, 15.0, 10.0, 10.0 }, + new double[] { 100.0, 200.0, 300.0, 100.0 } + ) + ); + assertEquals( + expectedM, + WellKnownText.fromWKT(validator, true, "POLYGON M ((20.0 10.0 100.0, 30.0 15.0 200.0, 40.0 10.0 300.0, 20.0 10.0 100.0))") + ); + + Polygon expectedZAutoClose = new Polygon( + new LinearRing( + new double[] { 20.0, 30.0, 40.0, 20.0 }, + new double[] { 10.0, 15.0, 10.0, 10.0 }, + new double[] { 100.0, 200.0, 300.0, 100.0 } + ) + ); + assertEquals( + expectedZAutoClose, + WellKnownText.fromWKT(validator, true, "POLYGON Z ((20.0 10.0 100.0, 30.0 15.0 200.0, 40.0 10.0 300.0))") + ); + } + + public void testParsePolygonZorMWithTwoCoordinatesThrowsException() { + GeometryValidator validator = GeographyValidator.instance(true); + List polygonsWkt = List.of( + "POLYGON Z ((20.0 10.0, 30.0 15.0, 40.0 10.0, 20.0 10.0))", + "POLYGON M ((20.0 10.0, 30.0 15.0, 40.0 10.0, 20.0 10.0))" + ); + for (String polygon : polygonsWkt) { + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> WellKnownText.fromWKT(validator, true, polygon) + ); + assertEquals(ZorMMustIncludeThreeValuesMsg, ex.getMessage()); + } + } + @Override protected Polygon mutateInstance(Polygon instance) { return null;// TODO implement https://github.com/elastic/elasticsearch/issues/25929 From a8366fd54db716fcffea9f992985c1772d227648 Mon Sep 17 00:00:00 2001 From: Omric Date: Thu, 3 Apr 2025 15:36:43 +0300 Subject: [PATCH 2/3] EOF handling on Z/M parsing --- .../geometry/utils/WellKnownText.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/libs/geo/src/main/java/org/elasticsearch/geometry/utils/WellKnownText.java b/libs/geo/src/main/java/org/elasticsearch/geometry/utils/WellKnownText.java index b77d9e6e24d2b..48f55846ca1bc 100644 --- a/libs/geo/src/main/java/org/elasticsearch/geometry/utils/WellKnownText.java +++ b/libs/geo/src/main/java/org/elasticsearch/geometry/utils/WellKnownText.java @@ -723,13 +723,19 @@ private static boolean isNumberNext(StreamTokenizer stream) throws IOException { return type == StreamTokenizer.TT_WORD; } - private static boolean isZOrMNext(StreamTokenizer stream) throws ParseException, IOException { - String token = nextWord(stream); - if (token.equals(Z) || token.equals(M)) { - return true; + private static boolean isZOrMNext(StreamTokenizer stream) { + String token; + try { + token = nextWord(stream); + if (token.equals(Z) || token.equals(M)) { + return true; + } + stream.pushBack(); + return false; + } catch (ParseException | IOException e) { + return false; } - stream.pushBack(); - return false; + } private static String nextEmptyOrOpen(StreamTokenizer stream) throws IOException, ParseException { From d3ad00435202cd3351a660ac90d7a7642d7136ee Mon Sep 17 00:00:00 2001 From: Ignacio Vera Date: Thu, 3 Apr 2025 15:07:46 +0200 Subject: [PATCH 3/3] Add yaml changelog file --- docs/changelog/125896.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/125896.yaml diff --git a/docs/changelog/125896.yaml b/docs/changelog/125896.yaml new file mode 100644 index 0000000000000..92c2d2712f853 --- /dev/null +++ b/docs/changelog/125896.yaml @@ -0,0 +1,5 @@ +pr: 125896 +summary: Support explicit Z/M attributes using WKT geometry +area: Geo +type: enhancement +issues: [123111]