[sis] branch geoapi-4.0 updated: fix(Feature): On SQLMM functions, allow SRID/CRS argument to be optional.

[am...@apache.org]

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 c90e69905a fix(Feature): On SQLMM functions, allow SRID/CRS argument to be optional.
c90e69905a is described below

commit c90e69905a8ba43b4baeaa7450b9337686a3aff5
Author: Alexis Manin <al...@geomatys.com>
AuthorDate: Wed Jun 8 12:29:14 2022 +0200

    fix(Feature): On SQLMM functions, allow SRID/CRS argument to be optional.
    
    The case was already there, but the previous code was not checking the "maybe" flag that activate optional CRS.
---
 .../internal/filter/sqlmm/FunctionWithSRID.java    |  5 +++-
 .../sis/internal/filter/sqlmm/SQLMMTest.java       | 35 ++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/FunctionWithSRID.java b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/FunctionWithSRID.java
index fedb772b6c..237588b1c9 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/FunctionWithSRID.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/sqlmm/FunctionWithSRID.java
@@ -97,8 +97,11 @@ abstract class FunctionWithSRID<R> extends SpatialFunction<R> {
      * @todo The {@code MAYBE} flag could be removed if we know the type of value evaluated by the expression.
      *       For now it exists mostly because the last parameter given to {@code ST_Point} can be of various types.
      */
-    FunctionWithSRID(final SQLMM operation, final Expression<? super R, ?>[] parameters, final int hasSRID) {
+    FunctionWithSRID(final SQLMM operation, final Expression<? super R, ?>[] parameters, int hasSRID) {
         super(operation, parameters);
+        if (hasSRID == MAYBE && parameters.length < operation.maxParamCount) {
+            hasSRID = ABSENT;
+        }
         if (hasSRID == ABSENT) {
             literalCRS = true;
             srid = null;
diff --git a/core/sis-feature/src/test/java/org/apache/sis/internal/filter/sqlmm/SQLMMTest.java b/core/sis-feature/src/test/java/org/apache/sis/internal/filter/sqlmm/SQLMMTest.java
index 05966c05d6..0c41d149ad 100644
--- a/core/sis-feature/src/test/java/org/apache/sis/internal/filter/sqlmm/SQLMMTest.java
+++ b/core/sis-feature/src/test/java/org/apache/sis/internal/filter/sqlmm/SQLMMTest.java
@@ -16,6 +16,10 @@
  */
 package org.apache.sis.internal.filter.sqlmm;
 
+import java.util.function.BiFunction;
+import org.apache.sis.internal.feature.jts.JTS;
+import org.apache.sis.referencing.crs.HardCodedCRS;
+import org.locationtech.jts.geom.Point;
 import org.opengis.feature.Feature;
 import org.opengis.filter.Expression;
 import org.opengis.filter.FilterFactory;
@@ -24,6 +28,9 @@ import org.apache.sis.filter.DefaultFilterFactory;
 import org.apache.sis.referencing.CommonCRS;
 import org.apache.sis.test.TestCase;
 import org.junit.Test;
+import org.opengis.filter.Literal;
+import org.opengis.referencing.crs.CoordinateReferenceSystem;
+import org.opengis.util.FactoryException;
 
 import static org.opengis.test.Assert.*;
 
@@ -77,4 +84,32 @@ public final strictfp class SQLMMTest extends TestCase {
         assertEquals(wkt, polygon.toText());
         assertEquals(CommonCRS.WGS84.geographic(), polygon.getUserData());
     }
+
+    @Test
+    public void testOptionalCrsInSTPoint() throws Exception {
+        // Ensure that when argument array is of size 2, the FunctionWithSRID constructor will not fail with an ArrayIndexOutOfBoundsException.
+        // This is important. This case has already happen, making it a regression test.
+        assertPoint(null, (x, y) -> new Expression[] { x, y });
+        // Ensure point function will correctly interpret a literal with a null value as "no crs available"
+        assertPoint(null, (x, y) -> new Expression[] { x, y, factory.literal(null) });
+        // Ensure CRS is fetched properly
+        assertPoint(HardCodedCRS.WGS84, (x, y) -> new Expression[]{ x, y, factory.literal(HardCodedCRS.WGS84) });
+    }
+
+    /**
+     * Verify that a point function properly build a point with expected CRS and coordinate.
+     */
+    private void assertPoint(CoordinateReferenceSystem expectedCrs, BiFunction<Expression<Feature, Double>, Expression<Feature, Double>, Expression[]> argumentBundler) throws FactoryException {
+        final Literal<Feature, Double> x = factory.literal(1.0);
+        final Literal<Feature, Double> y = factory.literal(2.0);
+        Expression<Feature, ?> fn = factory.function("ST_Point", argumentBundler.apply(x, y));
+        Object rawPoint = fn.apply(null);
+        assertInstanceOf("ST_Point should create a Point geometry", Point.class, rawPoint);
+        Point point = (Point) rawPoint;
+        final CoordinateReferenceSystem pointCrs = JTS.getCoordinateReferenceSystem(point);
+        if (expectedCrs == null) assertNull("Point CRS", pointCrs);
+        else assertEquals("Point CRS", expectedCrs, pointCrs);
+        assertEquals(point.getX(), x.getValue(), 1e-1);
+        assertEquals(point.getY(), y.getValue(), 1e-1);
+    }
 }