sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ama...@apache.org
Subject [sis] 10/22: fix(SQLStore): improve tests and checkstyle
Date Thu, 14 Nov 2019 11:46:44 GMT
This is an automated email from the ASF dual-hosted git repository.

amanin pushed a commit to branch refactor/sql-store
in repository https://gitbox.apache.org/repos/asf/sis.git

commit 2980aff451cb668851382e5d0ac9294ead808526
Author: Alexis Manin <amanin@apache.org>
AuthorDate: Wed Oct 9 17:01:47 2019 +0200

    fix(SQLStore): improve tests and checkstyle
---
 .../main/java/org/apache/sis/feature/Features.java |  4 +--
 .../java/org/apache/sis/filter/DefaultBBOX.java    | 12 ++++----
 .../java/org/apache/sis/filter/ST_Envelope.java    | 10 +++----
 .../java/org/apache/sis/internal/feature/ESRI.java |  4 +--
 .../apache/sis/internal/feature/Geometries.java    |  6 ++--
 .../java/org/apache/sis/internal/feature/JTS.java  |  2 +-
 .../org/apache/sis/internal/feature/Java2D.java    | 17 +++++++++--
 .../sis/internal/sql/feature/ANSIInterpreter.java  | 16 +++++------
 .../sql/feature/FilterInterpreterTest.java         | 33 ++++++++++++++++------
 .../apache/sis/internal/storage/SubsetAdapter.java | 10 +++----
 10 files changed, 73 insertions(+), 41 deletions(-)

