From f74a5616e89f03e13a025401475ac1101bb159ae Mon Sep 17 00:00:00 2001 From: Alan Kennedy Date: Sun, 25 Nov 2012 21:01:27 +0000 Subject: Save has_one associations only if record has changes Prevents save related callbacks such as `after_commit` being triggered when `has_one` objects are already persisted and have no changes. --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/lib/active_record/autosave_association.rb | 5 +++-- activerecord/test/cases/autosave_association_test.rb | 15 +++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 53dc1374b4..f31b8a36ea 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Only save has_one associations if record has changes. + Previously after save related callbacks, such as `#after_commit`, were triggered when the has_one + object did not get saved to the db. + + *Alan Kennedy* + * ActiveRecord::Base#attribute_for_inspect now truncates long arrays (more than 10 elements) *Jan Bernacki* diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index e9622ca0c1..e9e35f4e21 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -377,14 +377,15 @@ module ActiveRecord def save_has_one_association(reflection) association = association_instance_get(reflection.name) record = association && association.load_target + if record && !record.destroyed? autosave = reflection.options[:autosave] if autosave && record.marked_for_destruction? record.destroy - else + elsif autosave != false key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id - if autosave != false && (autosave || new_record? || record_changed?(reflection, record, key)) + if (autosave && record.changed_for_autosave?) || new_record? || record_changed?(reflection, record, key) unless reflection.through_reflection record[reflection.foreign_key] = key diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 517d2674a7..acb19032ea 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -626,10 +626,25 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase end end + @ship.pirate.catchphrase = "Changed Catchphrase" + assert_raise(RuntimeError) { assert !@pirate.save } assert_not_nil @pirate.reload.ship end + def test_should_save_changed_has_one_changed_object_if_child_is_saved + @pirate.ship.name = "NewName" + @pirate.ship.expects(:save).once.returns(true) + + assert @pirate.save + end + + def test_should_not_save_changed_has_one_unchanged_object_if_child_is_saved + @pirate.ship.expects(:save).never + + assert @pirate.save + end + # belongs_to def test_should_destroy_a_parent_association_as_part_of_the_save_transaction_if_it_was_marked_for_destroyal assert !@ship.pirate.marked_for_destruction? -- cgit v1.2.3