Merge commit 'tb/master'

Rakefile

@@ -12,7 +12,7 @@ task :default => [:clean, :test]
 desc 'Test the paperclip plugin.'
 Rake::TestTask.new(:test) do |t|
   t.libs << 'lib' << 'profile'
-  t.pattern = 'test/**/test_*.rb'
+  t.pattern = 'test/**/*_test.rb'
   t.verbose = true
 end
 
@@ -46,6 +46,7 @@ task :clean do |t|
 end
 
 spec = Gem::Specification.new do |s| 
+  s.rubygems_version  = "1.2.0"
   s.name              = "paperclip"
   s.version           = Paperclip::VERSION
   s.author            = "Jon Yurek"
@@ -65,6 +66,9 @@ spec = Gem::Specification.new do |s|
   s.extra_rdoc_files  = ["README"]
   s.rdoc_options << '--line-numbers' << '--inline-source'
   s.requirements << "ImageMagick"
+  s.add_runtime_dependency 'right_aws'
+  s.add_development_dependency 'Shoulda'
+  s.add_development_dependency 'mocha'
 end
  
 Rake::GemPackageTask.new(spec) do |pkg| 

generators/paperclip/USAGE

 Usage:
 
-  script/generate attachment Class attachment1 attachment2
+  script/generate paperclip Class attachment1 (attachment2 ...)
   
 This will create a migration that will add the proper columns to your class's table.
\ No newline at end of file

lib/paperclip.rb

@@ -113,6 +113,12 @@ module Paperclip
     def has_attached_file name, options = {}
       include InstanceMethods
 
+      %w(file_name content_type).each do |field|
+        unless column_names.include?("#{name}_#{field}")
+          raise PaperclipError.new("#{self} model does not have required column '#{name}_#{field}'")
+        end
+      end
+
       write_inheritable_attribute(:attachment_definitions, {}) if attachment_definitions.nil?
       attachment_definitions[name] = {:validations => []}.merge(options)
 
@@ -133,7 +139,7 @@ module Paperclip
       end
 
       validates_each(name) do |record, attr, value|
-        value.send(:flush_errors)
+        value.send(:flush_errors) unless value.valid?
       end
     end
 
@@ -180,7 +186,11 @@ module Paperclip
     
     # Places ActiveRecord-style validations on the content type of the file assigned. The
     # possible options are:
-    # * +content_type+: Allowed content types.  Can be a single content type or an array.  Allows all by default.
+    # * +content_type+: Allowed content types.  Can be a single content type or an array.
+    #   Each type can be a String or a Regexp. It should be noted that Internet Explorer uploads
+    #   files with content_types that you may not expect. For example, JPEG images are given
+    #   image/pjpeg and PNGs are image/x-png, so keep that in mind when determining how you match.
+    #   Allows all by default.
     # * +message+: The message to display when the uploaded file has an invalid content type.
     def validates_attachment_content_type name, options = {}
       attachment_definitions[name][:validations] << lambda do |attachment, instance|
@@ -190,7 +200,7 @@ module Paperclip
           unless options[:content_type].blank?
             content_type = instance[:"#{name}_content_type"]
             unless valid_types.any?{|t| t === content_type }
-              options[:message] || ActiveRecord::Errors.default_error_messages[:inclusion]
+              options[:message] || "is not one of the allowed file types."
             end
           end
         end

lib/paperclip/attachment.rb

@@ -43,6 +43,8 @@ module Paperclip
 
       normalize_style_definition
       initialize_storage
+
+      logger.info("[paperclip] Paperclip attachment #{name} on #{instance.class} initialized.")
     end
 
     # What gets called when you call instance.attachment = File. It clears errors,
@@ -50,6 +52,7 @@ module Paperclip
     # the previous file for deletion, to be flushed away on #save of its host.
     def assign uploaded_file
       return nil unless valid_assignment?(uploaded_file)
+      logger.info("[paperclip] Assigning #{uploaded_file} to #{name}")
 
       queue_existing_for_delete
       @errors            = []
@@ -57,6 +60,7 @@ module Paperclip
 
       return nil if uploaded_file.nil?
 
