sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ama...@apache.org
Subject [sis] branch geoapi-4.0 updated: lfix(Feature): improve envelope operation to ensure attribute envelopes have a CRS defined if possible
Date Fri, 29 May 2020 16:05:37 GMT
This is an automated email from the ASF dual-hosted git repository.

amanin 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 6722ec6  lfix(Feature): improve envelope operation to ensure attribute envelopes
have a CRS defined if possible
6722ec6 is described below

commit 6722ec6172dcb7b3329e9917bb0e0f436e4ea536
Author: Alexis Manin <amanin@apache.org>
AuthorDate: Fri May 29 18:04:18 2020 +0200

    lfix(Feature): improve envelope operation to ensure attribute envelopes have a CRS defined
if possible
---
 .../org/apache/sis/feature/EnvelopeOperation.java  |  34 +++--
 .../apache/sis/feature/EnvelopeOperationTest.java  | 154 +++++++++++++++++++++
 2 files changed, 173 insertions(+), 15 deletions(-)

diff --git a/core/sis-feature/src/main/java/org/apache/sis/feature/EnvelopeOperation.java
b/core/sis-feature/src/main/java/org/apache/sis/feature/EnvelopeOperation.java
index b932010..ad14ad0 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/feature/EnvelopeOperation.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/feature/EnvelopeOperation.java
@@ -22,6 +22,7 @@ import java.util.Map;
 import java.util.LinkedHashMap;
 import java.util.Objects;
 import java.util.Optional;
+import org.apache.sis.util.Utilities;
 import org.opengis.util.GenericName;
 import org.opengis.util.FactoryException;
 import org.opengis.geometry.Envelope;
@@ -130,7 +131,7 @@ final class EnvelopeOperation extends AbstractOperation {
         final String characteristicName = AttributeConvention.CRS_CHARACTERISTIC.toString();
         /*
          * Get all property names without duplicated values. If a property is a link to an
attribute,
-         * then the key will be the name of the referenced attribute instead than the operation
name.
+         * then the key will be the name of the referenced attribute instead of the operation
name.
          * The intent is to avoid querying the same geometry twice if the attribute is also
specified
          * explicitly in the array of properties.
          *
@@ -190,10 +191,8 @@ final class EnvelopeOperation extends AbstractOperation {
                     if (crs == null) {
                         crs = value;                                    // Fallback if default
geometry has no CRS.
                     }
-                    final CoordinateOperation op = CRS.findOperation(value, crs, null);
-                    if (!op.getMathTransform().isIdentity()) {
-                        attributeToCRS[i] = op;
-                    }
+                    // even in case of identity operation, we keep it to be able to fetch
characteristic CRS of the attribute.
+                    attributeToCRS[i] = CRS.findOperation(value, crs, null);
                 }
             }
         }
@@ -287,7 +286,7 @@ final class EnvelopeOperation extends AbstractOperation {
             final String[] attributeNames = EnvelopeOperation.this.attributeNames;
             GeneralEnvelope envelope = null;                                        // Union
of all envelopes.
             for (int i=0; i<attributeNames.length; i++) {
-                Envelope genv;                                                      // Envelope
of a single geometry.
+                GeneralEnvelope genv;                                               // Envelope
of a single geometry.
                 final String name = attributeNames[i];
                 if (attributeToCRS == null) {
                     /*
@@ -319,28 +318,33 @@ final class EnvelopeOperation extends AbstractOperation {
                     try {
                         if (at == null) {
                             final CoordinateOperation op = attributeToCRS[i];
-                            if (op != null) {                           // Null operation
means identity transform.
-                                genv = Envelopes.transform(op, genv);
+                            if (op != null) {
+                                // Ensure attribute envelope has a CRS by forcing CRS found
as characteristic
+                                if (op.getMathTransform().isIdentity() && op.getSourceCRS()
!= null) genv.setCoordinateReferenceSystem(op.getSourceCRS());
+                                else genv = Envelopes.transform(op, genv);
                             }
                         } else {                                                        //
Should be a rare case.
                             final Object geomCRS = at.getValue();
                             if (!(geomCRS instanceof CoordinateReferenceSystem)) {
                                 throw new IllegalStateException(Errors.format(Errors.Keys.UnspecifiedCRS));
                             }
-                            ((GeneralEnvelope) genv).setCoordinateReferenceSystem((CoordinateReferenceSystem)
geomCRS);
-                            genv = Envelopes.transform(genv, crs);
+                            genv.setCoordinateReferenceSystem((CoordinateReferenceSystem)
geomCRS);
+                            genv = GeneralEnvelope.castOrCopy(Envelopes.transform(genv, crs));
                         }
                     } catch (TransformException e) {
                         throw new IllegalStateException(Errors.format(Errors.Keys.CanNotTransformEnvelope),
e);
                     }
                 }
+
+                /* Add current attribute envelope in result one. For now, we only allow union
of envelopes in the same
+                 * crs because we were not able to deduce output space from feature type
characteristics. However, in
+                 * the future, we could find a common space to use.
+                 */
                 if (envelope == null) {
-                    envelope = GeneralEnvelope.castOrCopy(genv);        // Should always
be a cast without copy.
-                    // Ensure that default CRS is set in case envelope returned by geometry
was not specified
-                    if (envelope.getCoordinateReferenceSystem() == null && crs !=
null) envelope.setCoordinateReferenceSystem(crs);
-                } else {
+                    envelope = genv;
+                } else if (Utilities.equalsIgnoreMetadata(genv.getCoordinateReferenceSystem(),
envelope.getCoordinateReferenceSystem())) {
                     envelope.add(genv);
-                }
+                } else throw new IllegalStateException(Errors.format(Errors.Keys.MismatchedCRS));
             }
             return envelope;
         }
