This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit 855247d94dccb0570024d7658cb9c030d7e1b340 Author: jsorel <[email protected]> AuthorDate: Wed Sep 9 11:49:54 2026 +0200 feat(SHP): review how ids are used, preserve them after a delete operation. Add Store.compact method to clean deleted records --- .../sis/storage/shapefile/ShapefileStore.java | 361 ++++++++++++++------- .../apache/sis/storage/shapefile/package-info.java | 9 + .../sis/storage/shapefile/ShapefileStoreTest.java | 140 +++++++- 3 files changed, 384 insertions(+), 126 deletions(-) diff --git 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 index 90f5f1096a..9d9f525d78 100644 --- 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 @@ -264,6 +264,26 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur return featureSetView.getFileSet(); } + /** + * Rewrite the files, removing all records marked as deleted. + * + * Removing features does not shrink the files, deleted records are only + * flagged as such in the dbf file to preserve the record numbers, and therefore + * the feature identifiers, of the remaining features. + * This method physically drops those records to reduce the files size. + * + * <p> + * <b>Warning :</b> the remaining records are renumbered from one, consequently + * their {@code sis:identifier} values change. Any identifier obtained before this + * call must be considered obsolete. + * </p> + * + * @throws DataStoreException if an error occurred while rewriting the files. + */ + public void compact() throws DataStoreException { + featureSetView.compact(); + } + private final class AsFeatureSet extends AbstractFeatureSet implements WritableFeatureSet { private final Rectangle2D.Double filter; @@ -423,6 +443,54 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur 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, + DBFHeader header, int geomSrid, boolean generateId, String baseId) + { + final Feature 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 @@ -435,6 +503,13 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur return super.getEnvelope(); } + /** + * {@inheritDoc } + * + * The returned value is the number of records stored in the dbf file, + * it includes the records marked as deleted. Call {@link ShapefileStore#compact()} + * to remove them and obtain an exact count. + */ @Override public OptionalLong getFeatureCount() { try { @@ -455,7 +530,9 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur final DBFReader dbfreader; try { shpreader = readShp ? new ShapeReader(ShpFiles.openReadChannel(files.shpFile), filter) : null; - dbfreader = (dbfPropertiesIndex.length > 0) ? new DBFReader(ShpFiles.openReadChannel(files.getDbf(false)), charset, timezone, dbfPropertiesIndex) : null; + //the dbf file is always read, even if no property is requested : + //the record state tag is the only place where deleted records are flagged. + dbfreader = new DBFReader(ShpFiles.openReadChannel(files.getDbf(false)), charset, timezone, dbfPropertiesIndex); } catch (IOException ex) { throw new DataStoreException("Faild to open shp and dbf files.", ex); } @@ -471,57 +548,29 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur final boolean generateId = mustGenerateId(); final String baseId = type.getName().tip().toString() +"."; + final DBFHeader header = dbfreader.getHeader(); final Spliterator spliterator; - if (readShp && dbfPropertiesIndex.length > 0) { - //read both shp and dbf - final DBFHeader header = dbfreader.getHeader(); - + if (readShp) { + //read the shp and the matching dbf record spliterator = new Spliterators.AbstractSpliterator(Long.MAX_VALUE, Spliterator.ORDERED) { @Override public boolean tryAdvance(Consumer action) { try { - final ShapeRecord shpRecord = shpreader.next(); - if (shpRecord == null) return false; - //move dbf to record offset, some shp record might have been skipped because of filter - long offset = (long)header.headerSize + ((long)(shpRecord.recordNumber-1)) * ((long)header.recordSize); - dbfreader.moveToOffset(offset); - final Object[] dbfRecord = dbfreader.next(); - final Feature next = type.newInstance(); - if (shpRecord.geometry != null) { - shpRecord.geometry.setUserData(crs); - shpRecord.geometry.setSRID(geomSrid); - } - next.setPropertyValue(GEOMETRY_NAME, shpRecord.geometry); - for (int i = 0; i < dbfPropertiesIndex.length; i++) { - next.setPropertyValue(header.fields[dbfPropertiesIndex[i]].fieldName, dbfRecord[i]); + for (;;) { + final ShapeRecord shpRecord = shpreader.next(); + if (shpRecord == null) return false; + //move dbf to record offset, some shp record might have been skipped because of filter + long offset = (long)header.headerSize + ((long)(shpRecord.recordNumber-1)) * ((long)header.recordSize); + dbfreader.moveToOffset(offset); + final Object[] dbfRecord = dbfreader.next(); + if (dbfRecord == null) return false; + //skip deleted records, they only exist to preserve the record numbers + if (dbfRecord == DBFReader.DELETED_RECORD) continue; + action.accept(toFeature(type, shpRecord.recordNumber, shpRecord.geometry, + dbfRecord, header, geomSrid, generateId, baseId)); + return true; } - if (generateId) next.setPropertyValue(AttributeConvention.IDENTIFIER, baseId + shpRecord.recordNumber); - - action.accept(next); - return true; - } catch (IOException ex) { - throw new BackingStoreException(ex.getMessage(), ex); - } - } - }; - } else if (readShp) { - //read only the shp - spliterator = new Spliterators.AbstractSpliterator(Long.MAX_VALUE, Spliterator.ORDERED) { - @Override - public boolean tryAdvance(Consumer action) { - try { - final ShapeRecord shpRecord = shpreader.next(); - if (shpRecord == null) return false; - final Feature next = type.newInstance(); - if (shpRecord.geometry != null) { - shpRecord.geometry.setUserData(crs); - shpRecord.geometry.setSRID(geomSrid); - } - next.setPropertyValue(GEOMETRY_NAME, shpRecord.geometry); - if (generateId) next.setPropertyValue(AttributeConvention.IDENTIFIER, baseId + shpRecord.recordNumber); - action.accept(next); - return true; } catch (IOException ex) { throw new BackingStoreException(ex.getMessage(), ex); } @@ -529,21 +578,21 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur }; } else { //read only dbf - final AtomicInteger nextId = new AtomicInteger(); - final DBFHeader header = dbfreader.getHeader(); + final AtomicInteger recordNumber = new AtomicInteger(); spliterator = new Spliterators.AbstractSpliterator(Long.MAX_VALUE, Spliterator.ORDERED) { @Override public boolean tryAdvance(Consumer action) { try { - final Object[] dbfRecord = dbfreader.next(); - if (dbfRecord == null) return false; - final Feature next = type.newInstance(); - for (int i = 0; i < dbfPropertiesIndex.length; i++) { - next.setPropertyValue(header.fields[dbfPropertiesIndex[i]].fieldName, dbfRecord[i]); + for (;;) { + //deleted records are counted, identifiers must match those of the shp records + final int number = recordNumber.incrementAndGet(); + final Object[] dbfRecord = dbfreader.next(); + if (dbfRecord == null) return false; + if (dbfRecord == DBFReader.DELETED_RECORD) continue; + action.accept(toFeature(type, number, null, + dbfRecord, header, geomSrid, generateId, baseId)); + return true; } - if (generateId) next.setPropertyValue(AttributeConvention.IDENTIFIER, baseId + nextId.incrementAndGet()); - action.accept(next); - return true; } catch (IOException ex) { throw new BackingStoreException(ex.getMessage(), ex); } @@ -826,81 +875,102 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur @Override public void add(Iterator<? extends Feature> features) 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."); - } - final Writer writer = new Writer(charset); - try { - //write existing features - try (Stream<Feature> stream = features(false)) { - Iterator<Feature> iterator = stream.iterator(); - while (iterator.hasNext()) { - writer.write(iterator.next()); - } - } - - //write new features - while (features.hasNext()) { - writer.write(features.next()); - } - - writer.finish(true); - } catch (IOException ex) { - try { - writer.finish(false); - } catch (IOException e) { - ex.addSuppressed(e); - } - throw new DataStoreException("Writing failed", ex); - } + rewrite(null, null, features, false); } @Override public void removeIf(Predicate<? super Feature> filter) 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."); - } - final Writer writer = new Writer(charset); - try { - //write existing features not matching filter - try (Stream<Feature> stream = features(false)) { - Iterator<Feature> iterator = stream.filter(filter.negate()).iterator(); - while (iterator.hasNext()) { - writer.write(iterator.next()); - } - } - writer.finish(true); - } catch (IOException ex) { - try { - writer.finish(false); - } catch (IOException e) { - ex.addSuppressed(e); - } - throw new DataStoreException("Writing failed", ex); - } + rewrite(filter, null, null, false); } @Override public void replaceIf(Predicate<? super Feature> filter, UnaryOperator<Feature> 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. + * + * @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 + { 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 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; + final Writer writer = new Writer(charset); try { - //write existing features applying modifications - try (Stream<Feature> stream = features(false)) { - Iterator<Feature> iterator = stream.iterator(); - while (iterator.hasNext()) { - Feature feature = iterator.next(); - if (filter.test(feature)) { - feature = updater.apply(feature); + //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; + 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); + } + } + + if (feature == null) { + //record is deleted, keep its slot to preserve the following record numbers + if (!dropDeleted) writer.writeDeleted(shpRecord.recordNumber); + } else if (dropDeleted) { + writer.write(feature); + } else { + writer.write(feature, shpRecord.recordNumber); } - if (feature != null) writer.write(feature); } } + + //append new features + while (newFeatures != null && newFeatures.hasNext()) { + writer.write(newFeatures.next()); + } + writer.finish(true); } catch (IOException ex) { try { @@ -910,6 +980,9 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur } throw new DataStoreException("Writing failed", ex); } + //files have been replaced, number of records and bounding box have changed + files.scan(); + reloadHeaders(); } @Override @@ -1185,7 +1258,10 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur private final ShapeHeader shpHeader; private final DBFHeader dbfHeader; private String defaultGeomName = null; - private int inc = 0; + /** + * Highest record number written, new records are appended after it. + */ + private int lastRecordNumber = 0; private Writer(Charset charset) throws DataStoreException{ try { @@ -1227,8 +1303,19 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur } + /** + * Write a feature, appended after the last written record. + */ private void write(Feature feature) throws IOException { - inc++; //number starts at 1 + write(feature, lastRecordNumber + 1); + } + + /** + * Write a feature with the given record number. + * + * @param recordNumber record number, starts at one + */ + private void write(Feature feature, int recordNumber) throws IOException { final ShapeRecord shpRecord = new ShapeRecord(); final long recordStartPosition = shpWriter.getSteamPosition(); @@ -1248,21 +1335,16 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur } } - //write geometry + //write geometry, a null geometry is written as a null shape + shpRecord.recordNumber = recordNumber; Object value = feature.getPropertyValue(defaultGeomName); if (value instanceof Geometry) { shpRecord.geometry = (Geometry) value; - shpRecord.recordNumber = inc; - } else { + } else if (value != null) { throw new IOException("Feature geometry property is not a geometry"); } shpWriter.writeRecord(shpRecord); - final long recordEndPosition = shpWriter.getSteamPosition(); - - //write index - final int recordStartPositionWord = Math.toIntExact(recordStartPosition / 2); // divide by 2 for word size - final int recordEndPositionWord = Math.toIntExact(recordEndPosition / 2); // divide by 2 for word size - shxWriter.writeRecord(recordStartPositionWord, recordEndPositionWord - recordStartPositionWord); + writeIndex(recordStartPosition); //copy dbf fields Object[] fields = new Object[dbfHeader.fields.length]; @@ -1270,6 +1352,37 @@ public final class ShapefileStore extends URIDataStore implements WritableFeatur fields[i] = feature.getPropertyValue(dbfHeader.fields[i].fieldName); } dbfWriter.writeRecord(fields); + lastRecordNumber = Math.max(lastRecordNumber, recordNumber); + } + + /** + * Write a record marked as deleted. + * + * The record is preserved in the files to keep the record numbers of the + * following records unchanged. It contains a null shape and blank dbf fields. + * + * @param recordNumber record number, starts at one + */ + private void writeDeleted(int recordNumber) throws IOException { + final ShapeRecord shpRecord = new ShapeRecord(); + shpRecord.recordNumber = recordNumber; + final long recordStartPosition = shpWriter.getSteamPosition(); + shpWriter.writeRecord(shpRecord); + writeIndex(recordStartPosition); + dbfWriter.writeDeletedRecord(); + lastRecordNumber = Math.max(lastRecordNumber, recordNumber); + } + + /** + * Write the shx entry of the record which starts at given position. + * + * @param recordStartPosition position of the record in the shp file, in bytes + */ + private void writeIndex(long recordStartPosition) throws IOException { + final long recordEndPosition = shpWriter.getSteamPosition(); + final int recordStartPositionWord = Math.toIntExact(recordStartPosition / 2); // divide by 2 for word size + final int recordEndPositionWord = Math.toIntExact(recordEndPosition / 2); // divide by 2 for word size + shxWriter.writeRecord(recordStartPositionWord, recordEndPositionWord - recordStartPositionWord); } /** diff --git a/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/package-info.java b/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/package-info.java index e2ac355205..a47315a625 100644 --- a/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/package-info.java +++ b/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/package-info.java @@ -24,6 +24,15 @@ * <h2>Writing example</h2> *{@snippet class="org.apache.sis.storage.shapefile.Snippets" region="write"} * + * <h2>Feature identifiers</h2> + * The shapefile format does not store feature identifiers, they are derived from + * the record number. To keep those identifiers stable, removing a feature does not + * shift the following records : the record is flagged as deleted in the dbf file and + * its slot is preserved, holding a null shape and blank fields. New features are always + * appended after the last record number, an identifier is therefore never reused. + * Use {@link org.apache.sis.storage.shapefile.ShapefileStore#compact()} to drop those + * records and reduce the files size, at the cost of renumbering the remaining records. + * * For raw access to DBF and SHP, use the related packages : * <ul> * <li>{@link org.apache.sis.storage.shapefile.shp}</li> diff --git 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 index 45e53c70cf..461b5a7b74 100644 --- 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 @@ -19,6 +19,7 @@ package org.apache.sis.storage.shapefile; import java.io.IOException; import java.net.URISyntaxException; import java.net.URL; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.time.LocalDate; @@ -298,12 +299,121 @@ public class ShapefileStoreTest { Object[] result = store.features(false).toArray(); assertEquals(1, result.length); - //because of incremental id, feature2 will now have sis:identifer=0 - feature2.setPropertyValue(AttributeConvention.IDENTIFIER, "test.1"); + //the deleted record is preserved in the files, feature2 keeps its identifier assertEquals(feature2, result[0]); } } + /** + * 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(); + 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(); + 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(); + store.add(List.of(createFeature1(type), createFeature2(type), createFeature3(type)).iterator()); + + final FilterFactory<Feature, 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); + 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(); + store.add(List.of(createFeature1(type), createFeature2(type), createFeature3(type)).iterator()); + + final FilterFactory<Feature, 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) -> { + 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")); + } + } + + /** + * 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(); + store.add(List.of(createFeature1(type), createFeature2(type), createFeature3(type)).iterator()); + + final FilterFactory<Feature, 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")); + } + } + /** * Test replacing features in a shapefile. */ @@ -363,6 +473,20 @@ public class ShapefileStoreTest { } + /** + * 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(); + for (final String id : expected) { + assertTrue(ite.hasNext(), "missing feature " + id); + assertEquals(id, ite.next().getPropertyValue(AttributeConvention.IDENTIFIER)); + } + assertFalse(ite.hasNext(), "unexpected additional feature"); + } + } + private static FeatureType createType() { final FeatureTypeBuilder ftb = new FeatureTypeBuilder(); ftb.setName("test"); @@ -398,4 +522,16 @@ public class ShapefileStoreTest { feature.setPropertyValue("date", LocalDate.of(2030, 6, 21)); return feature; } + + private static Feature createFeature3(FeatureType type) { + Feature 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; + } }
