Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Andy Atkinson June 18th, 2009 @ 06:36 AM

  I was not able to reproduce your error with the automated tests in the attached app, or in the console. Did I get the test conditions correct?

  • 2808bug.tgz 77.7 KB

• You flagged this item as spam.
  

  Tjalling van der Wal June 18th, 2009 @ 02:21 PM

  Hi, thanks for your interest.

  To reproduce the error I made the following changes:

  First, instead of defining 'y' in Foo, add it to Object in an initializer (in our project it is some utility function inject by a plugin)
  

  class Object
    private 
    def y
  
  
  
  
  
  
  

  "The value of y"
  

  
  
  
  
    end
  end
  

  Second, I rewrote your tests:
  

  def test_aaaa_raise_error_when_y_is_called_the_first_time
    puts "running: test_raise_error_when_y_is_called_the_first_time"
    assert_raise NoMethodError do
  
  
  
  
  
  
  

  Foo.new.y
  

  
  
  
  
    end
  end
  
  
  
  def test_bbbb_access_the_private_call
    puts "running: test_access_the_private_call"
    assert "The value of y", Foo.new.send(:y)
  end
  
  
  
  def test_cccc_first_call_x_the_y
    puts "running: test_first_call_x_the_y"
    f = Foo.new
    f.x
    assert_equal "The value of y", f.y    # This one should fail as the AR-attribute has no value set.
  end
  

  Note that the order of the tests matters because calling 'x' has side effects. Therefore I added explicit output to the tests to be sure they are run in the correct order. I added the aaaa, bbbc, cccc prefixes to the tests names because (apparently) tests are sorted alphabetically.

  The 3rd test you wrote ("fetch the value accessing the private method the second time") should indeed fail. Calling the private y method with send in the 2nd test does not generate the public method.

• You flagged this item as spam.
  

  Sam Goldstein July 1st, 2009 @ 09:16 PM

  I'm having a similar issue with a Model that has a system attribute. Since this is an inherited private method an error is sometimes thrown when the accessor is called (but only if no other attribute accessor has been called.)

  Is anyone working on a patch for this?

• You flagged this item as spam.
  

  Tjalling van der Wal July 7th, 2009 @ 08:49 AM

  I've commented the check for calling private methods out in our project. I don't know if the check is really needed, since method_missing performs other checks too.

  The only nice solution to this problem is to resolve the naming conflict.

• You flagged this item as spam.
  

  Sam Goldstein July 8th, 2009 @ 08:01 PM

  • Assigned user set to “CancelProfileIsBroken”

  I've created a patch for this issue.

  The root problem here was that ActiveRecord was using two inconsistent methods for determining if an attribute method had been implemented.

  When checking if access control allowed an attribute method to be called AR looked to see if a private method with that name was defined, raising an error if it was.

  When checking to determine whether an attribute method was already implemented AR looked to see if the method was defined by an ancestor below ActiveRecord in the inheritance chain. Methods that shared their names with attributes but were inherited from ancestors above ActiveRecord (e.g. Object, Module, Kernel) were overridden.

  The existing AR method which defines this check is shown below.

        # Checks whether the method is defined in the model or any of its subclasses
        # that also derive from Active Record. Raises DangerousAttributeError if the
        # method is defined by Active Record though.
        def instance_method_already_implemented?(method_name)
          method_name = method_name.to_s
          return true if method_name =~ /^id(=$|\?$|$)/
          @_defined_class_methods         ||= ancestors.first(ancestors.index(ActiveRecord::Base)).sum([]) { |m| m.public_instance_methods(false) | m.private_instance_methods(false) | m.protected_instance_methods(false) }.map {|m| m.to_s }.to_set
          @@_defined_activerecord_methods ||= (ActiveRecord::Base.public_instance_methods(false) | ActiveRecord::Base.private_instance_methods(false) | ActiveRecord::Base.protected_instance_methods(false)).map{|m| m.to_s }.to_set
          raise DangerousAttributeError, "#{method_name} is defined by ActiveRecord" if @@_defined_activerecord_methods.include?(method_name)
          @_defined_class_methods.include?(method_name)
        end
  

  I've updated ActiveRecord to respect access control, using this same check. Private methods inherited from Object (for example) will be overridden, while those implemented by an ActiveRecord descendant are respected. This avoids situations where an read attribute method will behave differently depending on whether another attribute was called before it (as detailed in this ticket.)

  • bug2808.diff 3.4 KB

