fix race condition creating and deleting dirs when doing concurrent w…

…rites

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);

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<String, Path> 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<InventoryUpdater.RemoveFileResult> removeFiles) {
         removeFiles.forEach(remove -> {
             var stagingPath = stagingFullPath(remove.getPathUnderContentDir());

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);
         }

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<String, Path> 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<String, Path> 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();

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) {

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()));
     }