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);
+    }
 }


Reply via email to