Fixed up some issues regarding file names and deleting attachments.

git-svn-id: https://svn.thoughtbot.com/plugins/paperclip/trunk@169 7bbfaf0e-4d1d-0410-9690-a8bb5f8ef2aa

lib/paperclip.rb

@@ -21,6 +21,7 @@ module Thoughtbot
           attachments[attr] = (attachments[attr] || {:name => attr}).merge(options)
 
           define_method "#{attr}=" do |uploaded_file|
+            return unless is_a_file? uploaded_file
             attachments[attr].merge!({
               :dirty        => true,
               :files        => {:original => uploaded_file},
@@ -39,6 +40,10 @@ module Thoughtbot
             uploaded_file
           end
           
+          define_method attr do
+            self.send("#{attr}_file_name")
+          end
+          
           define_method "#{attr}_attachment" do
             attachments[attr]
           end
@@ -61,9 +66,15 @@ module Thoughtbot
             end
           end
           
+          define_method "destroy_#{attr}" do
+            if attachments[attr].keys.any?
+              delete_attachment attachments[attr]
+            end
+          end
+
           define_method "#{attr}_after_save" do
             if attachments[attr].keys.any?
-              write_attachment attachments[attr]
+              write_attachment attachments[attr] if attachments[attr][:files]
               attachments[attr][:dirty] = false
               attachments[attr][:files] = nil
             end
@@ -84,6 +95,8 @@ module Thoughtbot
 
     module InstanceMethods
       
+      private
+      
       def path_for attachment, style = :original
         prefix = File.join(attachment[:path_prefix], attachment[:path])
         prefix.gsub!(/:rails_root/, RAILS_ROOT)
@@ -103,7 +116,7 @@ module Thoughtbot
       end
       
       def ensure_directories_for attachment
-        attachment[:files].keys.each do |style|
+        attachment[:files].each do |style, file|
           dirname = File.dirname(path_for(attachment, style))
           FileUtils.mkdir_p dirname
         end
@@ -121,12 +134,15 @@ module Thoughtbot
       
       def delete_attachment attachment
         (attachment[:thumbnails].keys + [:original]).each do |style|
-          FileUtils.rm path_for(attachment, style)
+          file_path = path_for(attachment, style)
+          FileUtils.rm(file_path)
         end
+        self.update_attribute "#{attachment[:name]}_file_name", nil
+        self.update_attribute "#{attachment[:name]}_content_type", nil
       end
 
       def make_thumbnail orig_io, geometry
-        thumb = IO.popen("convert - -scale '#{geometry}' - > /dev/stdout", "w+") do |io|
+        thumb = IO.popen("convert - -scale '#{geometry}' -", "w+") do |io|
           orig_io.rewind
           io.write(orig_io.read)
           io.close_write
@@ -136,6 +152,12 @@ module Thoughtbot
         StringIO.new(thumb)
       end
       
+      def is_a_file? data
+        [:size, :content_type, :original_filename].map do |meth|
+          data.respond_to? meth
+        end.all?
+      end
+
       def sanitize_filename filename
         File.basename(filename).gsub(/[^\w\.\_]/,'_')
       end
@@ -155,6 +177,10 @@ module Thoughtbot
       def original_filename
         self.path
       end
+      
+      def size
+        File.size(self)
+      end
     end
   end
 end

test/paperclip_images_test.rb

@@ -17,9 +17,9 @@ class PaperclipImagesTest < Test::Unit::TestCase
 
   def test_should_save_the_file_and_its_thumbnails
     assert @foo.save
-    assert File.exists?( @foo.image_filename(:original) )
-    assert File.exists?( @foo.image_filename(:medium) )
-    assert File.exists?( @foo.image_filename(:thumb) )
+    assert File.exists?( @foo.image_filename(:original) ), @foo.image_filename(:original)
+    assert File.exists?( @foo.image_filename(:medium) ), @foo.image_filename(:medium)
+    assert File.exists?( @foo.image_filename(:thumb) ), @foo.image_filename(:thumb)
     assert File.size?(   @foo.image_filename(:original) )
     assert File.size?(   @foo.image_filename(:medium) )
     assert File.size?(   @foo.image_filename(:thumb) )
@@ -36,10 +36,9 @@ class PaperclipImagesTest < Test::Unit::TestCase
   
   def test_should_delete_all_thumbnails_on_destroy
     assert @foo.save
+    names = [:original, :medium, :thumb].map{|style| @foo.image_filename(style) }
     assert @foo.destroy
-    assert !File.exists?( @foo.image_filename(:original) )
-    assert !File.exists?( @foo.image_filename(:medium) )
-    assert !File.exists?( @foo.image_filename(:thumb) )
+    names.each {|path| assert !File.exists?( path ), path }
   end
   
 end
\ No newline at end of file

test/paperclip_test.rb

@@ -33,10 +33,11 @@ class PaperclipTest < Test::Unit::TestCase
   
   def test_should_delete_files_on_destroy
     assert @bar.save
-    assert File.exists?( @bar.document_filename )
+    assert File.exists?( @bar.document_filename ), @bar.document_filename
     
+    document_filename = @bar.document_filename
     assert @bar.destroy
-    assert !File.exists?( @bar.document_filename )
+    assert !File.exists?( document_filename ), document_filename
   end
 
 end
\ No newline at end of file