Rollback the transaction when a before_* callback returns false.

Previously this would have committed the transaction but not carried out save or destroy operation.

[#891 state:committed]
Signed-off-by: Michael Koziarski <michael@koziarski.com>

2 files changed, 26 insertions, 2 deletions

diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index be2621fdb6..eec531c514 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -169,6 +169,18 @@ module ActiveRecord
   # If a <tt>before_*</tt> callback returns +false+, all the later callbacks and the associated action are cancelled. If an <tt>after_*</tt> callback returns
   # +false+, all the later callbacks are cancelled. Callbacks are generally run in the order they are defined, with the exception of callbacks
   # defined as methods on the model, which are called last.
+  #
+  # == Transactions
+  #
+  # The entire callback chain of a +save+, <tt>save!</tt>, or +destroy+ call runs
+  # within a transaction. That includes <tt>after_*</tt> hooks. If everything
+  # goes fine a COMMIT is executed once the chain has been completed.
+  #
+  # If a <tt>before_*</tt> callback cancels the action a ROLLBACK is issued. You
+  # can also trigger a ROLLBACK raising an exception in any of the callbacks,
+  # including <tt>after_*</tt> hooks. Note, however, that in that case the client
+  # needs to be aware of it because an ordinary +save+ will raise such exception
+  # instead of quietly returning +false+.
   module Callbacks
     CALLBACKS = %w(
       after_find after_initialize before_save after_save before_create after_create before_update after_update before_validation
diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index 0531afbb52..81462a2917 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -91,11 +91,11 @@ module ActiveRecord
     end
 
     def destroy_with_transactions #:nodoc:
-      transaction { destroy_without_transactions }
+      with_transaction_returning_status(:destroy_without_transactions)
     end
 
     def save_with_transactions(perform_validation = true) #:nodoc:
-      rollback_active_record_state! { transaction { save_without_transactions(perform_validation) } }
+      rollback_active_record_state! { with_transaction_returning_status(:save_without_transactions, perform_validation) }
     end
 
     def save_with_transactions! #:nodoc:
@@ -118,5 +118,17 @@ module ActiveRecord
       end
       raise
     end
+
+    # Executes +method+ within a transaction and captures its return value as a
+    # status flag. If the status is true the transaction is committed, otherwise
+    # a ROLLBACK is issued. In any case the status flag is returned.
+    def with_transaction_returning_status(method, *args)
+      status = nil
+      transaction do
+        status = send(method, *args)
+        raise ActiveRecord::Rollback unless status
+      end
+      status
+    end
   end
 end