Fixed STI classes not defining an attribute method if there is a

conflicting private method defined on its ancestors.

The problem is that `method_defined_within?(name, klass, superklass)`
only works correclty when `klass` and `superklass` are both `Class`es.

If both `klass` and `superklass` are both `Class`es, they share the
same inheritance chain, so if a method is defined on `klass` but not
`superklass`, this method must be introduced at some point between
`klass` and `superklass`.

This does not work when `superklass` is a `Module`. A `Module`'s
inheritance chain contains just itself. So if a method is defined on
`klass` but not on `superklass`, the method could still be defined
somewhere upstream, e.g. in `Object`.

This fix works by avoiding calling `method_defined_within?` with a
module while still fufilling the requirement (checking that the
method is defined withing `superclass` but not is not a generated
attribute method).

4d8ee28 is likely an attempted partial fix for this problem. This
unrolls that fix and properly check the `superclass` as intended.

Fixes #11569.

Conflicts:
	activerecord/CHANGELOG.md
	activerecord/test/cases/attribute_methods_test.rb
	activerecord/test/schema/schema.rb

activerecord/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   Fixed STI classes not defining an attribute method if there is a
+    conflicting private method defined on its ancestors.
+
+    Fixes #11569.
+
+    *Godfrey Chan*
+
 *   Fix validation on uniqueness of empty association.
 
     *Evgeny Li*

activerecord/lib/active_record/attribute_methods.rb

@@ -86,8 +86,9 @@ def instance_method_already_implemented?(method_name)
           super
         else
           # If B < A and A defines its own attribute method, then we don't want to overwrite that.
-          defined = method_defined_within?(method_name, superclass, superclass.generated_attribute_methods)
-          defined && !ActiveRecord::Base.method_defined?(method_name) || super
+          defined = method_defined_within?(method_name, superclass) &&
+            superclass.instance_method(method_name).owner != superclass.generated_attribute_methods
+          defined || super
         end
       end
 

activerecord/test/cases/attribute_methods_test.rb

@@ -710,19 +710,27 @@ def test_bulk_update_respects_access_control
     assert_raise(ActiveRecord::UnknownAttributeError) { @target.new.attributes = { :title => "Ants in pants" } }
   end
 
-  def test_read_attribute_overwrites_private_method_not_considered_implemented
-    # simulate a model with a db column that shares its name an inherited
-    # private method (e.g. Object#system)
-    #
-    Object.class_eval do
-      private
-      def title; "private!"; end
+  def test_global_methods_are_overwritten
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = 'computers'
+    end
+
+    assert !klass.instance_method_already_implemented?(:system)
+    computer = klass.new
+    assert_nil computer.system
+  end
+
+  def test_global_methods_are_overwritte_when_subclassing
+    klass = Class.new(ActiveRecord::Base) { self.abstract_class = true }
+
+    subklass = Class.new(klass) do
+      self.table_name = 'computers'
     end
-    assert !@target.instance_method_already_implemented?(:title)
-    topic = @target.new
-    assert_nil topic.title
 
-    Object.send(:undef_method, :title) # remove test method from object
+    assert !klass.instance_method_already_implemented?(:system)
+    assert !subklass.instance_method_already_implemented?(:system)
+    computer = subklass.new
+    assert_nil computer.system
   end
 
   def test_instance_method_should_be_defined_on_the_base_class

activerecord/test/fixtures/computers.yml

@@ -1,4 +1,5 @@
 workstation:
   id: 1
+  system: 'Linux'
   developer: 1
   extendedWarranty: 1

activerecord/test/schema/schema.rb

@@ -197,6 +197,7 @@ def create_table(*args, &block)
   create_table :computers, :force => true do |t|
     t.integer :developer, :null => false
     t.integer :extendedWarranty, :null => false
+    t.string :system
   end
 
   create_table :contracts, :force => true do |t|