Changed validations from an array to a hash in prep for block specifications

lib/paperclip.rb

@@ -145,7 +145,7 @@ module Paperclip
       include InstanceMethods
       include InstanceMethods
 
 
       write_inheritable_attribute(:attachment_definitions, {}) if attachment_definitions.nil?
       write_inheritable_attribute(:attachment_definitions, {}) if attachment_definitions.nil?
-      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 :destroy_attached_files
@@ -175,23 +175,14 @@ module Paperclip
     # * +greater_than+: equivalent to :in => options[:greater_than]..Infinity
     # * +greater_than+: equivalent to :in => options[:greater_than]..Infinity
     # * +message+: error message to display, use :min and :max as replacements
     # * +message+: error message to display, use :min and :max as replacements
     def validates_attachment_size name, options = {}
     def validates_attachment_size name, options = {}
-      attachment_definitions[name][:validations] << lambda do |attachment, instance|
-        unless options[:greater_than].nil?
-          options[:in] = (options[:greater_than]..(1/0)) # 1/0 => Infinity
-        end
-        unless options[:less_than].nil?
-          options[:in] = (0..options[:less_than])
-        end
-        
-        if attachment.file? && !options[:in].include?(attachment.instance_read(:file_size).to_i)
-          min = options[:in].first
-          max = options[:in].last
+      min     = options[:greater_than] || (options[:in] && options[:in].first) || 0
+      max     = options[:less_than]    || (options[:in] && options[:in].last)  || (1.0/0)
+      range   = (min..max)
+      message = options[:message] || "file size must be between :min and :max bytes."
 
 
-          if options[:message]
-            options[:message].gsub(/:min/, min.to_s).gsub(/:max/, max.to_s)
-          else
-            "file size is not between #{min} and #{max} bytes."
-          end
+      attachment_definitions[name][:validations][:size] = lambda do |attachment, instance|
+        if attachment.file? && !range.include?(attachment.instance_read(:file_size).to_i)
+          message.gsub(/:min/, min.to_s).gsub(/:max/, max.to_s)
         end
         end
       end
       end
     end
     end
@@ -203,10 +194,9 @@ module Paperclip
 
 
     # Places ActiveRecord-style validations on the presence of a file.
     # Places ActiveRecord-style validations on the presence of a file.
     def validates_attachment_presence name, options = {}
     def validates_attachment_presence name, options = {}
-      attachment_definitions[name][:validations] << lambda do |attachment, instance|
-        unless attachment.file?
-          options[:message] || "must be set."
-        end
+      message = options[:message] || "must be set."
+      attachment_definitions[name][:validations][:presence] = lambda do |attachment, instance|
+        message unless attachment.file?
       end
       end
     end
     end
     
     
@@ -219,7 +209,7 @@ module Paperclip
     #   Allows all by default.
     #   Allows all by default.
     # * +message+: The message to display when the uploaded file has an invalid content type.
     # * +message+: The message to display when the uploaded file has an invalid content type.
     def validates_attachment_content_type name, options = {}
     def validates_attachment_content_type name, options = {}
-      attachment_definitions[name][:validations] << lambda do |attachment, instance|
+      attachment_definitions[name][:validations][:content_type] = lambda do |attachment, instance|
         valid_types = [options[:content_type]].flatten
         valid_types = [options[:content_type]].flatten
         
         
         unless attachment.original_filename.blank?
         unless attachment.original_filename.blank?

lib/paperclip/attachment.rb

@@ -10,7 +10,7 @@ module Paperclip
         :styles        => {},
         :styles        => {},
         :default_url   => "/:attachment/:style/missing.png",
         :default_url   => "/:attachment/:style/missing.png",
         :default_style => :original,
         :default_style => :original,
-        :validations   => [],
+        :validations   => {},
         :storage       => :filesystem
         :storage       => :filesystem
       }
       }
     end
     end
@@ -38,7 +38,7 @@ module Paperclip
       @options           = options
       @options           = options
       @queued_for_delete = []
       @queued_for_delete = []
       @queued_for_write  = {}
       @queued_for_write  = {}
-      @errors            = []
+      @errors            = {}
       @validation_errors = nil
       @validation_errors = nil
       @dirty             = false
       @dirty             = false
 
 
@@ -69,7 +69,7 @@ module Paperclip
 
 
       uploaded_file.binmode if uploaded_file.respond_to? :binmode
       uploaded_file.binmode if uploaded_file.respond_to? :binmode
       queue_existing_for_delete
       queue_existing_for_delete
-      @errors            = []
+      @errors            = {}
       @validation_errors = nil
       @validation_errors = nil
 
 
       return nil if uploaded_file.nil?
       return nil if uploaded_file.nil?
@@ -122,7 +122,7 @@ module Paperclip
 
 
     # Returns an array containing the errors on this attachment.
     # Returns an array containing the errors on this attachment.
     def errors
     def errors
-      @errors.compact.uniq
+      @errors
     end
     end
 
 
     # Returns true if there are changes that need to be saved.
     # Returns true if there are changes that need to be saved.
@@ -230,10 +230,12 @@ module Paperclip
 
 
     def validate #:nodoc:
     def validate #:nodoc:
       unless @validation_errors
       unless @validation_errors