+      logger.info("[paperclip] Writing attributes for #{name}")
       @queued_for_write[:original]        = uploaded_file.to_tempfile
       @instance[:"#{@name}_file_name"]    = uploaded_file.original_filename.strip.gsub /[^\w\d\.\-]+/, '_'
       @instance[:"#{@name}_content_type"] = uploaded_file.content_type.strip
@@ -95,6 +99,7 @@ module Paperclip
 
     # Returns true if there are any errors on this attachment.
     def valid?
+      validate
       errors.length == 0
     end
 
@@ -112,11 +117,13 @@ module Paperclip
     # the instance's errors and returns false, cancelling the save.
     def save
       if valid?
+        logger.info("[paperclip] Saving files for #{name}")
         flush_deletes
         flush_writes
         @dirty = false
         true
       else
+        logger.info("[paperclip] Errors on #{name}. Not saving.")
         flush_errors
         false
       end
@@ -166,18 +173,27 @@ module Paperclip
     # again.
     def reprocess!
       new_original = Tempfile.new("paperclip-reprocess")
-      old_original = to_file(:original)
-      new_original.write( old_original.read )
-      new_original.rewind
+      if old_original = to_file(:original)
+        new_original.write( old_original.read )
+        new_original.rewind
 
-      @queued_for_write = { :original => new_original }
-      post_process
+        @queued_for_write = { :original => new_original }
+        post_process
 
-      old_original.close if old_original.respond_to?(:close)
+        old_original.close if old_original.respond_to?(:close)
+
+        save
+      else
+        true
+      end
     end
 
     private
 
+    def logger
+      instance.logger
+    end
+
     def valid_assignment? file #:nodoc:
       file.nil? || (file.respond_to?(:original_filename) && file.respond_to?(:content_type))
     end
@@ -189,6 +205,7 @@ module Paperclip
         end.flatten.compact.uniq
         @errors += @validation_errors
       end
+      @validation_errors
     end
 
     def normalize_style_definition
@@ -206,9 +223,11 @@ module Paperclip
 
     def post_process #:nodoc:
       return if @queued_for_write[:original].nil?
+      logger.info("[paperclip] Post-processing #{name}")
       @styles.each do |name, args|
         begin
           dimensions, format = args
