Fix #4142: Missing images after upload.

app/jobs/delete_upload_files_job.rb

@@ -3,6 +3,6 @@ class DeleteUploadFilesJob < ApplicationJob
   queue_with_priority 20
 
   def perform(md5, file_ext, upload_id)
-    UploadService::Utils.delete_file(md5, file_ext, upload_id)
+    # do nothing
   end
 end

app/logical/danbooru_logger.rb

@@ -3,7 +3,8 @@ class DanbooruLogger
     Rails.logger.info(message)
 
     if defined?(::NewRelic)
-      ::NewRelic::Agent.record_custom_event(:spam, message: message, **params)
+      params = flatten_hash(params).symbolize_keys
+      ::NewRelic::Agent.record_custom_event(:info, message: message, **params)
     end
   end
 

app/logical/danbooru_maintenance.rb

@@ -9,7 +9,7 @@ module DanbooruMaintenance
 
   def daily
     PostPruner.new.prune!
-    Upload.where('created_at < ?', 1.day.ago).delete_all
+    Upload.prune!
     Delayed::Job.where('created_at < ?', 45.days.ago).delete_all
     PostDisapproval.prune!
     ForumSubscription.process_all!

app/logical/upload_service/utils.rb

@@ -24,31 +24,6 @@ class UploadService
       end
     end
 
-    def delete_file(md5, file_ext, upload_id = nil)     
-      if Post.where(md5: md5).exists?
-        if upload_id.present? && Upload.where(id: upload_id).exists?
-          CurrentUser.as_system do
-            Upload.find(upload_id).update(status: "completed")
-          end
-        end
-
-        return
-      end
-
-      if upload_id.present? && Upload.where(id: upload_id).exists?
-        CurrentUser.as_system do
-          Upload.find(upload_id).update(status: "preprocessed + deleted")
-        end
-      end
-
-      Danbooru.config.storage_manager.delete_file(nil, md5, file_ext, :original)
-      Danbooru.config.storage_manager.delete_file(nil, md5, file_ext, :large)
-      Danbooru.config.storage_manager.delete_file(nil, md5, file_ext, :preview)
-      Danbooru.config.backup_storage_manager.delete_file(nil, md5, file_ext, :original)
-      Danbooru.config.backup_storage_manager.delete_file(nil, md5, file_ext, :large)
-      Danbooru.config.backup_storage_manager.delete_file(nil, md5, file_ext, :preview)
-    end
-
     def calculate_ugoira_dimensions(source_path)
       folder = Zip::File.new(source_path)
       Tempfile.open("ugoira-dim-") do |tempfile|
@@ -163,10 +138,6 @@ class UploadService
         crop_file.try(:close!)
         sample_file.try(:close!)
       end
-
-      # in case this upload never finishes processing, we need to delete the
-      # distributed files in the future
-      DeleteUploadFilesJob.set(wait: 24.hours).perform_later(upload.md5, upload.file_ext, upload.id)
     end
 
     # these methods are only really used during upload processing even 

app/models/upload.rb

@@ -67,6 +67,9 @@ class Upload < ApplicationRecord
   validates :md5, confirmation: true, if: -> (rec) { rec.md5_confirmation.present? }
   validates_with FileValidator, on: :file
   serialize :context, JSON
+
+  after_destroy_commit :delete_files
+
   scope :preprocessed, -> { where(status: "preprocessed") }
 
   def initialize_attributes
@@ -75,6 +78,10 @@ class Upload < ApplicationRecord
     self.server = Danbooru.config.server_host
   end
 
+  def self.prune!(date = 1.day.ago)
+    where("created_at < ?", date).lock.destroy_all
+  end
+
   module FileMethods
     def is_image?
       %w(jpg gif png).include?(file_ext)
@@ -91,6 +98,21 @@ class Upload < ApplicationRecord
     def is_ugoira?
       %w(zip).include?(file_ext)
     end
+
+    def delete_files
+      # md5 is blank if the upload errored out before downloading the file.
+      if md5.blank? || Upload.where(md5: md5).exists? || Post.where(md5: md5).exists?
+        return
+      end
+
+      DanbooruLogger.info("Uploads: Deleting files for upload md5=#{md5}", upload: as_json)
+      Danbooru.config.storage_manager.delete_file(nil, md5, file_ext, :original)
+      Danbooru.config.storage_manager.delete_file(nil, md5, file_ext, :large)
+      Danbooru.config.storage_manager.delete_file(nil, md5, file_ext, :preview)
+      Danbooru.config.backup_storage_manager.delete_file(nil, md5, file_ext, :original)
+      Danbooru.config.backup_storage_manager.delete_file(nil, md5, file_ext, :large)
+      Danbooru.config.backup_storage_manager.delete_file(nil, md5, file_ext, :preview)
+    end
   end
 
   module StatusMethods

test/models/upload_service_test.rb

@@ -1233,4 +1233,53 @@ class UploadServiceTest < ActiveSupport::TestCase
     end
 
   end
+
+  context "Upload#prune!" do
+    setup do
+      @user = create(:user, created_at: 1.year.ago)
+    end
+
+    should "delete stale upload records" do
+      @upload = as(@user) { UploadService.new(file: upload_file("test/files/test.jpg")).start! }
+
+      assert_difference("Upload.count", -1) { Upload.prune!(0.seconds.ago) }
+    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?(Danbooru.config.storage_manager.file_path(@upload.md5, "jpg", :original)))
+
+      @upload.destroy!
+      refute(File.exists?(Danbooru.config.storage_manager.file_path(@upload.md5, "jpg", :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?(Danbooru.config.storage_manager.file_path(@upload.md5, "jpg", :original)))
+
+      @upload.destroy!
+      assert(File.exists?(Danbooru.config.storage_manager.file_path(@upload.md5, "jpg", :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?(Danbooru.config.storage_manager.file_path(@upload1.md5, "jpg", :original)))
+
+      @upload1.destroy!
+      assert(File.exists?(Danbooru.config.storage_manager.file_path(@upload1.md5, "jpg", :original)))
+
+      @upload2.destroy!
+      refute(File.exists?(Danbooru.config.storage_manager.file_path(@upload2.md5, "jpg", :original)))
+    end
+
+    should "work on uploads without a file" do
+      @upload = as(@user) { UploadService.new(source: "http://14903gf0vm3g134yjq3n535yn3n.com/does_not_exist.jpg").start! }
+
+      assert(@upload.is_errored?)
+      assert_nil(@upload.md5)
+      assert_difference("Upload.count", -1) { @upload.destroy! }
+    end
+  end
 end