Close and unlink tempfiles after flush write

Fixes #902

lib/paperclip/attachment.rb

@@ -440,6 +440,10 @@ module Paperclip
 
 
     # called by storage after the writes are flushed and before @queued_for_writes is cleared
     # called by storage after the writes are flushed and before @queued_for_writes is cleared
     def after_flush_writes
     def after_flush_writes
+       @queued_for_write.each do |style, file|
+         file.close unless file.closed?
+         file.unlink if file.respond_to?(:unlink) && file.path.present? && File.exist?(file.path)
+       end
     end
     end
 
 
     def cleanup_filename(filename)
     def cleanup_filename(filename)

lib/paperclip/io_adapters/abstract_adapter.rb

@@ -3,7 +3,7 @@ require 'active_support/core_ext/module/delegation'
 module Paperclip
 module Paperclip
   class AbstractAdapter
   class AbstractAdapter
     attr_reader :content_type, :original_filename, :size
     attr_reader :content_type, :original_filename, :size
-    delegate :close, :closed?, :eof?, :path, :rewind, :to => :@tempfile
+    delegate :close, :closed?, :eof?, :path, :rewind, :unlink, :to => :@tempfile
 
 
     def fingerprint
     def fingerprint
       @fingerprint ||= Digest::MD5.file(path).to_s
       @fingerprint ||= Digest::MD5.file(path).to_s

test/io_adapters/abstract_adapter_test.rb

@@ -33,7 +33,7 @@ class AbstractAdapterTest < Test::Unit::TestCase
       @adapter.tempfile = stub("Tempfile")
       @adapter.tempfile = stub("Tempfile")
     end
     end
 
 
-    [:close, :closed?, :eof?, :path, :rewind].each do |method|
+    [:close, :closed?, :eof?, :path, :rewind, :unlink].each do |method|
       should "delegate #{method} to @tempfile" do
       should "delegate #{method} to @tempfile" do
         @adapter.tempfile.stubs(method)
         @adapter.tempfile.stubs(method)
         @adapter.public_send(method)
         @adapter.public_send(method)

test/io_adapters/attachment_adapter_test.rb

@@ -63,7 +63,8 @@ class AttachmentAdapterTest < Test::Unit::TestCase
 
 
       @attachment.assign(@file)
       @attachment.assign(@file)
     
     
-      @thumb = @attachment.queued_for_write[:thumb]
+      @thumb = Tempfile.new("thumbnail").tap(&:binmode)
+      FileUtils.cp @attachment.queued_for_write[:thumb].path, @thumb.path
     
     
       @attachment.save
       @attachment.save
       @subject = Paperclip.io_adapters.for(@attachment.styles[:thumb])
       @subject = Paperclip.io_adapters.for(@attachment.styles[:thumb])
@@ -71,6 +72,7 @@ class AttachmentAdapterTest < Test::Unit::TestCase
 
 
     teardown do
     teardown do
       @file.close
       @file.close
+      @thumb.close
     end
     end
 
 
     should "get the original filename" do
     should "get the original filename" do

test/storage/filesystem_test.rb

@@ -28,10 +28,19 @@ class FileSystemTest < Test::Unit::TestCase
       end
       end
 
 
       should "be rewinded after flush_writes" do
       should "be rewinded after flush_writes" do
-        files = @dummy.avatar.queued_for_write.map{ |style, file| file }
+        @dummy.avatar.instance_eval "def after_flush_writes; end"
+
+        files = @dummy.avatar.queued_for_write.values
         @dummy.save
         @dummy.save
         assert files.none?(&:eof?), "Expect all the files to be rewinded."
         assert files.none?(&:eof?), "Expect all the files to be rewinded."
       end
       end
+
+      should "be removed after after_flush_writes" do
+        paths = @dummy.avatar.queued_for_write.values.map(&:path)
+        @dummy.save
+        assert paths.none?{ |path| File.exists?(path) },
+          "Expect all the files to be deleted."
+      end
     end
     end
 
 
     context "with file that has space in file name" do
     context "with file that has space in file name" do

test/storage/fog_test.rb

@@ -112,11 +112,20 @@ class FogTest < Test::Unit::TestCase
       end
       end
 
 
       should "be rewinded after flush_writes" do
       should "be rewinded after flush_writes" do
-        files = @dummy.avatar.queued_for_write.map{ |style, file| file }
+        @dummy.avatar.instance_eval "def after_flush_writes; end"
+
+        files = @dummy.avatar.queued_for_write.values
         @dummy.save
         @dummy.save
         assert files.none?(&:eof?), "Expect all the files to be rewinded."
         assert files.none?(&:eof?), "Expect all the files to be rewinded."
       end
       end
 
 
+      should "be removed after after_flush_writes" do
+        paths = @dummy.avatar.queued_for_write.values.map(&:path)
+        @dummy.save
+        assert paths.none?{ |path| File.exists?(path) },
+          "Expect all the files to be deleted."
+      end
+
       should "pass the content type to the Fog::Storage::AWS::Files instance" do
       should "pass the content type to the Fog::Storage::AWS::Files instance" do
         Fog::Storage::AWS::Files.any_instance.expects(:create).with do |hash|
         Fog::Storage::AWS::Files.any_instance.expects(:create).with do |hash|
           hash[:content_type]
           hash[:content_type]

test/storage/s3_test.rb

@@ -586,11 +586,20 @@ class S3Test < Test::Unit::TestCase
       end
       end
 
 
       should "be rewinded after flush_writes" do
       should "be rewinded after flush_writes" do
-        files = @dummy.avatar.queued_for_write.map{ |style, file| file.tap(&:read) }
+        @dummy.avatar.instance_eval "def after_flush_writes; end"
+
+        files = @dummy.avatar.queued_for_write.values.each(&:read)
         @dummy.save
         @dummy.save
         assert files.none?(&:eof?), "Expect all the files to be rewinded."
         assert files.none?(&:eof?), "Expect all the files to be rewinded."
       end
       end
 
 
+      should "be removed after after_flush_writes" do
+        paths = @dummy.avatar.queued_for_write.values.map(&:path)
+        @dummy.save
+        assert paths.none?{ |path| File.exists?(path) },
+          "Expect all the files to be deleted."
+      end
+
       context "and saved" do
       context "and saved" do
         setup do
         setup do
           object = stub
           object = stub