Skip to content

Delete remote uploads

Jan Provaznik requested to merge jprovazn-remote-upload-destroy into master

What does this MR do?

Delete remote uploads

Are there points in the code the reviewer needs to double check?

  • make sure that files are deleted in object store
  • make sure that set of uploaders used in destroy_file_uploads is proper (that we don't use not-mounted uploaders of other types)

Why was this MR needed?

ObjectStore uploader requires presence of associated uploads record when deleting the upload file (through the carrierwave's after_commit hook) because we keep info whether file is LOCAL or REMOTE in upload object.

For this reason we can not destroy uploads as "dependent: :destroy" hook because these would be deleted too soon. Instead we rely on carrierwave's hook to destroy uploads in after_commit hook.

But in before_destroy hook we still have to delete not-mounted uploads (which don't use carrierwave's destroy hook). This has to be done in before_Destroy instead of after_commit because FileUpload requires existence of model's object on destroy action.

This is not ideal state of things, in a next step we should investigate how to unify model dependencies so we can use same workflow for all uploads.

Note: this patch addresses only object deletion in object store (issue #45425 (closed)), it still uses before_destroy hook (which is used by dependent: :destroy too. But follow-up improvement, which uses after_commit hook will be needed - this is a follow-up issue #46069 (closed).

What are the relevant issue numbers?

Closes #45425 (closed)

Edited by Jan Provaznik

Merge request reports