sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ama...@apache.org
Subject [sis] 06/07: fix(Feature): fix a corner case where an expression optimisation was not properly unwrapped.
Date Fri, 30 Jul 2021 16:49:42 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

commit 410d2c2b14c760d05cee7e2a3cc1e1a7485c581a
Author: Alexis Manin <alexis.manin@geomatys.com>
AuthorDate: Fri Jul 30 18:37:39 2021 +0200

    fix(Feature): fix a corner case where an expression optimisation was not properly unwrapped.
    
    For optimisation purposes, some expressions are  transformed for internal usage. However,
we must ensure that users get the original expressions when queried.
---
 .../java/org/apache/sis/cql/FilterReadingTest.java | 30 ++++++++++------------
 .../apache/sis/filter/BinaryGeometryFilter.java    | 13 +++++-----
 .../java/org/apache/sis/internal/filter/Node.java  |  5 ++--
 .../org/apache/sis/filter/SpatialTestCase.java     | 15 +++++++++++
 4 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/core/sis-cql/src/test/java/org/apache/sis/cql/FilterReadingTest.java b/core/sis-cql/src/test/java/org/apache/sis/cql/FilterReadingTest.java
index 314ae38..1a88493 100644
--- a/core/sis-cql/src/test/java/org/apache/sis/cql/FilterReadingTest.java
+++ b/core/sis-cql/src/test/java/org/apache/sis/cql/FilterReadingTest.java
@@ -26,20 +26,11 @@ import java.text.ParseException;
 import javax.measure.Quantity;
 import javax.measure.quantity.Length;
 
+import org.apache.sis.geometry.AbstractEnvelope;
+import org.apache.sis.geometry.GeneralEnvelope;
+import org.opengis.geometry.Envelope;
 import org.opengis.util.CodeList;
 import org.opengis.feature.Feature;
-import org.opengis.filter.Filter;
-import org.opengis.filter.Literal;
-import org.opengis.filter.LogicalOperator;
-import org.opengis.filter.LogicalOperatorName;
-import org.opengis.filter.SpatialOperator;
-import org.opengis.filter.SpatialOperatorName;
-import org.opengis.filter.DistanceOperator;
-import org.opengis.filter.DistanceOperatorName;
-import org.opengis.filter.TemporalOperator;
-import org.opengis.filter.TemporalOperatorName;
-import org.opengis.filter.BinaryComparisonOperator;
-import org.opengis.filter.ComparisonOperatorName;
 
 import org.apache.sis.measure.Units;
 import org.apache.sis.measure.Quantities;
@@ -57,6 +48,8 @@ import org.locationtech.jts.geom.Coordinate;
 import org.locationtech.jts.geom.Geometry;
 import org.locationtech.jts.geom.LinearRing;
 
+// Branch dependant imports
+import org.opengis.filter.*;
 
 /**
  * Test reading CQL filters.
@@ -163,7 +156,7 @@ public final strictfp class FilterReadingTest extends CQLTestCase {
                             FF.contains(FF.property("BoundingBox"), FF.literal(baseGeometryPoint))
                             )
                     ),
-                    FF.bbox(FF.property("BoundingBox"), new Envelope2D(null, 10, 20, 30-10,
40-20))
+                    FF.bbox(FF.property("BoundingBox"), new GeneralEnvelope(new Envelope2D(null,
10, 20, 30-10, 40-20)))
                 ),
                 filter);
     }
@@ -307,11 +300,16 @@ public final strictfp class FilterReadingTest extends CQLTestCase {
         testBBOX("BBOX(geometry,-10,-20,10,20)", "geometry", env);
     }
 
-    private void testBBOX(final String cql, final String att, final Envelope2D env) throws
CQLException {
+    private void testBBOX(final String cql, final String att, final Envelope env) throws
CQLException {
         final Filter<?> filter = CQL.parseFilter(cql);
-        assertInstanceOf(null, SpatialOperator.class, filter);
+        assertInstanceOf(null, BinarySpatialOperator.class, filter);
         assertEquals(SpatialOperatorName.BBOX, filter.getOperatorType());
-        assertEquals(FF.bbox(FF.property(att), env), filter);
+        assertEquals(FF.property(att), ((BinarySpatialOperator)filter).getOperand1());
+        assertEquals(FF.property(att), ((BinarySpatialOperator)filter).getOperand1());
+        final Expression<?, ?> arg2 = ((BinarySpatialOperator) filter).getOperand2();
+        assertTrue("Second argument should be a literal expression", arg2 instanceof Literal);
+        final AbstractEnvelope argEnv = AbstractEnvelope.castOrCopy((Envelope) ((Literal<?,
?>) arg2).getValue());
+        assertTrue(argEnv.equals(env, 1e-2, false));
     }
 
     /**
diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/BinaryGeometryFilter.java
b/core/sis-feature/src/main/java/org/apache/sis/filter/BinaryGeometryFilter.java
index 8fd29c4..47e22d5 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/filter/BinaryGeometryFilter.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/filter/BinaryGeometryFilter.java
@@ -29,6 +29,7 @@ import org.apache.sis.internal.filter.Node;
 import org.apache.sis.util.ArgumentChecks;
 
 // Branch-dependent imports
+import org.opengis.filter.BinarySpatialOperator;
 import org.opengis.filter.Filter;
 import org.opengis.filter.Literal;
 import org.opengis.filter.Expression;
@@ -60,14 +61,14 @@ abstract class BinaryGeometryFilter<R,G> extends Node implements
SpatialOperator
     /**
      * The first of the two expressions to be used by this function.
      *
-     * @see #getExpression1()
+     * @see BinarySpatialOperator#getOperand1()
      */
     protected final Expression<? super R, GeometryWrapper<G>> expression1;
 
     /**
      * The second of the two expressions to be used by this function.
      *
-     * @see #getExpression2()
+     * @see BinarySpatialOperator#getOperand2()
      */
     protected final Expression<? super R, GeometryWrapper<G>> expression2;
 
