Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix transaction pointer in rollback

Ensures pointer is correct when called from outside #commit_db_transaction_with_callback
  • Loading branch information...
commit 3b164bf88fc61cc2ead7366a96e1636d82cbcc4b 1 parent b5748d3
@mocoso mocoso authored committed
View
8 lib/after_commit/after_savepoint.rb
@@ -51,7 +51,11 @@ def release_savepoint_with_callback
trigger_after_commit_on_update_callbacks
trigger_after_commit_on_destroy_callbacks
rescue
- rollback_to_savepoint unless committed
+ unless committed
+ decrement_transaction_pointer
+ rollback_to_savepoint
+ increment_transaction_pointer
+ end
ensure
AfterCommit.cleanup(self)
decrement_transaction_pointer
@@ -63,6 +67,7 @@ def release_savepoint_with_callback
# should recieve the after_commit callback, but do fire the after_rollback
# callback for each record that failed to be committed.
def rollback_to_savepoint_with_callback
+ increment_transaction_pointer
begin
trigger_before_rollback_callbacks
rollback_to_savepoint_without_callback
@@ -70,6 +75,7 @@ def rollback_to_savepoint_with_callback
ensure
AfterCommit.cleanup(self)
end
+ decrement_transaction_pointer
end
alias_method_chain :rollback_to_savepoint, :callback
end
View
11 lib/after_commit/connection_adapters.rb
@@ -37,6 +37,16 @@ def commit_db_transaction_with_callback
trigger_after_commit_on_update_callbacks
trigger_after_commit_on_destroy_callbacks
result
+ rescue
+ if committed
+ result
+ else
+ # Need to decrement the transaction pointer before calling
+ # rollback... to ensure it is not incremented twice
+ decrement_transaction_pointer
+ rollback_db_transaction
+ increment_transaction_pointer
+ end
ensure
AfterCommit.cleanup(self)
decrement_transaction_pointer
@@ -60,6 +70,7 @@ def rollback_db_transaction_with_callback
AfterCommit.cleanup(self)
decrement_transaction_pointer
end
+ decrement_transaction_pointer
end
alias_method_chain :rollback_db_transaction, :callback
View
22 test/after_commit_test.rb
@@ -71,7 +71,7 @@ class Bar < ActiveRecord::Base
end
class UnsavableRecord < ActiveRecord::Base
- attr_accessor :after_commit_called
+ attr_accessor :after_commit_called, :after_rollback_called
set_table_name 'mock_records'
@@ -86,10 +86,14 @@ def after_save
end
after_commit :after_commit
-
def after_commit
self.after_commit_called = true
end
+
+ after_rollback :after_rollback
+ def after_rollback
+ self.after_rollback_called = true
+ end
end
class AfterCommitTest < Test::Unit::TestCase
@@ -124,7 +128,12 @@ def test_after_commit_on_destroy_is_called
def test_after_commit_does_not_trigger_when_transaction_rolls_back
record = UnsavableRecord.new
begin; record.save; rescue; end
-
+
+ # Ensure that commit of another transaction doesn't then trigger the
+ # after_commit hook on previously rolled back record
+ another_record = MockRecord.create!
+ another_record.save
+
assert_equal false, record.after_commit_called
end
@@ -141,6 +150,13 @@ def test_after_commit_does_not_trigger_when_unrelated_transaction_commits
assert_equal 1, CountingRecord.counter
end
+ def test_after_rollback_triggered_when_transaction_rolls_back
+ record = UnsavableRecord.new
+ begin; record.save; rescue; end
+
+ assert record.after_rollback_called
+ end
+
def test_two_transactions_are_separate
Bar.delete_all
foo = Foo.create
Please sign in to comment.
Something went wrong with that request. Please try again.