This is an automated email from the ASF dual-hosted git repository. desruisseaux 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 4ce25c9 Complete the implementation of `GeometryGetter` with the use of `InfoStatements` for fetching the CRS from a SRID code. 4ce25c9 is described below commit 4ce25c9f9211c0df8fa6fbf1d6e32ddd2bbb5160 Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Sat Dec 25 04:17:44 2021 +0100 Complete the implementation of `GeometryGetter` with the use of `InfoStatements` for fetching the CRS from a SRID code. --- .../apache/sis/internal/sql/feature/Analyzer.java | 8 +-- .../apache/sis/internal/sql/feature/Column.java | 6 +- .../apache/sis/internal/sql/feature/Database.java | 76 +++++++++++++++------- .../sis/internal/sql/feature/FeatureAdapter.java | 5 +- .../sis/internal/sql/feature/FeatureIterator.java | 26 ++++++-- .../sis/internal/sql/feature/GeometryGetter.java | 51 ++++----------- .../sis/internal/sql/feature/ValueGetter.java | 51 +++++++++------ .../apache/sis/internal/sql/postgis/Postgres.java | 10 +-- .../internal/sql/feature/GeometryGetterTest.java | 51 ++++++++++++--- .../sis/internal/sql/feature/ResultSetMock.java | 68 +++++++++++++++++++ .../sis/internal/sql/postgis/PostgresTest.java | 71 ++++++++++++++++++-- .../sis/internal/sql/postgis/SpatialFeatures.sql | 14 ++-- 12 files changed, 312 insertions(+), 125 deletions(-) diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Analyzer.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Analyzer.java index 11ae100..2a288ea 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Analyzer.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Analyzer.java @@ -125,12 +125,10 @@ final class Analyzer { * @param database information about the spatial database. * @param connection an existing connection to the database, used only for the lifetime of this {@code Analyzer}. * @param metadata value of {@code connection.getMetaData()} (provided because already known by caller). - * @param isSpatial whether the database contains "GEOMETRY_COLUMNS" and "SPATIAL_REF_SYS" tables. * @param customizer user-specified modification to the features, or {@code null} if none. */ Analyzer(final Database<?> database, final Connection connection, final DatabaseMetaData metadata, - final boolean isSpatial, final SchemaModifier customizer) - throws SQLException + final SchemaModifier customizer) throws SQLException { this.database = database; this.tables = new HashMap<>(); @@ -140,7 +138,7 @@ final class Analyzer { this.metadata = metadata; this.escape = metadata.getSearchStringEscape(); this.nameFactory = DefaultFactories.forBuildin(NameFactory.class); - spatialInformation = isSpatial ? database.createInfoStatements(connection) : null; + spatialInformation = database.isSpatial() ? database.createInfoStatements(connection) : null; } /** @@ -269,7 +267,7 @@ final class Analyzer { /** * Initializes the value getter on the given column. - * This method shall be invoked only after geometry columns have been identifier. + * This method shall be invoked only after geometry columns have been identified. */ final ValueGetter<?> setValueGetter(final Column column) { ValueGetter<?> getter = database.getMapping(column); diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Column.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Column.java index f6a558a..cd28d13 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Column.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Column.java @@ -39,6 +39,10 @@ import org.apache.sis.util.Localized; * possibly completed with information about a geometry column. * The aim is to describe all information about a column that is needed for mapping to feature model. * + * <h2>Multi-threading</h2> + * {@code Column} instances shall be kept unmodified after all fields have been initialized. + * The same instances may be read concurrently by many threads. + * * @author Alexis Manin (Geomatys) * @author Martin Desruisseaux (Geomatys) * @version 1.1 @@ -217,7 +221,7 @@ public final class Column { if (isNullable) { attribute.setMinimumOccurs(0); } - valueGetter.getCRS().ifPresent(attribute::setCRS); + valueGetter.getDefaultCRS().ifPresent(attribute::setCRS); return attribute; } diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Database.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Database.java index 8bd9f8b..7da6ed9 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Database.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Database.java @@ -17,8 +17,10 @@ package org.apache.sis.internal.sql.feature; import java.util.Set; +import java.util.Map; import java.util.List; import java.util.HashSet; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.WeakHashMap; import java.util.ArrayList; @@ -75,7 +77,7 @@ import org.apache.sis.util.Debug; * <ul> * <li>{@link #getMapping(Column)} for adding column types to recognize.</li> * <li>{@link #createInfoStatements(Connection)} for more info about spatial information.</li> - * <li>{@link #addIgnoredTables(Set)} for specifying more tables to ignore.</li> + * <li>{@link #addIgnoredTables(Map)} for specifying more tables to ignore.</li> * </ul> * * <h2>Multi-threading</h2> @@ -135,9 +137,18 @@ public class Database<G> extends Syntax { private Table[] tables; /** + * Whether the database contains "GEOMETRY_COLUMNS" and/or "SPATIAL_REF_SYS" tables. + * May also be set to {@code true} if some database-specific tables are found such as + * {@code "geography_columns"} and {@code "raster_columns"} in PostGIS. + * This field is initialized by {@link #analyze analyze(…)} and shall not be modified after that point. + * + * @see #isSpatial() + */ + private boolean isSpatial; + + /** * {@code true} if this database contains at least one geometry column. - * This field is initialized by {@link #analyze(SQLStore, Connection, ResourceDefinition...)} - * and shall not be modified after that point. + * This field is initialized by {@link #analyze analyze(…)} and shall not be modified after that point. * * @see #hasGeometry() */ @@ -303,23 +314,23 @@ public class Database<G> extends Syntax { tableCRS = tableCRS .toLowerCase(Locale.US).intern(); tableGeom = tableGeom.toLowerCase(Locale.US).intern(); } - final Set<String> ignoredTables = new HashSet<>(8); - ignoredTables.add(tableCRS); - ignoredTables.add(tableGeom); + final Map<String,Boolean> ignoredTables = new HashMap<>(8); + ignoredTables.put(tableCRS, Boolean.TRUE); + ignoredTables.put(tableGeom, Boolean.TRUE); addIgnoredTables(ignoredTables); - final boolean isSpatial = hasTable(metadata, tableTypes, ignoredTables); + isSpatial = hasTable(metadata, tableTypes, ignoredTables); /* * Collect the names of all tables specified by user, ignoring the tables * used for database internal working (for example by PostGIS). */ - final Analyzer analyzer = new Analyzer(this, connection, metadata, isSpatial, customizer); + final Analyzer analyzer = new Analyzer(this, connection, metadata, customizer); final Set<TableReference> declared = new LinkedHashSet<>(); for (final GenericName tableName : tableNames) { final String[] names = TableReference.splitName(tableName); try (ResultSet reflect = metadata.getTables(names[2], names[1], names[0], tableTypes)) { while (reflect.next()) { final String table = analyzer.getUniqueString(reflect, Reflection.TABLE_NAME); - if (ignoredTables.contains(table)) { + if (ignoredTables.containsKey(table)) { continue; } declared.add(new TableReference( @@ -376,8 +387,8 @@ public class Database<G> extends Syntax { } /** - * Returns {@code true} if the database contains all specified tables. This method updates - * {@link #schemaOfSpatialTables} and {@link #catalogOfSpatialTables} for the tables found. + * Returns {@code true} if the database contains at least one specified tables associated to {@code Boolean.TRUE}. + * This method updates {@link #schemaOfSpatialTables} and {@link #catalogOfSpatialTables} for the tables found. * If many occurrences of the same table are found, this method searches for a common pair * of catalog and schema names. All tables should be in the same (catalog, schema) pair. * If this is not the case, no (catalog,schema) will be used and the search for the tables @@ -388,23 +399,26 @@ public class Database<G> extends Syntax { * @param tables name of the table to search. * @return whether the given table has been found. */ - private boolean hasTable(final DatabaseMetaData metadata, final String[] tableTypes, final Set<String> tables) + private boolean hasTable(final DatabaseMetaData metadata, final String[] tableTypes, final Map<String,Boolean> tables) throws SQLException { // `SimpleImmutableEntry` used as a way to store a (catalog,schema) pair of strings. final FrequencySortedSet<SimpleImmutableEntry<String,String>> schemas = new FrequencySortedSet<>(true); int count = 0; - for (final String name : tables) { - boolean found = false; - try (ResultSet reflect = metadata.getTables(null, null, name, tableTypes)) { - while (reflect.next()) { - found = true; - schemas.add(new SimpleImmutableEntry<>( - reflect.getString(Reflection.TABLE_CAT), - reflect.getString(Reflection.TABLE_SCHEM))); + for (final Map.Entry<String,Boolean> entry : tables.entrySet()) { + if (entry.getValue()) { + String name = entry.getKey(); + boolean found = false; + try (ResultSet reflect = metadata.getTables(null, null, name, tableTypes)) { + while (reflect.next()) { + found = true; + schemas.add(new SimpleImmutableEntry<>( + reflect.getString(Reflection.TABLE_CAT), + reflect.getString(Reflection.TABLE_SCHEM))); + } } + if (found) count++; } - if (found) count++; } if (count == 0) { return false; @@ -455,6 +469,16 @@ public class Database<G> extends Syntax { } /** + * Returns {@code true} if this database is a spatial database. + * Tables such as "SPATIAL_REF_SYS" are used as sentinel values. + * + * @return whether this database is a spatial database. + */ + public final boolean isSpatial() { + return isSpatial; + } + + /** * Returns {@code true} if this database contains at least one geometry column. * * @return whether at least one geometry column has been found. @@ -540,7 +564,6 @@ public class Database<G> extends Syntax { final GeometryType type = columnDefinition.getGeometryType(); final Class<? extends G> geometryClass = geomLibrary.getGeometryClass(type).asSubclass(geomLibrary.rootClass); return new GeometryGetter<>(geomLibrary, geometryClass, columnDefinition.getGeometryCRS(), getBinaryEncoding(columnDefinition)); - // TODO: need to invoke GeometryGetter.setSridResolver(statements(…)) somewhere. } /** @@ -555,13 +578,16 @@ public class Database<G> extends Syntax { } /** - * Adds to the given set a list of tables to ignore when searching for feature tables. - * The given set already contains the {@code "SPATIAL_REF_SYS"} and {@code "GEOMETRY_COLUMNS"} + * Adds to the given map a list of tables to ignore when searching for feature tables. + * The given map already contains the {@code "SPATIAL_REF_SYS"} and {@code "GEOMETRY_COLUMNS"} * entries when this method is invoked. The default implementation adds nothing. * + * <p>Values tells whether the table can be used as a sentinel value for determining + * that this database {@linkplain #isSpatial is a spatial database}.</p> + * * @param ignoredTables where to add names of tables to ignore. */ - protected void addIgnoredTables(final Set<String> ignoredTables) { + protected void addIgnoredTables(final Map<String,Boolean> ignoredTables) { } /** diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/FeatureAdapter.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/FeatureAdapter.java index ac85330..fc3a6f2 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/FeatureAdapter.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/FeatureAdapter.java @@ -318,15 +318,16 @@ final class FeatureAdapter { * Creates a feature with attribute values initialized to values fetched from the given result set. * This method does not follow associations. * + * @param stmts prepared statements for fetching CRS from SRID, or {@code null} if none. * @param result the result set from which to get attribute values. * @return the feature with attribute values initialized. * @throws Exception if an error occurred while reading the database or converting values. */ - final Feature createFeature(final ResultSet result) throws Exception { + final Feature createFeature(final InfoStatements stmts, final ResultSet result) throws Exception { final Feature feature = featureType.newInstance(); for (int i=0; i<attributes.length; i++) { final Column column = attributes[i]; - final Object value = column.valueGetter.getValue(result, i+1); + final Object value = column.valueGetter.getValue(stmts, result, i+1); if (value != null) { feature.setPropertyValue(column.propertyName, value); } diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/FeatureIterator.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/FeatureIterator.java index fba5727..844821a 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/FeatureIterator.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/FeatureIterator.java @@ -43,9 +43,13 @@ import org.opengis.filter.SortBy; * Each {@code FeatureIterator} iterator is created for one specific SQL query * and can be used for only one iteration. * + * <h2>Parallelism</h2> + * Current implementation of {@code FeatureIterator} does not support parallelism. + * This iterator is not thread-safe and the {@link #trySplit()} method always returns {@code null}. + * * @author Martin Desruisseaux (Geomatys) * @author Alexis Manin (Geomatys) - * @version 1.1 + * @version 1.2 * @since 1.0 * @module */ @@ -83,6 +87,13 @@ final class FeatureIterator implements Spliterator<Feature>, AutoCloseable { private final long estimatedSize; /** + * A cache of statements for fetching spatial information such as geometry columns or SRID. + * This is non-null only if the {@linkplain Database#isSpatial() database is spatial}. + * The same instance is shared by all dependencies of this {@code FeatureIterator}. + */ + private final InfoStatements spatialInformation; + + /** * The feature sets referenced through foreigner keys, or an empty array if none. * This includes the associations inferred from both the imported and exported keys. * The first {@link FeatureAdapter#importCount} iterators are for imported keys, @@ -110,6 +121,7 @@ final class FeatureIterator implements Spliterator<Feature>, AutoCloseable { throws SQLException, InternalDataStoreException { adapter = table.adapter(connection); + spatialInformation = table.database.isSpatial() ? table.database.createInfoStatements(connection) : null; String sql = adapter.sql; if (distinct || filter != null || sort != null || offset > 0 || count > 0) { final SQLBuilder builder = new SQLBuilder(table.database).append(sql); @@ -153,7 +165,10 @@ final class FeatureIterator implements Spliterator<Feature>, AutoCloseable { * @param offset number of rows to skip in underlying SQL query, or ≤ 0 for none. * @param count maximum number of rows to return, or ≤ 0 for no limit. */ - private FeatureIterator(final FeatureAdapter adapter, final Connection connection) throws SQLException { + private FeatureIterator(final FeatureAdapter adapter, final Connection connection, + final InfoStatements spatialInformation) throws SQLException + { + this.spatialInformation = spatialInformation; this.adapter = adapter; statement = connection.prepareStatement(adapter.sql); dependencies = new FeatureIterator[adapter.dependencies.length]; @@ -166,7 +181,7 @@ final class FeatureIterator implements Spliterator<Feature>, AutoCloseable { private FeatureIterator dependency(final int i) throws SQLException { FeatureIterator dependency = dependencies[i]; if (dependency == null) { - dependency = new FeatureIterator(adapter.dependencies[i], result.getStatement().getConnection()); + dependency = new FeatureIterator(adapter.dependencies[i], result.getStatement().getConnection(), spatialInformation); dependencies[i] = dependency; } return dependency; @@ -236,7 +251,7 @@ final class FeatureIterator implements Spliterator<Feature>, AutoCloseable { */ private boolean fetch(final Consumer<? super Feature> action, final boolean all) throws Exception { while (result.next()) { - final Feature feature = adapter.createFeature(result); + final Feature feature = adapter.createFeature(spatialInformation, result); for (int i=0; i < dependencies.length; i++) { WeakValueHashMap<?,Object> instances = null; Object key = null, value = null; @@ -311,6 +326,9 @@ final class FeatureIterator implements Spliterator<Feature>, AutoCloseable { */ @Override public void close() throws SQLException { + if (spatialInformation != null) { + spatialInformation.close(); + } /* * Only one of `statement` and `result` should be non-null. The connection should be closed by * the `FeatureIterator` instance having a non-null `result` because it is the main one created diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/GeometryGetter.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/GeometryGetter.java index 1151690..5d6f2c6 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/GeometryGetter.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/GeometryGetter.java @@ -41,6 +41,9 @@ import org.apache.sis.internal.feature.Geometries; * <li><a href="http://postgis.net/docs/using_postgis_dbmanagement.html#EWKB_EWKT">PostGIS extended format</a></li> * </ul> * + * <h2>Multi-threading</h2> + * {@code GeometryGetter} instances shall be thread-safe. + * * @author Johann Sorel (Geomatys) * @author Alexis Manin (Geomatys) * @author Martin Desruisseaux (Geomatys) @@ -59,14 +62,7 @@ final class GeometryGetter<G, V extends G> extends ValueGetter<V> { private final Geometries<G> geometryFactory; /** - * The mapper to use for resolving a Spatial Reference Identifier (SRID) integer - * as Coordinate Reference System (CRS) object. - * This is {@code null} if there is no mapping to apply. - */ - private InfoStatements fromSridToCRS; - - /** - * The Coordinate Reference System if {@link #fromSridToCRS} can not map the SRID. + * The Coordinate Reference System if {@link InfoStatements} can not map the SRID. * This is {@code null} if there is no default. */ private final CoordinateReferenceSystem defaultCRS; @@ -96,21 +92,11 @@ final class GeometryGetter<G, V extends G> extends ValueGetter<V> { } /** - * Sets the mapper to use for resolving a Spatial Reference Identifier (SRID) integer - * as Coordinate Reference System (CRS) object. - * - * @param fromSridToCRS mapper for resolving SRID integers as CRS objects, or {@code null} if none. - */ - final void setSridResolver(final InfoStatements fromSridToCRS) { - this.fromSridToCRS = fromSridToCRS; - } - - /** * Returns the default coordinate reference system for this column. * The default CRS is declared in the {@code "GEOMETRY_COLUMNS"} table. */ @Override - public Optional<CoordinateReferenceSystem> getCRS() { + public Optional<CoordinateReferenceSystem> getDefaultCRS() { return Optional.ofNullable(defaultCRS); } @@ -119,6 +105,7 @@ final class GeometryGetter<G, V extends G> extends ValueGetter<V> { * The given result set must have its cursor position on the line to read. * This method does not modify the cursor position. * + * @param stmts prepared statements for fetching CRS from SRID, or {@code null} if none. * @param source the result set from which to get the value. * @param columnIndex index of the column in which to get the value. * @return geometry value in the given column. May be {@code null}. @@ -128,32 +115,20 @@ final class GeometryGetter<G, V extends G> extends ValueGetter<V> { * find a way to ensure that SQL queries use {@code ST_AsBinary} function. */ @Override - public V getValue(final ResultSet source, final int columnIndex) throws Exception { + public V getValue(final InfoStatements stmts, final ResultSet source, final int columnIndex) throws Exception { final byte[] wkb = encoding.getBytes(source, columnIndex); if (wkb == null) return null; - final GeometryWrapper<G> geom = read(wkb); - return valueType.cast(geom.implementation()); - } - - /** - * Parses a WKB stored in the given byte array. - * - * @param wkb the array containing the WKB to decode. Should neither be null nor empty. - * @return geometry parsed from the given array of bytes. Never null, never empty. - * @throws Exception if the WKB can not be parsed. The exception type depends on the geometry implementation. - */ - final GeometryWrapper<G> read(final byte[] wkb) throws Exception { - final GeometryWrapper<G> wrapper = geometryFactory.parseWKB(ByteBuffer.wrap(wkb)); + final GeometryWrapper<G> geom = geometryFactory.parseWKB(ByteBuffer.wrap(wkb)); CoordinateReferenceSystem crs = defaultCRS; - if (fromSridToCRS != null) { - final OptionalInt srid = wrapper.getSRID(); + if (stmts != null) { + final OptionalInt srid = geom.getSRID(); if (srid.isPresent()) { - crs = fromSridToCRS.fetchCRS(srid.getAsInt()); + crs = stmts.fetchCRS(srid.getAsInt()); } } if (crs != null) { - wrapper.setCoordinateReferenceSystem(crs); + geom.setCoordinateReferenceSystem(crs); } - return wrapper; + return valueType.cast(geom.implementation()); } } diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ValueGetter.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ValueGetter.java index 4920b95..ad0adfc 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ValueGetter.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ValueGetter.java @@ -38,9 +38,12 @@ import org.apache.sis.util.ArgumentChecks; * The {@code ValueGetter} getter method will typically delegate to the most appropriate {@link ResultSet} getter * method for the column type, but may also perform some conversions such as parsing geometry Well-Known Binary (WKB). * - * <p>The {@link #getValue(ResultSet, int)} method is invoked with the result set cursor placed on the row of interest. - * The index of the column to read must be specified. It allows to reuse the same {@code ValueGetter} instance for an - * arbitrary amount of columns.</p> + * <p>The {@link #getValue(InfoStatements, ResultSet, int)} method is invoked with the result set cursor placed on the + * row of interest. The index of the column to read must be specified. It allows to reuse the same {@code ValueGetter} + * instance for an arbitrary amount of columns.</p> + * + * <h2>Multi-threading</h2> + * {@code ValueGetter} instances shall be thread-safe. * * @param <T> type of values in the column. * @@ -73,12 +76,18 @@ public abstract class ValueGetter<T> { * The given result set must have its cursor position on the line to read. * This method does not modify the cursor position. * + * <div class="note"><b>Note:</b> + * The {@code stmts} is the same reference for all features created by a new {@link FeatureIterator} instance, + * including its dependencies. But the {@code source} will vary depending on whether we are iterating over the + * main feature or one of its dependencies.</div> + * + * @param stmts prepared statements for fetching CRS from SRID, or {@code null} if none. * @param source the result set from which to get the value. * @param columnIndex index of the column in which to get the value. * @return value in the given column. May be {@code null}. * @throws Exception if an error occurred. May be an SQL error, a WKB parsing error, <i>etc.</i> */ - public abstract T getValue(ResultSet source, int columnIndex) throws Exception; + public abstract T getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws Exception; /** * Returns the default coordinate reference system for this column. @@ -90,7 +99,7 @@ public abstract class ValueGetter<T> { * * @return the default coordinate reference system for values in this column. */ - public Optional<CoordinateReferenceSystem> getCRS() { + public Optional<CoordinateReferenceSystem> getDefaultCRS() { return Optional.empty(); } @@ -104,7 +113,7 @@ public abstract class ValueGetter<T> { private AsObject() {super(Object.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Object getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public Object getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { return source.getObject(columnIndex); } } @@ -119,7 +128,7 @@ public abstract class ValueGetter<T> { private AsString() {super(String.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public String getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public String getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { return source.getString(columnIndex); } } @@ -141,7 +150,7 @@ public abstract class ValueGetter<T> { } /** Fetches the value from the specified column in the given result set. */ - @Override public byte[] getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public byte[] getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { return encoding.getBytes(source, columnIndex); } } @@ -157,7 +166,7 @@ public abstract class ValueGetter<T> { private AsByte() {super(Byte.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Byte getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public Byte getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { byte value = source.getByte(columnIndex); return source.wasNull() ? null : value; } @@ -174,7 +183,7 @@ public abstract class ValueGetter<T> { private AsShort() {super(Short.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Short getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public Short getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { short value = source.getShort(columnIndex); return source.wasNull() ? null : value; } @@ -191,7 +200,7 @@ public abstract class ValueGetter<T> { private AsInteger() {super(Integer.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Integer getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public Integer getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { int value = source.getInt(columnIndex); return source.wasNull() ? null : value; } @@ -208,7 +217,7 @@ public abstract class ValueGetter<T> { private AsLong() {super(Long.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Long getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public Long getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { long value = source.getLong(columnIndex); return source.wasNull() ? null : value; } @@ -225,7 +234,7 @@ public abstract class ValueGetter<T> { private AsFloat() {super(Float.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Float getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public Float getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { float value = source.getFloat(columnIndex); return source.wasNull() ? null : value; } @@ -242,7 +251,7 @@ public abstract class ValueGetter<T> { private AsDouble() {super(Double.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Double getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public Double getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { double value = source.getDouble(columnIndex); return source.wasNull() ? null : value; } @@ -258,7 +267,7 @@ public abstract class ValueGetter<T> { private AsBigDecimal() {super(BigDecimal.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public BigDecimal getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public BigDecimal getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { return source.getBigDecimal(columnIndex); } } @@ -274,7 +283,7 @@ public abstract class ValueGetter<T> { private AsBoolean() {super(Boolean.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Boolean getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public Boolean getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { boolean value = source.getBoolean(columnIndex); return source.wasNull() ? null : value; } @@ -292,7 +301,7 @@ public abstract class ValueGetter<T> { private AsDate() {super(Date.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Date getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public Date getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { return source.getDate(columnIndex); } } @@ -310,7 +319,7 @@ public abstract class ValueGetter<T> { private AsLocalTime() {super(LocalTime.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public LocalTime getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public LocalTime getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { final Time time = source.getTime(columnIndex); return (time != null) ? time.toLocalTime() : null; } @@ -329,7 +338,7 @@ public abstract class ValueGetter<T> { private AsInstant() {super(Instant.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Instant getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public Instant getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { final Timestamp time = source.getTimestamp(columnIndex); return (time != null) ? time.toInstant() : null; } @@ -348,7 +357,7 @@ public abstract class ValueGetter<T> { private AsOffsetDateTime() {super(OffsetDateTime.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public OffsetDateTime getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public OffsetDateTime getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { final Timestamp time = source.getTimestamp(columnIndex); if (time == null) return null; final int offsetMinute = time.getTimezoneOffset(); @@ -369,7 +378,7 @@ public abstract class ValueGetter<T> { private AsOffsetTime() {super(OffsetTime.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public OffsetTime getValue(ResultSet source, int columnIndex) throws SQLException { + @Override public OffsetTime getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { final Time time = source.getTime(columnIndex); if (time == null) return null; final int offsetMinute = time.getTimezoneOffset(); diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/postgis/Postgres.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/postgis/Postgres.java index cb36120..86f7f77 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/postgis/Postgres.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/postgis/Postgres.java @@ -16,7 +16,7 @@ */ package org.apache.sis.internal.sql.postgis; -import java.util.Set; +import java.util.Map; import java.sql.Types; import java.sql.Connection; import java.sql.Statement; @@ -150,10 +150,10 @@ public final class Postgres<G> extends Database<G> { * @param ignoredTables where to add names of tables to ignore. */ @Override - protected void addIgnoredTables(final Set<String> ignoredTables) { - ignoredTables.add("geography_columns"); // Postgis 1+ - ignoredTables.add("raster_columns"); // Postgis 2 - ignoredTables.add("raster_overviews"); + protected void addIgnoredTables(final Map<String,Boolean> ignoredTables) { + ignoredTables.put("geography_columns", Boolean.TRUE); // Postgis 1+ + ignoredTables.put("raster_columns", Boolean.TRUE); // Postgis 2 + ignoredTables.put("raster_overviews", Boolean.FALSE); } /** diff --git a/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/GeometryGetterTest.java b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/GeometryGetterTest.java index e57e0c5..4e197d6 100644 --- a/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/GeometryGetterTest.java +++ b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/GeometryGetterTest.java @@ -20,10 +20,14 @@ import java.nio.ByteBuffer; import java.sql.Connection; import java.sql.Statement; import java.sql.ResultSet; +import org.opengis.util.FactoryException; +import org.opengis.referencing.crs.CoordinateReferenceSystem; import org.apache.sis.internal.feature.Geometries; import org.apache.sis.internal.feature.GeometryWrapper; -import org.apache.sis.referencing.crs.HardCodedCRS; +import org.apache.sis.internal.feature.GeometryWithCRS; +import org.apache.sis.referencing.CRS; import org.apache.sis.referencing.CommonCRS; +import org.apache.sis.referencing.crs.HardCodedCRS; import org.apache.sis.setup.GeometryLibrary; import org.apache.sis.test.TestCase; import org.junit.Test; @@ -103,9 +107,18 @@ public strictfp final class GeometryGetterTest extends TestCase { // Read the point. point.rewind(); - final GeometryWrapper<?> read = createReader(library, BinaryEncoding.RAW).read(point.array()); - assertSame(HardCodedCRS.WGS84, read.getCoordinateReferenceSystem()); - assertEquals(GF.createPoint(42.2, 43.3), read.implementation()); + final ResultSet r = ResultSetMock.create(point.array()); + final Object geometry = createReader(library, BinaryEncoding.RAW).getValue(null, r, 1); + assertEquals(GF.createPoint(42.2, 43.3), geometry); + final GeometryWrapper<?> wrapper = Geometries.implementation(library).castOrWrap(geometry); + /* + * If the wrapper is an instance of `GeometryWithCRS`, then the CRS is stored + * with the wrapper instead of the geometry implementation. In such case, the + * CRS is lost on `GeometryWrapper.implementation()` and can not be tested. + */ + if (!(wrapper instanceof GeometryWithCRS)) { + assertSame(HardCodedCRS.WGS84, wrapper.getCoordinateReferenceSystem()); + } } /** @@ -115,25 +128,43 @@ public strictfp final class GeometryGetterTest extends TestCase { * another test class} having a connection to a database. * * @param connection connection to the database. - * @param spatialRefSys helper method for fetching CRS from SRID codes. + * @param fromSridToCRS the resolver of Spatial Reference Identifier (SRID) to CRS, or {@code null}. * @param encoding the way binary data are encoded (raw or hexadecimal). * @throws Exception if an error occurred while querying the database or parsing the WKT or WKB. */ - public void testFromDatabase(final Connection connection, final InfoStatements spatialRefSys, + public void testFromDatabase(final Connection connection, final InfoStatements fromSridToCRS, final BinaryEncoding encoding) throws Exception { final GeometryGetter<?,?> reader = createReader(GeometryLibrary.JTS, encoding); - reader.setSridResolver(spatialRefSys); try (Statement stmt = connection.createStatement(); - ResultSet results = stmt.executeQuery("SELECT \"WKT\",\"WKB\" FROM features.\"Geometries\"")) + ResultSet results = stmt.executeQuery("SELECT \"WKT\",\"WKB\",\"SRID\" FROM features.\"Geometries\"")) { while (results.next()) { final String wkt = results.getString(1); - final Geometry geometry = (Geometry) reader.getValue(results, 2); + final Geometry geometry = (Geometry) reader.getValue(fromSridToCRS, results, 2); final GeometryWrapper<?> expected = GF.parseWKT(wkt); assertEquals("WKT and WKB parsings gave different results.", expected.implementation(), geometry); - assertSame("SRID", CommonCRS.WGS84.normalizedGeographic(), GF.castOrWrap(geometry).getCoordinateReferenceSystem()); + assertSame("SRID", getExpectedCRS(results.getInt(3)), + GF.castOrWrap(geometry).getCoordinateReferenceSystem()); } } } + + /** + * Returns the expected CRS for the given SRID. Note that a SRID is not necessary an EPSG code. + * This method accepts only the SRID used in the {@code "SpatialFeatures"} test schema. + * The mapping from SRID to CRS is hard-coded to the same mapping than the spatial database + * used for the test. Other databases may have different mapping. + * + * @param srid the SRID for which to get the CRS. + * @return the CRS for the given SRID. + * @throws FactoryException if an error occurred while fetching the CRS. + */ + public static CoordinateReferenceSystem getExpectedCRS(final int srid) throws FactoryException { + switch (srid) { + case 3395: return CRS.forCode("EPSG:3395"); + case 4326: return CommonCRS.WGS84.normalizedGeographic(); + default: throw new AssertionError(srid); + } + } } diff --git a/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/ResultSetMock.java b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/ResultSetMock.java new file mode 100644 index 0000000..746a4f1 --- /dev/null +++ b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/ResultSetMock.java @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sis.internal.sql.feature; + +import java.sql.ResultSet; +import java.lang.reflect.Proxy; +import java.lang.reflect.Method; +import java.lang.reflect.InvocationHandler; + + +/** + * An implementation of {@link ResultSet} where the {@code getBytes(1)} method returns a + * predefined array of bytes. Because of the large amount of methods in {@code ResultSet}, + * this class implements that interface using reflection. + * + * @author Martin Desruisseaux (Geomatys) + * @version 1.2 + * @since 1.2 + * @module + */ +final class ResultSetMock implements InvocationHandler { + /** + * The bytes to return on invocation on {@link ResultSet#getBytes(int)}. + */ + private final byte[] wkb; + + /** + * Creates a new handler for a {@link ResultSet} mock. + */ + private ResultSetMock(final byte[] wkb) { + this.wkb = wkb; + } + + /** + * Creates a mock where {@link ResultSet#getBytes(int)} returns the given array of bytes. + */ + static ResultSet create(final byte[] wkb) { + return (ResultSet) Proxy.newProxyInstance(ResultSetMock.class.getClassLoader(), + new Class<?>[] {ResultSet.class}, new ResultSetMock(wkb)); + } + + /** + * Implementation of all {@link ResultSet} methods. Only {@code ResultSet.getBytes(1)} + * is currently supported. All other method invocations cause an {@link AssertionError} + * to be thrown. + */ + @Override + public Object invoke(final Object proxy, final Method method, final Object[] args) { + if (method.getName().equals("getBytes") && args.length == 1 && Integer.valueOf(1).equals(args[0])) { + return wkb; + } + throw new AssertionError(method); + } +} diff --git a/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/postgis/PostgresTest.java b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/postgis/PostgresTest.java index 8eed2b7..7b67836 100644 --- a/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/postgis/PostgresTest.java +++ b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/postgis/PostgresTest.java @@ -23,9 +23,11 @@ import java.nio.ByteBuffer; import java.nio.channels.Channels; import java.nio.channels.ReadableByteChannel; import java.lang.reflect.Method; -import org.opengis.referencing.crs.ProjectedCRS; +import java.util.stream.Stream; +import org.opengis.util.FactoryException; import org.apache.sis.setup.OptionKey; import org.apache.sis.setup.GeometryLibrary; +import org.apache.sis.storage.DataStoreException; import org.apache.sis.storage.sql.SQLStore; import org.apache.sis.storage.sql.SQLStoreProvider; import org.apache.sis.storage.sql.ResourceDefinition; @@ -34,6 +36,8 @@ import org.apache.sis.storage.sql.SQLStoreTest; import org.apache.sis.internal.storage.io.ChannelDataInput; import org.apache.sis.internal.sql.feature.BinaryEncoding; import org.apache.sis.internal.sql.feature.GeometryGetterTest; +import org.apache.sis.internal.feature.jts.JTS; +import org.apache.sis.referencing.CRS; import org.apache.sis.referencing.crs.HardCodedCRS; import org.apache.sis.test.sql.TestDatabase; import org.apache.sis.test.DependsOn; @@ -41,7 +45,14 @@ import org.apache.sis.test.TestCase; import org.apache.sis.util.Version; import org.junit.Test; -import static org.opengis.test.Assert.*; +// Branch-dependent imports +import org.opengis.feature.Feature; + +// Optional dependencies +import org.locationtech.jts.geom.Point; +import org.locationtech.jts.geom.Geometry; + +import static org.junit.Assert.*; /** @@ -92,6 +103,7 @@ public final strictfp class PostgresTest extends TestCase { testInfoStatements(info); testGeometryGetter(info, connection); testRasterReader(TestRaster.USHORT, info, connection); + testFeatures(store); } } } @@ -102,9 +114,9 @@ public final strictfp class PostgresTest extends TestCase { * * @throws Exception if an error occurred while testing the database. */ - private void testInfoStatements(final ExtendedInfo info) throws Exception { + private static void testInfoStatements(final ExtendedInfo info) throws Exception { assertEquals("findSRID", 4326, info.findSRID(HardCodedCRS.WGS84)); - assertInstanceOf("fetchCRS", ProjectedCRS.class, info.fetchCRS(3395)); + assertSame("fetchCRS", CRS.forCode("EPSG:3395"), info.fetchCRS(3395)); } /** @@ -113,15 +125,17 @@ public final strictfp class PostgresTest extends TestCase { * * @throws Exception if an error occurred while testing the database. */ - private void testGeometryGetter(final ExtendedInfo info, final Connection connection) throws Exception { + private static void testGeometryGetter(final ExtendedInfo info, final Connection connection) throws Exception { final GeometryGetterTest test = new GeometryGetterTest(); test.testFromDatabase(connection, info, BinaryEncoding.HEXADECIMAL); } /** - * Tests {@link RasterReader}. + * Tests {@link RasterReader} in the context of a database. */ - private void testRasterReader(final TestRaster test, final ExtendedInfo info, final Connection connection) throws Exception { + private static void testRasterReader(final TestRaster test, final ExtendedInfo info, final Connection connection) + throws Exception + { final BinaryEncoding encoding = BinaryEncoding.HEXADECIMAL; final RasterReader reader = new RasterReader(info); try (PreparedStatement stmt = connection.prepareStatement("SELECT image FROM features.\"SpatialData\" WHERE filename=?")) { @@ -134,4 +148,47 @@ public final strictfp class PostgresTest extends TestCase { assertFalse(r.next()); } } + + /** + * Tests the construction of feature instances. + */ + private static void testFeatures(final SQLStore store) throws DataStoreException { + try (Stream<Feature> features = store.findResource("SpatialData").features(false)) { + features.forEach(PostgresTest::validate); + } + } + + /** + * Invoked for each feature instances for performing some checks on the feature. + * This method performs only a superficial verification of geometries. + */ + private static void validate(final Feature feature) { + final String filename = feature.getPropertyValue("filename").toString(); + final Geometry geometry = (Geometry) feature.getPropertyValue("geometry"); + final int geomSRID; + switch (filename) { + case "raster-ushort.wkb": { + assertNull(geometry); + return; + } + case "point-prj": { + final Point p = (Point) geometry; + assertEquals(2, p.getX(), STRICT); + assertEquals(3, p.getY(), STRICT); + geomSRID = 3395; + break; + } + case "polygon-prj": geomSRID = 3395; break; + case "linestring": + case "polygon": + case "multi-linestring": + case "multi-polygon": geomSRID = 4326; break; + default: throw new AssertionError(filename); + } + try { + assertEquals(GeometryGetterTest.getExpectedCRS(geomSRID), JTS.getCoordinateReferenceSystem(geometry)); + } catch (FactoryException e) { + throw new AssertionError(e); + } + } } diff --git a/storage/sis-sqlstore/src/test/resources/org/apache/sis/internal/sql/postgis/SpatialFeatures.sql b/storage/sis-sqlstore/src/test/resources/org/apache/sis/internal/sql/postgis/SpatialFeatures.sql index 7302488..2dec522 100644 --- a/storage/sis-sqlstore/src/test/resources/org/apache/sis/internal/sql/postgis/SpatialFeatures.sql +++ b/storage/sis-sqlstore/src/test/resources/org/apache/sis/internal/sql/postgis/SpatialFeatures.sql @@ -32,16 +32,16 @@ INSERT INTO features."SpatialData" ("filename", "image") VALUES -- Geometries with arbitrary coordinate values. -- INSERT INTO features."SpatialData" ("filename", "geometry") VALUES - ('hexa-wkb.csv:1', ST_GeomFromText('POINT(0 0)', 4326)), - ('hexa-wkb.csv:2', ST_GeomFromText('LINESTRING(-71.160281 42.258729,-71.160837 42.259113,-71.161144 42.25932)', 4326)), - ('hexa-wkb.csv:3', ST_GeomFromText('POLYGON((0 0,0 1,1 1,1 0,0 0))', 4326)), - ('hexa-wkb.csv:4', ST_GeomFromText('POLYGON' + ('point-prj', ST_GeomFromText('POINT(2 3)', 3395)), + ('linestring', ST_GeomFromText('LINESTRING(-71.160281 42.258729,-71.160837 42.259113,-71.161144 42.25932)', 4326)), + ('polygon-prj', ST_GeomFromText('POLYGON((0 0,0 1,1 1,1 0,0 0))', 3395)), + ('polygon', ST_GeomFromText('POLYGON' || '((-71.1776585052917 42.3902909739571,-71.1776820268866 42.3903701743239,-71.1776063012595 42.3903825660754,' || '-71.1775826583081 42.3903033653531,-71.1776585052917 42.3902909739571))', 4326)), - ('hexa-wkb.csv:5', ST_GeomFromText('MULTILINESTRING' + ('multi-linestring', ST_GeomFromText('MULTILINESTRING' || '((-71.160281 42.258729,-71.160837 42.259113,-71.161144 42.25932),' || '(-71.1031627617667 42.3152960829043,-71.102923838298 42.3149156848307))', 4326)), - ('hexa-wkb.csv:6', ST_GeomFromText('MULTIPOLYGON(' + ('multi-polygon', ST_GeomFromText('MULTIPOLYGON(' || '((-71.1031880899493 42.3152774590236,-71.1031627617667 42.3152960829043,-71.102923838298 42.3149156848307,' || '-71.1023097974109 42.3151969047397,-71.1019285062273 42.3147384934248,-71.102505233663 42.3144722937587,' || '-71.1027748747100 42.3141658254797,-71.1031139451630 42.3142739188902,-71.103248764160 42.3140248998700,' @@ -65,6 +65,6 @@ INSERT INTO features."SpatialData" ("filename", "geometry") VALUES -- Used for parsing the same geometry in two ways and comparing the results. -- CREATE VIEW features."Geometries" AS - SELECT ST_AsText(geometry) AS "WKT", geometry AS "WKB" + SELECT ST_AsText(geometry) AS "WKT", geometry AS "WKB", ST_SRID(geometry) AS "SRID" FROM features."SpatialData" WHERE geometry IS NOT NULL;