Skip to content

Commit

Permalink
fix bug where empty content directories were being created
Browse files Browse the repository at this point in the history
  • Loading branch information
pwinckles committed Apr 5, 2021
1 parent 1832e7d commit 9248c1b
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,13 @@ private void getObjectInternal(Inventory inventory, VersionNum versionNum, Path

protected void writeNewVersion(Inventory inventory, Path stagingDir) {
var finalInventory = writeInventory(inventory, stagingDir);

// Versions should not contain empty content directories
var contentDir = stagingDir.resolve(inventory.resolveContentDirectory());
if (!FileUtil.hasChildren(contentDir)) {
UncheckedFiles.delete(contentDir);
}

objectLock.doInWriteLock(inventory.getId(), () -> storage.storeNewVersion(finalInventory, stagingDir));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,15 @@ private void verifyPriorInventory(Inventory inventory, Path sidecarPath) {
}
}

private void versionContentCheck(Inventory inventory, ObjectPaths.ObjectRoot objectRoot, Path contentPath, boolean checkFixity) {
private void versionContentCheck(Inventory inventory,
ObjectPaths.ObjectRoot objectRoot,
Path contentPath,
boolean checkFixity) {
// The content directory will not exist if the version does not introduce new files
if (Files.notExists(contentPath)) {
return;
}

var version = inventory.getHeadVersion();
var fileIds = inventory.getFileIdsForMatchingFiles(objectRoot.path().relativize(contentPath));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ public static void safeDeleteDirectory(Path path) {
}
}


/**
* Recursive delete that deletes everything under and including the specified directory. Failures deleting individual
* files are logged and a single exception is thrown at the end.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
import edu.wisc.library.ocfl.core.cache.NoOpCache;
import edu.wisc.library.ocfl.core.extension.UnsupportedExtensionBehavior;
import edu.wisc.library.ocfl.core.extension.storage.layout.HashedNTupleLayoutExtension;
import edu.wisc.library.ocfl.core.extension.storage.layout.config.FlatLayoutConfig;
import edu.wisc.library.ocfl.core.extension.storage.layout.config.HashedNTupleIdEncapsulationLayoutConfig;
import edu.wisc.library.ocfl.core.extension.storage.layout.config.HashedNTupleLayoutConfig;
import edu.wisc.library.ocfl.core.util.FileUtil;
import edu.wisc.library.ocfl.core.util.UncheckedFiles;
import edu.wisc.library.ocfl.itest.ITestHelper;
import edu.wisc.library.ocfl.itest.OcflITest;
import edu.wisc.library.ocfl.test.TestHelper;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledOnOs;
import org.junit.jupiter.api.condition.OS;
Expand All @@ -39,6 +42,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;

public class FileSystemOcflITest extends OcflITest {

Expand Down Expand Up @@ -166,6 +170,27 @@ public void allowPathsWithDifficultCharsWhenNoRestrictionsApplied() throws IOExc
}
}

@Test
public void shouldNotCreateEmptyContentDirWhenVersionHasNoContent() {
var repoName = "empty-content";
var repo = defaultRepo(repoName, builder -> builder.defaultLayoutConfig(new FlatLayoutConfig()));

var objectId = "object";

repo.updateObject(ObjectVersionId.head(objectId), null, updater -> {
updater.writeFile(streamString("asdf"), "file.txt");
});

repo.updateObject(ObjectVersionId.head(objectId), null, updater -> {
updater.removeFile("file.txt");
});

var root = repoDir(repoName);
var v2ContentPath = root.resolve(objectId).resolve("v2/content");

assertFalse(Files.exists(v2ContentPath), "empty content directories should not exist");
}

@Override
protected void onBefore() {
reposDir = UncheckedFiles.createDirectories(tempRoot.resolve("repos"));
Expand Down

0 comments on commit 9248c1b

Please sign in to comment.