Merge pull request #3158 from evazion/fix-expunge-post

Fix #3156: Expunging posts is broken

Gemfile

@@ -75,4 +75,5 @@ group :test do
   gem "simplecov", :require => false
   gem "timecop"
   gem "fakeweb"
+  gem "test_after_commit" # XXX remove me after upgrading to rails 5.
 end

Gemfile.lock

@@ -341,6 +341,8 @@ GEM
       rest-client (~> 1.4)
     term-ansicolor (1.3.2)
       tins (~> 1.0)
+    test_after_commit (1.1.0)
+      activerecord (>= 3.2)
     therubyracer (0.12.3)
       libv8 (~> 3.16.14.15)
       ref
@@ -438,6 +440,7 @@ DEPENDENCIES
   streamio-ffmpeg
   stripe
   term-ansicolor
+  test_after_commit
   therubyracer
   timecop
   twitter
@@ -448,4 +451,4 @@ DEPENDENCIES
   whenever
 
 BUNDLED WITH
-   1.14.5
+   1.14.6

app/models/post.rb

@@ -29,8 +29,8 @@ class Post < ActiveRecord::Base
   after_save :update_parent_on_save
   after_save :apply_post_metatags
   after_save :expire_essential_tag_string_cache
-  after_destroy :remove_iqdb_async
-  after_destroy :delete_files
+  after_commit :delete_files, :on => :destroy
+  after_commit :remove_iqdb_async, :on => :destroy
   after_commit :update_iqdb_async, :on => :create
   after_commit :notify_pubsub
 
@@ -65,11 +65,13 @@ class Post < ActiveRecord::Base
     extend ActiveSupport::Concern
 
     module ClassMethods
-      def delete_files(post_id, file_path, large_file_path, preview_file_path)
-        post = Post.find(post_id)
+      def delete_files(post_id, file_path, large_file_path, preview_file_path, force: false)
+        unless force
+          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.")
+          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
         end
 
         # the large file and the preview don't necessarily exist. if so errors will be ignored.
@@ -84,7 +86,7 @@ class Post < ActiveRecord::Base
     end
 
     def delete_files
-      Post.delete_files(id, file_path, large_file_path, preview_file_path)
+      Post.delete_files(id, file_path, large_file_path, preview_file_path, force: true)
     end
 
     def distribute_files
@@ -1711,15 +1713,7 @@ class Post < ActiveRecord::Base
     end
 
     def remove_iqdb_async
-      if File.exists?(preview_file_path) && Post.iqdb_enabled?
-        Post.iqdb_sqs_service.send_message("remove\n#{id}")
-      end
-    end
-
-    def update_iqdb
-      if Post.iqdb_enabled? && Post.iqdb_enabled?
-        Post.iqdb_sqs_service.send_message("update\n#{id}\n#{complete_preview_file_url}")
-      end
+      Post.remove_iqdb(id)
     end
   end
 

test/helpers/iqdb_test_helper.rb

@@ -16,6 +16,7 @@ module IqdbTestHelper
 
     service = mock_sqs_service.new
     Post.stubs(:iqdb_sqs_service).returns(service)
+    Post.stubs(:iqdb_enabled?).returns(true)
 
     Danbooru.config.stubs(:iqdbs_auth_key).returns("hunter2")
     Danbooru.config.stubs(:iqdbs_server).returns("http://localhost:3004")

test/test_helper.rb

@@ -48,6 +48,7 @@ class ActionController::TestCase
 end
 
 Delayed::Worker.delay_jobs = false
+TestAfterCommit.enabled = false
 
 require "helpers/reportbooru_helper"
 class ActiveSupport::TestCase

test/unit/post_test.rb

@@ -33,7 +33,32 @@ class PostTest < ActiveSupport::TestCase
   context "Deletion:" do
     context "Expunging a post" do
       setup do
-        @post = FactoryGirl.create(:post)
+        @upload = FactoryGirl.create(:jpg_upload)
+        @upload.process!
+        @post = @upload.post
+      end
+
+      should "delete the files" do
+        assert_equal(true, File.exists?(@post.preview_file_path))
+        assert_equal(true, File.exists?(@post.large_file_path))
+        assert_equal(true, File.exists?(@post.file_path))
+
+        TestAfterCommit.with_commits(true) do
+          @post.expunge!
+        end
+
+        assert_equal(false, File.exists?(@post.preview_file_path))
+        assert_equal(false, File.exists?(@post.large_file_path))
+        assert_equal(false, File.exists?(@post.file_path))
+      end
+
+      should "remove the post from iqdb" do
+        mock_iqdb_service!
+        Post.iqdb_sqs_service.expects(:send_message).with("remove\n#{@post.id}")
+
+        TestAfterCommit.with_commits(true) do
+          @post.expunge!
+        end
       end
 
       context "that is status locked" do