OCFL / ocfl-java

A Java OCFL implementation
MIT License
16 stars 12 forks source link

MutableOcflRepository fails on writing files with the same hash #105

Closed Mewel closed 4 months ago

Mewel commented 4 months ago

Writing a file with the same hash leads to a IllegalArgumentException due to the fact that the source directory is empty. Before copying, the MutableOcflRepository should check if the hash is already present in the inventory.json.

java.lang.IllegalArgumentException: Source must exist and be a directory: /tmp/ocfl3291760920107648701/ocfl-work/e03731555a6eece977a708aaf6a3c780-3248953293/content/r1

    at io.ocfl.core.util.FileUtil.moveDirectory(FileUtil.java:90)
    at io.ocfl.core.storage.filesystem.FileSystemStorage.moveDirectoryInto(FileSystemStorage.java:266)
    at io.ocfl.core.storage.DefaultOcflStorage.moveToRevisionDirectory(DefaultOcflStorage.java:756)
    at io.ocfl.core.storage.DefaultOcflStorage.storeNewMutableHeadVersion(DefaultOcflStorage.java:694)
    at io.ocfl.core.storage.DefaultOcflStorage.storeNewVersion(DefaultOcflStorage.java:253)
        ...
public class OCFLTestCase {

    @Test
    public void testMutable() throws IOException {
        Path tempDirectory = Files.createTempDirectory("ocfl");
        Path repoDirectoryPath = tempDirectory.resolve("ocfl-repo");
        Path workDirectoryPath = tempDirectory.resolve("ocfl-work");

        Files.createDirectory(repoDirectoryPath);
        Files.createDirectory(workDirectoryPath);

        MutableOcflRepository repository = new OcflRepositoryBuilder()
            .defaultLayoutConfig(new HashedNTupleLayoutConfig())
            .storage(storage -> storage.fileSystem(repoDirectoryPath))
            .workDir(workDirectoryPath)
            .buildMutable();

        String objectId = "object_1";
        ObjectVersionId head = ObjectVersionId.head(objectId);
        repository.stageChanges(head, new VersionInfo(), (updater) -> {
            updater.writeFile(new ByteArrayInputStream(new byte[] { 1 }), "info_1.txt");
        });
        repository.commitStagedChanges(objectId, new VersionInfo());

        repository.stageChanges(head, new VersionInfo(), (updater) -> {
            updater.writeFile(new ByteArrayInputStream(new byte[] { 1 }), "info_2.txt");
        });
        repository.commitStagedChanges(objectId, new VersionInfo());
    }

}

Changing one of the byte[] { 1 } to byte[] { 2 } will work as expected.

pwinckles commented 4 months ago

@Mewel Thanks for the report! I created a PR that you're welcome to review if you'd like before I merge.

Mewel commented 4 months ago

I checked out your branch, but my test case now leads to this exception:

io.ocfl.api.exception.OcflNoSuchFileException: NoSuchFileException: /tmp/ocfl12980879452098006446/ocfl-repo/e92/48d/1a4/e9248d1a4ec5483ad18cef2b10d200dfdc7d429d925a6820a6fb1f5057ba5378/extensions/0005-mutable-head/head/inventory.json

    at io.ocfl.api.exception.OcflIOException.from(OcflIOException.java:48)
    at io.ocfl.core.storage.filesystem.FileSystemStorage.copyFileInto(FileSystemStorage.java:241)
    at io.ocfl.core.storage.DefaultOcflStorage.lambda$storeMutableHeadInventory$4(DefaultOcflStorage.java:807)
    at net.jodah.failsafe.Functions.lambda$toSupplier$10(Functions.java:262)
    at net.jodah.failsafe.Functions.lambda$get$0(Functions.java:48)
    at net.jodah.failsafe.RetryPolicyExecutor.lambda$supply$0(RetryPolicyExecutor.java:66)
    at net.jodah.failsafe.Execution.executeSync(Execution.java:128)
    at net.jodah.failsafe.FailsafeExecutor.call(FailsafeExecutor.java:379)
    at net.jodah.failsafe.FailsafeExecutor.run(FailsafeExecutor.java:212)
    at io.ocfl.core.storage.DefaultOcflStorage.storeMutableHeadInventory(DefaultOcflStorage.java:806)
    at io.ocfl.core.storage.DefaultOcflStorage.storeNewMutableHeadVersion(DefaultOcflStorage.java:699)
    at io.ocfl.core.storage.DefaultOcflStorage.storeNewVersion(DefaultOcflStorage.java:253)
    at io.ocfl.core.storage.CachingOcflStorage.storeNewVersion(CachingOcflStorage.java:95)
    at io.ocfl.core.DefaultOcflRepository.lambda$writeNewVersion$7(DefaultOcflRepository.java:640)
    at io.ocfl.core.lock.InMemoryObjectLock.lambda$doInWriteLock$0(InMemoryObjectLock.java:68)
    at io.ocfl.core.lock.InMemoryObjectLock.doInLock(InMemoryObjectLock.java:86)
    at io.ocfl.core.lock.InMemoryObjectLock.doInWriteLock(InMemoryObjectLock.java:79)
    at io.ocfl.core.lock.InMemoryObjectLock.doInWriteLock(InMemoryObjectLock.java:67)
    at io.ocfl.core.DefaultOcflRepository.writeNewVersion(DefaultOcflRepository.java:639)
    at io.ocfl.core.DefaultMutableOcflRepository.stageChanges(DefaultMutableOcflRepository.java:128)
    at OCFLTestCase.testMutable(OCFLTestCase.java:37)
    ...
Caused by: java.nio.file.NoSuchFileException: /tmp/ocfl12980879452098006446/ocfl-repo/e92/48d/1a4/e9248d1a4ec5483ad18cef2b10d200dfdc7d429d925a6820a6fb1f5057ba5378/extensions/0005-mutable-head/head/inventory.json
    at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:92)
    at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
    at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
    at java.base/sun.nio.fs.UnixCopyFile.copyFile(UnixCopyFile.java:246)
    at java.base/sun.nio.fs.UnixCopyFile.copy(UnixCopyFile.java:603)
    at java.base/sun.nio.fs.UnixFileSystemProvider.copy(UnixFileSystemProvider.java:257)
    at java.base/java.nio.file.Files.copy(Files.java:1305)
    at io.ocfl.core.storage.filesystem.FileSystemStorage.copyFileInto(FileSystemStorage.java:239)
    ... 89 more
pwinckles commented 4 months ago

@Mewel Thanks, I should have run your test case. I missed that your code was doing a commit between the writes, so there are two bugs here. Will update the PR shortly.

pwinckles commented 4 months ago

@Mewel are you good with the fix now?

pwinckles commented 4 months ago

The fix is in version 2.0.1, which I just released. Let me know if you have any further issues.

Mewel commented 4 months ago

Looks good! Thanks for the fast fix!