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 f511e51bbccab3363b4f640b6e0fa7d862bda654 Author: Alexis Manin <[email protected]> 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; }
