Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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

…cations
  • Loading branch information...
commit cd02525e96f9023706f459f920d84f970e47c3f5 1 parent b48cea7
@jyurek jyurek authored
View
36 lib/paperclip.rb
@@ -145,7 +145,7 @@ def has_attached_file name, options = {}
include InstanceMethods
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
before_destroy :destroy_attached_files
@@ -175,23 +175,14 @@ def has_attached_file name, options = {}
# * +greater_than+: equivalent to :in => options[:greater_than]..Infinity
# * +message+: error message to display, use :min and :max as replacements
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
-
- 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
+ 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."
+
+ 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
@@ -203,10 +194,9 @@ def validates_attachment_thumbnails name, options = {}
# Places ActiveRecord-style validations on the presence of a file.
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
@@ -219,7 +209,7 @@ def validates_attachment_presence name, options = {}
# 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|
+ attachment_definitions[name][:validations][:content_type] = lambda do |attachment, instance|
valid_types = [options[:content_type]].flatten
unless attachment.original_filename.blank?
View
24 lib/paperclip/attachment.rb
@@ -10,7 +10,7 @@ def self.default_options
:styles => {},
:default_url => "/:attachment/:style/missing.png",
:default_style => :original,
- :validations => [],
+ :validations => {},
:storage => :filesystem
}
end
@@ -38,7 +38,7 @@ def initialize name, instance, options = {}
@options = options
@queued_for_delete = []
@queued_for_write = {}
- @errors = []
+ @errors = {}
@validation_errors = nil
@dirty = false
@@ -69,7 +69,7 @@ def assign uploaded_file
uploaded_file.binmode if uploaded_file.respond_to? :binmode
queue_existing_for_delete
- @errors = []
+ @errors = {}
@validation_errors = nil
return nil if uploaded_file.nil?
@@ -122,7 +122,7 @@ def valid?
# Returns an array containing the errors on this attachment.
def errors
- @errors.compact.uniq
+ @errors
end
# Returns true if there are changes that need to be saved.
@@ -230,10 +230,12 @@ def valid_assignment? file #:nodoc:
def validate #:nodoc:
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
@validation_errors
end
@@ -268,7 +270,7 @@ def post_process #:nodoc:
extra_options_for(name),
@whiny_thumbnails)
rescue PaperclipError => e
- @errors << e.message if @whiny_thumbnails
+ (@errors[:processing] ||= []) << e.message if @whiny_thumbnails
end
end
end
@@ -296,8 +298,8 @@ def queue_existing_for_delete #:nodoc:
end
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
View
2  test/integration_test.rb
@@ -215,7 +215,7 @@ class IntegrationTest < Test::Unit::TestCase
Dummy.validates_attachment_presence :avatar
@d2 = Dummy.find(@dummy.id)
@d2.avatar = @file
- assert @d2.valid?
+ assert @d2.valid?, @d2.errors.full_messages.inspect
@d2.avatar = @bad_file
assert ! @d2.valid?
@d2.avatar = nil
View
61 test/paperclip_test.rb
@@ -135,45 +135,52 @@ class ::SubDummy < Dummy; 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
- 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
end
-
- context "and a valid file" do
+ context "and assigned a valid file" do
setup do
- @file = args[2] && File.new(File.join(FIXTURES_DIR, args[2]))
+ @dummy.avatar = valid_file
+ @dummy.valid?
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
+ should "not have an error on the attachment" do
+ assert_nil @dummy.errors.on(:avatar)
end
end
-
- context "and an invalid file" do
+ context "and assigned an invalid file" do
setup do
- @file = args[3] && File.new(File.join(FIXTURES_DIR, args[3]))
+ @dummy.avatar = invalid_file
+ @dummy.valid?
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
+ should "have an error on the attachment" do
+ assert @dummy.errors.on(:avatar)
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

3 comments on commit cd02525

@joshpencheon

I’m having some problems with Paperclip 2.1.2 (sudo gem update isn’t finding newer versions…)

When I try to validate_attachment_presense :image it doesn’t work. I’ve managed to hack it, like so:

  before_validation :validate_image

  private
  
 # This then seems to trigger #flush_errors...
  def validate_image
    image.send(:validate)
  end

Am I doing something wrong, or is this a bug?

Thanks, Josh

@joshpencheon

To clarify, when I say ’doesn’t work’ I mean that with

validates_attachment_presense :image

…I get:

my_file = AttachedFile.new
my_file.valid? # => true

Doing this doesn’t seem very nice:

# Untested
my_file = AttachedFile.new
my_file.document = nil if my_file.docuement.original_filename.blank?
my_file.valid? # => false
@glennpow

This change broke paperclip, since now the validations that return nil messages (I.E. when a validation succeeds) are not stripped away as they were with the .compact call on the array of @validation_errors previously. Should be something like:

def validate #:nodoc: unless @validation_errors @validation_errors = @validations.inject({}) do |errors, validation| name, block = validation errors[name] = block.call(self, instance) if block errors end.reject { |name, error| error.nil? } errors.merge!(validation_errors) end @validation_errors end
Please sign in to comment.
Something went wrong with that request. Please try again.