diff --git a/ocfl-java-core/src/main/java/io/ocfl/core/DefaultMutableOcflRepository.java b/ocfl-java-core/src/main/java/io/ocfl/core/DefaultMutableOcflRepository.java index 64d33f76..6c4beb0f 100644 --- a/ocfl-java-core/src/main/java/io/ocfl/core/DefaultMutableOcflRepository.java +++ b/ocfl-java-core/src/main/java/io/ocfl/core/DefaultMutableOcflRepository.java @@ -143,7 +143,7 @@ public ObjectVersionId stageChanges( try { objectUpdater.accept(updater); var newInventory = buildNewInventory(inventoryUpdater, versionInfo); - writeNewVersion(newInventory, stagingDir, false); + writeNewVersion(newInventory, stagingDir, false, updater.checkForEmptyDirs()); return ObjectVersionId.version(objectVersionId.getObjectId(), newInventory.getHead()); } finally { FileUtil.safeDeleteDirectory(stagingDir); @@ -231,7 +231,7 @@ private Inventory createAndPersistEmptyVersion(ObjectVersionId objectId) { .build()) .build(); - writeNewVersion(inventory, stagingDir, false); + writeNewVersion(inventory, stagingDir, false, false); return inventory; } finally { FileUtil.safeDeleteDirectory(stagingDir); diff --git a/ocfl-java-core/src/main/java/io/ocfl/core/DefaultOcflObjectUpdater.java b/ocfl-java-core/src/main/java/io/ocfl/core/DefaultOcflObjectUpdater.java index 6797e7c6..ec73120f 100644 --- a/ocfl-java-core/src/main/java/io/ocfl/core/DefaultOcflObjectUpdater.java +++ b/ocfl-java-core/src/main/java/io/ocfl/core/DefaultOcflObjectUpdater.java @@ -48,6 +48,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -64,6 +65,7 @@ public class DefaultOcflObjectUpdater implements OcflObjectUpdater { private final AddFileProcessor addFileProcessor; private final FileLocker fileLocker; private final Map stagedFileMap; + private final AtomicBoolean checkForEmptyDirs; public DefaultOcflObjectUpdater( Inventory inventory, @@ -77,6 +79,7 @@ public DefaultOcflObjectUpdater( this.addFileProcessor = Enforce.notNull(addFileProcessor, "addFileProcessor cannot be null"); this.fileLocker = Enforce.notNull(fileLocker, "fileLocker cannot be null"); this.stagedFileMap = new ConcurrentHashMap<>(); + this.checkForEmptyDirs = new AtomicBoolean(false); } @Override @@ -145,7 +148,7 @@ public OcflObjectUpdater writeFile(InputStream input, String destinationPath, Oc ((FixityCheckInputStream) input).checkFixity(); } catch (FixityCheckException e) { FileUtil.safeDelete(stagingFullPath); - FileUtil.deleteDirAndParentsIfEmpty(stagingFullPath.getParent(), stagingDir); + checkForEmptyDirs.set(true); throw e; } } @@ -168,7 +171,7 @@ public OcflObjectUpdater writeFile(InputStream input, String destinationPath, Oc stagingFullPath, digest); UncheckedFiles.delete(stagingFullPath); - FileUtil.deleteDirAndParentsIfEmpty(stagingFullPath.getParent(), stagingDir); + checkForEmptyDirs.set(true); } else { stagedFileMap.put(destinationPath, stagingFullPath); } @@ -315,6 +318,16 @@ public OcflObjectUpdater clearFixityBlock() { return this; } + /** + * Returns true if the processor deleted a file and thus we need to look for empty directories to delete prior to + * writing the version. + * + * @return true if we need to look for empty directories + */ + public boolean checkForEmptyDirs() { + return checkForEmptyDirs.get() || addFileProcessor.checkForEmptyDirs(); + } + private void removeUnneededStagedFiles(Set removeFiles) { removeFiles.forEach(remove -> { var stagingPath = stagingFullPath(remove.getPathUnderContentDir()); diff --git a/ocfl-java-core/src/main/java/io/ocfl/core/DefaultOcflRepository.java b/ocfl-java-core/src/main/java/io/ocfl/core/DefaultOcflRepository.java index 4ff35bd9..d82ddfc2 100644 --- a/ocfl-java-core/src/main/java/io/ocfl/core/DefaultOcflRepository.java +++ b/ocfl-java-core/src/main/java/io/ocfl/core/DefaultOcflRepository.java @@ -184,7 +184,7 @@ public ObjectVersionId putObject( var newInventory = buildNewInventory(inventoryUpdater, versionInfo); try { - writeNewVersion(newInventory, stagingDir, upgrade); + writeNewVersion(newInventory, stagingDir, upgrade, fileProcessor.checkForEmptyDirs()); return ObjectVersionId.version(objectVersionId.getObjectId(), newInventory.getHead()); } finally { FileUtil.safeDeleteDirectory(stagingDir); @@ -223,7 +223,7 @@ public ObjectVersionId updateObject( objectUpdater.accept(updater); var upgrade = inventoryUpdater.upgradeInventory(config); var newInventory = buildNewInventory(inventoryUpdater, versionInfo); - writeNewVersion(newInventory, stagingDir, upgrade); + writeNewVersion(newInventory, stagingDir, upgrade, updater.checkForEmptyDirs()); return ObjectVersionId.version(objectVersionId.getObjectId(), newInventory.getHead()); } finally { FileUtil.safeDeleteDirectory(stagingDir); @@ -400,7 +400,7 @@ public ObjectVersionId replicateVersionAsHead(ObjectVersionId objectVersionId, V createStagingContentDir(inventory, stagingDir); try { - writeNewVersion(newInventory, stagingDir, upgrade); + writeNewVersion(newInventory, stagingDir, upgrade, false); return ObjectVersionId.version(objectVersionId.getObjectId(), newInventory.getHead()); } finally { FileUtil.safeDeleteDirectory(stagingDir); @@ -633,10 +633,16 @@ private void getObjectInternal(Inventory inventory, VersionNum versionNum, Path } } - protected void writeNewVersion(Inventory inventory, Path stagingDir, boolean upgradedOcflVersion) { + protected void writeNewVersion( + Inventory inventory, Path stagingDir, boolean upgradedOcflVersion, boolean checkForEmptyDirs) { var finalInventory = writeInventory(inventory, stagingDir); var contentDir = stagingDir.resolve(inventory.resolveContentDirectory()); + + if (checkForEmptyDirs) { + FileUtil.deleteEmptyDirs(contentDir); + } + if (!FileUtil.hasChildren(contentDir)) { UncheckedFiles.delete(contentDir); } diff --git a/ocfl-java-core/src/main/java/io/ocfl/core/inventory/AddFileProcessor.java b/ocfl-java-core/src/main/java/io/ocfl/core/inventory/AddFileProcessor.java index b8e4349b..ec495a41 100644 --- a/ocfl-java-core/src/main/java/io/ocfl/core/inventory/AddFileProcessor.java +++ b/ocfl-java-core/src/main/java/io/ocfl/core/inventory/AddFileProcessor.java @@ -45,6 +45,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,6 +61,7 @@ public class AddFileProcessor { private final FileLocker fileLocker; private final Path stagingDir; private final DigestAlgorithm digestAlgorithm; + private final AtomicBoolean checkForEmptyDirs; public static Builder builder() { return new Builder(); @@ -92,6 +94,7 @@ public AddFileProcessor( this.fileLocker = Enforce.notNull(fileLocker, "fileLocker cannot be null"); this.stagingDir = Enforce.notNull(stagingDir, "stagingDir cannot be null"); this.digestAlgorithm = Enforce.notNull(digestAlgorithm, "digestAlgorithm cannot be null"); + this.checkForEmptyDirs = new AtomicBoolean(false); } /** @@ -178,7 +181,7 @@ public Map processPath(Path sourcePath, String destinationPath, Oc stagingFullPath, digest); UncheckedFiles.delete(stagingFullPath); - FileUtil.deleteDirAndParentsIfEmpty(stagingFullPath.getParent(), stagingDir); + checkForEmptyDirs.set(true); } } } @@ -243,6 +246,16 @@ public Map processFileWithDigest( }); } + /** + * Returns true if the processor deleted a file and thus we need to look for empty directories to delete prior to + * writing the version. + * + * @return true if we need to look for empty directories + */ + public boolean checkForEmptyDirs() { + return checkForEmptyDirs.get(); + } + private String destinationPath(String path, Path sourcePath) { if (path.isBlank() && Files.isRegularFile(sourcePath)) { return sourcePath.getFileName().toString(); diff --git a/ocfl-java-core/src/main/java/io/ocfl/core/util/FileUtil.java b/ocfl-java-core/src/main/java/io/ocfl/core/util/FileUtil.java index a1408ebe..0c7fcb73 100644 --- a/ocfl-java-core/src/main/java/io/ocfl/core/util/FileUtil.java +++ b/ocfl-java-core/src/main/java/io/ocfl/core/util/FileUtil.java @@ -222,7 +222,9 @@ public static void deleteChildren(Path root) { */ public static void deleteEmptyDirs(Path root) { try (var files = Files.find(root, Integer.MAX_VALUE, (file, attrs) -> attrs.isDirectory())) { - files.filter(f -> !f.equals(root)).forEach(FileUtil::deleteDirIfEmpty); + files.filter(f -> !f.equals(root)) + .sorted(Comparator.reverseOrder()) + .forEach(FileUtil::deleteDirIfEmpty); } catch (NoSuchFileException e) { // ignore } catch (IOException e) { diff --git a/ocfl-java-core/src/test/java/io/ocfl/core/util/FileUtilTest.java b/ocfl-java-core/src/test/java/io/ocfl/core/util/FileUtilTest.java index 86e0e9f1..9e9c61f7 100644 --- a/ocfl-java-core/src/test/java/io/ocfl/core/util/FileUtilTest.java +++ b/ocfl-java-core/src/test/java/io/ocfl/core/util/FileUtilTest.java @@ -97,9 +97,8 @@ public void shouldDeleteAllEmptyDirectories() throws IOException { FileUtil.deleteEmptyDirs(tempRoot); - assertThat(tempRoot.resolve("a/b/c").toFile(), anExistingDirectory()); + assertThat(tempRoot.resolve("a/b/c").toFile(), not(anExistingDirectory())); assertThat(tempRoot.resolve("a/c/file3").toFile(), anExistingFile()); - assertThat(tempRoot.resolve("a/b/c/d").toFile(), not(anExistingDirectory())); assertThat(tempRoot.resolve("a/d").toFile(), not(anExistingDirectory())); }