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 2304ae050f Fix `AbstractCoordinateOperation.equals(…)` when one object
is a defining conversion and the other is a fully defined conversion. In such
case, we need to complete the defining conversion before the comparison can be
done.
2304ae050f is described below
commit 2304ae050f1559820d2bcbfec4ef56bf647d9fcd
Author: Martin Desruisseaux <[email protected]>
AuthorDate: Sun Aug 16 20:10:18 2026 +0200
Fix `AbstractCoordinateOperation.equals(…)` when one object is a defining
conversion and the other is a fully defined conversion.
In such case, we need to complete the defining conversion before the
comparison can be done.
---
.../operation/AbstractCoordinateOperation.java | 111 ++++++++++++++-------
.../operation/AbstractSingleOperation.java | 7 +-
.../referencing/operation/DefiningConversion.java | 69 +++++++++++++
.../org/apache/sis/referencing/CommonCRSTest.java | 22 +++-
4 files changed, 167 insertions(+), 42 deletions(-)
diff --git
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/AbstractCoordinateOperation.java
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/AbstractCoordinateOperation.java
index 088f11239a..d8968ec4d5 100644
---
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/AbstractCoordinateOperation.java
+++
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/AbstractCoordinateOperation.java
@@ -51,6 +51,7 @@ import org.apache.sis.io.wkt.Formatter;
import org.apache.sis.io.wkt.FormattableObject;
import org.apache.sis.io.wkt.Convention;
import org.apache.sis.util.Classes;
+import org.apache.sis.util.Utilities;
import org.apache.sis.util.ComparisonMode;
import org.apache.sis.util.UnsupportedImplementationException;
import org.apache.sis.util.collection.Containers;
@@ -72,7 +73,6 @@ import
org.apache.sis.metadata.internal.shared.ImplementationHelper;
import org.apache.sis.util.internal.shared.Constants;
import org.apache.sis.system.Semaphores;
import org.apache.sis.system.Loggers;
-import static org.apache.sis.util.Utilities.deepEquals;
// Specific to the geoapi-3.1 and geoapi-4.0 branches:
import org.opengis.referencing.crs.DerivedCRS;
@@ -593,7 +593,7 @@ check: for (int isTarget=0; ; isTarget++) { //
0 == source check; 1
* <li>If at least one {@linkplain
org.apache.sis.metadata.iso.quality.DefaultQuantitativeResult quantitative
* result} is found with a linear unit, then returns the largest
result value converted to metres.</li>
*
- * <li>Otherwise if the operation is a {@linkplain DefaultConversion
conversion},
+ * <li>Otherwise if the operation is a {@linkplain Conversion
conversion},
* then returns 0 since a conversion is by definition accurate up to
rounding errors.</li>
*
* <li>Otherwise if the operation is a {@linkplain DefaultTransformation
transformation},
@@ -828,6 +828,18 @@ check: for (int isTarget=0; ; isTarget++) { //
0 == source check; 1
}
}
+ /**
+ * Returns a coordinate operation to substitute to this operation for
comparison purposes.
+ * A substitution may be necessary for making possible to compare the
{@link #transform}
+ * if one of the operations to compare is a defining operation.
+ *
+ * @param other the other operation which will be compared with this
operation.
+ * @return an operation which can be compared with {@code other}.
+ */
+ CoordinateOperation comparableTo(final CoordinateOperation other) {
+ return this;
+ }
+
/**
* Compares this coordinate operation with the specified object for
equality. If the {@code mode} argument
* is {@link ComparisonMode#STRICT} or {@link ComparisonMode#BY_CONTRACT
BY_CONTRACT}, then all available
@@ -865,37 +877,46 @@ check: for (int isTarget=0; ; isTarget++) {
// 0 == source check; 1
* - Scope, domain and accuracy properties only if NOT in
"ignore metadata" mode.
* - Interpolation CRS in all cases (regardless if ignoring
metadata or not).
*/
- final var that = (CoordinateOperation) object;
+ CoordinateOperation that = (CoordinateOperation) object;
if ((mode.isIgnoringMetadata() ||
- (deepEquals(getCoordinateOperationAccuracy(),
that.getCoordinateOperationAccuracy(), mode))) &&
- deepEquals(getInterpolationCRS(),
that.getInterpolationCRS(), mode))
+ (Utilities.deepEquals(getCoordinateOperationAccuracy(),
that.getCoordinateOperationAccuracy(), mode))) &&
+ Utilities.deepEquals(getInterpolationCRS(),
that.getInterpolationCRS(), mode))
{
+ /*
+ * If one of the operation is a defining conversion, i.e.
a conversion where the transform
+ * has not yet been computed, while the other operation is
a fully-defined conversion, then
+ * we need to complete the defining operation before we
can compare the transforms.
+ */
+ if (that instanceof AbstractCoordinateOperation) {
+ that = ((AbstractCoordinateOperation)
that).comparableTo(this);
+ }
+ final CoordinateOperation self = comparableTo(that);
/*
* At this point all metadata match or can be ignored.
First, compare the targetCRS.
* We need to perform this comparison only if this
`equals(…)` method is not invoked
- * from AbstractDerivedCRS, otherwise we would fall in an
infinite recursive loop
- * (because targetCRS is the DerivedCRS, which in turn
wants to compare this operation).
+ * from `AbstractDerivedCRS.equals(…)`, otherwise the
latter would call this method
+ * again for comparing conversions and we would fall in an
infinite recursive loop.
*
- * We also opportunistically use this "anti-recursion"
check for another purpose.
- * The Semaphores.COMPARING flag should be set only when
AbstractDerivedCRS is comparing
+ * We also opportunistically use this anti-recursion check
for another purpose.
+ * The `Semaphores.COMPARING` flag should be set only when
`AbstractDerivedCRS` is comparing
* its "from base" conversion. The flag should never be
set in any other circumstance,
* since this is an internal Apache SIS mechanism. If we
know that we are comparing the
- * AbstractDerivedCRS.fromBase conversion, then (in the
way Apache SIS is implemented)
- * this.sourceCRS == AbstractDerivedCRS.baseCRS.
Consequently, we can relax the check of
- * sourceCRS axis order if the mode is
ComparisonMode.IGNORE_METADATA.
+ * `AbstractDerivedCRS.fromBase` conversion, then (in the
way Apache SIS is implemented)
+ * `this.sourceCRS == AbstractDerivedCRS.baseCRS`.
Consequently, we can relax the check
+ * of `sourceCRS` axis order if the mode is
`ComparisonMode.IGNORE_METADATA`.
*/
boolean debug = false;
- if (Semaphores.COMPARING_CONVERSION_OR_DERIVED_CRS.set())
try {
- if (!deepEquals(getTargetCRS(), that.getTargetCRS(),
mode)) {
- return false;
- }
- } finally {
- Semaphores.COMPARING_CONVERSION_OR_DERIVED_CRS.clear();
- } else {
- if (mode.isIgnoringMetadata()) {
- debug = (mode == ComparisonMode.DEBUG);
- mode = ComparisonMode.ALLOW_VARIANT;
+ if (Semaphores.COMPARING_CONVERSION_OR_DERIVED_CRS.set()) {
+ try {
+ if (!Utilities.deepEquals(self.getTargetCRS(),
that.getTargetCRS(), mode)) {
+ return false;
+ }
+ } finally {
+
Semaphores.COMPARING_CONVERSION_OR_DERIVED_CRS.clear();
}
+ } else if (mode.isIgnoringMetadata()) {
+ debug = (mode == ComparisonMode.DEBUG);
+ mode = ComparisonMode.ALLOW_VARIANT;
}
/*
* Now compare the sourceCRS, potentially with a relaxed
ComparisonMode (see above comment).
@@ -903,24 +924,23 @@ check: for (int isTarget=0; ; isTarget++) {
// 0 == source check; 1
* need to take in account those difference before to
compare the MathTransform. We proceed
* by modifying `tr2` as if it was a MathTransform with
crs1 as the source instead of crs2.
*/
- final CoordinateReferenceSystem crs1 = this.getSourceCRS();
+ final CoordinateReferenceSystem crs1 = self.getSourceCRS();
final CoordinateReferenceSystem crs2 = that.getSourceCRS();
- if (deepEquals(crs1, crs2, mode)) {
- MathTransform tr1 = this.getMathTransform();
+ if (Utilities.deepEquals(crs1, crs2, mode)) {
+ MathTransform tr1 = self.getMathTransform();
MathTransform tr2 = that.getMathTransform();
- if (mode.ordinal() >=
ComparisonMode.ALLOW_VARIANT.ordinal()) try {
- final MathTransform before = MathTransforms.linear(
-
CoordinateSystems.swapAndScaleAxes(crs1.getCoordinateSystem(),
-
crs2.getCoordinateSystem()));
- final MathTransform after = MathTransforms.linear(
-
CoordinateSystems.swapAndScaleAxes(that.getTargetCRS().getCoordinateSystem(),
-
this.getTargetCRS().getCoordinateSystem()));
- tr2 = MathTransforms.concatenate(before, tr2,
after);
- } catch (IncommensurableException | RuntimeException
e) {
- Logging.ignorableException(LOGGER,
AbstractCoordinateOperation.class, "equals", e);
+ if (tr2 != null && mode.ordinal() >=
ComparisonMode.ALLOW_VARIANT.ordinal()) {
+ final MathTransform before =
swapAndScaleAxes(crs1, crs2);
+ final MathTransform after =
swapAndScaleAxes(that.getTargetCRS(), self.getTargetCRS());
+ if (before != null) tr2 =
MathTransforms.concatenate(before, tr2);
+ if (after != null) tr2 =
MathTransforms.concatenate(tr2, after);
+ }
+ if (Utilities.deepEquals(tr1, tr2, mode)) {
+ return true;
+ } else if (debug) {
+ // Try to identify the mismatched property.
+ assert Utilities.deepEquals(tr1, tr2,
ComparisonMode.DEBUG);
}
- if (deepEquals(tr1, tr2, mode)) return true;
- assert !debug || deepEquals(tr1, tr2,
ComparisonMode.DEBUG); // For locating the mismatch.
}
}
}
@@ -928,6 +948,23 @@ check: for (int isTarget=0; ; isTarget++) { //
0 == source check; 1
return false;
}
+ /**
+ * Returns a transform for applying a change of axis order and units of
measurement between two <abbr>CRS</abbr>s.
+ * If any <abbr>CRS</abbr> is {@code null}, which may happen with defining
operations, then returns {@code null}.
+ */
+ private static MathTransform swapAndScaleAxes(final
CoordinateReferenceSystem crs1,
+ final
CoordinateReferenceSystem crs2)
+ {
+ if (crs1 != null && crs2 != null) try {
+ return
MathTransforms.linear(CoordinateSystems.swapAndScaleAxes(crs1.getCoordinateSystem(),
+
crs2.getCoordinateSystem()));
+ } catch (IncommensurableException | RuntimeException e) {
+ // Declare `equals` as the source method becayse it is the public
API.
+ Logging.ignorableException(LOGGER,
AbstractCoordinateOperation.class, "equals", e);
+ }
+ return null;
+ }
+
/**
* Invoked by {@code hashCode()} for computing the hash code when first
needed.
* See {@link AbstractIdentifiedObject#computeHashCode()} for more
information.
diff --git
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/AbstractSingleOperation.java
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/AbstractSingleOperation.java
index 2fe5df6bf1..c97b956b88 100644
---
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/AbstractSingleOperation.java
+++
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/AbstractSingleOperation.java
@@ -303,10 +303,9 @@ class AbstractSingleOperation extends
AbstractCoordinateOperation implements Sin
}
}
/*
- * We consider the operation method as metadata. One could argue that
OperationMethod's `sourceDimension` and
- * `targetDimension` are not metadata, but their values should be
identical to the `sourceCRS` and `targetCRS`
- * dimensions, already checked above. We could also argue that
`OperationMethod.parameters` are not metadata,
- * but their values should have been taken in account for the
MathTransform creation, compared above.
+ * We consider the operation method and parameters as metadata.
Conceptually, they are not metadata
+ * because a change of method or parameter value may change the result
of coordinate operations.
+ * But Apache SIS implementation verifies equivalence by comparing the
`MathTransform`s instead.
*
* Comparing the MathTransforms instead of parameters avoid the
problem of implicit parameters. For example, in
* a ProjectedCRS, the "semiMajor" and "semiMinor" axis lengths are
sometimes provided as explicit parameters,
diff --git
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefiningConversion.java
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefiningConversion.java
index c5e05da4bc..3f39d18684 100644
---
a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefiningConversion.java
+++
b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/DefiningConversion.java
@@ -18,15 +18,20 @@ package org.apache.sis.referencing.operation;
import java.util.Map;
import jakarta.xml.bind.annotation.XmlTransient;
+import org.opengis.util.FactoryException;
import org.opengis.parameter.ParameterValueGroup;
import org.opengis.referencing.crs.DerivedCRS;
import org.opengis.referencing.crs.CoordinateReferenceSystem;
+import org.opengis.referencing.operation.MathTransformFactory;
+import org.opengis.referencing.operation.CoordinateOperation;
import org.opengis.referencing.operation.OperationMethod;
import org.opengis.referencing.operation.MathTransform;
+import org.opengis.referencing.operation.Conversion;
import org.apache.sis.referencing.cs.AxesConvention;
import org.apache.sis.referencing.internal.Resources;
import
org.apache.sis.referencing.operation.transform.DefaultMathTransformFactory;
import org.apache.sis.util.collection.Containers;
+import org.apache.sis.util.logging.Logging;
/**
@@ -79,6 +84,13 @@ public class DefiningConversion extends DefaultConversion {
*/
private final boolean normalized;
+ /**
+ * Cached result of the call to {@code specialize(…)}.
+ *
+ * @see #specialize(CoordinateReferenceSystem, CoordinateReferenceSystem,
MathTransformFactory)
+ */
+ private transient volatile Conversion cached;
+
/**
* Creates a defining conversion from the given transform and/or
parameters.
* This conversion has no source and target <abbr>CRS</abbr> since those
elements
@@ -160,4 +172,61 @@ public class DefiningConversion extends DefaultConversion {
public boolean normalized() {
return normalized;
}
+
+ /**
+ * Returns a specialization of this conversion with non-null
<abbr>CRS</abbr>s.
+ * This method should be invoked when more information become available
about the conversion to create.
+ *
+ * @param sourceCRS the source <abbr>CRS</abbr>.
+ * @param targetCRS the target <abbr>CRS</abbr>.
+ * @param factory the factory to use for creating a transform from the
parameters
+ * or for performing axis changes, or {@code null} for the default
factory.
+ * @return conversion which declares the given <abbr>CRS</abbr>s as the
source and target.
+ * @throws FactoryException if the creation of a {@link MathTransform}
from the {@linkplain #getParameterValues()
+ * parameter values} failed.
+ */
+ @Override
+ public Conversion specialize(final CoordinateReferenceSystem sourceCRS,
+ final CoordinateReferenceSystem targetCRS,
+ MathTransformFactory factory) throws
FactoryException
+ {
+ Conversion specialized;
+ final boolean cache = (factory == null) || (factory ==
DefaultMathTransformFactory.provider());
+ if (cache) {
+ specialized = cached;
+ if (specialized != null
+ && specialized.getSourceCRS().equals(sourceCRS)
+ && specialized.getTargetCRS().equals(targetCRS))
+ {
+ return specialized;
+ }
+ }
+ specialized = super.specialize(sourceCRS, targetCRS, factory);
+ if (cache) {
+ cached = specialized;
+ }
+ return specialized;
+ }
+
+ /**
+ * Returns a conversion which can be compared with the given object.
+ * IF {@code other} is not a defining conversion, then this method returns
a fully-defined conversion
+ * resolved with the same source and target <abbr>CRS</abbr>s as {@code
other}.
+ * This is necessary for allowing the comparison of {@link
#getMathTransform()}.
+ *
+ * @param other the other operation which will be compared with this
defining conversion.
+ * @return an operation which can be compared with {@code other}.
+ */
+ @Override
+ final CoordinateOperation comparableTo(final CoordinateOperation other) {
+ if (getSourceCRS() == null && getTargetCRS() == null) { //
Verified by precaution.
+ CoordinateReferenceSystem crs1, crs2;
+ if ((crs1 = other.getSourceCRS()) != null && (crs2 =
other.getTargetCRS()) != null) try {
+ return specialize(crs1, crs2, null);
+ } catch (FactoryException e) {
+ Logging.ignorableException(LOGGER, DefiningConversion.class,
"equals", e);
+ }
+ }
+ return this;
+ }
}
diff --git
a/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/CommonCRSTest.java
b/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/CommonCRSTest.java
index acf655e18d..c8fc3ba2fc 100644
---
a/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/CommonCRSTest.java
+++
b/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/CommonCRSTest.java
@@ -36,8 +36,10 @@ import org.opengis.referencing.cs.CoordinateSystem;
import org.opengis.referencing.cs.EllipsoidalCS;
import org.opengis.referencing.datum.TemporalDatum;
import org.opengis.referencing.datum.VerticalDatum;
+import org.opengis.referencing.operation.Conversion;
import org.apache.sis.metadata.iso.citation.Citations;
import org.apache.sis.metadata.internal.shared.AxisNames;
+import org.apache.sis.referencing.factory.IdentifiedObjectFinder;
import org.apache.sis.referencing.internal.VerticalDatumTypes;
import org.apache.sis.util.internal.shared.Constants;
import static org.apache.sis.util.internal.shared.Constants.UTC;
@@ -331,7 +333,7 @@ public final class CommonCRSTest extends TestCase {
}
/**
- * Tests the URN lookup on temporal CRS.
+ * Tests the <abbr>URN</abbr> lookup on temporal <abbr>CRS</abbr>.
*
* @throws FactoryException if a call to {@link
IdentifiedObjects#lookupURN lookupURN(…)} failed.
*/
@@ -343,6 +345,24 @@ public final class CommonCRSTest extends TestCase {
assertEquals("urn:ogc:def:crs:OGC::TruncatedJulianDate",
IdentifiedObjects.lookupURN(crs, Citations.OGC));
}
+ /**
+ * Verifies the query of the <abbr>EPSG</abbr> of an <abbr>UTM</abbr>
projection.
+ *
+ * @throws FactoryException if a call to {@link
IdentifiedObjects#newFinder newFinder(…)} failed.
+ */
+ @Test
+ public void testLookupEPSG() throws FactoryException {
+ final ProjectedCRS crs = CommonCRS.WGS84.universal(45, 3); // UTM
zone 31N.
+ final IdentifiedObjectFinder finder =
IdentifiedObjects.newFinder(Constants.EPSG);
+ finder.setSearchDomain(IdentifiedObjectFinder.Domain.DECLARATION);
+ assertSame(crs, finder.findSingleton(crs));
+ assertEquals(32631, IdentifiedObjects.lookupEPSG(crs));
+
+ final Conversion fromBase = crs.getConversionFromBase();
+ assertEqualsIgnoreMetadata(fromBase, finder.findSingleton(fromBase));
+ assertEquals(16031, IdentifiedObjects.lookupEPSG(fromBase));
+ }
+
/**
* Tests {@link CommonCRS#universal(double, double)} with Universal
Transverse Mercator (UTM) projections.
*/
