validates_uniqueness_of behaves well with abstract superclasses. References #3833, closes #9886.

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

4 files changed, 68 insertions, 2 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 796c7274a7..26e0a7f9c2 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -29,7 +29,8 @@
 
 * MySQL: fix change_column on not-null columns that don't accept dfeault values of ''.  #6663 [Jonathan Viney, Tarmo Tänav]
 
-* validates_uniqueness_of behaves well with single-table inheritance.  #3833 [Gabriel Gironda, rramdas, François Beausoleil, Josh Peek, Tarmo Tänav]
+* validates_uniqueness_of behaves well with abstract superclasses and
+single-table inheritance.  #3833, #9886 [Gabriel Gironda, rramdas, François Beausoleil, Josh Peek, Tarmo Tänav, pat]
 
 * Raise ProtectedAttributeAssignmentError in development and test environments when mass-assigning to an attr_protected attribute.  #9802 [Henrik N]
 
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 6514c4077a..35a1f8d0d4 100755
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -633,7 +633,21 @@ module ActiveRecord
             condition_params << record.send(:id)
           end
 
-          if find(:first, :conditions => [condition_sql, *condition_params])
+          # 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 finder_class.find(:first, :conditions => [condition_sql, *condition_params])
             record.errors.add(attr_name, configuration[:message])
           end
         end
diff --git a/activerecord/test/fixtures/db_definitions/schema.rb b/activerecord/test/fixtures/db_definitions/schema.rb
index 88f99f51ac..dcb6222cd3 100644
--- a/activerecord/test/fixtures/db_definitions/schema.rb
+++ b/activerecord/test/fixtures/db_definitions/schema.rb
@@ -289,4 +289,10 @@ ActiveRecord::Schema.define do
     t.column :message, :string, :null=>false
     t.column :developer_id, :integer, :null=>false
   end
+
+  create_table :inept_wizards, :force => true do |t|
+    t.column :name, :string, :null => false
+    t.column :city, :string, :null => false
+    t.column :type, :string
+  end
 end
diff --git a/activerecord/test/validations_test.rb b/activerecord/test/validations_test.rb
index 49fc30eff7..2f67e9a36d 100755
--- a/activerecord/test/validations_test.rb
+++ b/activerecord/test/validations_test.rb
@@ -33,6 +33,22 @@ class Topic < ActiveRecord::Base
   has_many :silly_unique_replies, :dependent => :destroy, :foreign_key => "parent_id"
 end
 
+class Wizard < ActiveRecord::Base
+  self.abstract_class = true
+
+  validates_uniqueness_of :name
+end
+
+class IneptWizard < Wizard
+  validates_uniqueness_of :city
+end
+
+class Conjurer < IneptWizard
+end
+
+class Thaumaturgist < IneptWizard
+end
+
 class ValidationsTest < Test::Unit::TestCase
   fixtures :topics, :developers
 
@@ -402,6 +418,35 @@ class ValidationsTest < Test::Unit::TestCase
     assert t2.save, "should save with nil"
   end
 
+  def test_validate_straight_inheritance_uniqueness
+    w1 = IneptWizard.create(:name => "Rincewind", :city => "Ankh-Morpork")
+    assert w1.valid?, "Saving w1"
+
+    # Should use validation from base class (which is abstract)
+    w2 = IneptWizard.new(:name => "Rincewind", :city => "Quirm")
+    assert !w2.valid?, "w2 shouldn't be valid"
+    assert w2.errors.on(:name), "Should have errors for name"
+    assert_equal "has already been taken", w2.errors.on(:name), "Should have uniqueness message for name"
+
+    w3 = Conjurer.new(:name => "Rincewind", :city => "Quirm")
+    assert !w3.valid?, "w3 shouldn't be valid"
+    assert w3.errors.on(:name), "Should have errors for name"
+    assert_equal "has already been taken", w3.errors.on(:name), "Should have uniqueness message for name"
+
+    w4 = Conjurer.create(:name => "The Amazing Bonko", :city => "Quirm")
+    assert w4.valid?, "Saving w4"
+
+    w5 = Thaumaturgist.new(:name => "The Amazing Bonko", :city => "Lancre")
+    assert !w5.valid?, "w5 shouldn't be valid"
+    assert w5.errors.on(:name), "Should have errors for name"
+    assert_equal "has already been taken", w5.errors.on(:name), "Should have uniqueness message for name"
+
+    w6 = Thaumaturgist.new(:name => "Mustrum Ridcully", :city => "Quirm")
+    assert !w6.valid?, "w6 shouldn't be valid"
+    assert w6.errors.on(:city), "Should have errors for city"
+    assert_equal "has already been taken", w6.errors.on(:city), "Should have uniqueness message for city"
+  end
+
   def test_validate_format
     Topic.validates_format_of(:title, :content, :with => /^Validation\smacros \w+!$/, :message => "is bad data")