aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSean Griffin <sean@thoughtbot.com>2015-01-18 13:43:31 -0700
committerSean Griffin <sean@thoughtbot.com>2015-01-18 13:43:31 -0700
commitea721d7027c16f64c47395546c67858060a273e3 (patch)
treee3eb8855e41610c491fbfd9757ae2ff7d49d45cf
parentc542677310b7dbb205cf3bc24f74099ae929ca20 (diff)
downloadrails-ea721d7027c16f64c47395546c67858060a273e3.tar.gz
rails-ea721d7027c16f64c47395546c67858060a273e3.tar.bz2
rails-ea721d7027c16f64c47395546c67858060a273e3.zip
Don't calculate in-place changes on attribute assignment
When an attribute is assigned, we determine if it was already marked as changed so we can determine if we need to clear the changes, or mark it as changed. Since this only affects the `attributes_changed_by_setter` hash, in-place changes are irrelevant to this process. Since calculating in-place changes can be expensive, we can just skip it here. I also added a test for the only edge case I could think of that would be affected by this change.
-rw-r--r--activemodel/lib/active_model/dirty.rb1
-rw-r--r--activerecord/lib/active_record/attribute_methods/dirty.rb2
-rw-r--r--activerecord/test/cases/dirty_test.rb8
3 files changed, 10 insertions, 1 deletions
diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb
index afba9bab0d..614bc6a5d8 100644
--- a/activemodel/lib/active_model/dirty.rb
+++ b/activemodel/lib/active_model/dirty.rb
@@ -189,6 +189,7 @@ module ActiveModel
def changes_include?(attr_name)
attributes_changed_by_setter.include?(attr_name)
end
+ alias attribute_changed_by_setter? changes_include?
# Removes current changes and makes them accessible through +previous_changes+.
def changes_applied # :doc:
diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index ce7f575150..bce9c5e1e3 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -108,7 +108,7 @@ module ActiveRecord
end
def save_changed_attribute(attr, old_value)
- if attribute_changed?(attr)
+ if attribute_changed_by_setter?(attr)
clear_attribute_changes(attr) unless _field_changed?(attr, old_value)
else
set_attribute_was(attr, old_value) if _field_changed?(attr, old_value)
diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index ae4a8aab2c..192ba6f7cd 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -721,6 +721,14 @@ class DirtyTest < ActiveRecord::TestCase
assert record.save
end
+ test "mutating and then assigning doesn't remove the change" do
+ pirate = Pirate.create!(catchphrase: "arrrr")
+ pirate.catchphrase << " matey!"
+ pirate.catchphrase = "arrrr matey!"
+
+ assert pirate.catchphrase_changed?(from: "arrrr", to: "arrrr matey!")
+ end
+
private
def with_partial_writes(klass, on = true)
old = klass.partial_writes?