diff --git a/core/sis-feature/src/test/java/org/apache/sis/feature/EnvelopeOperationTest.java
b/core/sis-feature/src/test/java/org/apache/sis/feature/EnvelopeOperationTest.java
index 79482ee..e0b9739 100644
--- a/core/sis-feature/src/test/java/org/apache/sis/feature/EnvelopeOperationTest.java
+++ b/core/sis-feature/src/test/java/org/apache/sis/feature/EnvelopeOperationTest.java
@@ -20,8 +20,21 @@ import java.util.Arrays;
 import java.util.Map;
 import java.util.Collections;
 import com.esri.core.geometry.Point;
+import com.esri.core.geometry.Polyline;
 import com.esri.core.geometry.Polygon;
+import org.apache.sis.feature.builder.AttributeRole;
+import org.apache.sis.feature.builder.AttributeTypeBuilder;
+import org.apache.sis.feature.builder.CharacteristicTypeBuilder;
+import org.apache.sis.feature.builder.FeatureTypeBuilder;
+import org.apache.sis.internal.feature.Geometries;
+import org.apache.sis.internal.feature.GeometryWrapper;
+import org.apache.sis.referencing.CRS;
+import org.opengis.feature.Attribute;
+import org.opengis.feature.AttributeType;
+import org.opengis.feature.Feature;
+import org.opengis.feature.FeatureType;
 import org.opengis.geometry.Envelope;
+import org.opengis.metadata.extent.GeographicBoundingBox;
 import org.opengis.util.FactoryException;
 import org.opengis.referencing.crs.CoordinateReferenceSystem;
 import org.apache.sis.internal.feature.AttributeConvention;
