Merge pull request #16458 from chancancode/ar_fix_reserved_inheritance

Fixed issue w/custom accessors + reserved name + inheritance Conflicts: activerecord/CHANGELOG.md
rails · Aug 17, 2014 · a5e2d2e · a5e2d2e
2 parents b662273 + 2638d5c
commit a5e2d2e
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 5 deletions.
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
@@ -1,3 +1,11 @@
+*   Fixed an issue where custom accessor methods (such as those generated by
+    `enum`) with the same name as a global method are incorrectly overridden
+    when subclassing.
+
+    Fixes #16288.
+
+    *Godfrey Chan*
+
 *   `*_was` and `changes` now work correctly for in-place attribute changes as
     well.
 

diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
@@ -57,14 +57,16 @@ def method_body(method_name, const_name)
       end
     end
 
+    class GeneratedAttributeMethods < Module; end # :nodoc:
+
     module ClassMethods
       def inherited(child_class) #:nodoc:
         child_class.initialize_generated_modules
         super
       end
 
       def initialize_generated_modules # :nodoc:
-        @generated_attribute_methods = Module.new { extend Mutex_m }
+        @generated_attribute_methods = GeneratedAttributeMethods.new { extend Mutex_m }
         @attribute_methods_generated = false
         include @generated_attribute_methods
       end
@@ -113,10 +115,11 @@ def instance_method_already_implemented?(method_name)
         if superclass == Base
           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)
-          base_defined = Base.method_defined?(method_name) || Base.private_method_defined?(method_name)
-          defined && !base_defined || super
+          # If ThisClass < ... < SomeSuperClass < ... < Base and SomeSuperClass
+          # defines its own attribute method, then we don't want to overwrite that.
+          defined = method_defined_within?(method_name, superclass, Base) &&
+            ! superclass.instance_method(method_name).owner.is_a?(GeneratedAttributeMethods)
+          defined || super
         end
       end
 

diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb
@@ -810,6 +810,24 @@ def title=(val); self.author_name = val; end
     assert_equal "lol", topic.author_name
   end
 
+  def test_inherited_custom_accessors_with_reserved_names
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = 'computers'
+      self.abstract_class = true
+      def system; "omg"; end
+      def system=(val); self.developer = val; end
+    end
+
+    subklass = Class.new(klass)
+    [klass, subklass].each(&:define_attribute_methods)
+
+    computer = subklass.find(1)
+    assert_equal "omg", computer.system
+
+    computer.developer = 99
+    assert_equal 99, computer.developer
+  end
+
   def test_on_the_fly_super_invokable_generated_attribute_methods_via_method_missing
     klass = new_topic_like_ar_class do
       def title