diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 14d04a6388..6ecdab6eb0 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -167,8 +167,13 @@ module ActiveRecord def commit_transaction transaction = @stack.last - transaction.before_commit_records - @stack.pop + + begin + transaction.before_commit_records + ensure + @stack.pop + end + transaction.commit transaction.commit_records end diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index 637f89196e..8a7f19293d 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -34,6 +34,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase 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_create_commit { |record| record.do_after_commit(:create) } after_update_commit { |record| record.do_after_commit(:update) } @@ -47,6 +48,12 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @history ||= [] 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) @after_commit ||= {} @after_commit[on] ||= [] @@ -59,6 +66,11 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @after_rollback[on] << block 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) blocks = @after_commit[on] if defined?(@after_commit) blocks.each{|b| b.call(self)} if blocks @@ -74,6 +86,20 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @first = TopicWithCallbacks.find(1) 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 @first.after_commit_block{|r| r.history << :after_commit} @first.after_rollback_block{|r| r.history << :after_rollback}