Permalink
Browse files

When an exception is caused during a 'release_savepoint' call, after …

…handling the error, activerecord re-raises so outer code has a chance to see the error. However, active_callback was acting as a sink, catching exceptions and not re-throwing to prevent active record from triggering a rollback on

an already committed savepoint.  This submission makes an attempt to save commit progress in a non-transient manner, so exceptions can be passed to active record while allowing rollbacks to be skipped if a savepoint has already been released.  The immediate benefit are unit/functional tests
properly reporting errors that occur during after_commit callbacks triggered during a release_savepoint call.
  • Loading branch information...
1 parent 573f3f7 commit 622707f20fe82883c6d221df5ecea04ce6d5e17b Keith Simmons committed Apr 26, 2011
Showing with 22 additions and 12 deletions.
  1. +21 −12 lib/after_commit/after_savepoint.rb
  2. +1 −0 lib/after_commit/connection_adapters.rb
@@ -34,9 +34,19 @@ def include_after_savepoint_extension(adapter)
module TestConnectionAdapters
def self.included(base)
base.class_eval do
+
+ def after_callback_transaction_committed?
+ committed = (Thread.current[:after_callback_committed] || {})[unique_transaction_key]
+ return !committed.nil? && committed
+ end
+
+ def after_callback_mark_committed committed
+ (Thread.current[:after_callback_committed] ||= {})[unique_transaction_key] = committed
+ end
+
def release_savepoint_with_callback
increment_transaction_pointer
- committed = false
+ after_callback_mark_committed false
begin
trigger_before_commit_callbacks
trigger_before_commit_on_create_callbacks
@@ -45,21 +55,17 @@ def release_savepoint_with_callback
trigger_before_commit_on_destroy_callbacks
release_savepoint_without_callback
- committed = true
+ after_callback_mark_committed true
trigger_after_commit_callbacks
trigger_after_commit_on_create_callbacks
trigger_after_commit_on_save_callbacks
trigger_after_commit_on_update_callbacks
trigger_after_commit_on_destroy_callbacks
- rescue Exception => e
- unless committed
- decrement_transaction_pointer
- rollback_to_savepoint
- increment_transaction_pointer
- end
ensure
- AfterCommit.cleanup(self)
+ if after_callback_transaction_committed?
+ AfterCommit.cleanup(self)
+ end
decrement_transaction_pointer
end
end
@@ -71,9 +77,12 @@ def release_savepoint_with_callback
def rollback_to_savepoint_with_callback
increment_transaction_pointer
begin
- trigger_before_rollback_callbacks
- rollback_to_savepoint_without_callback
- trigger_after_rollback_callbacks
+ # Only rollback if we have not already released rollback
+ unless after_callback_transaction_committed?
+ trigger_before_rollback_callbacks
+ rollback_to_savepoint_without_callback
+ trigger_after_rollback_callbacks
+ end
ensure
AfterCommit.cleanup(self)
end
@@ -183,6 +183,7 @@ def decrement_transaction_pointer
Thread.current[:after_commit_pointer] ||= 0
Thread.current[:after_commit_pointer] -= 1
end
+
end
end
end

0 comments on commit 622707f

Please sign in to comment.