omeka-s-modules / FileSideload

GNU General Public License v3.0
2 stars 3 forks source link

Sideload shouldn't override temp file location #14

Closed zerocrates closed 4 years ago

zerocrates commented 4 years ago

Currently, the module works by creating a TempFile, which reserves a filename in the system temp directory, and overriding the and setting the "deleteTempFile" flag to correspond with the module configuration of whether to delete files after they're loaded.

This leads to a problem if we make an improvement to the core: there are situations where, if we encounter an error like the file's extension or mime not being allowed or there being errors at various stages of the ingest/thumbnail/storage process, the original temp file gets left behind. Fixing this means inserting some cleanup code to correctly delete the temp file after an error.

For all other ingesters this works fine: the temp file in all cases is just a copy of a file uploaded from a user or downloaded from a URL... but it would cause issues with the sideload module since it's sort of "abusing" the TempFile class for non-temporary files: with the core "fixed," files in the sideload directory would get deleted after these kinds of errors, which is not what users would expect to happen. Even with the module configured to delete sideloaded files, that's intended only to happen after they're successfully loaded.

Instead of doing the override of the TempFile path it's currently doing, FileSideload should copy the file to be sideloaded to the TempFile path, leave the parameter to delete the temp file to its default of true, and then handle deleting the file from the sideload directory on its own, according to the module config.