FilesByMd5Service: in addition to confirming that a file exists before including it in the array #call returns, confirm that the file is of the expected size #935
Conversation
…e including it in the array #call returns, confirm that the file is of the expected size fixes sul-dlss/dor-services-app#5191
| @@ -24,12 +24,17 @@ def shelve_file_map | |||
|
|
|||
| # @return [Array<Hash<String, String>>] array of hashes with md5 as key and filename as value for shelved files | |||
| # For example: [ | |||
| # { "5b79c8570b7ef582735f912aa24ce5f2" => "2542A.tiff" }, | |||
| # { "cd5ca5c4666cfd5ce0e9dc8c83461d7a" => "2542A.jp2" } | |||
| # { "5b79c8570b7ef582735f912aa24ce5f2" => { "filename" => "2542A.tiff", "size" => 456 } }, | |||
There was a problem hiding this comment.
I now regret the original data structure which has morphed into ^^. To avoid confusing our future selves, can we define a struct (or similar) to hold file details (with md5, filename, and size attributes) and then return an array of file details?
There was a problem hiding this comment.
(Happy to discuss more / pair.)
There was a problem hiding this comment.
yup, that makes sense to me. will give it a shot and discuss more or ask for pairing if i get stuck
There was a problem hiding this comment.
ok, this is ready for another review. i definitely prefer things with the struct suggestion, feels easier to read than the one element hash. left the change as a separate commit so that it'd be easy to undo if the change i made wasn't what you had in mind.
i realized after i finished that commit that i didn't switch FilesByMd5Service.call from returning the md5 keyed hash list, but i guess since the return value gets used in a controller that other services presumably depend on, best to leave that structure as is?
… a FileDetails struct instead of a hash keyed on md5 (has was always one element and no lookup was ever done)
|
passed integration tests, merging |
fixes sul-dlss/dor-services-app#5191
assumes
sizemetadata will always be present, but that seems like a safe bet?