diff options
author | David Heinemeier Hansson <david@loudthinking.com> | 2014-08-17 12:53:15 -0700 |
---|---|---|
committer | David Heinemeier Hansson <david@loudthinking.com> | 2014-08-17 12:53:15 -0700 |
commit | 8f15565de879bd40c8390884d1d31e52de240323 (patch) | |
tree | c6c30c4a07a5d11f46d50853613b909ee0e8c6ae /activerecord | |
parent | 9f6e82ee4783e491c20f5244a613fdeb4024beb5 (diff) | |
parent | e158ee50e64e2ae2460cd26be53039922f588300 (diff) | |
download | rails-8f15565de879bd40c8390884d1d31e52de240323.tar.gz rails-8f15565de879bd40c8390884d1d31e52de240323.tar.bz2 rails-8f15565de879bd40c8390884d1d31e52de240323.zip |
Merge branch 'master' of github.com:rails/rails
Diffstat (limited to 'activerecord')
-rw-r--r-- | activerecord/CHANGELOG.md | 13 | ||||
-rw-r--r-- | activerecord/lib/active_record/associations/has_many_association.rb | 2 | ||||
-rw-r--r-- | activerecord/lib/active_record/attribute.rb | 8 | ||||
-rw-r--r-- | activerecord/lib/active_record/attribute_methods.rb | 13 | ||||
-rw-r--r-- | activerecord/lib/active_record/attribute_methods/dirty.rb | 25 | ||||
-rw-r--r-- | activerecord/lib/active_record/enum.rb | 4 | ||||
-rw-r--r-- | activerecord/lib/active_record/persistence.rb | 2 | ||||
-rw-r--r-- | activerecord/lib/active_record/timestamp.rb | 2 | ||||
-rw-r--r-- | activerecord/test/cases/attribute_methods_test.rb | 18 | ||||
-rw-r--r-- | activerecord/test/cases/dirty_test.rb | 21 |
10 files changed, 85 insertions, 23 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 02660ae4c2..b09d9e336b 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,16 @@ +* Fixed an issue where custom accessor methods (such as those generated by + `enum`) with the same name as a global method are incorrectly overridden + when subclassing. + + Fixes #16288. + + *Godfrey Chan* + +* `*_was` and `changes` now work correctly for in-place attribute changes as + well. + + *Sean Griffin* + * Fix regression on after_commit that didnt fire when having nested transactions. Fixes #16425 diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 79c3d2b0f5..1413efaf7f 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -103,7 +103,7 @@ module ActiveRecord if has_cached_counter?(reflection) counter = cached_counter_attribute_name(reflection) owner[counter] += difference - owner.changed_attributes.delete(counter) # eww + owner.send(:clear_attribute_changes, counter) # eww end end diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb index 15cbbcff68..8cc1904575 100644 --- a/activerecord/lib/active_record/attribute.rb +++ b/activerecord/lib/active_record/attribute.rb @@ -30,10 +30,14 @@ module ActiveRecord def value # `defined?` is cheaper than `||=` when we get back falsy values - @value = type_cast(value_before_type_cast) unless defined?(@value) + @value = original_value unless defined?(@value) @value end + def original_value + type_cast(value_before_type_cast) + end + def value_for_database type.type_cast_for_database(value) end @@ -54,7 +58,7 @@ module ActiveRecord self.class.from_database(name, value, type) end - def type_cast + def type_cast(*) raise NotImplementedError end diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 09e2faee86..1ff28ceccc 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -57,6 +57,8 @@ module ActiveRecord end end + class GeneratedAttributeMethods < Module; end # :nodoc: + module ClassMethods def inherited(child_class) #:nodoc: child_class.initialize_generated_modules @@ -64,7 +66,7 @@ module ActiveRecord end def initialize_generated_modules # :nodoc: - @generated_attribute_methods = Module.new { extend Mutex_m } + @generated_attribute_methods = GeneratedAttributeMethods.new { extend Mutex_m } @attribute_methods_generated = false include @generated_attribute_methods end @@ -113,10 +115,11 @@ module ActiveRecord 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) - base_defined = Base.method_defined?(method_name) || Base.private_method_defined?(method_name) - defined && !base_defined || super + # If ThisClass < ... < SomeSuperClass < ... < Base and SomeSuperClass + # defines its own attribute method, then we don't want to overwrite that. + defined = method_defined_within?(method_name, superclass, Base) && + ! superclass.instance_method(method_name).owner.is_a?(GeneratedAttributeMethods) + defined || super end end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index b58295a106..d3f4e51c33 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -51,14 +51,6 @@ module ActiveRecord super | changed_in_place end - def attribute_changed?(attr_name, options = {}) - result = super - # We can't change "from" something in place. Only setters can define - # "from" and "to" - result ||= changed_in_place?(attr_name) unless options.key?(:from) - result - end - def changes_applied super store_original_raw_attributes @@ -69,12 +61,16 @@ module ActiveRecord original_raw_attributes.clear end + def changed_attributes + super.reverse_merge(attributes_changed_in_place).freeze + end + private def calculate_changes_from_defaults @changed_attributes = nil self.class.column_defaults.each do |attr, orig_value| - changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value) + set_attribute_was(attr, orig_value) if _field_changed?(attr, orig_value) end end @@ -100,9 +96,9 @@ module ActiveRecord def save_changed_attribute(attr, old_value) if attribute_changed?(attr) - changed_attributes.delete(attr) unless _field_changed?(attr, old_value) + clear_attribute_changes(attr) unless _field_changed?(attr, old_value) else - changed_attributes[attr] = old_value if _field_changed?(attr, old_value) + set_attribute_was(attr, old_value) if _field_changed?(attr, old_value) end end @@ -132,6 +128,13 @@ module ActiveRecord @attributes[attr].changed_from?(old_value) end + def attributes_changed_in_place + changed_in_place.each_with_object({}) do |attr_name, h| + orig = @attributes[attr_name].original_value + h[attr_name] = orig + end + end + def changed_in_place self.class.attribute_names.select do |attr_name| changed_in_place?(attr_name) diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index f0ee433d0b..5958373e88 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -145,11 +145,11 @@ module ActiveRecord value = read_attribute(attr_name) if attribute_changed?(attr_name) if mapping[old] == value - changed_attributes.delete(attr_name) + clear_attribute_changes([attr_name]) end else if old != value - changed_attributes[attr_name] = mapping.key old + set_attribute_was(attr_name, mapping.key(old)) end end else diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 51b1931ed5..755ff2b2f1 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -466,7 +466,7 @@ module ActiveRecord changes[self.class.locking_column] = increment_lock if locking_enabled? - changed_attributes.except!(*changes.keys) + clear_attribute_changes(changes.keys) primary_key = self.class.primary_key self.class.unscoped.where(primary_key => self[primary_key]).update_all(changes) == 1 else diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index 417cd61f0c..936a18d99a 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -114,7 +114,7 @@ module ActiveRecord def clear_timestamp_attributes all_timestamp_attributes_in_model.each do |attribute_name| self[attribute_name] = nil - changed_attributes.delete(attribute_name) + clear_attribute_changes([attribute_name]) end end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 535928783f..b4917e727a 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -810,6 +810,24 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert_equal "lol", topic.author_name end + def test_inherited_custom_accessors_with_reserved_names + klass = Class.new(ActiveRecord::Base) do + self.table_name = 'computers' + self.abstract_class = true + def system; "omg"; end + def system=(val); self.developer = val; end + end + + subklass = Class.new(klass) + [klass, subklass].each(&:define_attribute_methods) + + computer = subklass.find(1) + assert_equal "omg", computer.system + + computer.developer = 99 + assert_equal 99, computer.developer + end + def test_on_the_fly_super_invokable_generated_attribute_methods_via_method_missing klass = new_topic_like_ar_class do def title diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 69a7f25213..0c77eedb52 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -661,6 +661,27 @@ class DirtyTest < ActiveRecord::TestCase assert_not model.foo_changed? end + test "in place mutation detection" do + pirate = Pirate.create!(catchphrase: "arrrr") + pirate.catchphrase << " matey!" + + assert pirate.catchphrase_changed? + expected_changes = { + "catchphrase" => ["arrrr", "arrrr matey!"] + } + assert_equal(expected_changes, pirate.changes) + assert_equal("arrrr", pirate.catchphrase_was) + assert pirate.catchphrase_changed?(from: "arrrr") + assert_not pirate.catchphrase_changed?(from: "anything else") + assert pirate.changed_attributes.include?(:catchphrase) + + pirate.save! + pirate.reload + + assert_equal "arrrr matey!", pirate.catchphrase + assert_not pirate.changed? + end + private def with_partial_writes(klass, on = true) old = klass.partial_writes? |