sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 01/02: Reduce the amount of exception that may be thrown when converting an Envelope to a GeographicBoundingBox for Envelopes.findOperation(…) purpose.
Date Sat, 15 Dec 2018 17:19:20 GMT
This is an automated email from the ASF dual-hosted git repository.

desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git

commit 5b26997a4e7de26e49bd32c8988c8ececc1f54c4
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Sat Dec 15 14:28:48 2018 +0100

    Reduce the amount of exception that may be thrown when converting an Envelope to a GeographicBoundingBox
for Envelopes.findOperation(…) purpose.
---
 .../sis/internal/metadata/ReferencingServices.java | 16 +++++--
 .../iso/extent/DefaultGeographicBoundingBox.java   |  3 +-
 .../org/apache/sis/geometry/EnvelopeReducer.java   | 23 ++++++----
 .../java/org/apache/sis/geometry/Envelopes.java    | 29 +++++++++---
 .../internal/referencing/ServicesForMetadata.java  | 51 ++++++++++++++++------
 .../referencing/ServicesForMetadataTest.java       |  4 +-
 6 files changed, 89 insertions(+), 37 deletions(-)

diff --git a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/ReferencingServices.java
b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/ReferencingServices.java
index 1e7898d..650a7a0 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/ReferencingServices.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/ReferencingServices.java
@@ -202,12 +202,22 @@ public class ReferencingServices extends OptionalDependency {
      * to a geographic CRS (without datum shift if possible). Otherwise, the envelope is
assumed already
      * in a geographic CRS using (<var>longitude</var>, <var>latitude</var>)
axis order.
      *
+     * <p>If {@code optional} is {@code true}, then this method will be executed in
optional mode:
+     * some failures will cause this method to return {@code null} instead than throwing
an exception.
+     * Note that {@link TransformException} may still be thrown but not directly by this
method.
+     * Warning may be logged, but in such case this method presumes that public caller is
+     * {@link org.apache.sis.geometry.Envelopes#findOperation(Envelope, Envelope)}.</p>
+     *
      * @param  envelope  the source envelope.
-     * @param  target    the target bounding box.
-     * @throws TransformException if the given envelope can't be transformed.
+     * @param  target    the target bounding box, or {@code null} for creating it automatically.
+     * @param  optional  {@code true} for replacing some (not all) exceptions by {@code
null} return value.
+     * @return the bounding box or {@code null} on failure. Never {@code null} if {@code
optional} argument is {@code false}.
      * @throws UnsupportedOperationException if the {@code "sis-referencing"} module has
not been found on the classpath.
+     * @throws TransformException if the given envelope can not be transformed.
      */
-    public void setBounds(Envelope envelope, DefaultGeographicBoundingBox target) throws
TransformException {
+    public DefaultGeographicBoundingBox setBounds(Envelope envelope, DefaultGeographicBoundingBox
target, boolean optional)
+            throws TransformException
+    {
         throw moduleNotFound();
     }
 
diff --git a/core/sis-metadata/src/main/java/org/apache/sis/metadata/iso/extent/DefaultGeographicBoundingBox.java
b/core/sis-metadata/src/main/java/org/apache/sis/metadata/iso/extent/DefaultGeographicBoundingBox.java
index 8cd58a1..2b17254 100644
--- a/core/sis-metadata/src/main/java/org/apache/sis/metadata/iso/extent/DefaultGeographicBoundingBox.java
+++ b/core/sis-metadata/src/main/java/org/apache/sis/metadata/iso/extent/DefaultGeographicBoundingBox.java
@@ -534,8 +534,7 @@ public class DefaultGeographicBoundingBox extends AbstractGeographicExtent
imple
     public void setBounds(final Envelope envelope) throws TransformException {
         ArgumentChecks.ensureNonNull("envelope", envelope);
         checkWritePermission(isNonEmpty());
-        ReferencingServices.getInstance().setBounds(envelope, this);
-        setInclusion(Boolean.TRUE);                                     // Set only on success.
+        ReferencingServices.getInstance().setBounds(envelope, this, false);
     }
 
     /**
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/geometry/EnvelopeReducer.java
b/core/sis-referencing/src/main/java/org/apache/sis/geometry/EnvelopeReducer.java
index e4b4752..87c580a 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/geometry/EnvelopeReducer.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/geometry/EnvelopeReducer.java
@@ -16,12 +16,13 @@
  */
 package org.apache.sis.geometry;
 
-import org.apache.sis.internal.referencing.Resources;
 import org.opengis.geometry.Envelope;
 import org.opengis.metadata.extent.GeographicBoundingBox;
 import org.opengis.referencing.crs.CoordinateReferenceSystem;
 import org.opengis.referencing.operation.TransformException;
 import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox;
+import org.apache.sis.internal.metadata.ReferencingServices;
+import org.apache.sis.internal.referencing.Resources;
 import org.apache.sis.referencing.CRS;
 import org.apache.sis.util.Utilities;
 
@@ -122,21 +123,25 @@ merge:  for (final Envelope envelope : envelopes) {
             case 1: return reduced[0];
         }
         /*
-         * Compute the geographic bounding box of all remaining elements to reduce.
-         * This will be used for choosing a common CRS.
+         * Compute the geographic bounding box of all remaining elements to reduce. This
will be used for
+         * choosing a common CRS. Note that if a warning is logged, ReferencingServices.setBounds(…)
will
+         * pretend that warning come from Envelopes.findOperation(…). This is not true
but not completely
+         * false neither since the purpose of this bounding box is to find a coordinate operation.
          */
+        final ReferencingServices converter = ReferencingServices.getInstance();
         CoordinateReferenceSystem[]  crs  = new CoordinateReferenceSystem[count];
         DefaultGeographicBoundingBox more = new DefaultGeographicBoundingBox();
         DefaultGeographicBoundingBox bbox = null;
         for (int i=0; i<count; i++) {
             final GeneralEnvelope e = reduced[i];
             crs[i] = e.getCoordinateReferenceSystem();
-            more.setBounds(e);
-            if (i == 0) {
-                bbox = more;
-                more = new DefaultGeographicBoundingBox();
-            } else {
-                reduce(bbox, more);
+            if (converter.setBounds(e, more, true) != null) {           // See above comment
about logging.
+                if (bbox == null) {
+                    bbox = more;
+                    more = new DefaultGeographicBoundingBox();
+                } else {
+                    reduce(bbox, more);
+                }
             }
         }
         /*
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/geometry/Envelopes.java b/core/sis-referencing/src/main/java/org/apache/sis/geometry/Envelopes.java
index e854b90..a0988fc 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/geometry/Envelopes.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/geometry/Envelopes.java
@@ -41,6 +41,7 @@ import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox;
 import org.apache.sis.referencing.CRS;
 import org.apache.sis.referencing.operation.AbstractCoordinateOperation;
 import org.apache.sis.referencing.operation.transform.AbstractMathTransform;
+import org.apache.sis.internal.metadata.ReferencingServices;
 import org.apache.sis.internal.referencing.CoordinateOperations;
 import org.apache.sis.internal.referencing.DirectPositionView;
 import org.apache.sis.internal.referencing.Formulas;
@@ -206,14 +207,27 @@ public final class Envelopes extends Static {
             if ((sourceCRS = source.getCoordinateReferenceSystem()) != null &&
                 (targetCRS = target.getCoordinateReferenceSystem()) != null)
             {
+                /*
+                 * Computing the area of interest (AOI) unconditionally would be harmless,
but it is useless if the CRS
+                 * are the same since the result will be identity anyway. Conversely we could
skip AOI computation more
+                 * often for example with the following condition instead of !=:
+                 *
+                 *     !Utilities.deepEquals(sourceCRS, targetCRS, ComparisonMode.ALLOW_VARIANT)
+                 *
+                 * but it would not be very advantageous if testing the condition is almost
as long as computing
+                 * the AOI. For now we keep != as a very cheap test which will work quite
often.
+                 */
                 DefaultGeographicBoundingBox areaOfInterest = null;
-                if (!sourceCRS.equals(targetCRS)) try {
-                    DefaultGeographicBoundingBox bbox = new DefaultGeographicBoundingBox();
-                    bbox.setBounds(source);
-                    areaOfInterest = bbox;                          // Set only on success.
-                    bbox = new DefaultGeographicBoundingBox();
-                    bbox.setBounds(target);
-                    areaOfInterest.add(bbox);
+                if (sourceCRS != targetCRS) try {
+                    final DefaultGeographicBoundingBox targetAOI;
+                    final ReferencingServices converter = ReferencingServices.getInstance();
+                    areaOfInterest = converter.setBounds(source, null, true);      // Should
be first.
+                    targetAOI      = converter.setBounds(target, null, true);
+                    if (areaOfInterest == null) {
+                        areaOfInterest = targetAOI;
+                    } else {
+                        areaOfInterest.add(targetAOI);
+                    }
                 } catch (TransformException e) {
                     /*
                      * Note: we may succeed to transform 'source' and fail to transform 'target'
to geographic bounding box,
@@ -314,6 +328,7 @@ public final class Envelopes extends Static {
                 } else {
                     // TODO: create an CoordinateOperationContext with the envelope as geographic
area.
                     //       May require that we optimize the search for CoordinateOperation
with non-null context first.
+                    //       See https://issues.apache.org/jira/browse/SIS-442
                     final CoordinateOperation operation;
                     try {
                         operation = CoordinateOperations.factory().createOperation(sourceCRS,
targetCRS);
diff --git a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/ServicesForMetadata.java
b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/ServicesForMetadata.java
index 1dba7a3..ecdd8a1 100644
--- a/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/ServicesForMetadata.java
+++ b/core/sis-referencing/src/main/java/org/apache/sis/internal/referencing/ServicesForMetadata.java
@@ -91,12 +91,14 @@ import org.apache.sis.internal.metadata.AxisDirections;
 import org.apache.sis.internal.metadata.ReferencingServices;
 import org.apache.sis.internal.referencing.provider.Affine;
 import org.apache.sis.internal.system.DefaultFactories;
+import org.apache.sis.internal.system.Modules;
 import org.apache.sis.internal.util.Constants;
 import org.apache.sis.internal.util.TemporalUtilities;
 import org.apache.sis.util.CharSequences;
 import org.apache.sis.util.collection.Containers;
 import org.apache.sis.util.resources.Vocabulary;
 import org.apache.sis.util.resources.Errors;
+import org.apache.sis.util.logging.Logging;
 import org.apache.sis.util.Exceptions;
 import org.apache.sis.util.Utilities;
 import org.apache.sis.util.iso.DefaultNameSpace;
@@ -148,14 +150,22 @@ public final class ServicesForMetadata extends ReferencingServices {
      * the horizontal extent. If the {@code crs} argument is null, then it is caller's responsibility
      * to ensure that the given envelope is two-dimensional.
      *
+     * <p>If {@code optional} is {@code true}, then this method will be executed in
optional mode:
+     * some failures will cause this method to return {@code null} instead than throwing
an exception.
+     * Note that {@link TransformException} may still be thrown but not directly by this
method.
+     * Warning may be logged, but in such case this method presumes that public caller is
+     * {@link Envelopes#findOperation(Envelope, Envelope)}.</p>
+     *
      * @param  envelope       the source envelope.
-     * @param  target         the target bounding box.
+     * @param  target         the target bounding box, or {@code null} for creating it automatically.
      * @param  crs            the envelope CRS, or {@code null} if unknown.
      * @param  normalizedCRS  the horizontal component of the given CRS, or null if the {@code
crs} argument is null.
+     * @param  optional       {@code true} for replacing some (not all) exceptions by {@code
null} return value.
+     * @return the bounding box or {@code null} on failure. Never {@code null} if {@code
optional} argument is {@code false}.
      * @throws TransformException if the given envelope can not be transformed.
      */
-    private void setGeographicExtent(Envelope envelope, final DefaultGeographicBoundingBox
target,
-            final CoordinateReferenceSystem crs, final GeographicCRS normalizedCRS) throws
TransformException
+    private DefaultGeographicBoundingBox setGeographicExtent(Envelope envelope, DefaultGeographicBoundingBox
target,
+            final CoordinateReferenceSystem crs, final GeographicCRS normalizedCRS, final
boolean optional) throws TransformException
     {
         if (normalizedCRS != null) {
             // No need to check for dimension, since GeodeticCRS can not have less than 2.
@@ -169,6 +179,11 @@ public final class ServicesForMetadata extends ReferencingServices {
                 try {
                     operation = factory.createOperation(crs, normalizedCRS);
                 } catch (FactoryException e) {
+                    if (optional) {
+                        // See javadoc for the assumption that optional mode is used only
by Envelopes.findOperation(…).
+                        Logging.recoverableException(Logging.getLogger(Modules.REFERENCING),
Envelopes.class, "findOperation", e);
+                        return null;
+                    }
                     throw new TransformException(Resources.format(Resources.Keys.CanNotTransformEnvelopeToGeodetic),
e);
                 }
                 envelope = Envelopes.transform(operation, envelope);
@@ -187,8 +202,12 @@ public final class ServicesForMetadata extends ReferencingServices {
             westBoundLongitude += rotation;
             eastBoundLongitude += rotation;
         }
+        if (target == null) {
+            target = new DefaultGeographicBoundingBox();
+        }
         target.setBounds(westBoundLongitude, eastBoundLongitude, southBoundLatitude, northBoundLatitude);
         target.setInclusion(Boolean.TRUE);
+        return target;
     }
 
     /**
@@ -238,26 +257,33 @@ public final class ServicesForMetadata extends ReferencingServices {
 
     /**
      * Sets a geographic bounding box from the specified envelope.
-     * If the envelope contains a CRS which is not geographic, then the bounding box will
be transformed
-     * to a geographic CRS (without datum shift if possible). Otherwise, the envelope is
assumed already
-     * in a geographic CRS using (<var>longitude</var>, <var>latitude</var>)
axis order.
+     * If the envelope has no CRS, then (<var>longitude</var>, <var>latitude</var>)
axis order is assumed.
+     * If the envelope CRS is not geographic, then the envelope will be transformed to a
geographic CRS.
+     * If {@code optional} is {@code true}, then some failures will cause this method to
return {@code null}
+     * instead than throwing an exception, and warning may be logged with assumption that
caller is
+     * {@link Envelopes#findOperation(Envelope, Envelope)}.
      *
      * @param  envelope  the source envelope.
-     * @param  target    the target bounding box where to store envelope information.
+     * @param  target    the target bounding box, or {@code null} for creating it automatically.
+     * @param  optional  {@code true} for replacing some (not all) exceptions by {@code
null} return value.
+     * @return the bounding box or {@code null} on failure. Never {@code null} if {@code
optional} argument is {@code false}.
      * @throws TransformException if the given envelope can not be transformed.
      */
     @Override
-    public void setBounds(Envelope envelope, final DefaultGeographicBoundingBox target) throws
TransformException {
+    public DefaultGeographicBoundingBox setBounds(final Envelope envelope, final DefaultGeographicBoundingBox
target,
+            final boolean optional) throws TransformException
+    {
         final CoordinateReferenceSystem crs = envelope.getCoordinateReferenceSystem();
         GeographicCRS normalizedCRS = ReferencingUtilities.toNormalizedGeographicCRS(crs);
         if (normalizedCRS == null) {
             if (crs != null) {
                 normalizedCRS = CommonCRS.defaultGeographic();
             } else if (envelope.getDimension() != 2) {
+                if (optional) return null;
                 throw new TransformException(dimensionNotFound(Resources.Keys.MissingHorizontalDimension_1,
crs));
             }
         }
-        setGeographicExtent(envelope, target, crs, normalizedCRS);
+        return setGeographicExtent(envelope, target, crs, normalizedCRS, optional);
     }
 
     /**
@@ -343,7 +369,7 @@ public final class ServicesForMetadata extends ReferencingServices {
             if (normalizedCRS == null) {
                 normalizedCRS = CommonCRS.defaultGeographic();
             }
-            setGeographicExtent(envelope, box, crs, normalizedCRS);
+            setGeographicExtent(envelope, box, crs, normalizedCRS, false);
         }
         /*
          * Other dimensions (vertical and temporal).
@@ -384,10 +410,7 @@ public final class ServicesForMetadata extends ReferencingServices {
             throw new TransformException(dimensionNotFound(Resources.Keys.MissingSpatioTemporalDimension_1,
crs));
         }
         if (horizontalCRS != null) {
-            final DefaultGeographicBoundingBox extent = new DefaultGeographicBoundingBox();
-            extent.setInclusion(Boolean.TRUE);
-            setBounds(envelope, extent);
-            target.getGeographicElements().add(extent);
+            target.getGeographicElements().add(setBounds(envelope, null, false));
         }
         if (verticalCRS != null) {
             final DefaultVerticalExtent extent = new DefaultVerticalExtent();
diff --git a/core/sis-referencing/src/test/java/org/apache/sis/internal/referencing/ServicesForMetadataTest.java
b/core/sis-referencing/src/test/java/org/apache/sis/internal/referencing/ServicesForMetadataTest.java
index 8778d6e..1ff2cad 100644
--- a/core/sis-referencing/src/test/java/org/apache/sis/internal/referencing/ServicesForMetadataTest.java
+++ b/core/sis-referencing/src/test/java/org/apache/sis/internal/referencing/ServicesForMetadataTest.java
@@ -102,7 +102,7 @@ public final strictfp class ServicesForMetadataTest extends TestCase {
     }
 
     /**
-     * Tests (indirectly) {@link ServicesForMetadata#setBounds(Envelope, DefaultGeographicBoundingBox)}
+     * Tests (indirectly) {@link ServicesForMetadata#setBounds(Envelope, DefaultGeographicBoundingBox,
boolean)}
      * from a three-dimensional envelope.
      *
      * @throws TransformException should never happen.
@@ -115,7 +115,7 @@ public final strictfp class ServicesForMetadataTest extends TestCase {
     }
 
     /**
-     * Tests (indirectly) {@link ServicesForMetadata#setBounds(Envelope, DefaultGeographicBoundingBox)}
+     * Tests (indirectly) {@link ServicesForMetadata#setBounds(Envelope, DefaultGeographicBoundingBox,
boolean)}
      * from a for-dimensional envelope.
      *
      * @throws TransformException should never happen.


Mime
View raw message