From 0317b93c17a46d7663a8c36edc26ad0ba3d75f85 Mon Sep 17 00:00:00 2001
From: Charles Bergeron <chuck.bergeron@gmail.com>
Date: Mon, 27 May 2013 23:07:05 -0700
Subject: Use Range#cover? for Numeric ranges (tests via endpoints) and use
 Range#include? for non-numeric ranges

added changelog message
---
 activemodel/CHANGELOG.md                                        | 4 ++++
 activemodel/lib/active_model/validations/clusivity.rb           | 7 ++++---
 activemodel/lib/active_model/validations/inclusion.rb           | 2 +-
 activemodel/test/cases/validations/inclusion_validation_test.rb | 1 +
 4 files changed, 10 insertions(+), 4 deletions(-)

(limited to 'activemodel')

diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md
index 8c7af2d078..b73d688964 100644
--- a/activemodel/CHANGELOG.md
+++ b/activemodel/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   `validates_inclusion_of` for ranges uses the speedy Range#cover for numerical ranges, and the accurate Range#include? for non-numerical ranges.
+
+    *Charles Bergeron*
+
 *   Deprecate `Validator#setup`. This should be done manually now in the validator's constructor.
 
     *Nick Sutterer*
diff --git a/activemodel/lib/active_model/validations/clusivity.rb b/activemodel/lib/active_model/validations/clusivity.rb
index 49df98d6c1..c5aacb010a 100644
--- a/activemodel/lib/active_model/validations/clusivity.rb
+++ b/activemodel/lib/active_model/validations/clusivity.rb
@@ -31,10 +31,11 @@ module ActiveModel
       end
 
       # In Ruby 1.9 <tt>Range#include?</tt> on non-numeric ranges checks all possible values in the
-      # range for equality, so it may be slow for large ranges. The new <tt>Range#cover?</tt>
-      # uses the previous logic of comparing a value with the range endpoints.
+      # range for equality, which is slower but more accurate. <tt>Range#cover?</tt> uses
+      # the previous logic of comparing a value with the range endpoints, which is fast
+      # but is only accurate on numeric ranges.
       def inclusion_method(enumerable)
-        enumerable.is_a?(Range) ? :cover? : :include?
+        (enumerable.is_a?(Range) && enumerable.first.is_a?(Numeric)) ? :cover? : :include?
       end
     end
   end
diff --git a/activemodel/lib/active_model/validations/inclusion.rb b/activemodel/lib/active_model/validations/inclusion.rb
index 1cfd86efee..24337614c5 100644
--- a/activemodel/lib/active_model/validations/inclusion.rb
+++ b/activemodel/lib/active_model/validations/inclusion.rb
@@ -28,7 +28,7 @@ module ActiveModel
       # Configuration options:
       # * <tt>:in</tt> - An enumerable object of available items. This can be
       #   supplied as a proc, lambda or symbol which returns an enumerable. If the
-      #   enumerable is a range the test is performed with <tt>Range#cover?</tt>,
+      #   enumerable is a numerical range the test is performed with <tt>Range#cover?</tt>,
       #   otherwise with <tt>include?</tt>.
       # * <tt>:within</tt> - A synonym(or alias) for <tt>:in</tt>
       # * <tt>:message</tt> - Specifies a custom error message (default is: "is
diff --git a/activemodel/test/cases/validations/inclusion_validation_test.rb b/activemodel/test/cases/validations/inclusion_validation_test.rb
index ceec9dc256..01a373d85d 100644
--- a/activemodel/test/cases/validations/inclusion_validation_test.rb
+++ b/activemodel/test/cases/validations/inclusion_validation_test.rb
@@ -14,6 +14,7 @@ class InclusionValidationTest < ActiveModel::TestCase
     Topic.validates_inclusion_of(:title, in: 'aaa'..'bbb')
     assert Topic.new("title" => "bbc", "content" => "abc").invalid?
     assert Topic.new("title" => "aa", "content" => "abc").invalid?
+    assert Topic.new("title" => "aaab", "content" => "abc").invalid?
     assert Topic.new("title" => "aaa", "content" => "abc").valid?
     assert Topic.new("title" => "abc", "content" => "abc").valid?
     assert Topic.new("title" => "bbb", "content" => "abc").valid?
-- 
cgit v1.2.3