From c52771e7a022a45f2b70bfc8f29b02d008fb9b15 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 31 Mar 2008 01:50:07 +0000 Subject: 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 --- activerecord/lib/active_record/validations.rb | 56 ++++++++++++++++++--------- activerecord/test/cases/validations_test.rb | 24 ++++++++++++ 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: # * message - Specifies a custom error message (default is: "has already been taken") - # * scope - One or more columns by which to limit the scope of the uniquness constraint. - # * case_sensitive - Looks for an exact match. Ignored by non-text columns (true by default). + # * scope - One or more columns by which to limit the scope of the uniqueness constraint. + # * case_sensitive - Looks for an exact match. Ignored by non-text columns (false by default). # * allow_nil - If set to true, skips this validation if the attribute is null (default is: false) # * allow_blank - If set to true, skips this validation if the attribute is blank (default is: false) # * if - 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" -- cgit v1.2.3