Fixing destroy callback to delete files just when model destroy method was…

Fixing destroy callback to delete files just when model destroy method was completed successfully (i.e. no exception like 'foreign key constraint violations' was raised)

lib/paperclip.rb

@@ -271,7 +271,8 @@ module Paperclip
       attachment_definitions[name] = {:validations => []}.merge(options)
       attachment_definitions[name] = {:validations => []}.merge(options)
 
 
       after_save :save_attached_files
       after_save :save_attached_files
-      before_destroy :destroy_attached_files
+      before_destroy :prepare_for_destroy
+      after_destroy :destroy_attached_files
 
 
       define_paperclip_callbacks :post_process, :"#{name}_post_process"
       define_paperclip_callbacks :post_process, :"#{name}_post_process"
 
 
@@ -407,10 +408,17 @@ module Paperclip
     def destroy_attached_files
     def destroy_attached_files
       Paperclip.log("Deleting attachments.")
       Paperclip.log("Deleting attachments.")
       each_attachment do |name, attachment|
       each_attachment do |name, attachment|
-        attachment.send(:queue_existing_for_delete)
         attachment.send(:flush_deletes)
         attachment.send(:flush_deletes)
       end
       end
     end
     end
+    
+    def prepare_for_destroy
+      Paperclip.log("Scheduling attachments for deletion.")
+      each_attachment do |name, attachment|
+        attachment.send(:queue_existing_for_delete)
+      end
+    end
+    
   end
   end
 
 
 end
 end

test/attachment_test.rb

@@ -1076,4 +1076,32 @@ class AttachmentTest < Test::Unit::TestCase
       assert_equal "hello", attachment.url
       assert_equal "hello", attachment.url
     end
     end
   end
   end
+  
+  context "An attached file" do
+    setup do
+      rebuild_model
+      @dummy = Dummy.new
+      @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "5k.png"), 'rb')
+      @dummy.avatar = @file
+      @dummy.save!
+      @attachment = @dummy.avatar 
+      @path = @attachment.path      
+    end
+
+    should "not be deleted when the model fails to destroy" do
+      @dummy.stubs(:destroy).raises(Exception)
+
+      assert_raise Exception do
+        @dummy.destroy
+      end
+
+      assert File.exists?(@path)
+    end
+
+    should "be deleted when the model is destroyed" do
+      @dummy.destroy
+      assert ! File.exists?(@path)
+    end
+  end
+  
 end
 end