Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MutableOcflRepository fails on writing files with the same hash #105

Closed
Mewel opened this issue Feb 28, 2024 · 6 comments · Fixed by #106
Closed

MutableOcflRepository fails on writing files with the same hash #105

Mewel opened this issue Feb 28, 2024 · 6 comments · Fixed by #106

Comments

@Mewel
Copy link
Contributor

Mewel commented Feb 28, 2024

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 added a commit to pwinckles/ocfl-java that referenced this issue Feb 29, 2024
Creating a mutable head revision that only contains a file with
identical contents to a file already in the mutable head is now
properly writen to the object.

Resolves OCFL#105
pwinckles added a commit to pwinckles/ocfl-java that referenced this issue Feb 29, 2024
Creating a mutable head revision that only contains a file with
identical contents to a file already in the mutable head is now
properly writen to the object.

Resolves OCFL#105
@pwinckles
Copy link
Collaborator

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

@Mewel
Copy link
Contributor Author

Mewel commented Feb 29, 2024

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
Copy link
Collaborator

@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 added a commit to pwinckles/ocfl-java that referenced this issue Feb 29, 2024
Now correctly stages a file with identical content to a file that is
already in the object. Previously, this would fail on the first
mutable head revision, when the revision did not contain any content
files.

Resolves OCFL#105
@pwinckles
Copy link
Collaborator

@Mewel are you good with the fix now?

pwinckles added a commit that referenced this issue Mar 2, 2024
Creating a mutable head revision that only contains a file with
identical contents to a file already in the mutable head is now
properly writen to the object.

Resolves #105
pwinckles added a commit that referenced this issue Mar 2, 2024
Now correctly stages a file with identical content to a file that is
already in the object. Previously, this would fail on the first
mutable head revision, when the revision did not contain any content
files.

Resolves #105
@pwinckles
Copy link
Collaborator

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

@Mewel
Copy link
Contributor Author

Mewel commented Mar 5, 2024

Looks good! Thanks for the fast fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants