diff options
| -rw-r--r-- | activerecord/CHANGELOG | 2 | ||||
| -rw-r--r-- | activerecord/lib/active_record/transactions.rb | 29 | ||||
| -rw-r--r-- | activerecord/test/transactions_test.rb | 4 |
3 files changed, 18 insertions, 17 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 720f203299..435aeac9b9 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -11,7 +11,7 @@ * Bring the sybase adapter up to scratch for 1.2 release. [jsheets] -* Rollback #new_record? and #id values for created records that rollback in an after_save callback. Closes #6910 [Ben Curren] +* Rollback new_record? and id when an exception is raised in a save callback. #6910 [Ben Curren, outerim] * Pushing a record on an association collection doesn't unnecessarily load all the associated records. [Obie Fernandez, Jeremy Kemper] diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 5ee644c0b4..77a8a14a2a 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -126,28 +126,27 @@ module ActiveRecord end def save_with_transactions(perform_validation = true) #:nodoc: - rollback_active_record_state { transaction { save_without_transactions(perform_validation) } } + rollback_active_record_state! { transaction { save_without_transactions(perform_validation) } } end def save_with_transactions! #:nodoc: - rollback_active_record_state { transaction { save_without_transactions! } } + rollback_active_record_state! { transaction { save_without_transactions! } } end - - # stores the current id and @new_record values so that they are reset - # after rolling the transaction back. - def rollback_active_record_state + + # Reset id and @new_record if the transaction rolls back. + def rollback_active_record_state! + id_present = has_attribute?(self.class.primary_key) + previous_id = id previous_new_record = @new_record - previous_id = self.id - response = yield - rescue - response = false - raise - ensure - unless response - @new_record = previous_new_record + yield + rescue Exception + @new_record = previous_new_record + if id_present self.id = previous_id + else + @attributes.delete(self.class.primary_key) end - response + raise end end end diff --git a/activerecord/test/transactions_test.rb b/activerecord/test/transactions_test.rb index ecc0da425d..3f3ee1adb7 100644 --- a/activerecord/test/transactions_test.rb +++ b/activerecord/test/transactions_test.rb @@ -133,8 +133,9 @@ class TransactionTest < Test::Unit::TestCase :content => "Have a nice day", :approved => false) new_record_snapshot = new_topic.new_record? + id_present = new_topic.has_attribute?(Topic.primary_key) id_snapshot = new_topic.id - + # Make sure the second save gets the after_create callback called. 2.times do begin @@ -146,6 +147,7 @@ class TransactionTest < Test::Unit::TestCase assert_equal "Make the transaction rollback", e.message assert_equal new_record_snapshot, new_topic.new_record?, "The topic should have its old new_record value" assert_equal id_snapshot, new_topic.id, "The topic should have its old id" + assert_equal id_present, new_topic.has_attribute?(Topic.primary_key) ensure remove_exception_raising_after_create_callback_to_topic end |