@@ -50,6 +63,13 @@ import org.opengis.feature.PropertyType;
  */
 @DependsOn(LinkOperationTest.class)
 public final strictfp class EnvelopeOperationTest extends TestCase {
+
+    private static final AttributeType<CoordinateReferenceSystem> CRS_CHARACTERISTIC
= new FeatureTypeBuilder()
+            .addAttribute(CoordinateReferenceSystem.class)
+            .setName(AttributeConvention.CRS_CHARACTERISTIC)
+            .setMinimumOccurs(0)
+            .build();
+
     /**
      * Creates a feature type with a bounds operation.
      * The feature contains the following properties:
@@ -171,4 +191,138 @@ public final strictfp class EnvelopeOperationTest extends TestCase {
     public void testSparseFeature() throws FactoryException {
         run(new SparseFeature(school(2)));
     }
+
+    /**
+     * If no characteristic is defined on properties, but geometries define different ones,
we should return an
+     * error, because it is an ambiguous case (Note: In the future, we could try to push
them all in a
+     * {@link CRS#suggestCommonTarget(GeographicBoundingBox, CoordinateReferenceSystem...)
common space}.
+     */
+    @Test
+    public void no_characteristic_but_different_geometry_crs() {
+        try {
+            final Envelope env = new CRSManagementUtil().test(HardCodedCRS.WGS84, HardCodedCRS.NTF);
+            fail("Ambiguity in CRS should have caused an error,  a value has been returned:
"+env);
+        } catch (IllegalStateException e) {
+            // Expected behavior
+        }
+    }
+
+    /**
+     * When CRS is not in characteristics, but can be found on geometries, returned envelope
should match it.
+     */
+    @Test
+    public void same_crs_on_geometries() {
+        final Envelope env = new CRSManagementUtil().test(HardCodedCRS.WGS84, HardCodedCRS.WGS84);
+        assertEquals(HardCodedCRS.WGS84, env.getCoordinateReferenceSystem());
+    }
+
+    /**
+     * When referencing is defined neither in characteristics nor on geometries, we should
assume all geometries are
+     * expressed in the same space. Therefore, an envelope with no CRS should be returned.
+     */
+    @Test
+    public void no_crs_defined() {
+        Envelope env = new CRSManagementUtil().test(null, null);
+        assertNull(env.getCoordinateReferenceSystem());
+    }
+
+    /**
+     * Ensure that returned envelope CRS is the default one specified by property type characteristics
if no geometry
+     * defines its CRS.
+     */
+    @Test
+    public void feature_type_characteristic_defines_crs() {
+        final Envelope env = new CRSManagementUtil(HardCodedCRS.WGS84, false, HardCodedCRS.WGS84,
false)
+                .test(null, null);
+        assertEquals(HardCodedCRS.WGS84, env.getCoordinateReferenceSystem());
+    }
+
+    @Test
+    public void feature_characteristic_define_crs() {
+        final CRSManagementUtil environment = new CRSManagementUtil(null, true, null, true);
+        Envelope env = environment
+                .test(HardCodedCRS.WGS84, true, HardCodedCRS.WGS84, true);
+        assertEquals(HardCodedCRS.WGS84, env.getCoordinateReferenceSystem());
+
+        try {
+            env = environment.test(HardCodedCRS.WGS84, true, HardCodedCRS.NTF, true);
+            fail("Envelope should not be computed due to different CRS in geometries: "+env);
+        } catch (IllegalStateException e) {
+            // expected behavior
+        }
+    }
+
+    private static class CRSManagementUtil {
+        final FeatureType type;
+
+        CRSManagementUtil() {
+            this(null, false, null, false);
+        }
+
+        /**
+         * Create a feature type containing two geometric fields. If given CRS are non null,
they will be specified as
+         * default CRS of each field through property type CRS characteristic.
+         * @param defaultCrs1 Default CRS of first property
+         * @param forceCharacteristic1 True if we want a CRS characteristic even with a null
CRS. False to omit
+         *                             characteristic i defaultCrs1 is null.
+         * @param defaultCrs2 Default CRS for second property
+         * @param forceCharacteristic2 True if we want a CRS characteristic even with a null
CRS. False to omit
+         *                             characteristic i defaultCrs2 is null.
+         */
+        CRSManagementUtil(final CoordinateReferenceSystem defaultCrs1, boolean forceCharacteristic1,
final CoordinateReferenceSystem defaultCrs2, boolean forceCharacteristic2) {
+            final FeatureTypeBuilder builder = new FeatureTypeBuilder().setName("test");
+            final AttributeTypeBuilder<GeometryWrapper> g1 = builder.addAttribute(GeometryWrapper.class).setName("g1");
+            if (defaultCrs1 != null || forceCharacteristic1) g1.setCRS(defaultCrs1);
+            g1.addRole(AttributeRole.DEFAULT_GEOMETRY);
+
+            final AttributeTypeBuilder<GeometryWrapper> g2 = builder.addAttribute(GeometryWrapper.class).setName("g2");
+            if (defaultCrs2 != null || forceCharacteristic2) g2.setCRS(defaultCrs2);
+
+            type = builder.build();
+        }
+
+        /**
+         * Compute the envelope of this feature, and ensure that lower/upper coordinates
are well-defined.
+         * The result is returned, so user can check the coordinate reference system on it.
+         *
+         * @param c1 CRS to put on the first geometry (not on property characteristic, but
geometry itself)
+         * @param c2 CRS to put on the second geometry (not on property characteristic, but
geometry itself)
+         * @return A non null envelope, result of the envelope operation.
+         */
+        Envelope test(final CoordinateReferenceSystem c1, final CoordinateReferenceSystem
c2) {
+            return test(c1, false, c2, false);
+        }
+
+        Envelope test(final CoordinateReferenceSystem c1, final boolean c1AsCharacteristic,
final CoordinateReferenceSystem c2, final boolean c2AsCharacteristic) {
+            final GeometryWrapper g1 = Geometries.wrap(new Point(4, 4))
+                    .orElseThrow(() -> new IllegalStateException("Cannot load ESRI binding"));
+            final GeometryWrapper g2 = Geometries.wrap(new Polyline(new Point(2, 2), new
Point(3, 3)))
+                    .orElseThrow(() -> new IllegalStateException("Cannot load ESRI binding"));
+
+            Feature f = type.newInstance();
+            set(f, "g1", g1, c1, c1AsCharacteristic);
+            set(f, "g2", g2, c2, c2AsCharacteristic);
+
+            Object result = f.getPropertyValue("sis:envelope");
+            assertNotNull(result);
+            assertTrue(result instanceof Envelope);
+            Envelope env = (Envelope) result;
+            assertArrayEquals(new double[]{2, 2}, env.getLowerCorner().getCoordinate(), 1e-4);
+            assertArrayEquals(new double[]{4, 4}, env.getUpperCorner().getCoordinate(), 1e-4);
+            return env;
+        }
+
+        private void set(final Feature target, final String propertyName, final GeometryWrapper
geometry, final CoordinateReferenceSystem crs, final boolean asCharacteristic) {
+            if (asCharacteristic) {
+                final Attribute g1p = (Attribute) target.getProperty(propertyName);
+                final Attribute<CoordinateReferenceSystem> crsCharacteristic = CRS_CHARACTERISTIC.newInstance();
+                crsCharacteristic.setValue(crs);
+                g1p.characteristics().put(AttributeConvention.CRS_CHARACTERISTIC.toString(),
crsCharacteristic);
+                g1p.setValue(geometry);
+            } else {
+                geometry.setCoordinateReferenceSystem(crs);
+                target.setPropertyValue(propertyName, geometry);
+            }
+        }
+    }
 }


Mime
View raw message