Permalink
Browse files

Merge pull request #13751 from chancancode/ar_rollback_fix

Restore ActiveRecord states after a rollback for models w/o callbacks
Conflicts:
	activerecord/CHANGELOG.md
  • Loading branch information...
1 parent 59b6686 commit 5621c0c540ff7dd9f53654eb3aa7f02c5c63ff1b @chancancode chancancode committed Jan 18, 2014
@@ -1,3 +1,11 @@
+* ActiveRecord states are now correctly restored after a rollback for
+ models that did not define any transactional callbacks (i.e.
+ `after_commit`, `after_rollback` or `after_create`).
+
+ Fixes #13744.
+
+ *Godfrey Chan*
+
* Add back `#yaml_initialize` method for Syck deserialization
`ActiveRecord::Core#yaml_initialize` has been added back to support deserialization using
@@ -23,6 +23,10 @@ def initialize(state = nil)
@parent = nil
end
+ def finalized?
+ @state
+ end
+
def committed?
@state == :committed
end
@@ -408,13 +408,10 @@ def sync_with_transaction_state
end
def update_attributes_from_transaction_state(transaction_state, depth)
- if transaction_state && !has_transactional_callbacks?
+ if transaction_state && transaction_state.finalized? && !has_transactional_callbacks?
unless @reflects_state[depth]
- if transaction_state.committed?
- committed!
- elsif transaction_state.rolledback?
- rolledback!
- end
+ restore_transaction_record_state if transaction_state.rolledback?
+ clear_transaction_record_state
@reflects_state[depth] = true
end
@@ -384,17 +384,26 @@ def test_rollback_when_saving_a_frozen_record
end
def test_restore_active_record_state_for_all_records_in_a_transaction
+ topic_without_callbacks = Class.new(ActiveRecord::Base) do
+ self.table_name = 'topics'
+ end
+
topic_1 = Topic.new(:title => 'test_1')
topic_2 = Topic.new(:title => 'test_2')
+ topic_3 = topic_without_callbacks.new(:title => 'test_3')
+
Topic.transaction do
assert topic_1.save
assert topic_2.save
+ assert topic_3.save
@first.save
@second.destroy
assert topic_1.persisted?, 'persisted'
assert_not_nil topic_1.id
assert topic_2.persisted?, 'persisted'
assert_not_nil topic_2.id
+ assert topic_3.persisted?, 'persisted'
+ assert_not_nil topic_3.id
assert @first.persisted?, 'persisted'
assert_not_nil @first.id
assert @second.destroyed?, 'destroyed'
@@ -405,6 +414,8 @@ def test_restore_active_record_state_for_all_records_in_a_transaction
assert_nil topic_1.id
assert !topic_2.persisted?, 'not persisted'
assert_nil topic_2.id
+ assert !topic_3.persisted?, 'not persisted'
+ assert_nil topic_3.id
assert @first.persisted?, 'persisted'
assert_not_nil @first.id
assert !@second.destroyed?, 'not destroyed'

0 comments on commit 5621c0c

Please sign in to comment.