This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/sis.git
commit 91b3d93094c687ae1adbd3abc57c82b58aaf79ab Merge: 260a742c08 fac1549bcd Author: Martin Desruisseaux <[email protected]> AuthorDate: Thu Oct 1 13:30:54 2026 +0200 Merge branch 'geoapi-3.1'. - Hardening against denial of service. - Work on Shapefile reader (incubator). README.md | 1 + .../apache/sis/coverage/grid/GridCRSBuilder.java | 2 +- .../org/apache/sis/filter/IdentifierFilter.java | 11 +- .../sis/pending/geoapi/filter/ResourceId.java} | 24 +- .../apache/sis/metadata/internal/Resources.java | 5 + .../sis/metadata/internal/Resources.properties | 1 + .../sis/metadata/internal/Resources_fr.properties | 1 + .../metadata/sql/internal/shared/SQLBuilder.java | 10 +- .../main/org/apache/sis/xml/IdentifierSpace.java | 4 +- .../main/org/apache/sis/xml/ReferenceResolver.java | 83 +++- .../main/org/apache/sis/xml/XML.java | 30 +- .../main/org/apache/sis/xml/bind/Context.java | 13 +- .../xml/internal/shared/ExternalLinkHandler.java | 31 +- .../sis/xml/internal/shared/InputFactory.java | 49 +-- .../apache/sis/xml/internal/shared/URISource.java | 48 ++- .../main/org/apache/sis/xml/package-info.java | 2 +- .../sql/internal/shared/SQLBuilderTest.java | 12 + .../org/apache/sis/metadata/xml/TestUsingFile.java | 17 + .../sis/metadata/xml/extern/UsingExternalXLink.xml | 51 +++ .../org/apache/sis/xml/ReferenceResolverMock.java | 1 + .../org/apache/sis/xml/ReferenceResolverTest.java | 108 ++++- .../main/org/apache/sis/io/wkt/WKTFormat.java | 5 +- .../sis/parameter/DefaultParameterValue.java | 60 +-- .../main/org/apache/sis/parameter/Parameters.java | 11 +- .../factory/ConcurrentAuthorityFactory.java | 8 +- .../referencing/factory/sql/EPSGDataAccess.java | 9 +- .../sis/referencing/factory/sql/EPSGFactory.java | 4 +- .../operation/AbstractSingleOperation.java | 2 +- .../referencing/operation/DefiningConversion.java | 100 +++-- .../referencing/operation/gridded/GridFile.java | 223 +++-------- .../referencing/operation/gridded/LoadedGrid.java | 6 +- .../provider/FranceGeocentricInterpolation.java | 12 +- .../sis/referencing/operation/provider/NADCON.java | 16 +- .../sis/referencing/operation/provider/NTv2.java | 35 +- .../transform/InterpolatedGeocentricTransform.java | 4 +- .../InterpolatedGeocentricTransform2D.java | 2 +- .../operation/transform/MathTransformBuilder.java | 59 ++- .../xml/bind/referencing/CC_OperationMethod.java | 4 +- .../org/apache/sis/xml/bind/referencing/Code.java | 3 +- .../test/org/apache/sis/io/wkt/ElementTest.java | 3 +- .../operation/gridded/GridFileTest.java | 117 ++++++ .../operation/provider/DatumShiftTestCase.java | 67 ---- .../FranceGeocentricInterpolationTest.java | 75 +++- .../provider/GeocentricTranslationTest.java | 2 +- .../referencing/operation/provider/NADCONTest.java | 52 ++- .../referencing/operation/provider/NTv2Test.java | 49 ++- .../provider/PositionVector7ParamTest.java | 3 +- .../InterpolatedGeocentricTransformTest.java | 47 +-- .../transform/InterpolatedTransformTest.java | 35 +- .../sis/test/integration/DatumShiftTest.java | 8 +- .../main/module-info.java | 2 +- .../main/org/apache/sis/storage/landsat/Band.java | 36 +- .../apache/sis/storage/landsat/LandsatStore.java | 14 +- .../apache/sis/storage/landsat/MetadataReader.java | 6 +- .../sis/storage/landsat}/internal/Resources.java | 113 +----- .../storage/landsat}/internal/Resources.properties | 12 +- .../storage/landsat/internal/Resources_en.java} | 25 +- .../storage/landsat/internal/Resources_fr.java} | 25 +- .../landsat/internal/Resources_fr.properties} | 17 +- .../storage/landsat/internal/package-info.java} | 21 +- .../apache/sis/storage/landsat/package-info.java | 2 +- .../storage/landsat/LandsatStoreProviderTest.java | 92 ++++- .../sis/storage/landsat/MetadataReaderTest.java | 1 + .../apache/sis/storage/geotiff/DeferredEntry.java | 29 +- .../org/apache/sis/storage/geotiff/Reader.java | 11 +- .../geotiff/reader/ImageMetadataBuilder.java | 1 + .../apache/sis/storage/netcdf/MetadataReader.java | 11 +- .../sis/storage/netcdf/classic/ChannelDecoder.java | 27 +- .../org.apache.sis.storage/main/module-info.java | 1 + .../main/org/apache/sis/io/stream/ChannelData.java | 11 +- .../org/apache/sis/io/stream/ChannelDataInput.java | 18 + .../main/org/apache/sis/io/stream/IOUtilities.java | 17 + .../org/apache/sis/storage/base/URIDataStore.java | 4 +- .../org/apache/sis/storage/event/WarningEvent.java | 18 +- .../org/apache/sis/storage/event/package-info.java | 4 +- .../main/org/apache/sis/storage/wkt/Store.java | 1 + .../main/org/apache/sis/storage/xml/Store.java | 4 +- .../main/org/apache/sis/io/Authorization.java | 71 ++++ .../main/org/apache/sis/io/package-info.java | 34 +- .../main/org/apache/sis/setup/Configuration.java | 38 +- .../main/org/apache/sis/system/DataURI.java | 226 +++++++++++ .../main/org/apache/sis/system/Environment.java | 8 + .../sis/util/internal/AutoMessageFormat.java | 22 +- .../apache/sis/util/internal/shared/Strings.java | 78 ++++ .../apache/sis/util/logging/MonolineFormatter.java | 44 +-- .../main/org/apache/sis/util/resources/Errors.java | 5 + .../apache/sis/util/resources/Errors.properties | 1 + .../apache/sis/util/resources/Errors_fr.properties | 3 +- .../sis/util/resources/IndexedResourceBundle.java | 18 +- .../main/module-info.java | 3 + .../storage/internal/shared/AggregationEvent.java | 61 +++ .../sis/storage/internal/shared/ContentEvent.java} | 26 +- .../internal/shared/FeatureSetContentEvent.java | 99 +++++ .../internal/shared/FeatureSetModelEvent.java | 100 +++++ .../sis/storage/internal/shared/ModelEvent.java} | 25 +- .../sis/storage/internal/shared/StorageEvent.java} | 26 +- .../internal/shared/TilingContentEvent.java | 88 +++++ .../storage/internal/shared/TilingModelEvent.java | 97 +++++ .../sis/storage/shapefile/ShapefileStore.java | 439 +++++++++++++++------ .../sis/storage/shapefile/dbf/DBFReader.java | 7 +- .../sis/storage/shapefile/dbf/DBFWriter.java | 14 + .../apache/sis/storage/shapefile/package-info.java | 9 + .../sis/storage/shapefile/shp/ShapeRecord.java | 26 +- .../sis/storage/shapefile/shp/ShapeWriter.java | 5 + .../shapefile/ShapefileStoreEventsTest.java | 372 +++++++++++++++++ .../sis/storage/shapefile/ShapefileStoreTest.java | 149 ++++++- .../sis/storage/shapefile/dbf/DBFIOTest.java | 48 +++ .../sis/storage/shapefile/shp/ShapeIOTest.java | 63 +++ netbeans-project/nbproject/project.xml | 1 + optional/src/org.apache.sis.gui/bundle/bin/sis | 1 + optional/src/org.apache.sis.gui/bundle/bin/sisfx | 1 + .../main/org/apache/sis/gui/setup/Inflater.java | 6 +- .../main/module-info.java | 2 +- .../referencing/factory/sql/epsg/package-info.java | 4 +- .../factory/sql/epsg/DataScriptUpdater.java | 4 + .../sis/referencing/factory/sql/epsg/README.md | 10 +- 116 files changed, 3268 insertions(+), 1014 deletions(-) diff --cc endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/grid/GridCRSBuilder.java index c1eecf0019,93cce3748d..cab79f7dc3 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/grid/GridCRSBuilder.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/coverage/grid/GridCRSBuilder.java @@@ -254,10 -254,10 +254,10 @@@ final class GridCRSBuilder extends Refe throws FactoryException { this.anchor = anchor; - properties.put(DefiningConversion.NORMALIZED_KEY, Boolean.FALSE); + properties.put(DefiningConversion.SIDE_PROCESSING_KEY, DefiningConversion.SideProcessing.NONE); - properties.put(ObjectDomain.SCOPE_KEY, SCOPE); + properties.put(Datum.SCOPE_KEY, SCOPE); grid.getGeographicExtent().ifPresent((domain) -> { - properties.put(ObjectDomain.DOMAIN_OF_VALIDITY_KEY, new DefaultExtent(null, domain, null, null)); + properties.put(Datum.DOMAIN_OF_VALIDITY_KEY, new DefaultExtent(null, domain, null, null)); }); fullGrid = grid; if (derived || grid.isDefined(GridGeometry.CRS | GridGeometry.GRID_TO_CRS)) try { diff --cc endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/IdentifierFilter.java index eb8c23652c,61af4ee1f5..04cb80edd7 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/IdentifierFilter.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/IdentifierFilter.java @@@ -24,8 -24,12 +24,9 @@@ import org.apache.sis.filter.base.Node import org.apache.sis.filter.base.XPathSource; import org.apache.sis.feature.internal.shared.AttributeConvention; -// Specific to the geoapi-3.1 and geoapi-4.0 branches: -import org.opengis.feature.Feature; -import org.opengis.feature.PropertyNotFoundException; -import org.opengis.filter.Expression; -import org.opengis.filter.ResourceId; -import org.opengis.filter.Filter; +// Specific to the main branch: +import org.apache.sis.feature.AbstractFeature; ++import org.apache.sis.pending.geoapi.filter.ResourceId; /** @@@ -36,7 -40,7 +37,7 @@@ * @author Martin Desruisseaux (Geomatys) */ final class IdentifierFilter extends Node - implements Filter<AbstractFeature>, XPathSource, Optimization.OnFilter<AbstractFeature> - implements ResourceId<Feature>, XPathSource, Optimization.OnFilter<Feature> ++ implements ResourceId<AbstractFeature>, XPathSource, Optimization.OnFilter<AbstractFeature> { /** * For cross-version compatibility. diff --cc endorsed/src/org.apache.sis.feature/main/org/apache/sis/pending/geoapi/filter/ResourceId.java index 37a0ea58ef,5585a056ad..36da42defb --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/pending/geoapi/filter/ResourceId.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/pending/geoapi/filter/ResourceId.java @@@ -14,22 -14,16 +14,16 @@@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.sis.geometries.internal.shared; ++package org.apache.sis.pending.geoapi.filter; - /** - * Earth observation stores. - * - * @author Rémi Maréchal (Geomatys) - * @author Thi Phuong Hao Nguyen (VNSC) - * @author Minh Chinh Vu (VNSC) - * @author Martin Desruisseaux (Geomatys) - * @version 1.4 - * @since 0.8 - */ - module org.apache.sis.storage.earthobservation { - requires transitive org.apache.sis.storage.geotiff; -// Test dependencies -import org.apache.sis.geometries.DataPointsTest; ++import org.apache.sis.filter.Filter; - provides org.apache.sis.storage.DataStoreProvider - with org.apache.sis.storage.landsat.LandsatStoreProvider; - exports org.apache.sis.storage.landsat; + /** - * Tests {@link ArrayDataPoints}. - * - * @author Johann Sorel (Geomatys) ++ * Placeholder for GeoAPI 3.1 interfaces (not yet released). ++ * Shall not be visible in public API, as it will be deleted after next GeoAPI release. + */ -public class ArrayDataPointsTest extends DataPointsTest { ++@SuppressWarnings("doclint:missing") ++public interface ResourceId<R> extends Filter<R> { ++ String getIdentifier(); } diff --cc endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/MathTransformBuilder.java index c168578bab,96a16c62c7..4a6c4c844e --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/MathTransformBuilder.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/referencing/operation/transform/MathTransformBuilder.java @@@ -41,10 -41,10 +47,10 @@@ import org.opengis.parameter.ParameterV * Then, the transform is created by a call to {@link #create()}. * * @author Martin Desruisseaux (Geomatys) - * @version 1.5 + * @version 1.7 * @since 1.5 */ -public abstract class MathTransformBuilder implements MathTransform.Builder { +public abstract class MathTransformBuilder { /** * The factory to use for building the transform. */ @@@ -83,86 -97,44 +102,124 @@@ return Optional.ofNullable(provider); } + /** + * Returns the parameter values of the transform to create. + * Those parameters are initialized to default values, which may be implementation or method depend. + * User-supplied values should be set directly in the returned instance with codes like + * <code>parameter(</code><var>name</var><code>).setValue(</code><var>value</var><code>)</code>. + * + * @return the parameter values of the transform to create. Values should be set in-place. + */ + public abstract ParameterValueGroup parameters(); + + /** + * Gives hints about axis lengths and their orientations in input coordinates. + * The action performed by this call depends on the {@linkplain #getMethod() operation method}. + * For map projections, the action may include something equivalent to the following code: + * + * {@snippet lang="java" : + * parameters().parameter("semi_major").setValue(ellipsoid.getSemiMajorAxis(), ellipsoid.getAxisUnit()); + * parameters().parameter("semi_minor").setValue(ellipsoid.getSemiMinorAxis(), ellipsoid.getAxisUnit()); + * } + * + * For geodetic datum shifts, the action may be similar to above code but with different parameter names: + * {@code "src_semi_major"} and {@code "src_semi_minor"}. Other operation methods may ignore the arguments. + * + * <h4>Axis order, units and direction</h4> + * By default, the source axes of a parameterized transform are normalized to <var>east</var>, + * <var>north</var>, <var>up</var> (if applicable) directions with units in degrees and meters. + * If this requirement is ambiguous, for example because the operation method uses incompatible + * axis directions or units, then the {@code cs} argument should be non-null for allowing the + * implementation to resolve that ambiguity. + * + * @param cs the coordinate system defining source axis order and units, or {@code null} if none. + * @param ellipsoid the ellipsoid providing source semi-axis lengths, or {@code null} if none. + */ + public void setSourceAxes(CoordinateSystem cs, Ellipsoid ellipsoid) { + } + + /** + * Gives hints about axis lengths and their orientations in output coordinates. + * The action performed by this call depends on the {@linkplain #getMethod() operation method}. + * For datum shifts, the action may include something equivalent to the following code: + * + * {@snippet lang="java" : + * parameters().parameter("tgt_semi_major").setValue(ellipsoid.getSemiMajorAxis(), ellipsoid.getAxisUnit()); + * parameters().parameter("tgt_semi_minor").setValue(ellipsoid.getSemiMinorAxis(), ellipsoid.getAxisUnit()); + * } + * + * <h4>Axis order, units and direction</h4> + * By default, the target axes of a parameterized transform are normalized to <var>east</var>, + * <var>north</var>, <var>up</var> (if applicable) directions with units in degrees and meters. + * If this requirement is ambiguous, for example because the operation method uses incompatible + * axis directions or units, then the {@code cs} argument should be non-null for allowing the + * implementation to resolve that ambiguity. + * + * @param cs the coordinate system defining target axis order and units, or {@code null} if none. + * @param ellipsoid the ellipsoid providing target semi-axis lengths, or {@code null} if none. + */ + public void setTargetAxes(CoordinateSystem cs, Ellipsoid ellipsoid) { + } + + /** + * Creates the parameterized transform. The operation method is given by {@link #getMethod()} + * and the parameter values should have been set on the group returned by {@link #parameters()} + * before to invoke this constructor. + * Example: + * + * {@snippet lang="java" : + * MathTransformFactory factory = ...; + * MathTransformBuilder builder = factory.builder("Transverse_Mercator"); + * ParameterValueGroup pg = builder.parameters(); + * pg.parameter("semi_major").setValue(6378137.000); + * pg.parameter("semi_minor").setValue(6356752.314); + * MathTransform mt = builder.create(); + * } + * + * @return the parameterized transform. + * @throws FactoryException if the transform creation failed. + * This exception is thrown if some required parameters have not been supplied, or have illegal values. + */ + public abstract MathTransform create() throws FactoryException; + + /** + * Returns a function which determines whether the <abbr>URI</abbr> specified in a parameter can be opened. + * The function will receive the following arguments: + * + * <ol> + * <li>a description of the <abbr>URI</abbr> parameter,</li> + * <li>the actual <abbr>URI</abbr> parameter value.</li> + * </ol> + * + * The default access control is a function returning {@link Authorization#GRANTED} + * in a {@linkplain Configuration#isTrustedEnvironment() trusted environment}, + * or {@link Authorization#DEFAULT} otherwise. + * The {@code DEFAULT} authorization grants access to files in the {@code $SIS_DATA/DatumChanges} + * directory for parameters that are datum shift grid files, and to files in the same directory as the + * <abbr>JSON</abbr>, <abbr>GML</abbr> or <abbr>WKT</abbr> document where the parameter value appears. + * + * @return a function deciding whether the <abbr>URI</abbr> can be opened. + * + * @see org.apache.sis.setup.Configuration#isTrustedEnvironment() + * + * @since 1.7 + */ + public BiFunction<ParameterDescriptor<URI>, URI, Authorization> getAccessControl() { + return accessControl; + } + + /** + * Sets a function which determines whether the <abbr>URI</abbr> specified in a parameter can be opened. + * See {@link #getAccessControl()} for more information. + * + * @param ac function telling whether the <abbr>URI</abbr> can be opened. + * + * @since 1.7 + */ + public void setAccessControl(BiFunction<ParameterDescriptor<URI>, URI, Authorization> ac) { + accessControl = Objects.requireNonNull(ac); + } + /** * Eventually replaces the given transform by a unique instance. The replacement is done * only if the {@linkplain #factory} is an instance of {@link DefaultMathTransformFactory} diff --cc endorsed/src/org.apache.sis.referencing/main/org/apache/sis/xml/bind/referencing/Code.java index 03266ddc83,010e70f2cf..57684d0d23 --- a/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/xml/bind/referencing/Code.java +++ b/endorsed/src/org.apache.sis.referencing/main/org/apache/sis/xml/bind/referencing/Code.java @@@ -227,14 -228,12 +228,14 @@@ public final class Code if (isEPSG) { code.codeSpace = Constants.IOGP; // Default value if we do not find a codespace below. if (authority != null) { - for (final Identifier id : authority.getIdentifiers()) { + for (final Identifier id : Containers.nonNull(authority.getIdentifiers())) { if (Constants.EPSG.equalsIgnoreCase(id.getCode())) { - final String cs = id.getCodeSpace(); - if (cs != null) { - code.codeSpace = cs; - break; + if (id instanceof ReferenceIdentifier) { + final String cs = ((ReferenceIdentifier) id).getCodeSpace(); + if (cs != null) { + code.codeSpace = cs; + break; + } } } } diff --cc endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/provider/FranceGeocentricInterpolationTest.java index bef2372ca9,2bee566881..c0aae0dc1d --- a/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/provider/FranceGeocentricInterpolationTest.java +++ b/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/provider/FranceGeocentricInterpolationTest.java @@@ -40,9 -44,10 +44,9 @@@ import org.apache.sis.referencing.opera * * @author Martin Desruisseaux (Geomatys) * - * @see GeocentricTranslationTest#testFranceGeocentricInterpolationPoint() * @see org.apache.sis.referencing.operation.transform.MolodenskyTransformTest#testFranceGeocentricInterpolationPoint() */ - public final class FranceGeocentricInterpolationTest extends DatumShiftTestCase { + public final class FranceGeocentricInterpolationTest extends TestCase { /** * Name of the file containing a small extract of the "{@code GR3DF97A.txt}" file. * The amount of data in this test file is less than 0.14% of the original file. diff --cc endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/provider/NADCONTest.java index b13123790c,91cc9ab133..e0d4a5194e --- a/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/provider/NADCONTest.java +++ b/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/provider/NADCONTest.java @@@ -36,9 -41,11 +41,11 @@@ import org.apache.sis.parameter.Paramet // Test dependencies import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; + import org.apache.sis.test.TestCase; + import org.apache.sis.referencing.operation.gridded.GridFileTest; -// Specific to the geoapi-3.1 and geoapi-4.0 branches: -import static org.opengis.test.Assertions.assertMatrixEquals; +// Specific to the main branch: +import static org.apache.sis.test.GeoapiAssert.assertMatrixEquals; /** diff --cc endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/provider/NTv2Test.java index 2d2c01391b,550f4dce9f..c9b36c362e --- a/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/provider/NTv2Test.java +++ b/endorsed/src/org.apache.sis.referencing/test/org/apache/sis/referencing/operation/provider/NTv2Test.java @@@ -43,9 -46,11 +46,11 @@@ import org.apache.sis.system.DataDirect // Test dependencies import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; + import org.apache.sis.test.TestCase; + import org.apache.sis.referencing.operation.gridded.GridFileTest; -// Specific to the geoapi-3.1 and geoapi-4.0 branches: -import static org.opengis.test.Assertions.assertMatrixEquals; +// Specific to the main branch: +import static org.apache.sis.test.GeoapiAssert.assertMatrixEquals; /** @@@ -54,9 -59,10 +59,9 @@@ * * @author Martin Desruisseaux (Geomatys) * - * @see GeocentricTranslationTest#testFranceGeocentricInterpolationPoint() * @see org.apache.sis.referencing.operation.transform.MolodenskyTransformTest#testFranceGeocentricInterpolationPoint() */ - public final class NTv2Test extends DatumShiftTestCase { + public final class NTv2Test extends TestCase { /** * Name of the file containing a small extract of the "{@code NTF_R93.gsb}" file. * The amount of data in this test file is less than 0.14% of the original file. diff --cc endorsed/src/org.apache.sis.storage.earthobservation/test/org/apache/sis/storage/landsat/MetadataReaderTest.java index d51b8ad3ad,795682b9f2..f7b9037bc7 --- a/endorsed/src/org.apache.sis.storage.earthobservation/test/org/apache/sis/storage/landsat/MetadataReaderTest.java +++ b/endorsed/src/org.apache.sis.storage.earthobservation/test/org/apache/sis/storage/landsat/MetadataReaderTest.java @@@ -30,7 -57,18 +30,8 @@@ import org.apache.sis.test.TestCase * @author Thi Phuong Hao Nguyen (VNSC) * @author Martin Desruisseaux (Geomatys) */ + @SuppressWarnings("exports") public final class MetadataReaderTest extends TestCase { - /** - * Helper class for verifying metadata content. - */ - private ContentVerifier verifier; - - /** - * A buffer for building paths to expected properties. - */ - private StringBuilder buffer; - /** * Creates a new test case. */ diff --cc incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/internal/shared/FeatureSetContentEvent.java index 0000000000,b0b7e8171e..650edf2e3f mode 000000,100644..100644 --- a/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/internal/shared/FeatureSetContentEvent.java +++ b/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/internal/shared/FeatureSetContentEvent.java @@@ -1,0 -1,98 +1,99 @@@ + /* + * 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.storage.internal.shared; + + import java.util.Set; + import org.apache.sis.filter.DefaultFilterFactory; + import org.apache.sis.storage.FeatureSet; + import org.apache.sis.storage.Resource; -import org.opengis.feature.Feature; -import org.opengis.filter.Filter; -import org.opengis.filter.ResourceId; ++ ++// Specific to the main branch: ++import org.apache.sis.filter.Filter; ++import org.apache.sis.feature.AbstractFeature; + + /** + * FeatureSet content event. + * + * @author Johann Sorel (Geomatys) + */ + public class FeatureSetContentEvent extends ContentEvent { + + public static enum Type{ + ADD, + UPDATE, + DELETE + }; + + private final Type type; + private Filter ids; + - public FeatureSetContentEvent(final FeatureSet source, final Type type, final Filter<Feature> identifiers) { ++ public FeatureSetContentEvent(final FeatureSet source, final Type type, final Filter<AbstractFeature> identifiers) { + super(source); + this.type = type; + this.ids = identifiers; + } + - public FeatureSetContentEvent(final FeatureSet source, final Type type, final Set<ResourceId> ids){ ++ public FeatureSetContentEvent(final FeatureSet source, final Type type, final Set<Filter> ids){ + this(source, type, resourceId(ids)); + } + - public static Filter<Feature> resourceId(final Set<ResourceId> ids) { ++ public static Filter<AbstractFeature> resourceId(final Set<Filter> ids) { + if (ids == null) { + return null; + } + switch (ids.size()) { + case 0: return Filter.exclude(); + case 1: return ids.iterator().next(); + default: return DefaultFilterFactory.forFeatures().or((Set) ids); + } + } + + /** + * Get the event type, can be Add, Update or Delete. + * @return Type of the event , never null. + */ + public Type getType() { + return type; + } + + /** + * Get the modified feature ids related to this event. + * This object may be null if the ids could not be retrieved. + * @return ResourceId or null + */ - public Filter<Feature> getIds() { ++ public Filter<AbstractFeature> getIds() { + return ids; + } + + @Override + public FeatureSetContentEvent copy(final Resource source){ + return new FeatureSetContentEvent((FeatureSet)source, type, ids); + } + - public static FeatureSetContentEvent createAddEvent(final FeatureSet source, final Filter<Feature> ids){ ++ public static FeatureSetContentEvent createAddEvent(final FeatureSet source, final Filter<AbstractFeature> ids){ + return new FeatureSetContentEvent(source, Type.ADD, ids); + } + - public static FeatureSetContentEvent createUpdateEvent(final FeatureSet source, final Filter<Feature> ids){ ++ public static FeatureSetContentEvent createUpdateEvent(final FeatureSet source, final Filter<AbstractFeature> ids){ + return new FeatureSetContentEvent(source, Type.UPDATE, ids); + } + - public static FeatureSetContentEvent createDeleteEvent(final FeatureSet source, final Filter<Feature> ids){ ++ public static FeatureSetContentEvent createDeleteEvent(final FeatureSet source, final Filter<AbstractFeature> ids){ + return new FeatureSetContentEvent(source, Type.DELETE, ids); + } + + } diff --cc incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/internal/shared/FeatureSetModelEvent.java index 0000000000,06868ebcca..19ee226d64 mode 000000,100644..100644 --- a/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/internal/shared/FeatureSetModelEvent.java +++ b/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/internal/shared/FeatureSetModelEvent.java @@@ -1,0 -1,98 +1,100 @@@ + /* + * 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.storage.internal.shared; + + import org.apache.sis.storage.FeatureSet; + import org.apache.sis.storage.Resource; + import static org.apache.sis.util.ArgumentChecks.*; -import org.opengis.feature.FeatureType; ++ ++// Specific to the main branch: ++import org.apache.sis.feature.DefaultFeatureType; + + /** + * FeatureSet management event. + * + * @todo work in progress + * @author Johann Sorel (Geomatys) + */ + public class FeatureSetModelEvent extends ModelEvent { + + public static enum Type{ + ADD, + UPDATE, + DELETE + }; + + private final Type type; - private final FeatureType oldType; - private final FeatureType newType; ++ private final DefaultFeatureType oldType; ++ private final DefaultFeatureType newType; + - private FeatureSetModelEvent(final FeatureSet source, final Type type, final FeatureType oldtype, final FeatureType newtype){ ++ private FeatureSetModelEvent(final FeatureSet source, final Type type, final DefaultFeatureType oldtype, final DefaultFeatureType newtype){ + super(source); + + ensureNonNull("type", type); + if(oldtype == null && newtype == null){ + throw new NullPointerException("Old and new feature type can not be both null."); + } + this.type = type; + this.oldType = oldtype; + this.newType = newtype; + } + + /** + * get the event type, can be Add, Update or Delete. + * @return Type of the event , never null. + */ + public Type getType() { + return type; + } + + /** + * Retrieve the newly created feature type or + * the updated feature type. + * @return FeatureType or null if event is a Delete + */ - public FeatureType getNewFeatureType() { ++ public DefaultFeatureType getNewFeatureType() { + return newType; + } + + /** + * Retrieve the deleted feature type or + * the old updated feature type. + * + * @return FeatureType or null if event is an Add + */ - public FeatureType getOldFeatureType() { ++ public DefaultFeatureType getOldFeatureType() { + return oldType; + } + + @Override + public FeatureSetModelEvent copy(Resource source) { + return new FeatureSetModelEvent((FeatureSet)source, type, oldType, newType); + } + - public static FeatureSetModelEvent createAddEvent(final FeatureSet source, final FeatureType type){ ++ public static FeatureSetModelEvent createAddEvent(final FeatureSet source, final DefaultFeatureType type){ + return new FeatureSetModelEvent(source, Type.ADD, null, type); + } + - public static FeatureSetModelEvent createUpdateEvent(final FeatureSet source, final FeatureType oldType, final FeatureType newType){ ++ public static FeatureSetModelEvent createUpdateEvent(final FeatureSet source, final DefaultFeatureType oldType, final DefaultFeatureType newType){ + return new FeatureSetModelEvent(source, Type.UPDATE, oldType, newType); + } + - public static FeatureSetModelEvent createDeleteEvent(final FeatureSet source, final FeatureType type){ ++ public static FeatureSetModelEvent createDeleteEvent(final FeatureSet source, final DefaultFeatureType type){ + return new FeatureSetModelEvent(source, Type.DELETE, type, null); + } + + } diff --cc incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/ShapefileStore.java index 0b3e89b78b,409e681984..920d5f1359 --- a/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/ShapefileStore.java +++ b/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/ShapefileStore.java @@@ -421,6 -448,54 +444,54 @@@ public final class ShapefileStore exten return type; } + /** + * Reload the shp and dbf headers. + * Must be called after the files have been rewritten, the number of records + * and the bounding box may have changed. + */ + private void reloadHeaders() throws DataStoreException { + if (readShp) { + try (final ShapeReader reader = new ShapeReader(ShpFiles.openReadChannel(files.shpFile), null)) { + shpHeader = new ShapeHeader(reader.getHeader()); + } catch (IOException ex) { + throw new DataStoreException("Failed to parse shape file header.", ex); + } + } + final Path dbfFile = files.getDbf(false); + if (dbfFile != null) { + try (DBFReader reader = new DBFReader(ShpFiles.openReadChannel(dbfFile), charset, timezone, null)) { + dbfHeader = new DBFHeader(reader.getHeader()); + } catch (IOException ex) { + throw new DataStoreException("Failed to parse dbf file header.", ex); + } + } + } + + /** + * Build a feature from a shp and dbf record. + * + * @param recordNumber record number, starts at one, used as feature identifier + * @param geometry record geometry, may be null + * @param dbfRecord dbf field values, restricted to the read properties + */ - private Feature toFeature(FeatureType type, int recordNumber, Geometry geometry, Object[] dbfRecord, ++ private AbstractFeature toFeature(DefaultFeatureType type, int recordNumber, Geometry geometry, Object[] dbfRecord, + DBFHeader header, int geomSrid, boolean generateId, String baseId) + { - final Feature next = type.newInstance(); ++ final AbstractFeature next = type.newInstance(); + if (readShp) { + if (geometry != null) { + geometry.setUserData(crs); + geometry.setSRID(geomSrid); + } + next.setPropertyValue(GEOMETRY_NAME, geometry); + } + for (int i = 0; i < dbfPropertiesIndex.length; i++) { + next.setPropertyValue(header.fields[dbfPropertiesIndex[i]].fieldName, dbfRecord[i]); + } + if (generateId) next.setPropertyValue(AttributeConvention.IDENTIFIER, baseId + recordNumber); + return next; + } + @Override public Optional<Envelope> getEnvelope() throws DataStoreException { getType();//force loading headers @@@ -825,18 -887,112 +885,112 @@@ } @Override - public void add(Iterator<? extends Feature> features) throws DataStoreException { + public void add(Iterator<? extends AbstractFeature> features) throws DataStoreException { + rewrite(null, null, features, false); + } + + @Override - public void removeIf(Predicate<? super Feature> filter) throws DataStoreException { ++ public void removeIf(Predicate<? super AbstractFeature> filter) throws DataStoreException { + rewrite(filter, null, null, false); + } + + @Override - public void replaceIf(Predicate<? super Feature> filter, UnaryOperator<Feature> updater) throws DataStoreException { ++ public void replaceIf(Predicate<? super AbstractFeature> filter, UnaryOperator<AbstractFeature> updater) throws DataStoreException { + rewrite(filter, updater, null, false); + } + + /** + * Rewrite the files without the records marked as deleted. + * + * @see ShapefileStore#compact() + */ + private void compact() throws DataStoreException { + rewrite(null, null, null, true); + } + + /** + * Rewrite all files applying given modifications. + * + * Existing records are copied one by one, preserving their record number, + * therefore preserving the feature identifiers. Removed features are not dropped + * but replaced by a record marked as deleted, holding a null shape and blank fields. + * New features are appended after the last record number. + * + * <p>A {@link FeatureSetContentEvent} is sent for each kind of modification which actually + * happened, once the new files have replaced the old ones. Nothing is sent when the call + * modified nothing, which is the case of an empty iterator of new features or of a + * predicate matching no feature.</p> + * + * @param remove predicate selecting the features to remove or to update, can be null + * @param updater operator applied to the features matching the predicate, can be null + * to remove them. A null result also removes the feature. + * @param newFeatures features to append at the end of the files, can be null + * @param dropDeleted true to drop the records marked as deleted and renumber the + * remaining records from one, changing the feature identifiers. + * @throws DataStoreException if an error occurred while writing the files + */ - private void rewrite(Predicate<? super Feature> remove, UnaryOperator<Feature> updater, - Iterator<? extends Feature> newFeatures, boolean dropDeleted) throws DataStoreException ++ private void rewrite(Predicate<? super AbstractFeature> remove, UnaryOperator<AbstractFeature> updater, ++ Iterator<? extends AbstractFeature> newFeatures, boolean dropDeleted) throws DataStoreException + { if (!isDefaultView()) throw new DataStoreException("Resource not writable in current filter state"); if (!Files.exists(locationAsPath)) { throw new DataStoreException("FeatureType do not exist, use updateType before modifying features."); } + //force loading the headers, the charset and the properties index - final FeatureType type = getType(); ++ final DefaultFeatureType type = getType(); + final boolean generateId = mustGenerateId(); + final String baseId = type.getName().tip().toString() + "."; + int srid = 0; + final Identifier id = IdentifiedObjects.getIdentifier(crs, Citations.EPSG); + if (id != null) try { + srid = Integer.parseInt(id.getCode()); + } catch (NumberFormatException e) { + // Ignore. Note: this is also the exception if id.getCode() is null. + } + final int geomSrid = srid; + /* + * Identifiers of the features modified by this call, collected while the new files are + * written and sent to the listeners once the write succeeded. They are recorded even + * when `generateId` is false, in which case they cannot designate anything and are + * dropped when the events are built, but their number still tells what happened. + */ + final var added = new ArrayList<String>(); + final var updated = new ArrayList<String>(); + final var removed = new ArrayList<String>(); + int dropped = 0; + final Writer writer = new Writer(charset); try { - //write existing features - try (Stream<AbstractFeature> stream = features(false)) { - Iterator<AbstractFeature> iterator = stream.iterator(); - while (iterator.hasNext()) { - writer.write(iterator.next()); + //copy existing records + try (ShapeReader shpreader = new ShapeReader(ShpFiles.openReadChannel(files.shpFile), null); + DBFReader dbfreader = new DBFReader(ShpFiles.openReadChannel(files.getDbf(false)), charset, timezone, dbfPropertiesIndex)) + { + final DBFHeader header = dbfreader.getHeader(); + for (ShapeRecord shpRecord = shpreader.next(); shpRecord != null; shpRecord = shpreader.next()) { + final long offset = (long)header.headerSize + ((long)(shpRecord.recordNumber-1)) * ((long)header.recordSize); + dbfreader.moveToOffset(offset); + final Object[] dbfRecord = dbfreader.next(); + if (dbfRecord == null) break; + - Feature feature = null; ++ AbstractFeature feature = null; + if (dbfRecord != DBFReader.DELETED_RECORD) { + feature = toFeature(type, shpRecord.recordNumber, shpRecord.geometry, + dbfRecord, header, geomSrid, generateId, baseId); + if (remove != null && remove.test(feature)) { + feature = (updater == null) ? null : updater.apply(feature); + (feature == null ? removed : updated).add(baseId + shpRecord.recordNumber); + } + } + + if (feature == null) { + //record is deleted, keep its slot to preserve the following record numbers + if (dropDeleted) dropped++; + else writer.writeDeleted(shpRecord.recordNumber); + } else if (dropDeleted) { + writer.write(feature); + } else { + writer.write(feature, shpRecord.recordNumber); + } } } @@@ -882,34 -1031,30 +1029,30 @@@ } } - @Override - public void replaceIf(Predicate<? super AbstractFeature> filter, UnaryOperator<AbstractFeature> updater) throws DataStoreException { - if (!isDefaultView()) throw new DataStoreException("Resource not writable in current filter state"); - if (!Files.exists(locationAsPath)) { - throw new DataStoreException("FeatureType do not exist, use updateType before modifying features."); + /** + * Sends a content event for the given feature identifiers, if there is any. + * + * @param type whether the features were added, updated or deleted. + * @param identifiers identifiers of the modified features, empty if none were. + * @param usable whether the feature type has an identifier property. + */ + private void fireContentEvent(final FeatureSetContentEvent.Type type, + final List<String> identifiers, final boolean usable) + { + if (identifiers.isEmpty()) { + return; } - final Writer writer = new Writer(charset); - try { - //write existing features applying modifications - try (Stream<AbstractFeature> stream = features(false)) { - Iterator<AbstractFeature> iterator = stream.iterator(); - while (iterator.hasNext()) { - AbstractFeature feature = iterator.next(); - if (filter.test(feature)) { - feature = updater.apply(feature); - } - if (feature != null) writer.write(feature); - } - } - writer.finish(true); - } catch (IOException ex) { - try { - writer.finish(false); - } catch (IOException e) { - ex.addSuppressed(e); - Filter<Feature> ids = null; ++ Filter<AbstractFeature> ids = null; + if (usable) { - final FilterFactory<Feature,Object,Object> ff = DefaultFilterFactory.forFeatures(); - final var rid = new LinkedHashSet<ResourceId>(); ++ final DefaultFilterFactory<AbstractFeature,Object,Object> ff = DefaultFilterFactory.forFeatures(); ++ final var rid = new LinkedHashSet<Filter>(); + for (final String identifier : identifiers) { + rid.add(ff.resourceId(identifier)); } - throw new DataStoreException("Writing failed", ex); + ids = FeatureSetContentEvent.resourceId(rid); } + ShapefileStore.this.listeners.fire(FeatureSetContentEvent.class, + new FeatureSetContentEvent(ShapefileStore.this, type, ids)); } @Override @@@ -1227,8 -1375,23 +1373,23 @@@ } - private void write(AbstractFeature feature) throws IOException { - inc++; //number starts at 1 + /** + * Write a feature, appended after the last written record. + * + * @return the record number given to the feature, which determines its identifier. + */ - private int write(Feature feature) throws IOException { ++ private int write(AbstractFeature feature) throws IOException { + final int recordNumber = lastRecordNumber + 1; + write(feature, recordNumber); + return recordNumber; + } + + /** + * Write a feature with the given record number. + * + * @param recordNumber record number, starts at one + */ - private void write(Feature feature, int recordNumber) throws IOException { ++ private void write(AbstractFeature feature, int recordNumber) throws IOException { final ShapeRecord shpRecord = new ShapeRecord(); final long recordStartPosition = shpWriter.getSteamPosition(); diff --cc incubator/src/org.apache.sis.storage.shapefile/test/org/apache/sis/storage/shapefile/ShapefileStoreEventsTest.java index 0000000000,4c0010f82c..3786b33727 mode 000000,100644..100644 --- a/incubator/src/org.apache.sis.storage.shapefile/test/org/apache/sis/storage/shapefile/ShapefileStoreEventsTest.java +++ b/incubator/src/org.apache.sis.storage.shapefile/test/org/apache/sis/storage/shapefile/ShapefileStoreEventsTest.java @@@ -1,0 -1,373 +1,372 @@@ + /* + * 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.storage.shapefile; + + import java.nio.file.Path; + import java.util.ArrayList; + import java.util.Collections; + import java.util.LinkedHashSet; + import java.util.List; + import java.util.Set; + import java.util.stream.Stream; + import org.apache.sis.feature.internal.shared.AttributeConvention; + import org.apache.sis.filter.DefaultFilterFactory; + import org.apache.sis.storage.DataStoreException; + import org.apache.sis.storage.FeatureSet; + import org.apache.sis.storage.StorageConnector; + import org.apache.sis.storage.event.StoreEvent; + import org.apache.sis.storage.event.StoreListener; + import org.apache.sis.storage.internal.shared.ContentEvent; + import org.apache.sis.storage.internal.shared.FeatureSetContentEvent; + import org.apache.sis.storage.internal.shared.FeatureSetModelEvent; + import org.apache.sis.storage.internal.shared.ModelEvent; + + // Test dependencies + import static org.junit.jupiter.api.Assertions.*; + import org.junit.jupiter.api.Test; + import org.junit.jupiter.api.io.TempDir; + -// Specific to the geoapi-3.1 and geoapi-4.0 branches: -import org.opengis.feature.Feature; -import org.opengis.feature.FeatureType; -import org.opengis.filter.Filter; -import org.opengis.filter.FilterFactory; -import org.opengis.filter.LogicalOperator; -import org.opengis.filter.ResourceId; ++// Specific to the main branch: ++import org.apache.sis.filter.Filter; ++import org.apache.sis.feature.AbstractFeature; ++import org.apache.sis.feature.DefaultFeatureType; ++import org.apache.sis.pending.geoapi.filter.ResourceId; ++import org.apache.sis.pending.geoapi.filter.LogicalOperator; + + + /** + * Tests the {@link FeatureSetModelEvent} and {@link FeatureSetContentEvent} sent by + * {@link ShapefileStore} when the shapefile is created or its features are modified. + * + * @author Johann Sorel (Geomatys) + */ + public final class ShapefileStoreEventsTest { + - private static final FilterFactory<Feature,Object,Object> FF = DefaultFilterFactory.forFeatures(); ++ private static final DefaultFilterFactory<AbstractFeature,Object,Object> FF = DefaultFilterFactory.forFeatures(); + + public ShapefileStoreEventsTest() { + } + + /** + * Collects the events sent to a listener, so that a test can assert what was sent and in + * which order. + */ + private static final class Recorder<E extends StoreEvent> implements StoreListener<E> { + /** + * All events received so far, in the order they were received. + */ + final List<E> events = new ArrayList<>(); + + @Override + public void eventOccurred(final E event) { + events.add(event); + } + + /** + * Returns the only event received, failing if a different number of events was received. + */ + E single() { + assertEquals(1, events.size(), () -> "Expected exactly one event but got " + events); + return events.get(0); + } + + /** + * Asserts that no event at all was received. + */ + void assertEmpty() { + assertTrue(events.isEmpty(), () -> "Expected no events but received: " + events); + } + } + + /** + * Creates a store for a shapefile which does not exist yet. + */ + private static ShapefileStore create(final Path folder) throws DataStoreException { + return new ShapefileStore(null, new StorageConnector(folder.resolve("test.shp"))); + } + + /** + * Creates a store holding the given number of features, from one to three. + * The features are numbered from one, as are the record numbers they are written to, + * so the feature identifiers are {@code "test.1"} to {@code "test.3"}. + */ + private static ShapefileStore createPopulated(final Path folder, final int count) throws DataStoreException { + final ShapefileStore store = create(folder); + store.updateType(ShapefileStoreTest.createType()); - final FeatureType type = store.getType(); - final var features = new ArrayList<Feature>(3); ++ final DefaultFeatureType type = store.getType(); ++ final var features = new ArrayList<AbstractFeature>(3); + if (count >= 1) features.add(ShapefileStoreTest.createFeature1(type)); + if (count >= 2) features.add(ShapefileStoreTest.createFeature2(type)); + if (count >= 3) features.add(ShapefileStoreTest.createFeature3(type)); + store.add(features.iterator()); + return store; + } + + /** + * Returns the identifiers designated by the filter of a content event. - * The filter is a single {@link ResourceId} when only one feature was modified, ++ * The filter is a single {@code ResourceId} when only one feature was modified, + * and the union of the identifiers of all the modified features otherwise. + */ - private static Set<String> identifiers(final Filter<Feature> filter) { ++ private static Set<String> identifiers(final Filter<AbstractFeature> filter) { + assertNotNull(filter, "Event carries no identifier."); + final var ids = new LinkedHashSet<String>(); + collect(filter, ids); + return ids; + } + + /** + * Adds to the given set the identifiers designated by the given filter. + */ + private static void collect(final Filter<?> filter, final Set<String> ids) { + if (filter instanceof ResourceId<?> rid) { + assertTrue(ids.add(rid.getIdentifier()), () -> "Duplicated identifier: " + rid.getIdentifier()); + } else if (filter instanceof LogicalOperator<?> op) { + for (final Filter<?> operand : op.getOperands()) { + collect(operand, ids); + } + } else { + fail("Filter is neither a ResourceId nor a union of them: " + filter); + } + } + + /** + * Asserts that the filter of a content event selects exactly the given features of the store, + * which is what makes the filter usable by the listener. Applicable only to the features which + * still exist after the modification, therefore not to a deletion. + */ - private static void assertSelects(final FeatureSet data, final Filter<Feature> filter, ++ private static void assertSelects(final FeatureSet data, final Filter<AbstractFeature> filter, + final String... expected) throws DataStoreException + { + final var selected = new ArrayList<String>(); - try (Stream<Feature> features = data.features(false)) { ++ try (Stream<AbstractFeature> features = data.features(false)) { + features.forEach((feature) -> { + if (filter.test(feature)) { + selected.add(feature.getPropertyValue(AttributeConvention.IDENTIFIER).toString()); + } + }); + } + assertArrayEquals(expected, selected.toArray(String[]::new), "Features selected by the event filter"); + } + + /** + * Tests the model event sent when a shapefile is created by {@code updateType(…)}. + * The type did not exist before, so the event is an addition. + */ + @Test + public void testCreateSendsModelEvent(@TempDir final Path folder) throws DataStoreException { + try (ShapefileStore store = create(folder)) { + final var model = new Recorder<FeatureSetModelEvent>(); + final var content = new Recorder<FeatureSetContentEvent>(); + store.addListener(FeatureSetModelEvent.class, model); + store.addListener(FeatureSetContentEvent.class, content); + + store.updateType(ShapefileStoreTest.createType()); + + final FeatureSetModelEvent event = model.single(); + assertSame(store, event.getSource(), "The store is the resource the users hold."); + assertEquals(FeatureSetModelEvent.Type.ADD, event.getType()); + assertNull(event.getOldFeatureType(), "Nothing existed before the creation."); + assertEquals(store.getType(), event.getNewFeatureType(), + "The event must carry the effective type, as read back from the files."); + content.assertEmpty(); + } + } + + /** + * Tests the content event sent when features are added. + */ + @Test + public void testAddSendsContentEvent(@TempDir final Path folder) throws DataStoreException { + try (ShapefileStore store = create(folder)) { + store.updateType(ShapefileStoreTest.createType()); - final FeatureType type = store.getType(); ++ final DefaultFeatureType type = store.getType(); + final var content = new Recorder<FeatureSetContentEvent>(); + final var model = new Recorder<FeatureSetModelEvent>(); + store.addListener(FeatureSetContentEvent.class, content); + store.addListener(FeatureSetModelEvent.class, model); + + store.add(List.of(ShapefileStoreTest.createFeature1(type), + ShapefileStoreTest.createFeature2(type)).iterator()); + + final FeatureSetContentEvent event = content.single(); + assertSame(store, event.getSource()); + assertEquals(FeatureSetContentEvent.Type.ADD, event.getType()); + assertEquals(Set.of("test.1", "test.2"), identifiers(event.getIds())); + assertSelects(store, event.getIds(), "test.1", "test.2"); + model.assertEmpty(); + } + } + + /** + * Verifies that the identifiers reported for added features are the record numbers actually + * used, which are not a simple count when the file holds records marked as deleted. + */ + @Test + public void testAddAfterRemoveReportsRealIdentifiers(@TempDir final Path folder) throws DataStoreException { + try (ShapefileStore store = createPopulated(folder, 2)) { + store.removeIf(FF.equal(FF.property("id"), FF.literal(1))); - final FeatureType type = store.getType(); ++ final DefaultFeatureType type = store.getType(); + final var content = new Recorder<FeatureSetContentEvent>(); + store.addListener(FeatureSetContentEvent.class, content); + + store.add(List.of(ShapefileStoreTest.createFeature3(type)).iterator()); + + final FeatureSetContentEvent event = content.single(); + assertEquals(FeatureSetContentEvent.Type.ADD, event.getType()); + assertEquals(Set.of("test.3"), identifiers(event.getIds()), + "The deleted record keeps its slot, so the new feature is the third one."); + assertSelects(store, event.getIds(), "test.3"); + } + } + + /** + * Tests the content event sent when features are removed. + */ + @Test + public void testRemoveSendsContentEvent(@TempDir final Path folder) throws DataStoreException { + try (ShapefileStore store = createPopulated(folder, 3)) { + final var content = new Recorder<FeatureSetContentEvent>(); + store.addListener(FeatureSetContentEvent.class, content); + + store.removeIf(FF.equal(FF.property("id"), FF.literal(2))); + + final FeatureSetContentEvent event = content.single(); + assertSame(store, event.getSource()); + assertEquals(FeatureSetContentEvent.Type.DELETE, event.getType()); + assertEquals(Set.of("test.2"), identifiers(event.getIds())); + } + } + + /** + * Tests the content event sent when features are updated. + */ + @Test + public void testReplaceSendsContentEvent(@TempDir final Path folder) throws DataStoreException { + try (ShapefileStore store = createPopulated(folder, 3)) { + final var content = new Recorder<FeatureSetContentEvent>(); + store.addListener(FeatureSetContentEvent.class, content); + + store.replaceIf(FF.equal(FF.property("id"), FF.literal(1)), (feature) -> { + feature.setPropertyValue("text", "modified"); + return feature; + }); + + final FeatureSetContentEvent event = content.single(); + assertEquals(FeatureSetContentEvent.Type.UPDATE, event.getType()); + assertEquals(Set.of("test.1"), identifiers(event.getIds())); + assertSelects(store, event.getIds(), "test.1"); + } + } + + /** + * Verifies that a replacement whose operator returns {@code null} is reported as a deletion, + * which is what it does to the file. + */ + @Test + public void testReplaceByNullSendsDeleteEvent(@TempDir final Path folder) throws DataStoreException { + try (ShapefileStore store = createPopulated(folder, 3)) { + final var content = new Recorder<FeatureSetContentEvent>(); + store.addListener(FeatureSetContentEvent.class, content); + + store.replaceIf(FF.equal(FF.property("id"), FF.literal(3)), (feature) -> null); + + final FeatureSetContentEvent event = content.single(); + assertEquals(FeatureSetContentEvent.Type.DELETE, event.getType()); + assertEquals(Set.of("test.3"), identifiers(event.getIds())); + } + } + + /** + * Tests the content event sent by a compaction, which renumbers every surviving record and + * therefore changes every identifier. No filter can express that mapping, so none is sent. + */ + @Test + public void testCompactSendsContentEvent(@TempDir final Path folder) throws DataStoreException { + try (ShapefileStore store = createPopulated(folder, 3)) { + store.removeIf(FF.equal(FF.property("id"), FF.literal(2))); + final var content = new Recorder<FeatureSetContentEvent>(); + store.addListener(FeatureSetContentEvent.class, content); + + store.compact(); + + final FeatureSetContentEvent event = content.single(); + assertSame(store, event.getSource()); + assertEquals(FeatureSetContentEvent.Type.UPDATE, event.getType()); + assertNull(event.getIds(), "Every identifier changed, no filter can designate them."); + } + } + + /** + * Verifies that the operations which change nothing send nothing. + */ + @Test + public void testNoChangeSendsNoEvent(@TempDir final Path folder) throws DataStoreException { + try (ShapefileStore store = createPopulated(folder, 2)) { + final var content = new Recorder<FeatureSetContentEvent>(); + store.addListener(FeatureSetContentEvent.class, content); + + store.add(Collections.emptyIterator()); + content.assertEmpty(); + + store.removeIf(FF.equal(FF.property("id"), FF.literal(999))); + content.assertEmpty(); + + store.replaceIf(FF.equal(FF.property("id"), FF.literal(999)), (feature) -> feature); + content.assertEmpty(); + + store.compact(); + content.assertEmpty(); + } + } + + /** + * Verifies that a listener registered for a parent event type receives the events, + * which is how a listener interested in any change of the resource registers itself. + */ + @Test + public void testListenerOnParentEventType(@TempDir final Path folder) throws DataStoreException { + try (ShapefileStore store = create(folder)) { + final var model = new Recorder<ModelEvent>(); + final var content = new Recorder<ContentEvent>(); + store.addListener(ModelEvent.class, model); + store.addListener(ContentEvent.class, content); + + store.updateType(ShapefileStoreTest.createType()); - final FeatureType type = store.getType(); ++ final DefaultFeatureType type = store.getType(); + store.add(List.of(ShapefileStoreTest.createFeature1(type)).iterator()); + + assertInstanceOf(FeatureSetModelEvent.class, model.single()); + assertInstanceOf(FeatureSetContentEvent.class, content.single()); + } + } + + /** + * Verifies that a removed listener stops receiving the events. + */ + @Test + public void testRemovedListener(@TempDir final Path folder) throws DataStoreException { + try (ShapefileStore store = createPopulated(folder, 2)) { + final var content = new Recorder<FeatureSetContentEvent>(); + store.addListener(FeatureSetContentEvent.class, content); + store.removeListener(FeatureSetContentEvent.class, content); + + store.removeIf(FF.equal(FF.property("id"), FF.literal(1))); + content.assertEmpty(); + } + } + + } diff --cc incubator/src/org.apache.sis.storage.shapefile/test/org/apache/sis/storage/shapefile/ShapefileStoreTest.java index c16307a6c5,97d4d33dc7..00b55833b0 --- a/incubator/src/org.apache.sis.storage.shapefile/test/org/apache/sis/storage/shapefile/ShapefileStoreTest.java +++ b/incubator/src/org.apache.sis.storage.shapefile/test/org/apache/sis/storage/shapefile/ShapefileStoreTest.java @@@ -46,11 -47,13 +47,12 @@@ import static org.junit.jupiter.api.Ass import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -// Specific to the geoapi-3.1 and geoapi-4.0 branches: -import org.opengis.feature.AttributeType; -import org.opengis.feature.Feature; -import org.opengis.feature.FeatureType; -import org.opengis.feature.PropertyType; -import org.opengis.filter.BinaryComparisonOperator; -import org.opengis.filter.FilterFactory; +// Specific to the main branch: +import org.apache.sis.feature.AbstractFeature; +import org.apache.sis.feature.DefaultFeatureType; +import org.apache.sis.feature.DefaultAttributeType; ++import org.apache.sis.feature.AbstractIdentifiedType; +import org.apache.sis.filter.Filter; /** @@@ -302,6 -304,116 +303,116 @@@ public class ShapefileStoreTest } } + /** + * Test that removing a feature does not change the identifiers of the remaining features. + * The deleted record is flagged in the dbf file and keeps its slot in the files. + */ + @Test + public void testRemovePreserveIdentifiers(@TempDir final Path folder) throws DataStoreException, IOException { + final Path temp = folder.resolve("test.shp"); + try (final ShapefileStore store = create(temp)) { + store.updateType(createType()); - final FeatureType type = store.getType(); ++ final DefaultFeatureType type = store.getType(); + store.add(List.of(createFeature1(type), createFeature2(type), createFeature3(type)).iterator()); + final long dbfSize = Files.size(folder.resolve("test.dbf")); + + //remove the feature in the middle - final FilterFactory<Feature, Object, Object> ff = DefaultFilterFactory.forFeatures(); ++ final DefaultFilterFactory<AbstractFeature, Object, Object> ff = DefaultFilterFactory.forFeatures(); + store.removeIf(ff.equal(ff.property("id"), ff.literal(2))); + + assertIdentifiers(store, "test.1", "test.3"); + //the deleted record still occupies its slot in the dbf file + assertEquals(dbfSize, Files.size(folder.resolve("test.dbf"))); + } + + //reopen the files to verify the deleted flag has been written and not only kept in memory + try (final ShapefileStore store = create(temp)) { + assertIdentifiers(store, "test.1", "test.3"); + } + } + + /** + * Test that a feature added after a removal is appended and does not reuse + * the record number of the deleted feature. + */ + @Test + public void testAddAfterRemove(@TempDir final Path folder) throws DataStoreException, IOException { + final Path temp = folder.resolve("test.shp"); + try (final ShapefileStore store = create(temp)) { + store.updateType(createType()); - FeatureType type = store.getType(); ++ DefaultFeatureType type = store.getType(); + store.add(List.of(createFeature1(type), createFeature2(type), createFeature3(type)).iterator()); + - final FilterFactory<Feature, Object, Object> ff = DefaultFilterFactory.forFeatures(); ++ final DefaultFilterFactory<AbstractFeature, Object, Object> ff = DefaultFilterFactory.forFeatures(); + store.removeIf(ff.equal(ff.property("id"), ff.literal(2))); + + //the new feature must not take the place of the removed one - final Feature feature4 = createFeature1(type); ++ final AbstractFeature feature4 = createFeature1(type); + feature4.setPropertyValue("id", 4); + store.add(List.of(feature4).iterator()); + + assertIdentifiers(store, "test.1", "test.3", "test.4"); + } + } + + /** + * Test that replacing features preserves the identifiers and the deleted records. + */ + @Test + public void testReplaceAfterRemove(@TempDir final Path folder) throws DataStoreException, IOException { + final Path temp = folder.resolve("test.shp"); + try (final ShapefileStore store = create(temp)) { + store.updateType(createType()); - final FeatureType type = store.getType(); ++ final DefaultFeatureType type = store.getType(); + store.add(List.of(createFeature1(type), createFeature2(type), createFeature3(type)).iterator()); + - final FilterFactory<Feature, Object, Object> ff = DefaultFilterFactory.forFeatures(); ++ final DefaultFilterFactory<AbstractFeature, Object, Object> ff = DefaultFilterFactory.forFeatures(); + store.removeIf(ff.equal(ff.property("id"), ff.literal(2))); - store.replaceIf(ff.equal(ff.property("id"), ff.literal(3)), (Feature feature) -> { ++ store.replaceIf(ff.equal(ff.property("id"), ff.literal(3)), (AbstractFeature feature) -> { + feature.setPropertyValue("text", "modified"); + return feature; + }); + + final Object[] result = store.features(false).toArray(); + assertEquals(2, result.length); - assertEquals("test.1", ((Feature) result[0]).getPropertyValue(AttributeConvention.IDENTIFIER)); - assertEquals("test.3", ((Feature) result[1]).getPropertyValue(AttributeConvention.IDENTIFIER)); - assertEquals("modified", ((Feature) result[1]).getPropertyValue("text")); ++ assertEquals("test.1", ((AbstractFeature) result[0]).getPropertyValue(AttributeConvention.IDENTIFIER)); ++ assertEquals("test.3", ((AbstractFeature) result[1]).getPropertyValue(AttributeConvention.IDENTIFIER)); ++ assertEquals("modified", ((AbstractFeature) result[1]).getPropertyValue("text")); + } + } + + /** + * Test compacting a shapefile, deleted records must be dropped + * and the remaining records renumbered. + */ + @Test + public void testCompact(@TempDir final Path folder) throws DataStoreException, IOException { + final Path temp = folder.resolve("test.shp"); + try (final ShapefileStore store = create(temp)) { + store.updateType(createType()); - final FeatureType type = store.getType(); ++ final DefaultFeatureType type = store.getType(); + store.add(List.of(createFeature1(type), createFeature2(type), createFeature3(type)).iterator()); + - final FilterFactory<Feature, Object, Object> ff = DefaultFilterFactory.forFeatures(); ++ final DefaultFilterFactory<AbstractFeature, Object, Object> ff = DefaultFilterFactory.forFeatures(); + store.removeIf(ff.equal(ff.property("id"), ff.literal(2))); + + final long shpSize = Files.size(temp); + final long dbfSize = Files.size(folder.resolve("test.dbf")); + + store.compact(); + + //deleted records are gone, remaining ones are renumbered + assertIdentifiers(store, "test.1", "test.2"); + assertTrue(Files.size(temp) < shpSize, "shp file should be smaller after compaction"); + assertTrue(Files.size(folder.resolve("test.dbf")) < dbfSize, "dbf file should be smaller after compaction"); + + //values must be preserved, only the identifiers change + final Object[] result = store.features(false).toArray(); - assertEquals(1, ((Feature) result[0]).getPropertyValue("id")); - assertEquals(3, ((Feature) result[1]).getPropertyValue("id")); ++ assertEquals(1, ((AbstractFeature) result[0]).getPropertyValue("id")); ++ assertEquals(3, ((AbstractFeature) result[1]).getPropertyValue("id")); + } + } + /** * Test replacing features in a shapefile. */ @@@ -343,15 -455,15 +454,15 @@@ final URL url = ShapefileStoreTest.class.getResource("/org/apache/sis/storage/shapefile/noid.shp"); try (final ShapefileStore store = create(url)) { - final FeatureType type = store.getType(); - final PropertyType generatedID = type.getProperty(AttributeConvention.IDENTIFIER); - assertTrue(generatedID instanceof AttributeType); + final DefaultFeatureType type = store.getType(); - final var generatedID = type.getProperty(AttributeConvention.IDENTIFIER); ++ final AbstractIdentifiedType generatedID = type.getProperty(AttributeConvention.IDENTIFIER); + assertTrue(generatedID instanceof DefaultAttributeType); assertEquals(5, type.getProperties(true).size()); - try (Stream<Feature> stream = store.features(false)) { - Iterator<Feature> iterator = stream.iterator(); + try (Stream<AbstractFeature> stream = store.features(false)) { + Iterator<AbstractFeature> iterator = stream.iterator(); assertTrue(iterator.hasNext()); - Feature feature1 = iterator.next(); + AbstractFeature feature1 = iterator.next(); assertEquals("noid.1", feature1.getPropertyValue(AttributeConvention.IDENTIFIER)); assertEquals("some text", feature1.getPropertyValue("text")); @@@ -361,7 -473,21 +472,21 @@@ } - private static DefaultFeatureType createType() { + /** + * Verify the identifiers of all features in the given store, in order. + */ + private static void assertIdentifiers(final ShapefileStore store, final String... expected) throws DataStoreException { - try (Stream<Feature> stream = store.features(false)) { - final Iterator<Feature> ite = stream.iterator(); ++ try (Stream<AbstractFeature> stream = store.features(false)) { ++ final Iterator<AbstractFeature> ite = stream.iterator(); + for (final String id : expected) { + assertTrue(ite.hasNext(), "missing feature " + id); + assertEquals(id, ite.next().getPropertyValue(AttributeConvention.IDENTIFIER)); + } + assertFalse(ite.hasNext(), "unexpected additional feature"); + } + } + - static FeatureType createType() { ++ static DefaultFeatureType createType() { final FeatureTypeBuilder ftb = new FeatureTypeBuilder(); ftb.setName("test"); ftb.addAttribute(Integer.class).setName("id"); @@@ -373,8 -499,8 +498,8 @@@ return ftb.build(); } - private static AbstractFeature createFeature1(DefaultFeatureType type) { - static Feature createFeature1(FeatureType type) { - Feature feature = type.newInstance(); ++ static AbstractFeature createFeature1(DefaultFeatureType type) { + AbstractFeature feature = type.newInstance(); feature.setPropertyValue("geometry", GF.createPoint(new Coordinate(10,20))); feature.setPropertyValue(AttributeConvention.IDENTIFIER, "test.1"); feature.setPropertyValue("id", 1); @@@ -385,8 -511,8 +510,8 @@@ return feature; } - private static AbstractFeature createFeature2(DefaultFeatureType type) { - static Feature createFeature2(FeatureType type) { - Feature feature = type.newInstance(); ++ static AbstractFeature createFeature2(DefaultFeatureType type) { + AbstractFeature feature = type.newInstance(); feature.setPropertyValue("geometry", GF.createPoint(new Coordinate(30,40))); feature.setPropertyValue(AttributeConvention.IDENTIFIER, "test.2");; feature.setPropertyValue("id", 2); @@@ -396,4 -522,16 +521,16 @@@ feature.setPropertyValue("date", LocalDate.of(2030, 6, 21)); return feature; } + - static Feature createFeature3(FeatureType type) { - Feature feature = type.newInstance(); ++ static AbstractFeature createFeature3(DefaultFeatureType type) { ++ AbstractFeature feature = type.newInstance(); + feature.setPropertyValue("geometry", GF.createPoint(new Coordinate(50,60))); + feature.setPropertyValue(AttributeConvention.IDENTIFIER, "test.3"); + feature.setPropertyValue("id", 3); + feature.setPropertyValue("text", "some text 3"); + feature.setPropertyValue("integer", 789); + feature.setPropertyValue("float", 789.123); + feature.setPropertyValue("date", LocalDate.of(2035, 7, 30)); + return feature; + } }
