Merge remote-tracking branch 'chancancode/fix_instance_method_already_implemented'

Conflicts:
	activerecord/CHANGELOG.md

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*
+
 *   Coerce strings when reading attributes.
     Fixes #10485.
 

activerecord/lib/active_record/attribute_methods.rb

@@ -105,8 +105,9 @@ module ActiveRecord
           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

@@ -746,19 +746,27 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     assert "unknown attribute: hello", error.message
   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 !@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)
+    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 !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

@@ -198,6 +198,7 @@ ActiveRecord::Schema.define do
   end
 
   create_table :computers, force: true do |t|
+    t.string :system
     t.integer :developer, null: false
     t.integer :extendedWarranty, null: false
   end