Fix case-sensitive validates_uniqueness_of. Closes #11366 [miloops]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9160 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

2 files changed, 61 insertions, 19 deletions

diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index aded3c0e85..5e2f710db6 100755
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -602,8 +602,8 @@ module ActiveRecord
       #
       # Configuration options:
       # * <tt>message</tt> - Specifies a custom error message (default is: "has already been taken")
-      # * <tt>scope</tt> - One or more columns by which to limit the scope of the uniquness constraint.
-      # * <tt>case_sensitive</tt> - Looks for an exact match.  Ignored by non-text columns (true by default).
+      # * <tt>scope</tt> - One or more columns by which to limit the scope of the uniqueness constraint.
+      # * <tt>case_sensitive</tt> - Looks for an exact match.  Ignored by non-text columns (false by default).
       # * <tt>allow_nil</tt> - If set to true, skips this validation if the attribute is null (default is: false)
       # * <tt>allow_blank</tt> - If set to true, skips this validation if the attribute is blank (default is: false)
       # * <tt>if</tt> - Specifies a method, proc or string to call to determine if the validation should
@@ -613,14 +613,30 @@ module ActiveRecord
       #   not occur (e.g. :unless => :skip_validation, or :unless => Proc.new { |user| user.signup_step <= 2 }).  The
       #   method, proc or string should return or evaluate to a true or false value.
       def validates_uniqueness_of(*attr_names)
-        configuration = { :message => ActiveRecord::Errors.default_error_messages[:taken], :case_sensitive => true }
+        configuration = { :message => ActiveRecord::Errors.default_error_messages[:taken] }
         configuration.update(attr_names.extract_options!)
 
         validates_each(attr_names,configuration) do |record, attr_name, value|
-          if value.nil? || (configuration[:case_sensitive] || !columns_hash[attr_name.to_s].text?)
+          # The check for an existing value should be run from a class that
+          # isn't abstract. This means working down from the current class
+          # (self), to the first non-abstract class. Since classes don't know
+          # their subclasses, we have to build the hierarchy between self and
+          # the record's class.
+          class_hierarchy = [record.class]
+          while class_hierarchy.first != self
+            class_hierarchy.insert(0, class_hierarchy.first.superclass)
+          end
+
+          # Now we can work our way down the tree to the first non-abstract
+          # class (which has a database table to query from).
+          finder_class = class_hierarchy.detect { |klass| !klass.abstract_class? }
+
+          if value.nil? || (configuration[:case_sensitive] || !finder_class.columns_hash[attr_name.to_s].text?)
             condition_sql = "#{record.class.quoted_table_name}.#{attr_name} #{attribute_condition(value)}"
             condition_params = [value]
           else
+            # sqlite has case sensitive SELECT query, while MySQL/Postgresql don't.
+            # Hence, this is needed only for sqlite.
             condition_sql = "LOWER(#{record.class.quoted_table_name}.#{attr_name}) #{attribute_condition(value)}"
             condition_params = [value.downcase]
           end
@@ -638,22 +654,24 @@ module ActiveRecord
             condition_params << record.send(:id)
           end
 
-          # The check for an existing value should be run from a class that
-          # isn't abstract. This means working down from the current class
-          # (self), to the first non-abstract class. Since classes don't know
-          # their subclasses, we have to build the hierarchy between self and
-          # the record's class.
-          class_hierarchy = [record.class]
-          while class_hierarchy.first != self
-            class_hierarchy.insert(0, class_hierarchy.first.superclass)
-          end
-
-          # Now we can work our way down the tree to the first non-abstract
-          # class (which has a database table to query from).
-          finder_class = class_hierarchy.detect { |klass| !klass.abstract_class? }
+          results = connection.select_all(
+            construct_finder_sql(
+              :select     => "#{attr_name}",
+              :from       => "#{finder_class.quoted_table_name}",
+              :conditions => [condition_sql, *condition_params]
+            )
+          )
+
+          unless results.length.zero?
+            found = true
+
+            # As MySQL/Postgres don't have case sensitive SELECT queries, we try to find duplicate
+            # column in ruby when case sensitive option
+            if configuration[:case_sensitive] && finder_class.columns_hash[attr_name.to_s].text?
+              found = results.any? { |a| a[attr_name.to_s] == value }
+            end
 
-          if finder_class.find(:first, :conditions => [condition_sql, *condition_params])
-            record.errors.add(attr_name, configuration[:message])
+            record.errors.add(attr_name, configuration[:message]) if found
           end
         end
       end
diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb
index d5b8b4d88b..d0b4902bce 100755
--- a/activerecord/test/cases/validations_test.rb
+++ b/activerecord/test/cases/validations_test.rb
@@ -437,6 +437,30 @@ class ValidationsTest < ActiveRecord::TestCase
     assert t2.save, "should save with nil"
   end
 
+  def test_validate_case_sensitive_uniqueness
+    Topic.validates_uniqueness_of(:title, :case_sensitive => true, :allow_nil => true)
+
+    t = Topic.new("title" => "I'm unique!")
+    assert t.save, "Should save t as unique"
+
+    t.content = "Remaining unique"
+    assert t.save, "Should still save t as unique"
+
+    t2 = Topic.new("title" => "I'M UNIQUE!")
+    assert t2.valid?, "Should be valid"
+    assert t2.save, "Should save t2 as unique"
+    assert !t2.errors.on(:title)
+    assert !t2.errors.on(:parent_id)
+    assert_not_equal "has already been taken", t2.errors.on(:title)
+
+    t3 = Topic.new("title" => "I'M uNiQUe!")
+    assert t3.valid?, "Should be valid"
+    assert t3.save, "Should save t2 as unique"
+    assert !t3.errors.on(:title)
+    assert !t3.errors.on(:parent_id)
+    assert_not_equal "has already been taken", t3.errors.on(:title)
+  end
+
   def test_validate_uniqueness_with_non_standard_table_names
     i1 = WarehouseThing.create(:value => 1000)
     assert !i1.valid?, "i1 should not be valid"