diff --git a/core/sis-feature/src/main/java/org/apache/sis/feature/Features.java b/core/sis-feature/src/main/java/org/apache/sis/feature/Features.java
index 7b9448d..8ad4957 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/feature/Features.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/feature/Features.java
@@ -240,11 +240,11 @@ public final class Features extends Static {
         // In case an operation also implements attribute type, we check it first.
         // TODO : cycle detection ?
         while (!(input instanceof AttributeType) && input instanceof Operation) {
-            input = ((Operation)input).getResult();
+            input = ((Operation) input).getResult();
         }
 
         if (input instanceof AttributeType) {
-            return Optional.of((AttributeType)input);
+            return Optional.of((AttributeType) input);
         }
 
         return Optional.empty();
diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java b/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java
index a465388..a597e45 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java
@@ -51,9 +51,9 @@ final class DefaultBBOX implements BBOX {
         this.right = right;
 
         if (left instanceof Literal) {
-            intersects = asOptimizedTest((Literal)left, right);
+            intersects = asOptimizedTest((Literal) left, right);
         } else if (right instanceof Literal) {
-            intersects = asOptimizedTest((Literal)right, left);
+            intersects = asOptimizedTest((Literal) right, left);
         } else intersects = this::nonOptimizedIntersect;
     }
 
@@ -78,8 +78,8 @@ final class DefaultBBOX implements BBOX {
     }
 
     private boolean nonOptimizedIntersect(Object candidate) {
-        Envelope leftEval = left == null? null : asEnvelope(left, candidate);
-        Envelope rightEval = left == null? null : asEnvelope(right, candidate);
+        Envelope leftEval = left == null ? null : asEnvelope(left, candidate);
+        Envelope rightEval = left == null ? null : asEnvelope(right, candidate);
         if (left == null) {
             return multiIntersect(candidate, rightEval);
         } else if (right == null) {
@@ -137,7 +137,7 @@ final class DefaultBBOX implements BBOX {
         } else {
             final Envelope env = Geometries.getEnvelope(candidate);
             if (env == null) throw new UnsupportedOperationException(
-                    "Candidate type unsupported: "+candidate == null? "null" : candidate.getClass().getCanonicalName()
+                    "Candidate type unsupported: "+candidate == null ? "null" : candidate.getClass().getCanonicalName()
             );
             return constEnv.intersects(env);
         }
@@ -161,7 +161,7 @@ final class DefaultBBOX implements BBOX {
 
     private static Predicate asOptimizedTest(Literal constant, Expression other) {
         final ImmutableEnvelope constEnv = new ImmutableEnvelope(asEnvelope(constant, null));
-        return other == null? it -> multiIntersect(it, constEnv) : it -> intersect(it,
other, constEnv);
+        return other == null ? it -> multiIntersect(it, constEnv) : it -> intersect(it,
other, constEnv);
     }
 
     /*
diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/ST_Envelope.java b/core/sis-feature/src/main/java/org/apache/sis/filter/ST_Envelope.java
index 0fa2761..98990ae 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/filter/ST_Envelope.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/filter/ST_Envelope.java
@@ -37,7 +37,7 @@ public class ST_Envelope extends AbstractFunction implements FeatureExpression
{
         if (parameters == null || parameters.length != 1) throw new MismatchedDimensionException(
                 String.format(
                     "Single parameter expected for %s operation: source Geometry. However,
%d arguments were provided",
-                    NAME, parameters == null? 0 : parameters.length
+                    NAME, parameters == null ? 0 : parameters.length
                 )
         );
 
@@ -68,7 +68,7 @@ public class ST_Envelope extends AbstractFunction implements FeatureExpression
{
         final AttributeType resultType;
 
         public LiteralEnvelope(Literal source) {
-            Object value = source == null? null : source.getValue();
+            Object value = source == null ? null : source.getValue();
             ensureNonNull("Source value", value);
             final Envelope tmpResult = tryGet(value);
 
@@ -91,7 +91,7 @@ public class ST_Envelope extends AbstractFunction implements FeatureExpression
{
         }
     }
 
-    private class FeatureEnvelope implements Worker {
+    private final class FeatureEnvelope implements Worker {
 
         final FeatureExpression source;
         final Function evaluator;
@@ -118,7 +118,7 @@ public class ST_Envelope extends AbstractFunction implements FeatureExpression
{
 
             final int minOccurs = attr.getMinimumOccurs();
             final AttributeType<?> crsCharacteristic = attr.characteristics().get(AttributeConvention.CRS_CHARACTERISTIC);
-            AttributeType[] crsParam = crsCharacteristic == null? null : new AttributeType[]{crsCharacteristic};
+            AttributeType[] crsParam = crsCharacteristic == null ? null : new AttributeType[]{crsCharacteristic};
             return new DefaultAttributeType<>(null, Envelope.class, Math.min(1, minOccurs),
1, null, crsParam);
         }
 
@@ -145,7 +145,7 @@ public class ST_Envelope extends AbstractFunction implements FeatureExpression
{
         if (value == null) return null;
 
         if (value instanceof GeographicBoundingBox) {
-            return new GeneralEnvelope((GeographicBoundingBox)value);
+            return new GeneralEnvelope((GeographicBoundingBox) value);
         } else if (value instanceof Envelope) {
             return (Envelope) value;
         } else if (value instanceof CharSequence) {
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java
index ce05495..80a71dab 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java
@@ -188,7 +188,7 @@ add:    for (;;) {
     }
 
     @Override
-    double[] getPoints(Object geometry) {
+    public double[] getPoints(Object geometry) {
         if (geometry instanceof GeometryWrapper) geometry = ((GeometryWrapper) geometry).geometry;
         ensureNonNull("Geometry", geometry);
         if (geometry instanceof MultiVertexGeometry) {
@@ -211,7 +211,7 @@ add:    for (;;) {
         for (final Object polr : polygonsOrLinearRings) {
             if (polr instanceof MultiPath) {
                 poly.add((MultiPath) polr, false);
-            } else throw new UnsupportedOperationException("Unsupported geometry type: "+polr
== null? "null" : polr.getClass().getCanonicalName());
+            } else throw new UnsupportedOperationException("Unsupported geometry type: "+polr
== null ? "null" : polr.getClass().getCanonicalName());
         }
 
         return poly;
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Geometries.java
b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Geometries.java
index 31094c9..f4c6507 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Geometries.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Geometries.java
@@ -252,7 +252,9 @@ public abstract class Geometries<G> {
      * @return the Well Known Text for the given geometry, or {@code null} if the given object
is unrecognized.
      */
     public static String formatWKT(Object geometry, double flatness) {
-        return findStrategy(g -> g.tryFormatWKT(geometry, flatness))
+        if (geometry instanceof GeometryWrapper) geometry = ((GeometryWrapper) geometry).geometry;
+        final Object fGeom = geometry;
+        return findStrategy(g -> g.tryFormatWKT(fGeom, flatness))
                 .orElse(null);
     }
 
@@ -476,7 +478,7 @@ public abstract class Geometries<G> {
         return findStrategy(g -> g.getPoints(geometry));
     }
 
-    abstract double[] getPoints(Object geometry);
+    public abstract double[] getPoints(Object geometry);
 
     abstract Object createMultiPolygonImpl(final Object... polygonsOrLinearRings);
 
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java
index 90fa183..2e9b184 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java
@@ -269,7 +269,7 @@ add:    for (;;) {
     }
 
     @Override
-    double[] getPoints(Object geometry) {
+    public double[] getPoints(Object geometry) {
         if (geometry instanceof GeometryWrapper) {
             geometry = ((GeometryWrapper) geometry).geometry;
         }
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java
index 38f78d8..991a723 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java
@@ -165,21 +165,34 @@ final class Java2D extends Geometries<Shape> {
         final Path2D path = isFloat ? new Path2D.Float (Path2D.WIND_NON_ZERO, length)
                                     : new Path2D.Double(Path2D.WIND_NON_ZERO, length);
         boolean lineTo = false;
+        double startX = Double.NaN, startY = Double.NaN;
+        double lastX = Double.NaN, lastY = Double.NaN;
         for (final Vector v : coordinates) {
             final int size = v.size();
             for (int i=0; i<size;) {
                 final double x = v.doubleValue(i++);
                 final double y = v.doubleValue(i++);
+                if (Double.isNaN(startX)) {
+                    startX = x;
+                    startY = y;
+                }
                 if (Double.isNaN(x) || Double.isNaN(y)) {
+                    if (lastX == startX && lastY == startY) path.closePath();
+                    startX = Double.NaN;
                     lineTo = false;
+                    startX = startY = Double.NaN;
                 } else if (lineTo) {
                     path.lineTo(x, y);
                 } else {
                     path.moveTo(x, y);
                     lineTo = true;
                 }
+                lastX = x; lastY = y;
             }
         }
+
+        if (lastX == startX && lastY == startY) path.closePath();
+
         return ShapeUtilities.toPrimitive(path);
     }
 
@@ -230,7 +243,7 @@ add:    for (;;) {
     }
 
     @Override
-    double[] getPoints(Object geometry) {
+    public double[] getPoints(Object geometry) {
         if (geometry instanceof GeometryWrapper) geometry = ((GeometryWrapper) geometry).geometry;
         ensureNonNull("Geometry", geometry);
         if (geometry instanceof Point2D) return getCoordinate(geometry);
@@ -275,7 +288,7 @@ add:    for (;;) {
     /**
      * An abstraction over {@link PathIterator} to use it in a streaming context.
      */
-    private static class PathSpliterator implements Spliterator<Segment> {
+    private static final class PathSpliterator implements Spliterator<Segment> {
 
         private final PathIterator source;
 
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ANSIInterpreter.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ANSIInterpreter.java
index 3a0c864..676fa1a 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ANSIInterpreter.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ANSIInterpreter.java
@@ -73,8 +73,8 @@ public class ANSIInterpreter implements FilterVisitor, ExpressionVisitor
{
             java.util.function.Function<Literal, CharSequence> valueFormatter,
             java.util.function.Function<PropertyName, CharSequence> nameFormatter
     ) {
-        ensureNonNull("Literal value formmatter", valueFormatter);
-        ensureNonNull("Property name formmatter", nameFormatter);
+        ensureNonNull("Literal value formatter", valueFormatter);
+        ensureNonNull("Property name formatter", nameFormatter);
         this.valueFormatter = valueFormatter;
         this.nameFormatter = nameFormatter;
     }
@@ -168,12 +168,12 @@ public class ANSIInterpreter implements FilterVisitor, ExpressionVisitor
{
 
     @Override
     public Object visit(PropertyIsNull filter, Object extraData) {
-        return evaluateMandatory(filter.getExpression(), extraData) + " = NULL";
+        return evaluateMandatory(filter.getExpression(), extraData) + " IS NULL";
     }
 
     @Override
     public Object visit(PropertyIsNil filter, Object extraData) {
-        return evaluateMandatory(filter.getExpression(), extraData) + " = NULL";
+        return evaluateMandatory(filter.getExpression(), extraData) + " IS NULL";
     }
 
     /*
@@ -394,13 +394,13 @@ public class ANSIInterpreter implements FilterVisitor, ExpressionVisitor
{
 
         // geometric special cases
         if (value instanceof GeographicBoundingBox) {
-            value = new GeneralEnvelope((GeographicBoundingBox)value);
+            value = new GeneralEnvelope((GeographicBoundingBox) value);
         }
         if (value instanceof Envelope) {
-            value = asGeometry((Envelope)value);
+            value = asGeometry((Envelope) value);
         }
         if (value instanceof Geometry) {
-            return format((Geometry)value);
+            return format((Geometry) value);
         }
 
         throw new UnsupportedOperationException("Not supported yet: Literal value of type
"+value.getClass());
@@ -505,7 +505,7 @@ public class ANSIInterpreter implements FilterVisitor, ExpressionVisitor
{
 
     protected static Geometry asGeometry(final Envelope source) {
         final double[] lower = source.getLowerCorner().getCoordinate();
-        final double[] upper = source.getLowerCorner().getCoordinate();
+        final double[] upper = source.getUpperCorner().getCoordinate();
         for (int i = 0 ; i < lower.length ; i++) {
             if (Double.isNaN(lower[i]) || Double.isNaN(upper[i])) {
                 throw new IllegalArgumentException("Cannot use envelope containing NaN for
filter");
diff --git a/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/FilterInterpreterTest.java
b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/FilterInterpreterTest.java
index ac3325f..7c73016 100644
--- a/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/FilterInterpreterTest.java
+++ b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/FilterInterpreterTest.java
@@ -1,5 +1,6 @@
 package org.apache.sis.internal.sql.feature;
 
+import org.opengis.filter.Filter;
 import org.opengis.filter.FilterFactory2;
 import org.opengis.filter.spatial.BBOX;
 
@@ -8,6 +9,7 @@ import org.apache.sis.geometry.GeneralEnvelope;
 import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox;
 import org.apache.sis.test.Assert;
 import org.apache.sis.test.TestCase;
+import org.apache.sis.util.iso.Names;
 
 import org.junit.Test;
 
@@ -18,19 +20,34 @@ public class FilterInterpreterTest extends TestCase {
     public void testGeometricFilter() {
         final ANSIInterpreter interpreter = new ANSIInterpreter();
         final BBOX filter = FF.bbox(FF.property("Toto"), new GeneralEnvelope(new DefaultGeographicBoundingBox(-12.3,
2.1, 43.3, 51.7)));
-        final Object result = filter.accept(interpreter, null);
-        Assert.assertTrue("Result filter should be a text", result instanceof CharSequence);
-        Assert.assertEquals(
-                "Filter as SQL condition: ",
-                "ST_Intersect(" +
+        assertConversion(filter,
+                "ST_Intersects(" +
                             "ST_Envelope(\"Toto\"), " +
                             "ST_Envelope(" +
                                 "ST_GeomFromText(" +
-                                    "POLYGON((-12.3 43.3, -12.3 51.7, 2.1 51.7, 2.1 43.3,
-12.3 43.3))" +
+                                    "POLYGON ((-12.3 43.3, -12.3 51.7, 2.1 51.7, 2.1 43.3,
-12.3 43.3))" +
                                 ")" +
                             ")" +
-                        ")",
-                result.toString()
+                        ")"
+        );
+    }
+
+    @Test
+    public void testSimpleFilter() {
+        Filter filter = FF.and(
+                FF.greater(FF.property(Names.createGenericName(null, ":", "mySchema", "myTable")),
FF.property("otter")),
+                FF.or(
+                        FF.isNull(FF.property("whatever")),
+                        FF.equals(FF.literal(3.14), FF.property("π"))
+                )
         );
+
+        assertConversion(filter, "((\"mySchema\".\"myTable\" > \"otter\") AND (\"whatever\"
IS NULL OR (3.14 = \"π\")))");
+    }
+
+    public void assertConversion(final Filter source, final String expected) {
+        final Object result = source.accept(new ANSIInterpreter(), null);
+        Assert.assertTrue("Result filter should be a text", result instanceof CharSequence);
+        Assert.assertEquals("Filter as SQL condition: ", expected, result.toString());
     }
 }
diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/SubsetAdapter.java
b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/SubsetAdapter.java
index 737d6d6..6f1a823 100644
--- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/SubsetAdapter.java
+++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/SubsetAdapter.java
@@ -51,10 +51,10 @@ public final class SubsetAdapter {
 
         final FeatureSet driverSubset = driver.build().orElse(source);
 
-        return isNoOp(remaining)? driverSubset : remaining.execute(driverSubset);
+        return isNoOp(remaining) ? driverSubset : remaining.execute(driverSubset);
     }
 
-    protected final static boolean isNoOp(final SimpleQuery in) {
+    protected static final boolean isNoOp(final SimpleQuery in) {
         return in.getOffset() <= 0
                 && in.getLimit() == UNLIMITED
                 && allColumnsIncluded(in)
@@ -62,17 +62,17 @@ public final class SubsetAdapter {
                 && !sortRequired(in);
     }
 
-    protected final static boolean sortRequired(final SimpleQuery in) {
+    protected static final boolean sortRequired(final SimpleQuery in) {
         final SortBy[] sortBy = in.getSortBy();
         return sortBy != null && sortBy.length > 0 && Arrays.stream(sortBy).anyMatch(Objects::nonNull);
     }
 
-    protected final static boolean allColumnsIncluded(final SimpleQuery in) {
+    protected static final boolean allColumnsIncluded(final SimpleQuery in) {
         final List<SimpleQuery.Column> cols = in.getColumns();
         return cols == null || cols.isEmpty();
     }
 
-    protected final static boolean filteringRequired(SimpleQuery in) {
+    protected static final boolean filteringRequired(SimpleQuery in) {
         final Filter filter = in.getFilter();
         return filter != Filter.INCLUDE;
     }


Mime
View raw message