Fetch files not listed in payload manifest #99

Activity

1. johnscancella

   added

   bug

   on Jan 12, 2018
2. johnscancella commented on Jan 12, 2018

   

   johnscancella

   on Jan 12, 2018

   Contributor

   More actions

   Hi Thomas,

   Thanks for submitting this. You are correct that the specification regarding the fetch component does not allow for hashes and that it needs to be put in the manifest file. I don't use the fetch feature that much so it doesn't get as much testing as it probably should. I took a look at the code, and there needs to be some logic added to PayloadWriter that checks for FetchItems and doesn't add them to the data directory. That way you can safely add your fetch items to the manifest.
3. johnscancella

   added a commit that references this issue on Jan 12, 2018

   refs #99 - don't write payload file to data directory if that file is…

   ...

   65f7fe8
4. johnscancella commented on Jan 12, 2018

   

   johnscancella

   on Jan 12, 2018

   Contributor

   More actions

   This should be fixed in release 5.0.4
5. johnscancella

   closed this as completedon Jan 12, 2018
6. johnscancella commented on Jan 12, 2018

   

   johnscancella

   on Jan 12, 2018

   Contributor

   More actions

   You can download it from https://bintray.com/johnscancella/maven/bagit-java/5.0.4 or https://github.com/LibraryOfCongress/bagit-java/releases/tag/v5.0.4
7. ThomasJejkal commented on Jan 15, 2018

   

   ThomasJejkal

   on Jan 15, 2018

   ContributorAuthor

   More actions

   Hi John,

   Thanks for adding the fix that fast. I've tested the new version but there still seems to be an issue as during "real usage" (outside the test world) the check for fetch paths does not work. I guess the reason are problems while dealing with relative paths, but let's go back to 'given-when-then' style:

   Given

   • I add a fetch file with path Paths.get("default/out.html") and a payload element with path Paths.get("/Users/jejkal/NetBeansProjects/RepoInteropTool/testBag/data/default/out.html"). I have to do it that way, because this is the only way to make the FetchFile check in PayloadWriter working as it searches for existing paths relative to the bag data dir.

   When

   • I write the bag to disk

   Then

   • manifest-md5.txt contains the correct entry with relative path data/default/out.html, as the payload path added before contains the absolute path which can be successfully relativized to the bag root.
   • fetch.txt contains the wrong entry with relative path ../default/out.html, as the relative path used for the FetchFile is assumed to be relative to the working directory, which is in my case located at '../' This, relativizing to the bag root cannot work as expected.

   All other combinations, e.g. using Paths.get("data/default/out.html") or Paths.get("/Users/jejkal/NetBeansProjects/RepoInteropTool/testBag/data/default/out.html") as FetchFile path cause PayloadWriter not to identify the payload as FetchFile as 'fetchPaths.contains(relativePayloadPath.normalize())' assumes to have paths relative to the bag data dir in the list.

   In the test work everything works fine as in PayloadTestWriterTest#testWritePayloadFilesMinusFetchFiles() you are providing 'rootDir' as argument 'bagDataDir' whereas PayloadWriter#writeVersionDependentPayloadFiles(final Bag bag, final Path outputDir) uses bag.getRootDir().resolve("data")

   Thanks for your help.

   Regards,
   Thomas
8. johnscancella

   reopened this on Jan 16, 2018
9. johnscancella

   added a commit that references this issue on Jan 16, 2018

   refs #99 - don't try to make the path relative for fetch files

   e7e76da
10. johnscancella commented on Jan 16, 2018

    

    johnscancella

    on Jan 16, 2018

    Contributor

    More actions

    Thanks for finding this. Now whatever path you give in the fetch item should be what it uses in the fetch.txt. This change will be in the 5.0.5 release
11. johnscancella

    closed this as completedon Jan 16, 2018
12. johnscancella

    added 3 commits that reference this issue on Jan 18, 2018

    refs #99 - fixing fetch files being listed in manifest files

    c6282b6

    refs #99 - fixing fetch files being listed in manifest files

    f83b15c

    refs #99 - reverted to using relative paths and added test to ensure

    ...

    b4b3ef4