mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Merge pull request #23407 from jeremy/corrupt-before-commit
Fix corrupt transaction state caused by `before_commit` exceptions
This commit is contained in:
commit
633969f159
2 changed files with 33 additions and 2 deletions
|
@ -167,8 +167,13 @@ module ActiveRecord
|
||||||
|
|
||||||
def commit_transaction
|
def commit_transaction
|
||||||
transaction = @stack.last
|
transaction = @stack.last
|
||||||
transaction.before_commit_records
|
|
||||||
@stack.pop
|
begin
|
||||||
|
transaction.before_commit_records
|
||||||
|
ensure
|
||||||
|
@stack.pop
|
||||||
|
end
|
||||||
|
|
||||||
transaction.commit
|
transaction.commit
|
||||||
transaction.commit_records
|
transaction.commit_records
|
||||||
end
|
end
|
||||||
|
|
|
@ -34,6 +34,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
|
||||||
|
|
||||||
has_many :replies, class_name: "ReplyWithCallbacks", foreign_key: "parent_id"
|
has_many :replies, class_name: "ReplyWithCallbacks", foreign_key: "parent_id"
|
||||||
|
|
||||||
|
before_commit { |record| record.do_before_commit(nil) }
|
||||||
after_commit { |record| record.do_after_commit(nil) }
|
after_commit { |record| record.do_after_commit(nil) }
|
||||||
after_create_commit { |record| record.do_after_commit(:create) }
|
after_create_commit { |record| record.do_after_commit(:create) }
|
||||||
after_update_commit { |record| record.do_after_commit(:update) }
|
after_update_commit { |record| record.do_after_commit(:update) }
|
||||||
|
@ -47,6 +48,12 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
|
||||||
@history ||= []
|
@history ||= []
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def before_commit_block(on = nil, &block)
|
||||||
|
@before_commit ||= {}
|
||||||
|
@before_commit[on] ||= []
|
||||||
|
@before_commit[on] << block
|
||||||
|
end
|
||||||
|
|
||||||
def after_commit_block(on = nil, &block)
|
def after_commit_block(on = nil, &block)
|
||||||
@after_commit ||= {}
|
@after_commit ||= {}
|
||||||
@after_commit[on] ||= []
|
@after_commit[on] ||= []
|
||||||
|
@ -59,6 +66,11 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
|
||||||
@after_rollback[on] << block
|
@after_rollback[on] << block
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def do_before_commit(on)
|
||||||
|
blocks = @before_commit[on] if defined?(@before_commit)
|
||||||
|
blocks.each{|b| b.call(self)} if blocks
|
||||||
|
end
|
||||||
|
|
||||||
def do_after_commit(on)
|
def do_after_commit(on)
|
||||||
blocks = @after_commit[on] if defined?(@after_commit)
|
blocks = @after_commit[on] if defined?(@after_commit)
|
||||||
blocks.each{|b| b.call(self)} if blocks
|
blocks.each{|b| b.call(self)} if blocks
|
||||||
|
@ -74,6 +86,20 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
|
||||||
@first = TopicWithCallbacks.find(1)
|
@first = TopicWithCallbacks.find(1)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# FIXME: Test behavior, not implementation.
|
||||||
|
def test_before_commit_exception_should_pop_transaction_stack
|
||||||
|
@first.before_commit_block { raise 'better pop this txn from the stack!' }
|
||||||
|
|
||||||
|
original_txn = @first.class.connection.current_transaction
|
||||||
|
|
||||||
|
begin
|
||||||
|
@first.save!
|
||||||
|
fail
|
||||||
|
rescue
|
||||||
|
assert_equal original_txn, @first.class.connection.current_transaction
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def test_call_after_commit_after_transaction_commits
|
def test_call_after_commit_after_transaction_commits
|
||||||
@first.after_commit_block{|r| r.history << :after_commit}
|
@first.after_commit_block{|r| r.history << :after_commit}
|
||||||
@first.after_rollback_block{|r| r.history << :after_rollback}
|
@first.after_rollback_block{|r| r.history << :after_rollback}
|
||||||
|
|
Loading…
Reference in a new issue