Hi all,
I''ve submitted a patch for a bug I came across in ActiveRecord.
https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2808-ar-attribute-collides-with-private-method-results-in-nomethoderror#ticket-2808-7
This is the first patch I''ve submitted to the Rails core and I would
love to hear any feedback.
I''ve described the logic behind the fix in the ticket, and also copied
it below.
Thanks
Sam Goldstein
http://drasticcode.com
## From Lighthouse ticket 2808
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.)