This is an automated email from the ASF dual-hosted git repository. afs pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/jena.git
commit ccc7fe90d067349140a9049ea2a2e7a7c9fd9874 Author: Edmond Chuc <[email protected]> AuthorDate: Mon Sep 14 16:46:31 2026 +1000 GH-4221: Preserve Z/M when converting coordinate sequences --- .../geosparql/implementation/DimensionInfo.java | 4 +- .../jts/CustomCoordinateSequence.java | 9 +- .../jts/CustomCoordinateSequenceFactory.java | 25 +++- .../implementation/GeometryWrapperFactoryTest.java | 41 +++++++ .../jts/CustomCoordinateSequenceFactoryTest.java | 129 +++++++++++++++++++++ 5 files changed, 204 insertions(+), 4 deletions(-) diff --git a/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/DimensionInfo.java b/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/DimensionInfo.java index ca1866c44f..d06566742b 100644 --- a/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/DimensionInfo.java +++ b/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/DimensionInfo.java @@ -91,7 +91,9 @@ public class DimensionInfo implements Serializable { } public static DimensionInfo find(Coordinate coordinate, Geometry geometry) { - CoordinateSequenceDimensions coordDims = CoordinateSequenceDimensions.find(coordinate); + // Empty geometries have no coordinate; use the existing XY default. + CoordinateSequenceDimensions coordDims = CoordinateSequenceDimensions.find( + coordinate == null ? XY_COORDINATE : coordinate); return new DimensionInfo(coordDims, geometry.getDimension()); } diff --git a/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequence.java b/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequence.java index 083e643ded..73ff6bdfbe 100644 --- a/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequence.java +++ b/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequence.java @@ -450,7 +450,8 @@ public class CustomCoordinateSequence implements CoordinateSequence, Serializabl case Y: return y[index]; case Z: - return z[index]; + // In XYM, ordinate 2 is M; the Z constant assumes an XYZ layout. + return dimensions == CoordinateSequenceDimensions.XYM ? m[index] : z[index]; case M: return m[index]; } @@ -473,7 +474,11 @@ public class CustomCoordinateSequence implements CoordinateSequence, Serializabl y[index] = value; break; case Z: - z[index] = value; + if (dimensions == CoordinateSequenceDimensions.XYM) { + m[index] = value; + } else { + z[index] = value; + } break; case M: m[index] = value; diff --git a/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequenceFactory.java b/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequenceFactory.java index 8d98dd0834..3cdf4ae636 100644 --- a/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequenceFactory.java +++ b/jena-geosparql/src/main/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequenceFactory.java @@ -48,7 +48,21 @@ public class CustomCoordinateSequenceFactory implements CoordinateSequenceFactor CustomCoordinateSequence customCoordSeq = (CustomCoordinateSequence) coordSeq; copyCoordSeq = customCoordSeq.copy(); } else { - copyCoordSeq = new CustomCoordinateSequence(coordSeq.toCoordinateArray()); + CoordinateSequenceDimensions dimensions; + if (coordSeq.hasM()) { + dimensions = coordSeq.hasZ() ? CoordinateSequenceDimensions.XYZM : CoordinateSequenceDimensions.XYM; + } else { + dimensions = coordSeq.hasZ() ? CoordinateSequenceDimensions.XYZ : CoordinateSequenceDimensions.XY; + } + copyCoordSeq = new CustomCoordinateSequence(coordSeq.size(), dimensions); + for (int i = 0; i < coordSeq.size(); i++) { + copyCoordSeq.setOrdinate(i, CoordinateSequence.X, coordSeq.getX(i)); + copyCoordSeq.setOrdinate(i, CoordinateSequence.Y, coordSeq.getY(i)); + if (coordSeq.hasZ()) + copyCoordSeq.setOrdinate(i, CoordinateSequence.Z, coordSeq.getZ(i)); + if (coordSeq.hasM()) + copyCoordSeq.setOrdinate(i, copyCoordSeq.getDimension() - copyCoordSeq.getMeasures(), coordSeq.getM(i)); + } } return copyCoordSeq; @@ -59,4 +73,13 @@ public class CustomCoordinateSequenceFactory implements CoordinateSequenceFactor return new CustomCoordinateSequence(size, dimension); } + @Override + public CoordinateSequence create(int size, int dimension, int measures) { + // Match JTS allocation limits: two or three spatial ordinates and at most one measure. + int spatialDimension = Math.max(2, Math.min(3, dimension - measures)); + int measureDimension = Math.min(1, measures); + return new CustomCoordinateSequence(size, CustomCoordinateSequence.findCoordinateSequenceDimensions( + spatialDimension + measureDimension, spatialDimension)); + } + } diff --git a/jena-geosparql/src/test/java/org/apache/jena/geosparql/implementation/GeometryWrapperFactoryTest.java b/jena-geosparql/src/test/java/org/apache/jena/geosparql/implementation/GeometryWrapperFactoryTest.java index 8dc419e318..2bde86bea7 100644 --- a/jena-geosparql/src/test/java/org/apache/jena/geosparql/implementation/GeometryWrapperFactoryTest.java +++ b/jena-geosparql/src/test/java/org/apache/jena/geosparql/implementation/GeometryWrapperFactoryTest.java @@ -23,6 +23,7 @@ package org.apache.jena.geosparql.implementation; import java.util.Arrays; import java.util.List; import org.apache.jena.geosparql.implementation.datatype.WKTDatatype; +import org.apache.jena.geosparql.implementation.jts.CoordinateSequenceDimensions; import org.apache.jena.geosparql.implementation.vocabulary.SRS_URI; import org.junit.After; import org.junit.AfterClass; @@ -31,6 +32,9 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.GeometryFactory; +import org.locationtech.jts.geom.impl.CoordinateArraySequence; /** * @@ -38,6 +42,43 @@ import org.locationtech.jts.geom.Coordinate; */ public class GeometryWrapperFactoryTest { + @Test + public void factoryHandlesEmptyGeometriesWithoutCoordinates() { + GeometryFactory factory = new GeometryFactory(); + Geometry[] members = { + factory.createPoint(new CoordinateArraySequence(0, 3, 0)), + factory.createPoint(new CoordinateArraySequence(0, 3, 1)) + }; + for (Geometry empty : new Geometry[] { + factory.createPoint(), + factory.createLineString(), + factory.createPolygon(), + factory.createGeometryCollection(), + factory.createGeometryCollection(new Geometry[] { factory.createGeometryCollection() }), + factory.createGeometryCollection(members) }) { + GeometryWrapper geometry = GeometryWrapperFactory.createGeometry(empty, + SRS_URI.DEFAULT_WKT_CRS84, WKTDatatype.URI); + assertEquals(true, geometry.getParsingGeometry().isEmpty()); + assertEquals(CoordinateSequenceDimensions.XY, geometry.getCoordinateSequenceDimensions()); + assertEquals(2, geometry.getCoordinateDimension()); + assertEquals(2, geometry.getSpatialDimension()); + } + } + + @Test + public void factoryKeepsOrdinaryJts2DGeometriesXY() { + GeometryFactory factory = new GeometryFactory(); + for (Geometry source : new Geometry[] { + factory.createPoint(new Coordinate(1, 2)), + factory.createLineString(new Coordinate[] { new Coordinate(1, 2), new Coordinate(3, 4) }) }) { + GeometryWrapper geometry = GeometryWrapperFactory.createGeometry(source, + SRS_URI.DEFAULT_WKT_CRS84, WKTDatatype.URI); + assertEquals(CoordinateSequenceDimensions.XY, geometry.getCoordinateSequenceDimensions()); + assertEquals(2, geometry.getCoordinateDimension()); + assertEquals(2, geometry.getSpatialDimension()); + } + } + public GeometryWrapperFactoryTest() { } diff --git a/jena-geosparql/src/test/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequenceFactoryTest.java b/jena-geosparql/src/test/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequenceFactoryTest.java new file mode 100644 index 0000000000..c1e63c3835 --- /dev/null +++ b/jena-geosparql/src/test/java/org/apache/jena/geosparql/implementation/jts/CustomCoordinateSequenceFactoryTest.java @@ -0,0 +1,129 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.apache.jena.geosparql.implementation.jts; + +import org.junit.Test; +import org.locationtech.jts.geom.CoordinateSequence; +import org.locationtech.jts.geom.CoordinateSequences; +import org.locationtech.jts.geom.impl.CoordinateArraySequence; +import org.locationtech.jts.geom.impl.CoordinateArraySequenceFactory; + +import static org.junit.Assert.*; + +public class CustomCoordinateSequenceFactoryTest { + @Test + public void allocationPreservesMeasuresAndMatchesJtsLayoutLimits() { + for (int size : new int[] { 0, 2 }) { + for (int[] layout : new int[][] { { 2, 0 }, { 3, 0 }, { 3, 1 }, { 4, 1 }, + { 1, 0 }, { 4, 0 }, { 5, 2 } }) { + CoordinateSequence expected = CoordinateArraySequenceFactory.instance().create(size, layout[0], layout[1]); + CoordinateSequence actual = new CustomCoordinateSequenceFactory().create(size, layout[0], layout[1]); + assertLayout(expected, actual); + if (size > 0 && actual.hasM()) { + actual.setOrdinate(0, actual.getDimension() - actual.getMeasures(), 8); + assertEquals(8, actual.getM(0), 0); + assertTrue(Double.isNaN(actual.getZ(0))); + } + } + } + } + + @Test + public void conversionPreservesLayoutsValuesAndIndependence() { + for (int[] layout : new int[][] { { 2, 0 }, { 3, 0 }, { 3, 1 }, { 4, 1 } }) { + CoordinateSequence source = new CoordinateArraySequence(2, layout[0], layout[1]); + for (int i = 0; i < source.size(); i++) { + source.setOrdinate(i, 0, 100 + i); + source.setOrdinate(i, 1, 10 + i); + if (source.hasZ()) + source.setOrdinate(i, 2, i == 0 ? Double.NaN : 9); + if (source.hasM()) + source.setOrdinate(i, source.getDimension() - 1, i == 0 ? Double.NaN : 8); + } + CoordinateSequence converted = new CustomCoordinateSequenceFactory().create(source); + assertLayout(source, converted); + for (int i = 0; i < source.size(); i++) { + assertEquals(source.getX(i), converted.getX(i), 0); + assertEquals(source.getY(i), converted.getY(i), 0); + assertEquals(source.getZ(i), converted.getZ(i), 0); + assertEquals(source.getM(i), converted.getM(i), 0); + } + converted.setOrdinate(1, CoordinateSequence.X, -1); + converted.setOrdinate(1, CoordinateSequence.Y, -1); + assertEquals(101, source.getX(1), 0); + assertEquals(11, source.getY(1), 0); + if (source.hasZ()) { + converted.setOrdinate(1, CoordinateSequence.Z, -1); + assertEquals(9, source.getZ(1), 0); + } + if (source.hasM()) { + converted.setOrdinate(1, converted.getDimension() - converted.getMeasures(), -1); + assertEquals(-1, converted.getM(1), 0); + assertEquals(8, source.getM(1), 0); + } + } + } + + @Test + public void convertedSequencesSupportGenericOrdinateCopying() { + for (int[] layout : new int[][] { { 2, 0 }, { 3, 0 }, { 3, 1 }, { 4, 1 } }) { + CoordinateSequence source = new CoordinateArraySequence(2, layout[0], layout[1]); + for (int i = 0; i < source.size(); i++) { + for (int ordinate = 0; ordinate < source.getDimension(); ordinate++) { + source.setOrdinate(i, ordinate, i == 0 && ordinate > 1 ? Double.NaN : 10 * i + ordinate); + } + } + CoordinateSequence converted = new CustomCoordinateSequenceFactory().create(source); + CoordinateSequence standardCopy = new CoordinateArraySequence(2, layout[0], layout[1]); + CoordinateSequences.copy(converted, 0, standardCopy, 0, source.size()); + CoordinateSequence customCopy = new CustomCoordinateSequenceFactory().create( + new CoordinateArraySequence(2, layout[0], layout[1])); + CoordinateSequences.copy(source, 0, customCopy, 0, source.size()); + for (CoordinateSequence result : new CoordinateSequence[] { converted, standardCopy, customCopy }) { + assertLayout(source, result); + for (int i = 0; i < source.size(); i++) { + for (int ordinate = 0; ordinate < source.getDimension(); ordinate++) { + assertEquals(source.getOrdinate(i, ordinate), result.getOrdinate(i, ordinate), 0); + } + assertEquals(source.getZ(i), result.getZ(i), 0); + assertEquals(source.getM(i), result.getM(i), 0); + } + } + } + } + + @Test + public void conversionPreservesEmptyLayouts() { + for (int[] layout : new int[][] { { 2, 0 }, { 3, 0 }, { 3, 1 }, { 4, 1 } }) { + CoordinateSequence source = new CoordinateArraySequence(0, layout[0], layout[1]); + assertLayout(source, new CustomCoordinateSequenceFactory().create(source)); + } + } + + private static void assertLayout(CoordinateSequence source, CoordinateSequence converted) { + assertNotSame(source, converted); + assertEquals(source.size(), converted.size()); + assertEquals(source.getDimension(), converted.getDimension()); + assertEquals(source.getMeasures(), converted.getMeasures()); + assertEquals(source.hasZ(), converted.hasZ()); + assertEquals(source.hasM(), converted.hasM()); + } +}
