From 44b0ba402ad9cec41b76169745b88eb3bd49582d Mon Sep 17 00:00:00 2001 From: ncordon Date: Thu, 20 Nov 2025 11:07:34 +0100 Subject: [PATCH 1/6] Updates jts version to 1.20.0 --- build-tools-internal/version.properties | 2 +- gradle/verification-metadata.xml | 5 +++++ modules/legacy-geo/src/main/java/module-info.java | 1 - 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/build-tools-internal/version.properties b/build-tools-internal/version.properties index 14acb95ed9b9a..93e1070eb1d7c 100644 --- a/build-tools-internal/version.properties +++ b/build-tools-internal/version.properties @@ -5,7 +5,7 @@ bundled_jdk_vendor = openjdk bundled_jdk = 25.0.1+8@2fbf10d8c78e40bd87641c434705079d # optional dependencies spatial4j = 0.7 -jts = 1.15.0 +jts = 1.20.0 jackson = 2.15.0 snakeyaml = 2.0 icu4j = 77.1 diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index 116721c29993b..b107bf0efdc6e 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -4483,6 +4483,11 @@ + + + + + diff --git a/modules/legacy-geo/src/main/java/module-info.java b/modules/legacy-geo/src/main/java/module-info.java index b5c62ae18b066..279b01f9e8dda 100644 --- a/modules/legacy-geo/src/main/java/module-info.java +++ b/modules/legacy-geo/src/main/java/module-info.java @@ -15,7 +15,6 @@ requires org.apache.lucene.core; requires org.apache.lucene.spatial_extras; requires org.apache.logging.log4j; - requires jts.core; requires spatial4j; exports org.elasticsearch.legacygeo.mapper; From 40c0f3ecbf7df018a0813c6555da4a6c9f8413ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Cord=C3=B3n?= Date: Thu, 20 Nov 2025 11:17:32 +0100 Subject: [PATCH 2/6] Update docs/changelog/138351.yaml --- docs/changelog/138351.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/138351.yaml diff --git a/docs/changelog/138351.yaml b/docs/changelog/138351.yaml new file mode 100644 index 0000000000000..c63c0bdbca8be --- /dev/null +++ b/docs/changelog/138351.yaml @@ -0,0 +1,5 @@ +pr: 138351 +summary: Bumps jts version to 1.20.0 +area: Geo +type: upgrade +issues: [] From 6c6c61abb9ef4e7958c513d6262a749bb6c969e6 Mon Sep 17 00:00:00 2001 From: ncordon Date: Fri, 21 Nov 2025 10:03:14 +0100 Subject: [PATCH 3/6] Small tweak to one of the tests --- modules/legacy-geo/src/main/java/module-info.java | 1 + .../elasticsearch/legacygeo/builders/PolygonBuilderTests.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/legacy-geo/src/main/java/module-info.java b/modules/legacy-geo/src/main/java/module-info.java index 279b01f9e8dda..8968e3b82418c 100644 --- a/modules/legacy-geo/src/main/java/module-info.java +++ b/modules/legacy-geo/src/main/java/module-info.java @@ -15,6 +15,7 @@ requires org.apache.lucene.core; requires org.apache.lucene.spatial_extras; requires org.apache.logging.log4j; + requires org.locationtech.jts; requires spatial4j; exports org.elasticsearch.legacygeo.mapper; diff --git a/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/builders/PolygonBuilderTests.java b/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/builders/PolygonBuilderTests.java index 463f795c13800..c2417854cbc00 100644 --- a/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/builders/PolygonBuilderTests.java +++ b/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/builders/PolygonBuilderTests.java @@ -168,7 +168,7 @@ public void testPolygonWithUndefinedOrientationDueToCollinearPoints() { new CoordinatesBuilder().coordinate(0.0, 0.0).coordinate(1.0, 1.0).coordinate(-1.0, -1.0).close() ); InvalidShapeException e = expectThrows(InvalidShapeException.class, pb::buildS4J); - assertEquals("Self-intersection at or near point (-1.0, -1.0, NaN)", e.getMessage()); + assertEquals("Self-intersection at or near point (0.0, 0.0, NaN)", e.getMessage()); } public void testCrossingDateline() { From 5e8a4f996d7d37f4f02dbb7caa0fc38b6f63b363 Mon Sep 17 00:00:00 2001 From: ncordon Date: Fri, 21 Nov 2025 17:25:06 +0100 Subject: [PATCH 4/6] Temp --- .../xpack/vectortile/feature/FeatureFactory.java | 7 +++++-- .../vectortile/feature/FeatureFactoryTests.java | 13 +++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java b/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java index b5f9088edc4be..23137e20aab66 100644 --- a/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java +++ b/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java @@ -332,8 +332,11 @@ private MvtCoordinateSequenceFilter(Envelope tileEnvelope, int extent) { @Override public void filter(CoordinateSequence seq, int i) { - seq.setOrdinate(i, 0, lon(seq.getOrdinate(i, 0))); - seq.setOrdinate(i, 1, lat(seq.getOrdinate(i, 1))); + if (i != 0 && i == seq.size() - 1 && seq.getCoordinate(0) == seq.getCoordinate(i)) { + return; + } + seq.setOrdinate(i, CoordinateSequence.X, lon(seq.getOrdinate(i, CoordinateSequence.X))); + seq.setOrdinate(i, CoordinateSequence.Y, lat(seq.getOrdinate(i, CoordinateSequence.Y))); } @Override diff --git a/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java b/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java index e8ea9bf06a4cc..94cf82b4200bc 100644 --- a/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java +++ b/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java @@ -292,6 +292,19 @@ private void assertParsing(InputStream is) throws IOException, ParseException { assertThat(builder.getFeatures(geometry), iterableWithSize(1)); } + public void testVectorTilesAreCorrect() throws IOException, ParseException { + // make sure we can parse big polygons + var is = new GZIPInputStream(getClass().getResourceAsStream("Antarctica.wkt.gz")); + final BufferedReader reader = new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8)); + final Geometry geometry = WellKnownText.fromWKT(StandardValidator.instance(true), true, reader.readLine()); + final FeatureFactory builder = new FeatureFactory(0, 0, 0, 4096, 5); + List bytes = builder.getFeatures(geometry); + assertThat(bytes.size(), equalTo(1)); + final VectorTile.Tile.Feature feature = VectorTile.Tile.Feature.parseFrom(bytes.get(0)); + assertThat(feature.getType(), Matchers.equalTo(VectorTile.Tile.GeomType.POLYGON)); + assertThat(feature.getGeometryCount(), equalTo(9071)); + } + public void testPolygonOrientation() throws IOException { Polygon polygon = new Polygon( new LinearRing(new double[] { -10, 10, 10, -10, -10 }, new double[] { -10, -10, 10, 10, -10 }), From 509cf298f070fcf6b8d3bbd3b86dc8b5db9ad42a Mon Sep 17 00:00:00 2001 From: ncordon Date: Tue, 25 Nov 2025 11:40:19 +0100 Subject: [PATCH 5/6] Adds tests to avoid bugs due to using the same reference --- .../vectortile/feature/FeatureFactory.java | 8 ++++ .../feature/FeatureFactoryTests.java | 38 ++++++++++++++++--- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java b/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java index 23137e20aab66..fb17ec72a2011 100644 --- a/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java +++ b/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java @@ -127,6 +127,14 @@ public List getFeatures(Geometry geometry) { return byteFeatures; } + /** + * Testing purposes only + */ + org.locationtech.jts.geom.Geometry getMvtGeometry(Geometry geometry) { + // Get geometry in pixel coordinates + return geometry.visit(mvtGeometryBuilder); + } + private record MVTGeometryBuilder( GeometryFactory geomFactory, org.locationtech.jts.geom.Geometry clipTile, diff --git a/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java b/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java index 94cf82b4200bc..cc40efee2ff5b 100644 --- a/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java +++ b/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java @@ -34,6 +34,7 @@ import java.nio.charset.StandardCharsets; import java.text.ParseException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.function.Consumer; @@ -292,17 +293,42 @@ private void assertParsing(InputStream is) throws IOException, ParseException { assertThat(builder.getFeatures(geometry), iterableWithSize(1)); } - public void testVectorTilesAreCorrect() throws IOException, ParseException { + public void testVectorTilesDoNotContainOriginWithSimplifiableGeometries() throws IOException, ParseException { + Polygon polygon = new Polygon( + new LinearRing( + new double[] { -94.491034, -94.493892, -94.399587, -94.483604, -94.491034 }, + new double[] { -72.522177, -72.491761, -72.607412, -72.545519, -72.522177 } + ) + ); + + final FeatureFactory builder = new FeatureFactory(0, 0, 0, 4096, 5); + + var mvtGeometry = builder.getMvtGeometry(polygon); + var coordinates = mvtGeometry.getCoordinates(); + var containsCenter = Arrays.stream(coordinates).anyMatch(c -> c.x == 2048 && c.y == 2048); + assertFalse(containsCenter); + } + + public void testVectorTilesDoNotContainOriginWithComplexGeometries() throws IOException, ParseException { // make sure we can parse big polygons var is = new GZIPInputStream(getClass().getResourceAsStream("Antarctica.wkt.gz")); final BufferedReader reader = new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8)); final Geometry geometry = WellKnownText.fromWKT(StandardValidator.instance(true), true, reader.readLine()); final FeatureFactory builder = new FeatureFactory(0, 0, 0, 4096, 5); - List bytes = builder.getFeatures(geometry); - assertThat(bytes.size(), equalTo(1)); - final VectorTile.Tile.Feature feature = VectorTile.Tile.Feature.parseFrom(bytes.get(0)); - assertThat(feature.getType(), Matchers.equalTo(VectorTile.Tile.GeomType.POLYGON)); - assertThat(feature.getGeometryCount(), equalTo(9071)); + + var mvtGeometry = builder.getMvtGeometry(geometry); + var coordinates = mvtGeometry.getCoordinates(); + var containsCenter = Arrays.stream(coordinates).anyMatch(c -> c.x == 2048 && c.y == 2048); + assertFalse(containsCenter); + } + + public void testVectorTilesDoNotContainOriginWithNonSimplifiableGeometries() throws IOException, ParseException { + Polygon polygon = new Polygon(new LinearRing(new double[] { -10, 10, 10, -10, -10 }, new double[] { -10, -10, 10, 10, -10 })); + final FeatureFactory builder = new FeatureFactory(0, 0, 0, 4096, 5); + var mvtGeometry = builder.getMvtGeometry(polygon); + var coordinates = mvtGeometry.getCoordinates(); + var containsCenter = Arrays.stream(coordinates).anyMatch(c -> c.x == 2048 && c.y == 2048); + assertFalse(containsCenter); } public void testPolygonOrientation() throws IOException { From e66f06569f059dddd0003dddec94b903fe744f49 Mon Sep 17 00:00:00 2001 From: ncordon Date: Tue, 25 Nov 2025 16:09:51 +0100 Subject: [PATCH 6/6] Addresses pr review comments --- gradle/verification-metadata.xml | 5 ----- .../xpack/vectortile/feature/FeatureFactory.java | 9 ++++++--- .../xpack/vectortile/feature/FeatureFactoryTests.java | 10 +++++----- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index b107bf0efdc6e..54fa25ff2f516 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -4478,11 +4478,6 @@ - - - - - diff --git a/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java b/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java index fb17ec72a2011..040c6e081cd03 100644 --- a/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java +++ b/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java @@ -111,7 +111,7 @@ public byte[] points(List multiPoint) { * Returns a List {@code byte[]} containing the mvt representation of the provided geometry */ public List getFeatures(Geometry geometry) { - // Get geometry in pixel coordinates + // Get clipped and simplified geometry in pixel coordinates final org.locationtech.jts.geom.Geometry mvtGeometry = geometry.visit(mvtGeometryBuilder); if (mvtGeometry == null) { return List.of(); @@ -128,10 +128,11 @@ public List getFeatures(Geometry geometry) { } /** - * Testing purposes only + * Testing purposes only, allowing us to test the impact of the geometry clipping and simplification + * without needing to parse the protobuffer results. */ org.locationtech.jts.geom.Geometry getMvtGeometry(Geometry geometry) { - // Get geometry in pixel coordinates + // Get clipped and simplified geometry in pixel coordinates return geometry.visit(mvtGeometryBuilder); } @@ -340,6 +341,8 @@ private MvtCoordinateSequenceFilter(Envelope tileEnvelope, int extent) { @Override public void filter(CoordinateSequence seq, int i) { + // JTS can sometimes close polygons using references to the starting coordinate, which can lead to that coordinate + // being converted twice. We need to detect if the end is the same actual Coordinate, and not do the conversion again. if (i != 0 && i == seq.size() - 1 && seq.getCoordinate(0) == seq.getCoordinate(i)) { return; } diff --git a/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java b/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java index cc40efee2ff5b..aebb1bf585648 100644 --- a/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java +++ b/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java @@ -293,7 +293,7 @@ private void assertParsing(InputStream is) throws IOException, ParseException { assertThat(builder.getFeatures(geometry), iterableWithSize(1)); } - public void testVectorTilesDoNotContainOriginWithSimplifiableGeometries() throws IOException, ParseException { + public void testVectorTilesDoNotContainOriginWithSimplifiableGeometries() { Polygon polygon = new Polygon( new LinearRing( new double[] { -94.491034, -94.493892, -94.399587, -94.483604, -94.491034 }, @@ -306,7 +306,7 @@ public void testVectorTilesDoNotContainOriginWithSimplifiableGeometries() throws var mvtGeometry = builder.getMvtGeometry(polygon); var coordinates = mvtGeometry.getCoordinates(); var containsCenter = Arrays.stream(coordinates).anyMatch(c -> c.x == 2048 && c.y == 2048); - assertFalse(containsCenter); + assertFalse("Coordinate (2048,2048) only occurs if we are converting the first / last coordinate twice", containsCenter); } public void testVectorTilesDoNotContainOriginWithComplexGeometries() throws IOException, ParseException { @@ -319,16 +319,16 @@ public void testVectorTilesDoNotContainOriginWithComplexGeometries() throws IOEx var mvtGeometry = builder.getMvtGeometry(geometry); var coordinates = mvtGeometry.getCoordinates(); var containsCenter = Arrays.stream(coordinates).anyMatch(c -> c.x == 2048 && c.y == 2048); - assertFalse(containsCenter); + assertFalse("Coordinate (2048,2048) only occurs if we are converting the first / last coordinate twice", containsCenter); } - public void testVectorTilesDoNotContainOriginWithNonSimplifiableGeometries() throws IOException, ParseException { + public void testVectorTilesDoNotContainOriginWithNonSimplifiableGeometries() { Polygon polygon = new Polygon(new LinearRing(new double[] { -10, 10, 10, -10, -10 }, new double[] { -10, -10, 10, 10, -10 })); final FeatureFactory builder = new FeatureFactory(0, 0, 0, 4096, 5); var mvtGeometry = builder.getMvtGeometry(polygon); var coordinates = mvtGeometry.getCoordinates(); var containsCenter = Arrays.stream(coordinates).anyMatch(c -> c.x == 2048 && c.y == 2048); - assertFalse(containsCenter); + assertFalse("Coordinate (2048,2048) only occurs if we are converting the first / last coordinate twice", containsCenter); } public void testPolygonOrientation() throws IOException {