bagit-java icon indicating copy to clipboard operation
bagit-java copied to clipboard
verified publisher icon LibraryOfCongress

Optimize control files creation and fixes files being truncated on some SMB file servers.

Open tiarossi opened this issue 7 years ago • 6 comments

This PR aims to fix the situation occourred when creating a BagIt package inside a SMB file system. By some strange situation Files.write is ignoring the StandardOpenOption.APPEND option and truncating files on these repositories.

Please ensure you have completed the following before submitting:

  • [x] Ran all tests to ensure existing functionality wasn't broken
  • [x] Ran all quality assurance checks and fixed any new errors or warnings, which include:

Note: you can complete both boxes by running and fixing warnings/errors with gradle clean check

  • [x] Code is self documenting or a short comment when self documenting isn't possible

tiarossi avatar Nov 13 '18 17:11 tiarossi

Great find! We should add a unit test that only runs on SMB to guard against this in the future. Have you filed a bug report with Oracle/Java? Once they have this as a bug, we should do an inline comment citing the bug so that future developers understand why we aren't using Files.write

jscancella avatar Nov 13 '18 17:11 jscancella

Coverage Status

Coverage increased (+0.004%) to 98.285% when pulling 3e7242cfa1e4dba2bf369bf20c9765faf3dfc1c7 on silegis-mg:master into 2b7002e62d721f5eb50461e4c4c70a6ef643ec1d on LibraryOfCongress:master.

coveralls avatar Nov 13 '18 17:11 coveralls

Coverage Status

Coverage increased (+0.004%) to 98.285% when pulling 3e7242cfa1e4dba2bf369bf20c9765faf3dfc1c7 on silegis-mg:master into 2b7002e62d721f5eb50461e4c4c70a6ef643ec1d on LibraryOfCongress:master.

coveralls avatar Nov 13 '18 17:11 coveralls

Coverage Status

Coverage increased (+0.004%) to 98.285% when pulling 3e7242cfa1e4dba2bf369bf20c9765faf3dfc1c7 on silegis-mg:master into 2b7002e62d721f5eb50461e4c4c70a6ef643ec1d on LibraryOfCongress:master.

coveralls avatar Nov 13 '18 17:11 coveralls

@jscancella, as i'm not sure if it is a Java bug, a Windows driver bug or a bug in the smb server we have been using, i didn't opened any bug report. Despite this, the change is a performance improvement as Files.write opens and closes the file at each access.

tiarossi avatar Nov 13 '18 19:11 tiarossi

I know it is easier to not figure out where the bug is coming from, but without full knowledge we won't understand how much of an impact this will have to the whole userbase(is it only windows 2000 SP1 using an old version of SMB, or all windows for example?). I am also personally against writing code to fix someone's bug without reporting the bug so it can be fixed because it shifts the maintenance burden onto me.

I stand by Jack Diderich:

I hate code, and I want as little of it as possible in our product

jscancella avatar Nov 14 '18 11:11 jscancella