OCFL / ocfl-java

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

A storage root containing only a `.DS_Store` file should be considered empty #56

Open mderuijter opened 2 years ago

mderuijter commented 2 years ago

For our project I have been trying to see if we can use this library to implement OCFL, and I ran into a small bug causing:

Exception in thread "main" edu.wisc.library.ocfl.api.exception.RepositoryConfigurationException: OCFL root is missing its namaste file, eg. 0=ocfl_1.0.

I think I traced the cause to DefaultOcflStorageInitializer.java#L92 which will always result to false

pwinckles commented 2 years ago

@mderuijter Can you give me more details about your situation? Are you using local storage or S3?

Given that exception, my expectation of what's happening is that you're trying to initialize a repository in a directory that is both not empty and not an OCFL storage root.

mderuijter commented 2 years ago

@pwinckles I'm using local storage and it's pointing to an empty directory. I tried to play with some of the example code(bit modified), which I expect should initialize a new OCFL repo, instead I get the error I described:

Path repoDir = Paths.get("src/test/resources/data/Outbox");
        Path workDir = Paths.get("src/test/resources/data/temp");

        var repo = new OcflRepositoryBuilder()
                .defaultLayoutConfig(new FlatOmitPrefixLayoutConfig().setDelimiter("urn:uuid:"))
                .storage(ocflStorageBuilder -> ocflStorageBuilder.fileSystem(repoDir))
                .workDir(workDir)
                .build();

Full stack trace:

Exception in thread "main" edu.wisc.library.ocfl.api.exception.RepositoryConfigurationException: OCFL root is missing its namaste file, eg. 0=ocfl_1.0.
    at edu.wisc.library.ocfl.core.storage.DefaultOcflStorageInitializer.validateOcflVersion(DefaultOcflStorageInitializer.java:131)
    at edu.wisc.library.ocfl.core.storage.DefaultOcflStorageInitializer.loadAndValidateExistingRepo(DefaultOcflStorageInitializer.java:108)
    at edu.wisc.library.ocfl.core.storage.DefaultOcflStorageInitializer.initializeStorage(DefaultOcflStorageInitializer.java:95)
    at edu.wisc.library.ocfl.core.storage.DefaultOcflStorage.doInitialize(DefaultOcflStorage.java:587)
    at edu.wisc.library.ocfl.core.storage.AbstractOcflStorage.initializeStorage(AbstractOcflStorage.java:62)
    at edu.wisc.library.ocfl.core.storage.CachingOcflStorage.doInitialize(CachingOcflStorage.java:59)
    at edu.wisc.library.ocfl.core.storage.AbstractOcflStorage.initializeStorage(AbstractOcflStorage.java:62)
    at edu.wisc.library.ocfl.core.OcflRepositoryBuilder.buildInternal(OcflRepositoryBuilder.java:399)
    at edu.wisc.library.ocfl.core.OcflRepositoryBuilder.build(OcflRepositoryBuilder.java:380)
mderuijter commented 2 years ago

@pwinckles also the code link I sent:

if (list("").isEmpty()) {
            layoutExtension = initNewRepo(ocflVersion, layoutConfig);
        } else {
            layoutExtension = loadAndValidateExistingRepo(ocflVersion, layoutConfig);
            // This is only validating currently and does not load anything
            loadRepositoryExtensions(supportEvaluator);
        }

list("").isEmpty() always evaluates to false, so layoutExtension = initNewRepo(ocflVersion, layoutConfig); can never be called

pwinckles commented 2 years ago

list("").isEmpty() only evaluates to false when there are files in the storage root.

I just copy and pasted your example and it works fine. There must be files in src/test/resources/data/Outbox. Do ls -a src/test/resources/data/Outbox.

mderuijter commented 2 years ago

okay, clearly i didn't understand that code properly

-> % ls -alh
total 16
drwxr-xr-x  3 menko  staff    96B Nov 17  2020 .
drwxr-xr-x  8 menko  staff   256B Dec  2 14:34 ..
-rw-r--r--@ 1 menko  staff   6.0K Nov 17  2020 .DS_Store

surely the .DS_Store file isn't causing it not to be empty?

mderuijter commented 2 years ago

@pwinckles removing the .DS_Store file worked... we can close the issue i think. Thanks for pointing that out!

pwinckles commented 2 years ago

I'm going to leave this open, and will update the repo initialization code to consider a directory containing only a .DS_Store file as empty. God, I wish Macs didn't litter the filesystem with that stuff.