Author: desruisseaux Date: Sat Aug 12 13:59:38 2017 New Revision: 1804866 URL: http://svn.apache.org/viewvc?rev=1804866&view=rev Log: Fix a bug that prevented CoordinateOperationRegistry registry to find an operation in the EPSG geodetic dataset when EPSG defines two versions of the same CRS with different axis order.
Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java?rev=1804866&r1=1804865&r2=1804866&view=diff ============================================================================== --- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java [UTF-8] (original) +++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/factory/IdentifiedObjectFinder.java [UTF-8] Sat Aug 12 13:59:38 2017 @@ -58,7 +58,7 @@ import org.apache.sis.util.Utilities; * is thread-safe. If concurrent searches are desired, then a new instance should be created for each thread. * * @author Martin Desruisseaux (IRD, Geomatys) - * @version 0.7 + * @version 0.8 * * @see GeodeticAuthorityFactory#newIdentifiedObjectFinder() * @see IdentifiedObjects#newFinder(String) @@ -302,7 +302,7 @@ public class IdentifiedObjectFinder { final AuthorityFactoryProxy<?> previous = proxy; proxy = AuthorityFactoryProxy.getInstance(object.getClass()); try { - if (!ignoreIdentifiers) { + if (!ignoreIdentifiers && !ignoreAxes) { /* * First check if one of the identifiers can be used to find directly an identified object. * Verify that the object that we found is actually equal to given one; we do not blindly Modified: sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java?rev=1804866&r1=1804865&r2=1804866&view=diff ============================================================================== --- sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java [UTF-8] (original) +++ sis/branches/JDK8/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/CoordinateOperationRegistry.java [UTF-8] Sat Aug 12 13:59:38 2017 @@ -257,19 +257,27 @@ class CoordinateOperationRegistry { } /** - * Finds the authority code for the given coordinate reference system. + * Finds the authority codes for the given coordinate reference system. * This method does not trust the code given by the user in its CRS - we verify it. - * This method may return a code even if the axis order does not match; + * This method may return codes even if the axis order does not match; * it will be caller's responsibility to make necessary adjustments. */ - private String findCode(final CoordinateReferenceSystem crs) throws FactoryException { + private List<String> findCode(final CoordinateReferenceSystem crs) throws FactoryException { + final List<String> codes = new ArrayList<>(); if (codeFinder != null) { - final Identifier identifier = IdentifiedObjects.getIdentifier(codeFinder.findSingleton(crs), null); - if (identifier != null) { - return identifier.getCode(); + for (final IdentifiedObject candidate : codeFinder.find(crs)) { + final Identifier identifier = IdentifiedObjects.getIdentifier(candidate, registry.getAuthority()); + if (identifier != null) { + final String code = identifier.getCode(); + if (Utilities.deepEquals(candidate, crs, ComparisonMode.APPROXIMATIVE)) { + codes.add(0, code); // If axis order match, give precedence to that CRS. + } else { + codes.add(code); + } + } } } - return null; + return codes; } /** @@ -366,56 +374,58 @@ class CoordinateOperationRegistry { final CoordinateReferenceSystem targetCRS) throws IllegalArgumentException, IncommensurableException, FactoryException { - final String sourceID = findCode(sourceCRS); - if (sourceID == null) { - return null; - } - final String targetID = findCode(targetCRS); - if (targetID == null) { - return null; - } - if (sourceID.equals(targetID)) { - /* - * Above check is necessary because this method may be invoked in some situations where the code - * are equal while the CRS are not. Such situation should be illegal, but unfortunately it still - * happen because many softwares are not compliant with EPSG definition of axis order. In such - * cases we will need to compute a transform from sourceCRS to targetCRS ignoring the source and - * target codes. The CoordinateOperationFinder class can do that, providing that we prevent this - * CoordinateOperationRegistry to (legitimately) claims that the operation from sourceCode to - * targetCode is the identity transform. - */ - return null; - } - final boolean inverse; - Collection<CoordinateOperation> operations; - boolean mdOnly = Semaphores.queryAndSet(Semaphores.METADATA_ONLY); // See comment for the same call inside the loop. - try { - try { - operations = registry.createFromCoordinateReferenceSystemCodes(sourceID, targetID); - inverse = Containers.isNullOrEmpty(operations); - if (inverse) { + final List<String> sources = findCode(sourceCRS); if (sources.isEmpty()) return null; + final List<String> targets = findCode(targetCRS); if (targets.isEmpty()) return null; + Collection<CoordinateOperation> operations = null; + boolean inverse = false; + for (final String sourceID : sources) { + for (final String targetID : targets) { + if (sourceID.equals(targetID)) { /* - * No operation from 'source' to 'target' available. But maybe there is an inverse operation. - * This is typically the case when the user wants to convert from a projected to a geographic CRS. - * The EPSG database usually contains transformation paths for geographic to projected CRS only. + * Above check is necessary because this method may be invoked in some situations where the code + * are equal while the CRS are not. Such situation should be illegal, but unfortunately it still + * happen because many softwares are not compliant with EPSG definition of axis order. In such + * cases we will need to compute a transform from sourceCRS to targetCRS ignoring the source and + * target codes. The CoordinateOperationFinder class can do that, providing that we prevent this + * CoordinateOperationRegistry to (legitimately) claims that the operation from sourceCode to + * targetCode is the identity transform. */ - operations = registry.createFromCoordinateReferenceSystemCodes(targetID, sourceID); - if (Containers.isNullOrEmpty(operations)) { - return null; - } + return null; } - } finally { - if (!mdOnly) { - Semaphores.clear(Semaphores.METADATA_ONLY); + final boolean mdOnly = Semaphores.queryAndSet(Semaphores.METADATA_ONLY); + try { + try { + operations = registry.createFromCoordinateReferenceSystemCodes(sourceID, targetID); + inverse = Containers.isNullOrEmpty(operations); + if (inverse) { + /* + * No operation from 'source' to 'target' available. But maybe there is an inverse operation. + * This is typically the case when the user wants to convert from a projected to a geographic CRS. + * The EPSG database usually contains transformation paths for geographic to projected CRS only. + */ + operations = registry.createFromCoordinateReferenceSystemCodes(targetID, sourceID); + if (Containers.isNullOrEmpty(operations)) { + continue; + } + } + } finally { + if (!mdOnly) { + Semaphores.clear(Semaphores.METADATA_ONLY); + } + } + } catch (NoSuchAuthorityCodeException | MissingFactoryResourceException e) { + /* + * sourceCode or targetCode is unknown to the underlying authority factory. + * Ignores the exception and fallback on the generic algorithm provided by + * CoordinateOperationFinder. + */ + log(null, e); + continue; } + break; // Stop on the first non-empty set of operations that we find. } - } catch (NoSuchAuthorityCodeException | MissingFactoryResourceException e) { - /* - * sourceCode or targetCode is unknown to the underlying authority factory. - * Ignores the exception and fallback on the generic algorithm provided by - * CoordinateOperationFinder. - */ - log(null, e); + } + if (operations == null) { return null; } /* @@ -443,7 +453,7 @@ class CoordinateOperationRegistry { * The non-public Semaphores.METADATA_ONLY mechanism instructs EPSGDataAccess to * instantiate DeferredCoordinateOperation instead of full coordinate operations. */ - mdOnly = Semaphores.queryAndSet(Semaphores.METADATA_ONLY); + final boolean mdOnly = Semaphores.queryAndSet(Semaphores.METADATA_ONLY); try { try { if (!it.hasNext()) break; Modified: sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java URL: http://svn.apache.org/viewvc/sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java?rev=1804866&r1=1804865&r2=1804866&view=diff ============================================================================== --- sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java [UTF-8] (original) +++ sis/branches/JDK8/core/sis-referencing/src/test/java/org/apache/sis/referencing/operation/CoordinateOperationRegistryTest.java [UTF-8] Sat Aug 12 13:59:38 2017 @@ -57,12 +57,16 @@ import static org.junit.Assume.*; * <ul> * <li><cite>"NTF (Paris) to WGS 84 (1)"</cite> operation (EPSG:8094), which implies a longitude rotation * followed by a geocentric translation in the geographic domain.</li> + * <li><cite>"Martinique 1938 to RGAF09 (1)"</cite> operation (EPSG:5491), which implies a datum shift + * 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> * </ul> * * The operations are tested with various axis order and dimension in source and target CRS. * * @author Martin Desruisseaux (Geomatys) - * @version 0.7 + * @version 0.8 * @since 0.7 * @module */ @@ -94,6 +98,11 @@ public final strictfp class CoordinateOp private static WKTFormat parser; /** + * The EPSG authority factory for CRS objects. Can be used as an alternative to {@link #parser}. + */ + private final CRSAuthorityFactory crsFactory; + + /** * The instance on which to execute the tests. */ private final CoordinateOperationRegistry registry; @@ -104,7 +113,7 @@ public final strictfp class CoordinateOp * @throws FactoryException if an error occurred while creating the factory to be tested. */ public CoordinateOperationRegistryTest() throws FactoryException { - final CRSAuthorityFactory crsFactory = CRS.getAuthorityFactory("EPSG"); + crsFactory = CRS.getAuthorityFactory("EPSG"); assumeTrue("EPSG factory required.", crsFactory instanceof CoordinateOperationAuthorityFactory); registry = new CoordinateOperationRegistry((CoordinateOperationAuthorityFactory) crsFactory, factory, null); } @@ -363,4 +372,39 @@ public final strictfp class CoordinateOp assertFalse("EPSG identifier not allowed for modified objects.", "EPSG".equalsIgnoreCase(id.getCodeSpace())); } } + + /** + * Tests <cite>"Martinique 1938 to RGAF09 (1)"</cite> operation with a target CRS fixed to EPSG:7086 + * instead of EPSG:5489. Both are <cite>"RGAF09"</cite>, but the former use (longitude, latitude) axis + * order instead than 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. + */ + @Test + public void testFindDespiteDifferentAxisOrder() throws FactoryException { + CoordinateReferenceSystem sourceCRS = crsFactory.createGeographicCRS("EPSG:4625"); + CoordinateReferenceSystem targetCRS = crsFactory.createGeographicCRS("EPSG:5489"); + CoordinateOperation operation = registry.createOperation(sourceCRS, targetCRS); + assertEpsgNameAndIdentifierEqual("Martinique 1938 to RGAF09 (1)", 5491, operation); + /* + * Above was only a verification using the source and target CRS expected by EPSG dataset. + * Now the interesting test: use a target CRS with different axis order. + */ + targetCRS = crsFactory.createGeographicCRS("EPSG:7086"); + operation = registry.createOperation(sourceCRS, targetCRS); + assertEpsgNameWithoutIdentifierEqual("Martinique 1938 to RGAF09 (1)", operation); + final ParameterValueGroup p = ((SingleOperation) 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. + */ + assertEquals("X-axis translation", 127.744, p.parameter("X-axis translation").doubleValue(), STRICT); + assertEquals("Y-axis translation", 547.069, p.parameter("Y-axis translation").doubleValue(), STRICT); + assertEquals("Z-axis translation", 118.359, p.parameter("Z-axis translation").doubleValue(), STRICT); + assertEquals("X-axis rotation", -3.1116, p.parameter("X-axis rotation") .doubleValue(), STRICT); + assertEquals("Y-axis rotation", 4.9509, p.parameter("Y-axis rotation") .doubleValue(), STRICT); + assertEquals("Z-axis rotation", -0.8837, p.parameter("Z-axis rotation") .doubleValue(), STRICT); + assertEquals("Scale difference", 14.1012, p.parameter("Scale difference") .doubleValue(), STRICT); + assertEquals("linearAccuracy", 0.1, CRS.getLinearAccuracy(operation), STRICT); + } }