Permalink
Browse files

Fix #4046.

  • Loading branch information...
jonleighton committed Dec 23, 2011
1 parent 30ce084 commit f1eb98f0fcca49c3f26a905a5c7b656ea22269d1
@@ -34,68 +34,59 @@ module ClassMethods
# accessors, mutators and query methods.
def define_attribute_methods
return if attribute_methods_generated?
-
- if base_class == self
- super(column_names)
- @attribute_methods_generated = true
- else
- base_class.define_attribute_methods
- end
+ superclass.define_attribute_methods unless self == base_class
+ super(column_names)
+ @attribute_methods_generated = true
end
def attribute_methods_generated?
- if base_class == self
- @attribute_methods_generated ||= false
- else
- base_class.attribute_methods_generated?
- end
- end
-
- def generated_attribute_methods
- @generated_attribute_methods ||= (base_class == self ? super : base_class.generated_attribute_methods)
+ @attribute_methods_generated ||= false
end
+ # We will define the methods as instance methods, but will call them as singleton
+ # methods. This allows us to use method_defined? to check if the method exists,
+ # which is fast and won't give any false positives from the ancestors (because
+ # there are no ancestors).
def generated_external_attribute_methods
- @generated_external_attribute_methods ||= begin
- if base_class == self
- # We will define the methods as instance methods, but will call them as singleton
- # methods. This allows us to use method_defined? to check if the method exists,
- # which is fast and won't give any false positives from the ancestors (because
- # there are no ancestors).
- Module.new { extend self }
- else
- base_class.generated_external_attribute_methods
- end
- end
+ @generated_external_attribute_methods ||= Module.new { extend self }
end
def undefine_attribute_methods
- if base_class == self
- super
- @attribute_methods_generated = false
- else
- base_class.undefine_attribute_methods
- end
+ super
+ @attribute_methods_generated = false
end
def instance_method_already_implemented?(method_name)
if dangerous_attribute_method?(method_name)
raise DangerousAttributeError, "#{method_name} is defined by ActiveRecord"
end
- super
+ if superclass == Base
+ super
+ else
+ method_defined_within?(method_name, superclass, superclass.generated_attribute_methods) || super
+ end
end
# A method name is 'dangerous' if it is already defined by Active Record, but
# not by any ancestors. (So 'puts' is not dangerous but 'save' is.)
- def dangerous_attribute_method?(method_name)
- active_record = ActiveRecord::Base
- superclass = ActiveRecord::Base.superclass
-
- (active_record.method_defined?(method_name) ||
- active_record.private_method_defined?(method_name)) &&
- !superclass.method_defined?(method_name) &&
- !superclass.private_method_defined?(method_name)
+ def dangerous_attribute_method?(name)
+ method_defined_within?(name, Base)
+ end
+
+ # Note that we could do this via klass.instance_methods(false), but this would require us
+ # to maintain a cached Set (for speed) and invalidate it at the correct time, which would
+ # be a pain. This implementation is also O(1) while avoiding maintaining a cached Set.
+ def method_defined_within?(name, klass, sup = klass.superclass)
+ if klass.method_defined?(name) || klass.private_method_defined?(name)
+ if sup.method_defined?(name) || sup.private_method_defined?(name)
+ klass.instance_method(name).owner != sup.instance_method(name).owner
+ else
+ true
+ end
+ else
+ false
+ end
end
def attribute_method?(attribute)
@@ -30,10 +30,8 @@ def cache_attribute?(attr_name)
end
def undefine_attribute_methods
- if base_class == self
- generated_external_attribute_methods.module_eval do
- instance_methods.each { |m| undef_method(m) }
- end
+ generated_external_attribute_methods.module_eval do
+ instance_methods.each { |m| undef_method(m) }
end
super
@@ -80,6 +78,7 @@ def __temp__
STR
generated_external_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
+ # raise if method_defined?('#{attr_name}')
def __temp__(v, attributes, attributes_cache, attr_name)
#{external_attribute_access_code(attr_name, cast_code)}
end
@@ -14,6 +14,7 @@ def type; :integer; end
def setup
@klass = Class.new do
+ def self.superclass; Base; end
def self.base_class; self; end
include ActiveRecord::AttributeMethods
@@ -711,6 +711,26 @@ def test_read_attribute_with_nil_should_not_asplode
assert_equal nil, Topic.new.read_attribute(nil)
end
+ # If B < A, and A defines an accessor for 'foo', we don't want to override
+ # that by defining a 'foo' method in the generated methods module for B.
+ # (That module will be inserted between the two, e.g. [B, <GeneratedAttributes>, A].)
+ def test_inherited_custom_accessors
+ klass = Class.new(ActiveRecord::Base) do
+ self.table_name = "topics"
+ self.abstract_class = true
+ def title; "omg"; end
+ def title=(val); self.author_name = val; end
+ end
+ subklass = Class.new(klass)
+ [klass, subklass].each(&:define_attribute_methods)
+
+ topic = subklass.find(1)
+ assert_equal "omg", topic.title
+
+ topic.title = "lol"
+ assert_equal "lol", topic.author_name
+ end
+
private
def cached_columns
@cached_columns ||= time_related_columns_on_topic.map(&:name)
@@ -1251,6 +1251,7 @@ def test_serialized_attribute_declared_in_subclass
important_topic.reload
assert_equal(hash, important_topic.important)
+ assert_equal(hash, important_topic.read_attribute(:important))
end
def test_serialized_time_attribute

0 comments on commit f1eb98f

Please sign in to comment.