+          dimensions = dimensions.call(instance) if dimensions.respond_to? :call
           @queued_for_write[name] = Thumbnail.make(@queued_for_write[:original], 
                                                    dimensions,
                                                    format, 
@@ -231,6 +250,7 @@ module Paperclip
 
     def queue_existing_for_delete #:nodoc:
       return if original_filename.blank?
+      logger.info("[paperclip] Queueing the existing files for #{name} for deletion.")
       @queued_for_delete += [:original, *@styles.keys].uniq.map do |style|
         path(style) if exists?(style)
       end.compact

lib/paperclip/geometry.rb

@@ -76,9 +76,9 @@ module Paperclip
     # overhanging image would be cropped. Useful for square thumbnail images. The cropping 
     # is weighted at the center of the Geometry.
     def transformation_to dst, crop = false
-      ratio = Geometry.new( dst.width / self.width, dst.height / self.height )
 
       if crop
+        ratio = Geometry.new( dst.width / self.width, dst.height / self.height )
         scale_geometry, scale = scaling(dst, ratio)
         crop_geometry         = cropping(dst, ratio, scale)
       else

lib/paperclip/storage.rb

@@ -9,7 +9,7 @@ module Paperclip
     #   almost all cases, should) be coordinated with the value of the +url+ option to
     #   allow files to be saved into a place where Apache can serve them without
     #   hitting your app. Defaults to 
-    #   ":rails_root/public/:class/:attachment/:id/:style_:filename". 
+    #   ":rails_root/public/:attachment/:id/:style/:basename.:extension"
     #   By default this places the files in the app's public directory which can be served 
     #   directly. If you are using capistrano for deployment, a good idea would be to 
     #   make a symlink to the capistrano-created system directory from inside your app's 
@@ -36,8 +36,10 @@ module Paperclip
       alias_method :to_io, :to_file
 
       def flush_writes #:nodoc:
+        logger.info("[paperclip] Writing files for #{name}")
         @queued_for_write.each do |style, file|
           FileUtils.mkdir_p(File.dirname(path(style)))
+          logger.info("[paperclip] -> #{path(style)}")
           result = file.stream_to(path(style))
           file.close
           result.close
@@ -46,8 +48,10 @@ module Paperclip
       end
 
       def flush_deletes #:nodoc:
+        logger.info("[paperclip] Deleting files for #{name}")
         @queued_for_delete.each do |path|
           begin
+            logger.info("[paperclip] -> #{path}")
             FileUtils.rm(path) if File.exist?(path)
           rescue Errno::ENOENT => e
             # ignore file-not-found, let everything else pass
@@ -102,6 +106,7 @@ module Paperclip
         base.class.interpolations[:s3_url] = lambda do |attachment, style|
           "https://s3.amazonaws.com/#{attachment.bucket_name}/#{attachment.path(style).gsub(%r{^/}, "")}"
         end
+        ActiveRecord::Base.logger.info("[paperclip] S3 Storage Initalized.")
       end
 
       def s3
@@ -135,8 +140,10 @@ module Paperclip
       alias_method :to_io, :to_file
 
       def flush_writes #:nodoc:
+        logger.info("[paperclip] Writing files for #{name}")
         @queued_for_write.each do |style, file|
           begin
+            logger.info("[paperclip] -> #{path(style)}")
             key = s3_bucket.key(path(style))
             key.data = file
             key.put(nil, @s3_permissions)
@@ -148,8 +155,10 @@ module Paperclip
       end
 
       def flush_deletes #:nodoc:
+        logger.info("[paperclip] Writing files for #{name}")
         @queued_for_delete.each do |path|
           begin
+            logger.info("[paperclip] -> #{path(style)}")
             if file = s3_bucket.key(path)
               file.delete
             end

lib/paperclip/upfile.rb

@@ -6,12 +6,15 @@ module Paperclip
 
     # Infer the MIME-type of the file from the extension.
     def content_type
-      type = self.path.match(/\.(\w+)$/)[1] rescue "octet-stream"
+      type = (self.path.match(/\.(\w+)$/)[1] rescue "octet-stream").downcase
       case type
-      when "jpg", "png", "gif" then "image/#{type}"
-      when "txt" then "text/plain"
-      when "csv", "xml", "html", "htm", "css", "js" then "text/#{type}"
-      else "x-application/#{type}"
+      when %r"jpe?g"                 then "image/jpeg"
+      when %r"tiff?"                 then "image/tiff"
+      when %r"png", "gif", "bmp"     then "image/#{type}"
+      when "txt"                     then "text/plain"
+      when %r"html?"                 then "text/html"
+      when "csv", "xml", "css", "js" then "text/#{type}"
+      else "application/x-#{type}"
       end
     end
 
@@ -31,3 +34,4 @@ end
 class File #:nodoc:
   include Paperclip::Upfile
 end
+

tasks/paperclip_tasks.rake

@@ -14,25 +14,66 @@ def obtain_attachments
   end
 end
 
+def for_all_attachments
+  klass = obtain_class
+  names = obtain_attachments
+  ids   = klass.connection.select_values("SELECT id FROM #{klass.table_name}")
+
+  ids.each do |id|
+    instance = klass.find(id)
+    names.each do |name|
+      result = if instance.send("#{ name }?")
+                 yield(instance, name)
+               else
+                 true
+               end
+      print result ? "." : "x"; $stdout.flush
+    end
+  end
+  puts " Done."
+end
+
 namespace :paperclip do
-  desc "Regenerates thumbnails for a given CLASS (and optional ATTACHMENT)"
-  task :refresh => :environment do
-    klass     = obtain_class
-    names     = obtain_attachments
-    instances = klass.find(:all)
-    
-    puts "Regenerating thumbnails for #{instances.length} instances of #{klass.name}:"
-    instances.each do |instance|
-      names.each do |name|
-        result = if instance.send("#{ name }?")
-          instance.send(name).reprocess!
-          instance.send(name).save
+  desc "Refreshes both metadata and thumbnails."
+  task :refresh => ["paperclip:refresh:metadata", "paperclip:refresh:thumbnails"]
+
+  namespace :refresh do
+    desc "Regenerates thumbnails for a given CLASS (and optional ATTACHMENT)."
+    task :thumbnails => :environment do
+      errors = []
+      for_all_attachments do |instance, name|
+        result = instance.send(name).reprocess!
+        errors << [instance.id, instance.errors] unless instance.errors.blank?
+        result
+      end
+      errors.each{|e| puts "#{e.first}: #{e.last.full_messages.inspect}" }
+    end
+
+    desc "Regenerates content_type/size metadata for a given CLASS (and optional ATTACHMENT)."
+    task :metadata => :environment do
+      for_all_attachments do |instance, name|
+        if file = instance.send(name).to_file
+          instance.send("#{name}_file_name=", instance.send("#{name}_file_name").strip)
+          instance.send("#{name}_content_type=", file.content_type.strip)
+          instance.send("#{name}_file_size=", file.size) if instance.respond_to?("#{name}_file_size")
+          instance.save(false)
         else
           true
         end
-        print result ? "." : "x"; $stdout.flush
       end
     end
-    puts " Done."
+  end
+
+  desc "Cleans out invalid attachments. Useful after you've added new validations."
+  task :clean => :environment do
+    for_all_attachments do |instance, name|
+      instance.send(name).send(:validate)
+      if instance.send(name).valid?
+        true
+      else
+        instance.send("#{name}=", nil)
+        instance.save
+      end
+    end
   end
 end

test/test_attachment.rb → test/attachment_test.rb

@@ -155,6 +155,7 @@ class AttachmentTest < Test::Unit::TestCase
       @instance.stubs(:[]).with(:test_content_type).returns(nil)
       @instance.stubs(:[]).with(:test_file_size).returns(nil)
       @instance.stubs(:[]).with(:test_updated_at).returns(nil)
+      @instance.stubs(:logger).returns(ActiveRecord::Base.logger)
       @attachment = Paperclip::Attachment.new(:test,
                                               @instance)
       @file = File.new(File.join(File.dirname(__FILE__),

test/test_paperclip.rb → test/paperclip_test.rb

@@ -7,6 +7,14 @@ class PaperclipTest < Test::Unit::TestCase
       @file = File.new(File.join(FIXTURES_DIR, "5k.png"))
     end
 
+    should "error when trying to also create a 'blah' attachment" do
+      assert_raises(Paperclip::PaperclipError) do
+        Dummy.class_eval do
+          has_attached_file :blah
+        end
+      end
+    end
+
     context "that is attr_protected" do
       setup do
         Dummy.class_eval do
@@ -64,6 +72,38 @@ class PaperclipTest < Test::Unit::TestCase
       assert Dummy.new.respond_to?(:avatar=)
     end
 
+    context "that is valid" do
+      setup do
+        @dummy = Dummy.new
+        @dummy.avatar = @file
+      end
+
+      should "be valid" do
+        assert @dummy.valid?
+      end
+
+      context "then has a validation added that makes it invalid" do
+        setup do
+          assert @dummy.save
+          Dummy.class_eval do
+            validates_attachment_content_type :avatar, :content_type => ["text/plain"]
+          end
+          @dummy2 = Dummy.find(@dummy.id)
+        end
+
+        should "be invalid when reloaded" do
+          assert ! @dummy2.valid?, @dummy2.errors.inspect
+        end
+
+        should "be able to call #valid? twice without having duplicate errors" do
+          @dummy2.avatar.valid?
+          first_errors = @dummy2.avatar.errors
+          @dummy2.avatar.valid?
+          assert_equal first_errors, @dummy2.avatar.errors
+        end
+      end
+    end
+
     [[:presence,      nil,                             "5k.png",   nil],
      [:size,          {:in => 1..10240},               "5k.png",   "12k.png"],
      [:size2,         {:in => 1..10240},               nil,        "12k.png"],
@@ -102,21 +142,6 @@ class PaperclipTest < Test::Unit::TestCase
             assert_equal 1, @dummy.avatar.errors.length
           end
         end
-        
-#        context "and an invalid file with :message" do
-#          setup do
-#            @file = args[3] && File.new(File.join(FIXTURES_DIR, args[3]))
-#          end
-#
-#          should "have errors" do
-#            if args[1] && args[1][:message] && args[4]
-#              @dummy.avatar = @file
-#              assert ! @dummy.avatar.valid?
-#              assert_equal 1, @dummy.avatar.errors.length
-#              assert_equal args[4], @dummy.avatar.errors[0]
-#            end
-#          end
-#        end
       end
     end
   end