This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
new 47e1f0fbf9 Add a test case for avoiding a regression in the
transformations from NAD27 to WGS84.
https://issues.apache.org/jira/browse/SIS-638
47e1f0fbf9 is described below
commit 47e1f0fbf99e82b64c89273c6d2b0eaf5f741084
Author: Martin Desruisseaux <[email protected]>
AuthorDate: Wed Oct 28 12:17:34 2026 +0100
Add a test case for avoiding a regression in the transformations from NAD27
to WGS84.
https://issues.apache.org/jira/browse/SIS-638
---
.../operation/CoordinateOperationRegistry.java | 4 +-
.../operation/DefaultConcatenatedOperation.java | 23 +++---
.../operation/CoordinateOperationRegistryTest.java | 85 ++++++++++++++++++----
.../test/org/apache/sis/test/LoggingWatcher.java | 17 +++++
4 files changed, 105 insertions(+), 24 deletions(-)
diff --git
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java
index ffe66b1401..b963d0f8d0 100644
---
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java
+++
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java
@@ -540,7 +540,7 @@ class CoordinateOperationRegistry {
* If at least one operation remains, we are done.
*/
if (decompose.source | decompose.target) {
- for (int i=operations.size(); --i >= 0;) {
+ for (int i = operations.size(); --i >= 0;) {
CoordinateOperation operation = operations.get(i);
operation = propagateVertical(sourceCRS,
targetCRS, operation, decompose);
if (operation != null) {
@@ -849,7 +849,7 @@ class CoordinateOperationRegistry {
private CoordinateOperation[] getSteps(final ConcatenatedOperation
operation, final boolean inverse)
throws NoninvertibleTransformException, FactoryException
{
- final var steps =
operation.getOperations().toArray(CoordinateOperation[]::new);
+ final CoordinateOperation[] steps =
operation.getOperations().toArray(CoordinateOperation[]::new);
CoordinateReferenceSystem previous = operation.getSourceCRS();
for (int i=0; i<steps.length; i++) {
final CoordinateOperation step = steps[i];
diff --git
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefaultConcatenatedOperation.java
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefaultConcatenatedOperation.java
index 8006da489e..da605cb36f 100644
---
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefaultConcatenatedOperation.java
+++
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefaultConcatenatedOperation.java
@@ -217,14 +217,14 @@ final class DefaultConcatenatedOperation extends
AbstractCoordinateOperation imp
* <h4>How coordinate operation accuracy is determined</h4>
* If {@code setAccuracy} is {@code true}, then this method copies
accuracy information found in the single
* {@link Transformation} instance. This method ignores instances of other
kinds for the following reason:
- * some {@link Conversion} instances declare an accuracy, which is
typically close to zero. If a concatenated
- * operation contains such conversion together with a transformation with
unknown accuracy, then we do not want
- * to declare "0 meter" as the concatenated operation accuracy; it would
be a false information.
+ * some {@link Conversion} instances declare an accuracy, which is
typically close to zero.
+ * If a concatenated operation contains such conversion together with a
transformation with unknown accuracy,
+ * we do not want to declare "0 meter" as the concatenated operation
accuracy; it would be a false information.
* Another reason is that a concatenated operation typically contains an
arbitrary number of conversions,
* but only one transformation. So considering only transformations
usually means to pickup only one operation
* in the given {@code operations} list, which make things clearer.
*
- * <h5>Note</h5>
+ * <h5>Compliance with <abbr>ISO</abbr> standard</h5>
* According ISO 19111, the accuracy attribute is allowed only for
transformations. However, this restriction
* is not enforced everywhere. For example, the EPSG database declares an
accuracy of 0 meter for conversions,
* which is conceptually exact. In this class we are departing from strict
interpretation of the specification
@@ -304,12 +304,14 @@ final class DefaultConcatenatedOperation extends
AbstractCoordinateOperation imp
} else {
/*
* We can skip identity transforms only if the operation is a
conversion.
- * We do not want to skip the tests for transformations,
because there is
- * may a change of datum and we want to keep the positional
accuracy metadata.
- *
- * Note: operation (source, target) may be in reverse order,
but it should be taken as metadata.
+ * We do not want to skip the step for transformations,
because it may be
+ * a change of datum and we want to keep the positional
accuracy metadata.
+ * We also need to ensure compatible source and target CRSs,
otherwise the
+ * `verifyStepChaining(…)` method may declare the operation
chain invalid.
*/
- if (!(step.isIdentity() && step instanceof Conversion)) {
+ if (!(step.isIdentity() && op instanceof Conversion
+ && Utilities.deepEquals(source, target,
ComparisonMode.APPROXIMATE)))
+ {
flattened.add(op);
}
previous = target; // For next iteration cycle.
@@ -345,6 +347,9 @@ final class DefaultConcatenatedOperation extends
AbstractCoordinateOperation imp
/**
* Verifies if a step of a concatenated operation can be chained after the
previous step.
+ * The source <abbr>CRS</abbr> should be equivalent to the target
<abbr>CRS</abbr> of the
+ * previous step, but this method accepts also swapped (<var>target</var>,
<var>source</var>)
+ * <abbr>CRS</abbr> pair.
*
* @param properties user-specified properties (for the locale of error
message), or {@code null} if none.
* @param step zero-based index of the operation step, used only
in case an exception it thrown.
diff --git
a/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java
b/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java
index 0af8b32bad..f75463460f 100644
---
a/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java
+++
b/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java
@@ -27,7 +27,9 @@ import org.opengis.referencing.operation.SingleOperation;
import org.opengis.referencing.operation.ConcatenatedOperation;
import org.opengis.referencing.operation.CoordinateOperation;
import org.opengis.referencing.operation.CoordinateOperationAuthorityFactory;
+import org.opengis.referencing.operation.NoninvertibleTransformException;
import org.opengis.referencing.operation.TransformException;
+import org.opengis.referencing.operation.MathTransform;
import org.apache.sis.referencing.CommonCRS;
import org.apache.sis.referencing.CRS;
import org.apache.sis.referencing.internal.shared.Formulas;
@@ -42,6 +44,8 @@ import org.junit.jupiter.api.TestInstance;
import static org.junit.jupiter.api.Assertions.*;
import org.junit.jupiter.api.parallel.Execution;
import org.junit.jupiter.api.parallel.ExecutionMode;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.apache.sis.test.LoggingWatcher;
import org.apache.sis.referencing.operation.transform.MathTransformTestCase;
import static
org.apache.sis.referencing.Assertions.assertEpsgNameAndIdentifierEqual;
import static org.apache.sis.test.TestCase.assumeConnectionToEPSG;
@@ -51,7 +55,7 @@ import org.opengis.metadata.Identifier;
/**
- * Tests {@link CoordinateOperationRegistry} using the EPSG geodetic dataset.
+ * Tests {@link CoordinateOperationRegistry} using the <abbr>EPSG</abbr>
geodetic dataset.
* If no EPSG geodetic dataset is available in the running environment, then
tests are skipped.
* For tests without the need of an EPSG database, see {@link
CoordinateOperationFinderTest}.
*
@@ -63,9 +67,13 @@ import org.opengis.metadata.Identifier;
* that does not go through WGS84. Furthermore, since the EPSG database
defines (λ,φ) axis order in
* addition to the usual (φ,λ) order for the target CRS, this tests
allows us to verify we can find
* this operation despite different axis order.</li>
+ * <li>All operations between <q>WGS 84</q> (EPSG:4326) and <q>NAD27</q>
(EPSG:4267).
+ * This is the opposite direction of the operations defined in the
<abbr>EPSG</abbr> database.
+ * <abbr>SIS</abbr> should nevertheless be able to create them by
creating the inverses of the
+ * operations from EPSG:4267 to EPSG:4326. We merely verifies that no
exception is thrown.</li>
* </ul>
*
- * The operations are tested with various axis order and dimension in source
and target CRS.
+ * The operations are tested with various axis order and dimension in source
and target <abbr>CRS</abbr>s.
*
* @author Martin Desruisseaux (Geomatys)
*/
@@ -73,6 +81,12 @@ import org.opengis.metadata.Identifier;
@Execution(ExecutionMode.SAME_THREAD)
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
public final class CoordinateOperationRegistryTest extends
MathTransformTestCase {
+ /**
+ * A JUnit extension for listening to log events.
+ */
+ @RegisterExtension
+ public final LoggingWatcher loggings;
+
/**
* The transformation factory to use for testing.
*/
@@ -101,6 +115,7 @@ public final class CoordinateOperationRegistryTest extends
MathTransformTestCase
* @throws FactoryException if an error occurred while creating the
factory to be tested.
*/
public CoordinateOperationRegistryTest() throws ParseException,
FactoryException {
+ loggings = new LoggingWatcher(AbstractCoordinateOperation.LOGGER);
crsFactory = CRS.getAuthorityFactory("EPSG");
assumeConnectionToEPSG(crsFactory instanceof
CoordinateOperationAuthorityFactory);
factory = new DefaultCoordinateOperationFactory();
@@ -120,6 +135,7 @@ public final class CoordinateOperationRegistryTest extends
MathTransformTestCase
@BeforeEach
public void reset() {
super.reset();
+ loggings.clear();
}
/**
@@ -143,6 +159,7 @@ public final class CoordinateOperationRegistryTest extends
MathTransformTestCase
/**
* Tests <q>NTF (Paris) to WGS 84 (1)</q> operation with source and target
CRS conform to EPSG definitions.
+ * This operation implies a longitude rotation followed by a geocentric
translation in the geographic domain.
*
* @throws ParseException if a CRS used in this test cannot be parsed.
* @throws FactoryException if the operation cannot be created.
@@ -175,12 +192,13 @@ public final class CoordinateOperationRegistryTest
extends MathTransformTestCase
verifyTransform(new double[] {54.271680278, 0.098269657}, // in
grads east of Paris
new double[] {48.844443528, 2.424952028}); // in
degrees east of Greenwich
validate();
+ loggings.assertNoUnexpectedLog();
}
/**
- * Tests <q>NTF (Paris) to WGS 84 (1)</q> operation with normalized source
and target CRS.
- * {@link CoordinateOperationRegistry} should be able to find the
operation despite the difference
- * in axis order an units.
+ * Tests <q>NTF (Paris) to WGS 84 (1)</q> operation with normalized source
and target <abbr>CRS</abbr>s.
+ * {@link CoordinateOperationRegistry} should be able to find the
operation despite the difference in axis
+ * order an units.
*
* @throws ParseException if a CRS used in this test cannot be parsed.
* @throws FactoryException if the operation cannot be created.
@@ -207,6 +225,7 @@ public final class CoordinateOperationRegistryTest extends
MathTransformTestCase
verifyTransform(new double[] {0.088442691, 48.844512250}, // in
degrees east of Paris
new double[] {2.424952028, 48.844443528}); // in
degrees east of Greenwich
validate();
+ loggings.assertNoUnexpectedLog();
}
/**
@@ -236,6 +255,7 @@ public final class CoordinateOperationRegistryTest extends
MathTransformTestCase
verifyTransform(new double[] {2.424952028, 48.844443528}, // in
degrees east of Greenwich
new double[] {0.098269657, 54.271680278}); // in
grads east of Paris
validate();
+ loggings.assertNoUnexpectedLog();
}
/**
@@ -270,11 +290,12 @@ public final class CoordinateOperationRegistryTest
extends MathTransformTestCase
verifyTransform(new double[] {54.271680278, 0.098269657, 20.00},
// in grads east of Paris
new double[] {48.844443528, 2.424952028, 63.15});
// in degrees east of Greenwich
validate();
+ loggings.assertNoUnexpectedLog();
}
/**
- * Tests <q>NTF (Paris) to WGS 84 (1)</q> operation with three-dimensional
source and target CRS
- * having different axis order and units than the ones declared in the
EPSG dataset.
+ * Tests <q>NTF (Paris) to WGS 84 (1)</q> operation with three-dimensional
source and target <abbr>CRS</abbr>s
+ * having different axis order and units than the ones declared in the
<abbr>EPSG</abbr> geodetic dataset.
*
* @throws ParseException if a CRS used in this test cannot be parsed.
* @throws FactoryException if the operation cannot be created.
@@ -304,6 +325,7 @@ public final class CoordinateOperationRegistryTest extends
MathTransformTestCase
verifyTransform(new double[] {0.088442691, 48.844512250, 20.00},
// in degrees east of Paris
new double[] {2.424952028, 48.844443528, 63.15});
// in degrees east of Greenwich
validate();
+ loggings.assertNoUnexpectedLog();
}
/**
@@ -313,11 +335,11 @@ public final class CoordinateOperationRegistryTest
extends MathTransformTestCase
* @param isEPSG {@code true} if the coordinate operation is expected to
contain EPSG identifiers.
*/
private static void verifyNTF(final CoordinateOperation operation, final
String domain, final boolean isEPSG) {
- assertInstanceOf(ConcatenatedOperation.class, operation, "Operation
should have two steps.");
- final List<? extends CoordinateOperation> steps =
((ConcatenatedOperation) operation).getOperations();
+ final List<? extends CoordinateOperation> steps =
+ assertInstanceOf(ConcatenatedOperation.class, operation,
"Operation should have two steps.").getOperations();
assertEquals(2, steps.size(), "Operation should have two steps.");
- final SingleOperation step1 = (SingleOperation) steps.get(0);
- final SingleOperation step2 = (SingleOperation) steps.get(1);
+ final SingleOperation step1 = assertInstanceOf(SingleOperation.class,
steps.get(0));
+ final SingleOperation step2 = assertInstanceOf(SingleOperation.class,
steps.get(1));
if (isEPSG) {
assertEpsgNameAndIdentifierEqual("NTF (Paris) to WGS 84 (1)",
8094, operation);
assertEpsgNameAndIdentifierEqual("NTF (Paris)",
4807, operation.getSourceCRS());
@@ -366,7 +388,7 @@ public final class CoordinateOperationRegistryTest extends
MathTransformTestCase
* instead of EPSG:5489. Both are <q>RGAF09</q>, but the former use
(longitude, latitude) axis
* order instead of the usual (latitude, longitude) order. The source CRS
stay fixed to EPSG:4625.
*
- * @throws FactoryException if an error occurred while creating a CRS or
operation.
+ * @throws FactoryException if an error occurred while creating a
<abbr>CRS</abbr> or an operation.
*/
@Test
public void testFindDespiteDifferentAxisOrder() throws FactoryException {
@@ -381,7 +403,7 @@ public final class CoordinateOperationRegistryTest extends
MathTransformTestCase
targetCRS = crsFactory.createGeographicCRS("EPSG:7086");
operation = createOperation(sourceCRS, targetCRS);
assertEpsgNameWithoutIdentifierEqual("Martinique 1938 to RGAF09 (1)",
operation);
- final ParameterValueGroup p = ((SingleOperation)
operation).getParameterValues();
+ final ParameterValueGroup p = assertInstanceOf(SingleOperation.class,
operation).getParameterValues();
/*
* Values below are copied from EPSG geodetic dataset 9.1. They may
need
* to be adjusted if a future version of EPSG dataset modify those
values.
@@ -394,5 +416,42 @@ public final class CoordinateOperationRegistryTest extends
MathTransformTestCase
assertEquals( -0.8837, p.parameter("Z-axis rotation")
.doubleValue());
assertEquals( 14.1012, p.parameter("Scale difference")
.doubleValue());
assertEquals( 0.1, CRS.getLinearAccuracy(operation));
+ loggings.assertNoUnexpectedLog();
+ }
+
+ /**
+ * Tests operations from <q>WGS 84</q> (EPSG:4326) to <q>NAD27</q>
(EPSG:4267).
+ * This is the opposite direction of the operations defined in the
<abbr>EPSG</abbr> database.
+ * <abbr>SIS</abbr> should nevertheless be able to create them by creating
the inverses of the
+ * operations from EPSG:4267 to EPSG:4326.
+ *
+ * @throws FactoryException if an error occurred while creating a
<abbr>CRS</abbr> or an operation.
+ * @throws NoninvertibleTransformException if a transform cannot be
inverted.
+ *
+ * @see <a href="https://issues.apache.org/jira/browse/SIS-638";>SIS-638</a>
+ */
+ @Test
+ public void testInverseOperations() throws FactoryException,
NoninvertibleTransformException {
+ registry.stopAtFirst = false;
+ CoordinateReferenceSystem source =
crsFactory.createGeographicCRS("EPSG:4326");
+ CoordinateReferenceSystem target =
crsFactory.createGeographicCRS("EPSG:4267");
+ List<CoordinateOperation> operations =
registry.createOperations(source, target);
+ /*
+ * The EPSG database version 13.101 contains 78 operations from
EPSG:4267 to EPSG:4326,
+ * but many of them require some NADCON or NTv2 files which may not be
available on the
+ * machine running this test. However, 30 operations do not require
any file and should
+ * be created in all cases.
+ */
+ final int count = operations.size();
+ assertTrue(count >= 30, () -> "Found " + count + " operations,
expected more than that.");
+ for (CoordinateOperation operation : operations) {
+ final String name = operation.getName().getCode();
+ transform = operation.getMathTransform();
+ MathTransform inverse = transform.inverse();
+ assertNotEquals(transform, inverse, name);
+ assertSame(transform, inverse.inverse(), name);
+ }
+ loggings.skipLogsWhile((message) -> message.contains("NADCON") ||
message.contains("NTv2"));
+ loggings.assertNoUnexpectedLog();
}
}
diff --git
a/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/LoggingWatcher.java
b/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/LoggingWatcher.java
index 5bcf2367f8..6e46d82825 100644
---
a/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/LoggingWatcher.java
+++
b/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/LoggingWatcher.java
@@ -23,6 +23,7 @@ import java.util.LinkedList;
import java.util.ConcurrentModificationException;
import java.util.Objects;
import java.util.function.Consumer;
+import java.util.function.Predicate;
import java.util.logging.Filter;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -298,6 +299,22 @@ public final class LoggingWatcher implements
BeforeEachCallback, AfterEachCallba
messages.add(new Message(formatter.formatMessage(record),
record.getThrown()));
}
+ /**
+ * Skips the next logs until the given condition become false.
+ * The filter receive a lot message in argument and returns whether that
log should be skipped.
+ *
+ * @param filter a predicate which, given a log message, returns whether
to skip that log.
+ */
+ public synchronized void skipLogsWhile(final Predicate<String> filter) {
+ Message message;
+ while ((message = messages.peek()) != null) {
+ if (!filter.test(message.text)) break;
+ if (messages.remove() != message) {
+ throw new ConcurrentModificationException();
+ }
+ }
+ }
+
/**
* Skips the next log messages if it contains all the given keywords.
* This method is used instead of {@link
#assertNextLogContains(String...)} when a log message may or
