diff options
Diffstat (limited to 'activerecord')
| -rw-r--r-- | activerecord/lib/active_record/attribute_methods.rb | 74 | ||||
| -rw-r--r-- | activerecord/lib/active_record/attribute_methods/read.rb | 6 | ||||
| -rw-r--r-- | activerecord/test/cases/attribute_methods/read_test.rb | 1 | ||||
| -rw-r--r-- | activerecord/test/cases/attribute_methods_test.rb | 20 | ||||
| -rw-r--r-- | activerecord/test/cases/base_test.rb | 23 | ||||
| -rw-r--r-- | activerecord/test/cases/reflection_test.rb | 10 | ||||
| -rw-r--r-- | activerecord/test/cases/yaml_serialization_test.rb | 6 | ||||
| -rw-r--r-- | activerecord/test/models/topic.rb | 4 | ||||
| -rw-r--r-- | activerecord/test/schema/schema.rb | 2 |
9 files changed, 94 insertions, 52 deletions
diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 131ef09f57..54099d3196 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -34,48 +34,26 @@ module ActiveRecord # 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) @@ -83,19 +61,31 @@ module ActiveRecord raise DangerousAttributeError, "#{method_name} is defined by ActiveRecord" end - super + if superclass == Base + 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 + 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 + + 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) diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index a83d944d1b..1548114580 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -30,10 +30,8 @@ module ActiveRecord 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 diff --git a/activerecord/test/cases/attribute_methods/read_test.rb b/activerecord/test/cases/attribute_methods/read_test.rb index 86a240d93c..7665f1c12e 100644 --- a/activerecord/test/cases/attribute_methods/read_test.rb +++ b/activerecord/test/cases/attribute_methods/read_test.rb @@ -14,6 +14,7 @@ module ActiveRecord def setup @klass = Class.new do + def self.superclass; Base; end def self.base_class; self; end include ActiveRecord::AttributeMethods diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 8e509a9792..b6cf26f978 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -711,6 +711,26 @@ class AttributeMethodsTest < ActiveRecord::TestCase 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) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index f102634ef1..6ff0c1355c 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1233,6 +1233,27 @@ class BasicsTest < ActiveRecord::TestCase assert_equal(myobj, topic.content) end + def test_serialized_attribute_in_base_class + Topic.serialize("content", Hash) + + hash = { 'content1' => 'value1', 'content2' => 'value2' } + important_topic = ImportantTopic.create("content" => hash) + assert_equal(hash, important_topic.content) + + important_topic.reload + assert_equal(hash, important_topic.content) + end + + def test_serialized_attribute_declared_in_subclass + hash = { 'important1' => 'value1', 'important2' => 'value2' } + important_topic = ImportantTopic.create("important" => hash) + assert_equal(hash, important_topic.important) + + important_topic.reload + assert_equal(hash, important_topic.important) + assert_equal(hash, important_topic.read_attribute(:important)) + end + def test_serialized_time_attribute myobj = Time.local(2008,1,1,1,0) topic = Topic.create("content" => myobj).reload @@ -1671,7 +1692,7 @@ class BasicsTest < ActiveRecord::TestCase def test_inspect_instance topic = topics(:first) - assert_equal %(#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "david@loudthinking.com", written_on: "#{topic.written_on.to_s(:db)}", bonus_time: "#{topic.bonus_time.to_s(:db)}", last_read: "#{topic.last_read.to_s(:db)}", content: "Have a nice day", approved: false, replies_count: 1, parent_id: nil, parent_title: nil, type: nil, group: nil, created_at: "#{topic.created_at.to_s(:db)}", updated_at: "#{topic.updated_at.to_s(:db)}">), topic.inspect + assert_equal %(#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "david@loudthinking.com", written_on: "#{topic.written_on.to_s(:db)}", bonus_time: "#{topic.bonus_time.to_s(:db)}", last_read: "#{topic.last_read.to_s(:db)}", content: "Have a nice day", important: nil, approved: false, replies_count: 1, parent_id: nil, parent_title: nil, type: nil, group: nil, created_at: "#{topic.created_at.to_s(:db)}", updated_at: "#{topic.updated_at.to_s(:db)}">), topic.inspect end def test_inspect_new_instance diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index b30db542a7..7fd15027eb 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -36,25 +36,25 @@ class ReflectionTest < ActiveRecord::TestCase def test_read_attribute_names assert_equal( - %w( id title author_name author_email_address bonus_time written_on last_read content group approved replies_count parent_id parent_title type created_at updated_at ).sort, + %w( id title author_name author_email_address bonus_time written_on last_read content important group approved replies_count parent_id parent_title type created_at updated_at ).sort, @first.attribute_names.sort ) end def test_columns - assert_equal 16, Topic.columns.length + assert_equal 17, Topic.columns.length end def test_columns_are_returned_in_the_order_they_were_declared column_names = Topic.columns.map { |column| column.name } - assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content approved replies_count parent_id parent_title type group created_at updated_at), column_names + assert_equal %w(id title author_name author_email_address written_on bonus_time last_read content important approved replies_count parent_id parent_title type group created_at updated_at), column_names end def test_content_columns content_columns = Topic.content_columns content_column_names = content_columns.map {|column| column.name} - assert_equal 12, content_columns.length - assert_equal %w(title author_name author_email_address written_on bonus_time last_read content group approved parent_title created_at updated_at).sort, content_column_names.sort + assert_equal 13, content_columns.length + assert_equal %w(title author_name author_email_address written_on bonus_time last_read content important group approved parent_title created_at updated_at).sort, content_column_names.sort end def test_column_string_type_and_limit diff --git a/activerecord/test/cases/yaml_serialization_test.rb b/activerecord/test/cases/yaml_serialization_test.rb index 5a38f2c6ee..2b4ec81199 100644 --- a/activerecord/test/cases/yaml_serialization_test.rb +++ b/activerecord/test/cases/yaml_serialization_test.rb @@ -5,10 +5,16 @@ class YamlSerializationTest < ActiveRecord::TestCase fixtures :topics def test_to_yaml_with_time_with_zone_should_not_raise_exception + tz = Time.zone Time.zone = ActiveSupport::TimeZone["Pacific Time (US & Canada)"] ActiveRecord::Base.time_zone_aware_attributes = true + topic = Topic.new(:written_on => DateTime.now) assert_nothing_raised { topic.to_yaml } + + ensure + Time.zone = tz + ActiveRecord::Base.time_zone_aware_attributes = false end def test_roundtrip diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index ede662450e..1a1a18166a 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -108,6 +108,10 @@ class Topic < ActiveRecord::Base def after_create_for_transaction; end end +class ImportantTopic < Topic + serialize :important, Hash +end + module Web class Topic < ActiveRecord::Base has_many :replies, :dependent => :destroy, :foreign_key => "parent_id", :class_name => 'Web::Reply' diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index bb08f5c181..5933e1f46e 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -607,8 +607,10 @@ ActiveRecord::Schema.define do # Oracle SELECT WHERE clause which causes many unit test failures if current_adapter?(:OracleAdapter) t.string :content, :limit => 4000 + t.string :important, :limit => 4000 else t.text :content + t.text :important end t.boolean :approved, :default => true t.integer :replies_count, :default => 0 |