• You flagged this item as spam.
  

  Sam Goldstein July 8th, 2009 @ 08:01 PM

  • Tag changed from “2-3-stable, active_record, attributes” to “2-3-stable, active_record, attributes, patch”
• 

  CancelProfileIsBroken July 8th, 2009 @ 10:11 PM

  • Assigned user cleared.
• 

  Repository July 9th, 2009 @ 05:47 AM

  • State changed from “new” to “committed”

  (from [d60d7edce462f4602bfc9996689087a235b034c9]) 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] http://github.com/rails/rails/commit/d60d7edce462f4602bfc9996689087...

• You flagged this item as spam.
  

  Eliot Sykes January 6th, 2010 @ 07:57 PM

  • Tag changed from “2-3-stable, active_record, attributes, patch” to “2, active_record, attributes, patch”

  Not sure if this is news, just been looking at the 2.3.5 tag to see if this fix is included and it doesn't seem to be

  http://github.com/rails/rails/blob/v2.3.5/activerecord/lib/active_r...

  Does anybody know if this is scheduled to go into a later version?

• You flagged this item as spam.
  

  Eliot Sykes January 6th, 2010 @ 07:58 PM

  • Tag changed from “2, active_record, attributes, patch” to “2-3-stable, active_record, attributes, patch”

• You flagged this item as spam.
  

  Eliot Sykes January 6th, 2010 @ 08:50 PM

  • Tag changed from “2-3-stable, active_record, attributes, patch” to “2, active_record, attributes, patch”

  I'm putting this patch I'm using on Rails 2.2.2 for completeness, in case anyone else hits this, plus I'm not sure if respond_to? has been fixed in line with how method_missing has above.

  Put this in config/initializers/active_record.rb

  
  class ActiveRecord::Base
    def method_missing_with_force_define_attribute_methods(method_id, *args, &block)
      self.class.define_attribute_methods unless self.class.generated_methods?
      method_missing_without_force_define_attribute_methods(method_id, *args, &block)
    end
    alias_method_chain :method_missing, :force_define_attribute_methods
    
    def respond_to_with_force_define_attribute_methods?(method, include_private_methods = false)
      self.class.define_attribute_methods unless self.class.generated_methods?
      respond_to_without_force_define_attribute_methods?(method, include_private_methods)
    end
    alias_method_chain :respond_to?, :force_define_attribute_methods
  end
  

• You flagged this item as spam.
  

  Eliot Sykes January 6th, 2010 @ 08:51 PM

  • Tag changed from “2, active_record, attributes, patch” to “2-3-stable, active_record, attributes, patch”

• You flagged this item as spam.
  

  Marcus Mateus January 26th, 2010 @ 12:16 AM

  I just ran into this issue as well and wasted a good bit of time tracing the odd behavior before finding this post. Please do incorporate it into the next release... it's sad that a commit has been out there since July.

  Eliot, thanks for the above patch as a quick temporary fix.

• You flagged this item as spam.
  

  Paul Barry March 26th, 2010 @ 02:42 AM

  I ran into this as well, because I have an attribute called link, but the problem only comes up when you try to call that method in a Rake task, because Rake defines a private link method on Object. I got around it simply by calling define_attribute_methods in the class definition.

• You flagged this item as spam.
  

  Jarl Friis June 10th, 2010 @ 12:48 PM

  I think I ran into this problem as well (using 2.3.5), see
  http://groups.google.com/group/rubyonrails-talk/browse_thread/threa...

  Jarl