@@ -152,11 +153,11 @@ abstract class BinaryGeometryFilter<R,G> extends Node implements
SpatialOperator
      * @return the unwrapped expression.
      */
     protected static <R,G> Expression<? super R, ?> original(final Expression<R,
? extends GeometryWrapper<G>> expression) {
-        if (expression instanceof LeafExpression.Transformed<?,?>) {
-            return ((LeafExpression.Transformed<R,?>) expression).original;
-        } else {
-            return unwrap(expression);
+        Expression<? super R, ?> unwrapped = unwrap(expression);
+        if (unwrapped instanceof LeafExpression.Transformed<?, ?>) {
+            unwrapped = ((LeafExpression.Transformed<R, ?>) unwrapped).original;
         }
+        return unwrapped;
     }
 
     /**
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Node.java b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Node.java
index d0da257..ea842dc 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Node.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Node.java
@@ -67,7 +67,7 @@ public abstract class Node implements Serializable {
      *
      * @see #createName(String)
      */
-    private static final LocalName SCOPE = Names.createLocalName("Apache", null, "sis");;
+    private static final LocalName SCOPE = Names.createLocalName("Apache", null, "sis");
 
     /**
      * Creates a new expression, operator or filter.
@@ -169,12 +169,11 @@ public abstract class Node implements Serializable {
      * returns the original expression. Otherwise returns the given expression.
      *
      * @param  <R>         the type of resources (e.g. {@link org.opengis.feature.Feature})
used as inputs.
-     * @param  <G>         the geometry implementation type.
      * @param  expression  the expression to unwrap.
      * @return the unwrapped expression.
      */
     @SuppressWarnings("unchecked")
-    protected static <R,G> Expression<? super R, ?> unwrap(final Expression<R,
? extends GeometryWrapper<G>> expression) {
+    protected static <R> Expression<? super R, ?> unwrap(final Expression<R,
?> expression) {
         if (expression instanceof GeometryConverter<?,?>) {
             return ((GeometryConverter<R,?>) expression).expression;
         } else {
diff --git a/core/sis-feature/src/test/java/org/apache/sis/filter/SpatialTestCase.java b/core/sis-feature/src/test/java/org/apache/sis/filter/SpatialTestCase.java
index 2035ace..91e3400 100644
--- a/core/sis-feature/src/test/java/org/apache/sis/filter/SpatialTestCase.java
+++ b/core/sis-feature/src/test/java/org/apache/sis/filter/SpatialTestCase.java
@@ -36,6 +36,7 @@ import static org.apache.sis.test.Assert.assertSerializedEquals;
 
 // Branch-dependent imports
 import org.opengis.feature.Feature;
+import org.opengis.filter.Expression;
 import org.opengis.filter.Literal;
 import org.opengis.filter.FilterFactory;
 import org.opengis.filter.DistanceOperator;
@@ -121,6 +122,20 @@ public abstract strictfp class SpatialTestCase<G> extends TestCase
{
     }
 
     /**
+     * Ensure that expressions provided as arguments for bbox filter are not hidden. We mean
that if they're wrapped for
+     * internal purpose, the wrappers should not be publicly exposed.
+     */
+    @Test
+    public void bbox_preserve_expression_type() {
+        final BinarySpatialOperator<Feature> bbox = factory.bbox(literal(Polygon.RIGHT),
new Envelope2D(null, 0, 0, 1, 1));
+        final Expression<? super Feature, ?> arg2 = bbox.getOperand2();
+        assertTrue("The two ways to acquire the second argument return different values",
arg2 == bbox.getExpressions().get(1));
+        assertTrue(
+                "Second argument value should be an envelope",
+                arg2 instanceof Literal && ((Literal<Feature, ?>) arg2).getValue()
instanceof Envelope);
+    }
+
+    /**
      * Tests {@link FilterFactory#beyond(Expression, Expression, Quantity)}
      */
     @Test

Mime
View raw message