diff --git a/activemodel/lib/active_model/validations/exclusion.rb b/activemodel/lib/active_model/validations/exclusion.rb index 0a44c6d54f..7ee718cf3c 100644 --- a/activemodel/lib/active_model/validations/exclusion.rb +++ b/activemodel/lib/active_model/validations/exclusion.rb @@ -2,7 +2,6 @@ module ActiveModel module Validations class ExclusionValidator < EachValidator def check_validity! - options[:in] ||= options.delete(:within) raise ArgumentError, "An object with the method include? is required must be supplied as the " << ":in option of the configuration hash" unless options[:in].respond_to?(:include?) end diff --git a/activemodel/lib/active_model/validations/inclusion.rb b/activemodel/lib/active_model/validations/inclusion.rb index 25b8c7866d..0c1334fe1b 100644 --- a/activemodel/lib/active_model/validations/inclusion.rb +++ b/activemodel/lib/active_model/validations/inclusion.rb @@ -2,7 +2,6 @@ module ActiveModel module Validations class InclusionValidator < EachValidator def check_validity! - options[:in] ||= options.delete(:within) raise ArgumentError, "An object with the method include? is required must be supplied as the " << ":in option of the configuration hash" unless options[:in].respond_to?(:include?) end diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb index f41ce34328..871f589af9 100644 --- a/activemodel/lib/active_model/validations/length.rb +++ b/activemodel/lib/active_model/validations/length.rb @@ -1,36 +1,43 @@ module ActiveModel module Validations class LengthValidator < EachValidator - OPTIONS = [ :is, :within, :in, :minimum, :maximum ].freeze MESSAGES = { :is => :wrong_length, :minimum => :too_short, :maximum => :too_long }.freeze CHECKS = { :is => :==, :minimum => :>=, :maximum => :<= }.freeze DEFAULT_TOKENIZER = lambda { |value| value.split(//) } - attr_reader :type def initialize(options) - @type = (OPTIONS & options.keys).first + if range = (options.delete(:in) || options.delete(:within)) + raise ArgumentError, ":in and :within must be a Range" unless range.is_a?(Range) + options[:minimum], options[:maximum] = range.begin, range.end + options[:maximum] -= 1 if range.exclude_end? + end + super(options.reverse_merge(:tokenizer => DEFAULT_TOKENIZER)) end def check_validity! - ensure_one_range_option! - ensure_argument_types! + keys = CHECKS.keys & options.keys + + if keys.empty? + raise ArgumentError, 'Range unspecified. Specify the :within, :maximum, :minimum, or :is option.' + end + + keys.each do |key| + value = options[key] + + unless value.is_a?(Integer) && value >= 0 + raise ArgumentError, ":#{key} must be a nonnegative Integer" + end + end end def validate_each(record, attribute, value) - checks = options.slice(:minimum, :maximum, :is) - value = options[:tokenizer].call(value) if value.kind_of?(String) + value = options[:tokenizer].call(value) if value.kind_of?(String) - if [:within, :in].include?(type) - range = options[type] - checks[:minimum], checks[:maximum] = range.begin, range.end - checks[:maximum] -= 1 if range.exclude_end? - end - - checks.each do |key, check_value| + CHECKS.each do |key, validity_check| + next unless check_value = options[key] custom_message = options[:message] || options[MESSAGES[key]] - validity_check = CHECKS[key] valid_value = if key == :maximum value.nil? || value.size.send(validity_check, check_value) @@ -38,33 +45,8 @@ def validate_each(record, attribute, value) value && value.size.send(validity_check, check_value) end - record.errors.add(attribute, MESSAGES[key], :default => custom_message, :count => check_value) unless valid_value - end - end - - protected - - def ensure_one_range_option! #:nodoc: - range_options = OPTIONS & options.keys - - case range_options.size - when 0 - raise ArgumentError, 'Range unspecified. Specify the :within, :maximum, :minimum, or :is option.' - when 1 - # Valid number of options; do nothing. - else - raise ArgumentError, 'Too many range options specified. Choose only one.' - end - end - - def ensure_argument_types! #:nodoc: - value = options[type] - - case type - when :within, :in - raise ArgumentError, ":#{type} must be a Range" unless value.is_a?(Range) - when :is, :minimum, :maximum - raise ArgumentError, ":#{type} must be a nonnegative Integer" unless value.is_a?(Integer) && value >= 0 + next if valid_value + record.errors.add(attribute, MESSAGES[key], :default => custom_message, :count => check_value) end end end diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb index f3ef5e648a..99d0268b67 100644 --- a/activemodel/test/cases/validations/length_validation_test.rb +++ b/activemodel/test/cases/validations/length_validation_test.rb @@ -221,10 +221,8 @@ def test_validates_length_of_using_bignum end def test_validates_length_of_nasty_params - assert_raise(ArgumentError) { Topic.validates_length_of(:title, :minimum=>6, :maximum=>9) } - assert_raise(ArgumentError) { Topic.validates_length_of(:title, :within=>6, :maximum=>9) } - assert_raise(ArgumentError) { Topic.validates_length_of(:title, :within=>6, :minimum=>9) } - assert_raise(ArgumentError) { Topic.validates_length_of(:title, :within=>6, :is=>9) } + assert_raise(ArgumentError) { Topic.validates_length_of(:title, :is=>-6) } + assert_raise(ArgumentError) { Topic.validates_length_of(:title, :within=>6) } assert_raise(ArgumentError) { Topic.validates_length_of(:title, :minimum=>"a") } assert_raise(ArgumentError) { Topic.validates_length_of(:title, :maximum=>"a") } assert_raise(ArgumentError) { Topic.validates_length_of(:title, :within=>"a") }