-        @validation_errors = @validations.collect do |v|
-          v.call(self, instance)
-        end.flatten.compact.uniq
-        @errors += @validation_errors
+        @validation_errors = @validations.inject({}) do |errors, validation|
+          name, block = validation
+          errors[name] = block.call(self, instance) if block
+          errors
+        end
+        @errors.merge!(@validation_errors)
       end
       end
       @validation_errors
       @validation_errors
     end
     end
@@ -268,7 +270,7 @@ module Paperclip
                                                    extra_options_for(name),
                                                    extra_options_for(name),
                                                    @whiny_thumbnails)
                                                    @whiny_thumbnails)
         rescue PaperclipError => e
         rescue PaperclipError => e
-          @errors << e.message if @whiny_thumbnails
+          (@errors[:processing] ||= []) << e.message if @whiny_thumbnails
         end
         end
       end
       end
     end
     end
@@ -296,8 +298,8 @@ module Paperclip
     end
     end
 
 
     def flush_errors #:nodoc:
     def flush_errors #:nodoc:
-      @errors.each do |error|
-        instance.errors.add(name, error)
+      @errors.each do |error, message|
+        instance.errors.add(name, message) if message
       end
       end
     end
     end
 
 

test/integration_test.rb

@@ -215,7 +215,7 @@ class IntegrationTest < Test::Unit::TestCase
       Dummy.validates_attachment_presence :avatar
       Dummy.validates_attachment_presence :avatar
       @d2 = Dummy.find(@dummy.id)
       @d2 = Dummy.find(@dummy.id)
       @d2.avatar = @file
       @d2.avatar = @file
-      assert   @d2.valid?
+      assert   @d2.valid?, @d2.errors.full_messages.inspect 
       @d2.avatar = @bad_file
       @d2.avatar = @bad_file
       assert ! @d2.valid?
       assert ! @d2.valid?
       @d2.avatar = nil
       @d2.avatar = nil

test/paperclip_test.rb

@@ -135,45 +135,52 @@ class PaperclipTest < Test::Unit::TestCase
       end
       end
     end
     end
 
 
-    [[:presence,      nil,                             "5k.png",   nil],
-     [:size,          {:in => 1..10240},               "5k.png",   "12k.png"],
-     [:size2,         {:in => 1..10240},               nil,        "12k.png"],
-     [:content_type1, {:content_type => "image/png"},  "5k.png",   "text.txt"],
-     [:content_type2, {:content_type => "text/plain"}, "text.txt", "5k.png"],
-     [:content_type3, {:content_type => %r{image/.*}}, "5k.png",   "text.txt"],
-     [:content_type4, {:content_type => "image/png"},  nil,        "text.txt"]].each do |args|
-      context "with #{args[0]} validations" do
+    def self.should_validate validation, options, valid_file, invalid_file
+      context "with #{validation} validation and #{options.inspect} options" do
         setup do
         setup do
-          Dummy.class_eval do
-            send(*[:"validates_attachment_#{args[0].to_s[/[a-z_]*/]}", :avatar, args[1]].compact)
-          end
+          Dummy.send(:"validates_attachment_#{validation}", :avatar, options)
           @dummy = Dummy.new
           @dummy = Dummy.new
         end
         end
-
-        context "and a valid file" do
+        context "and assigned a valid file" do
           setup do
           setup do
-            @file = args[2] && File.new(File.join(FIXTURES_DIR, args[2]))
+            @dummy.avatar = valid_file
+            @dummy.valid?
           end
           end
-
-          should "not have any errors" do
-            @dummy.avatar = @file
-            assert @dummy.avatar.valid?
-            assert_equal 0, @dummy.avatar.errors.length
+          should "not have an error when assigned a valid file" do
+            assert_nil @dummy.avatar.errors[validation]
           end
           end
+          should "not have an error on the attachment" do
+            assert_nil @dummy.errors.on(:avatar)
           end
           end
-
-        context "and an invalid file" do
+        end
+        context "and assigned an invalid file" do
           setup do
           setup do
-            @file = args[3] && File.new(File.join(FIXTURES_DIR, args[3]))
+            @dummy.avatar = invalid_file
+            @dummy.valid?
           end
           end
-
-          should "have errors" do
-            @dummy.avatar = @file
-            assert ! @dummy.avatar.valid?
-            assert_equal 1, @dummy.avatar.errors.length
+          should "have an error when assigned a valid file" do
+            assert_not_nil @dummy.avatar.errors[validation]
           end
           end
+          should "have an error on the attachment" do
+            assert @dummy.errors.on(:avatar)
           end
           end
         end
         end
       end
       end
     end
     end
+
+    [[:presence,      {},                              "5k.png",   nil],
+     [:size,          {:in => 1..10240},               nil,        "12k.png"],
+     [:size,          {:less_than => 10240},           "5k.png",   "12k.png"],
+     [:size,          {:greater_than => 8096},         "12k.png",  "5k.png"],
+     [:content_type,  {:content_type => "image/png"},  "5k.png",   "text.txt"],
+     [:content_type,  {:content_type => "text/plain"}, "text.txt", "5k.png"],
+     [:content_type,  {:content_type => %r{image/.*}}, "5k.png",   "text.txt"]].each do |args|
+      validation, options, valid_file, invalid_file = args
+      valid_file   &&= File.new(File.join(FIXTURES_DIR, valid_file))
+      invalid_file &&= File.new(File.join(FIXTURES_DIR, invalid_file))
+      
+      should_validate validation, options, valid_file, invalid_file
+    end
+
+  end
 end
 end