From 27aa9fe82a190249dc008389a638a09c29b266d2 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Tue, 13 Jun 2017 01:56:58 -0500
Subject: [PATCH] post replacements: don't delete files still in use.

Bug: if a user replaces a post with another image, then replaces the
post back to the original image, then the deletion job for the original
image will still run. The will delete the original file, but that file
is now in use again and should not be deleted.
---
 app/models/post.rb                 |  7 +++++++
 test/unit/post_replacement_test.rb | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/app/models/post.rb b/app/models/post.rb
index 2bd1abb01..5394e19e0 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -6,6 +6,7 @@ class Post < ActiveRecord::Base
   class DisapprovalError < Exception ; end
   class RevertError < Exception ; end
   class SearchError < Exception ; end
+  class DeletionError < Exception ; end
 
   before_validation :initialize_uploader, :on => :create
   before_validation :merge_old_changes
@@ -65,6 +66,12 @@ class Post < ActiveRecord::Base
 
     module ClassMethods
       def delete_files(post_id, file_path, large_file_path, preview_file_path)
+        post = Post.find(post_id)
+
+        if post.file_path == file_path || post.large_file_path == large_file_path || post.preview_file_path == preview_file_path
+          raise DeletionError.new("Files still in use; skipping deletion.")
+        end
+
         # the large file and the preview don't necessarily exist. if so errors will be ignored.
         FileUtils.rm_f(file_path)
         FileUtils.rm_f(large_file_path)
diff --git a/test/unit/post_replacement_test.rb b/test/unit/post_replacement_test.rb
index 293de1943..23c48bd7e 100644
--- a/test/unit/post_replacement_test.rb
+++ b/test/unit/post_replacement_test.rb
@@ -170,5 +170,25 @@ class PostReplacementTest < ActiveSupport::TestCase
         assert_equal([{"file"=>"000000.jpg", "delay"=>125}, {"file"=>"000001.jpg", "delay"=>125}], @post.pixiv_ugoira_frame_data.data)
       end
     end
+
+    context "a post that is replaced to another file then replaced back to the original file" do
+      should "not delete the original files" do
+        @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350")
+        @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364")
+        @post.replace!(replacement_url: "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350")
+
+        assert(File.exists?(@post.file_path))
+        assert(File.exists?(@post.preview_file_path))
+        assert(File.exists?(@post.large_file_path))
+
+        Timecop.travel(Time.now + PostReplacement::DELETION_GRACE_PERIOD + 1.day) do
+          Delayed::Worker.new.work_off
+        end
+
+        assert(File.exists?(@post.file_path))
+        assert(File.exists?(@post.preview_file_path))
+        assert(File.exists?(@post.large_file_path))
+      end
+    end
   end
 end