From d2587901996d99d6fbe6e64b6eb0188f7451a53c Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 24 Oct 2021 04:06:52 -0500 Subject: [PATCH] uploads: don't delete files of abandoned uploads. Just leave them. They don't take up that much space and they may be used in the future if someone else tries to upload the same file. --- app/models/upload.rb | 15 ----------- test/unit/upload_service_test.rb | 46 -------------------------------- 2 files changed, 61 deletions(-) diff --git a/app/models/upload.rb b/app/models/upload.rb index 12d636f65..069d2949d 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -75,8 +75,6 @@ class Upload < ApplicationRecord validates_with FileValidator, on: :file serialize :context, JSON - after_destroy_commit :delete_files - scope :pending, -> { where(status: "pending") } scope :preprocessed, -> { where(status: "preprocessed") } scope :completed, -> { where(status: "completed") } @@ -104,19 +102,6 @@ class Upload < ApplicationRecord end end - concerning :FileMethods do - def delete_files - # md5 is blank if the upload errored out before downloading the file. - if is_completed? || md5.blank? || Upload.exists?(md5: md5) || Post.exists?(md5: md5) - return - end - - media_asset&.destroy! - media_asset&.delete_files! - DanbooruLogger.info("Uploads: Deleting files for upload md5=#{md5}") - end - end - concerning :StatusMethods do def is_pending? status == "pending" diff --git a/test/unit/upload_service_test.rb b/test/unit/upload_service_test.rb index 73a18406d..7eb20debc 100644 --- a/test/unit/upload_service_test.rb +++ b/test/unit/upload_service_test.rb @@ -884,52 +884,6 @@ class UploadServiceTest < ActiveSupport::TestCase assert_difference("Upload.count", -1) { Upload.prune! } end - should "delete unused files after deleting the upload" do - @upload = as(@user) { UploadService::Preprocessor.new(file: upload_file("test/files/test.jpg")).start! } - assert_file_exists(@upload, :original) - - @upload.destroy! - assert_file_does_not_exist(@upload, :original) - end - - should "not delete files that are still in use by a post" do - @upload = as(@user) { UploadService.new(file: upload_file("test/files/test.jpg")).start! } - assert_file_exists(@upload, :original) - - @upload.destroy! - assert_file_exists(@upload, :original) - end - - should "not delete files if they're still in use by another upload" do - @upload1 = as(@user) { UploadService::Preprocessor.new(file: upload_file("test/files/test.jpg")).start! } - @upload2 = as(@user) { UploadService::Preprocessor.new(file: upload_file("test/files/test.jpg")).start! } - assert_equal(@upload1.md5, @upload2.md5) - assert_file_exists(@upload1, :original) - - @upload1.destroy! - assert_file_exists(@upload1, :original) - - @upload2.destroy! - assert_file_does_not_exist(@upload2, :original) - end - - should "not delete files that were replaced after upload and are still pending deletion" do - @upload = as(@user) { UploadService.new(file: upload_file("test/files/test.jpg")).start! } - assert(@upload.is_completed?) - - as(@user) { @upload.post.replace!(replacement_file: upload_file("test/files/test.png"), replacement_url: "") } - assert_not_equal(@upload.md5, @upload.post.md5) - - # after replacement the uploaded file is no longer in use, but it shouldn't be - # deleted yet. it should only be deleted by the replacer after the grace period. - @upload.destroy! - assert_file_exists(@upload, :original) - - travel (PostReplacement::DELETION_GRACE_PERIOD + 1).days - perform_enqueued_jobs - assert_file_does_not_exist(@upload, :original) - end - should "work on uploads without a file" do @upload = as(@user) { UploadService.new(source: "http://14903gf0vm3g134yjq3n535yn3n.com/does_not_exist.jpg").start! }