diff options
author | Godfrey Chan <godfreykfc@gmail.com> | 2014-02-13 11:49:19 -0800 |
---|---|---|
committer | Godfrey Chan <godfreykfc@gmail.com> | 2014-02-23 11:00:55 -0800 |
commit | 41554319f8454f667b5e315339d3e286dbf6e1a4 (patch) | |
tree | 6b429827488b2ca7f34095d5f74f208ea6a8301f | |
parent | 96759cf6c6a17053abb6a2b7cd87cdbd5a8420ba (diff) | |
download | rails-41554319f8454f667b5e315339d3e286dbf6e1a4.tar.gz rails-41554319f8454f667b5e315339d3e286dbf6e1a4.tar.bz2 rails-41554319f8454f667b5e315339d3e286dbf6e1a4.zip |
Fixed STI classes not defining an attribute method if there is a
conflicting private method defined on its ancestors.
The problem is that `method_defined_within?(name, klass, superklass)`
only works correclty when `klass` and `superklass` are both `Class`es.
If both `klass` and `superklass` are both `Class`es, they share the
same inheritance chain, so if a method is defined on `klass` but not
`superklass`, this method must be introduced at some point between
`klass` and `superklass`.
This does not work when `superklass` is a `Module`. A `Module`'s
inheritance chain contains just itself. So if a method is defined on
`klass` but not on `superklass`, the method could still be defined
somewhere upstream, e.g. in `Object`.
This fix works by avoiding calling `method_defined_within?` with a
module while still fufilling the requirement (checking that the
method is defined withing `superclass` but not is not a generated
attribute method).
4d8ee288 is likely an attempted partial fix for this problem. This
unrolls that fix and properly check the `superclass` as intended.
Fixes #11569.
-rw-r--r-- | activerecord/CHANGELOG.md | 7 | ||||
-rw-r--r-- | activerecord/lib/active_record/attribute_methods.rb | 5 | ||||
-rw-r--r-- | activerecord/test/cases/attribute_methods_test.rb | 30 | ||||
-rw-r--r-- | activerecord/test/fixtures/computers.yml | 1 | ||||
-rw-r--r-- | activerecord/test/schema/schema.rb | 1 |
5 files changed, 31 insertions, 13 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 881a4d2d85..7476b82e5f 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fixed STI classes not defining an attribute method if there is a + conflicting private method defined on its ancestors. + + Fixes #11569. + + *Godfrey Chan* + * Deprecate half-baked support for PostgreSQL range values with excluding beginnings. We currently map PostgreSQL ranges to Ruby ranges. This conversion is not fully possible because the Ruby range does not support excluded beginnings. diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 9326c9c117..57ceec2fc5 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -105,8 +105,9 @@ module ActiveRecord 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) - defined && !ActiveRecord::Base.method_defined?(method_name) || super + defined = method_defined_within?(method_name, superclass) && + superclass.instance_method(method_name).owner != superclass.generated_attribute_methods + defined || super end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 6c581a432f..aec1ea3972 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -728,19 +728,27 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert "unknown attribute: hello", error.message 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 + def test_global_methods_are_overwritten + klass = Class.new(ActiveRecord::Base) do + self.table_name = 'computers' + end + + assert !klass.instance_method_already_implemented?(:system) + computer = klass.new + assert_nil computer.system + end + + def test_global_methods_are_overwritte_when_subclassing + klass = Class.new(ActiveRecord::Base) { self.abstract_class = true } + + subklass = Class.new(klass) do + self.table_name = 'computers' end - assert !@target.instance_method_already_implemented?(:title) - topic = @target.new - assert_nil topic.title - Object.send(:undef_method, :title) # remove test method from object + assert !klass.instance_method_already_implemented?(:system) + assert !subklass.instance_method_already_implemented?(:system) + computer = subklass.new + assert_nil computer.system end def test_instance_method_should_be_defined_on_the_base_class diff --git a/activerecord/test/fixtures/computers.yml b/activerecord/test/fixtures/computers.yml index daf969d7da..7281a4d768 100644 --- a/activerecord/test/fixtures/computers.yml +++ b/activerecord/test/fixtures/computers.yml @@ -1,4 +1,5 @@ workstation: id: 1 + system: 'Linux' developer: 1 extendedWarranty: 1 diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 99a53434f6..5f2d6acbcb 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -198,6 +198,7 @@ ActiveRecord::Schema.define do end create_table :computers, force: true do |t| + t.string :system t.integer :developer, null: false t.integer :extendedWarranty, null: false end |