Make it so AR attributes which conflict with object-private methods (…

…e.g. system) don't 'randomly' cause NoMethodErrors

Previously if you called this attribute before others, you'd get exceptions.  But if it was the second-or-subsequent attribute you retrieved you'd get the correct behaviour.

Signed-off-by: Michael Koziarski <michael@koziarski.com>
[#2808 state:committed]

activerecord/lib/active_record/attribute_methods.rb

@@ -133,6 +133,7 @@ def cache_attribute?(attr_name)
       end
 
       private
+
         # Suffixes a, ?, c become regexp /(a|\?|c)$/
         def rebuild_attribute_method_regexp
           suffixes = attribute_method_suffixes.map { |s| Regexp.escape(s) }
@@ -238,19 +239,17 @@ def evaluate_attribute_method(attr_name, method_definition, method_name=attr_nam
     def method_missing(method_id, *args, &block)
       method_name = method_id.to_s
 
-      if self.class.private_method_defined?(method_name)
-        raise NoMethodError.new("Attempt to call private method", method_name, args)
-      end
-
       # If we haven't generated any methods yet, generate them, then
       # see if we've created the method we're looking for.
       if !self.class.generated_methods?
         self.class.define_attribute_methods
+        guard_private_attribute_method!(method_name, args)
         if self.class.generated_methods.include?(method_name)
           return self.send(method_id, *args, &block)
         end
       end
 
+      guard_private_attribute_method!(method_name, args)
       if self.class.primary_key.to_s == method_name
         id
       elsif md = self.class.match_attribute_method?(method_name)
@@ -371,6 +370,12 @@ def respond_to?(method, include_private_methods = false)
     end
 
     private
+      # prevent method_missing from calling private methods with #send
+      def guard_private_attribute_method!(method_name, args)
+        if self.class.private_method_defined?(method_name)
+          raise NoMethodError.new("Attempt to call private method", method_name, args)
+        end
+      end
 
       def missing_attribute(attr_name, stack)
         raise ActiveRecord::MissingAttributeError, "missing attribute: #{attr_name}", stack

activerecord/test/cases/attribute_methods_test.rb

@@ -277,6 +277,22 @@ 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
+    end
+    assert !@target.instance_method_already_implemented?(:title)
+    topic = @target.new
+    assert_equal nil, topic.title
+
+    Object.send(:undef_method, :title) # remove test method from object
+  end
+
+
   private
   def time_related_columns_on_topic
     Topic.columns.select{|c| [:time, :date, :datetime, :timestamp].include?(c.type)}.map(&:name)