after_commit runs after transactions with non-joinable parents

after_commit callbacks run after committing a transaction whose parent
is not `joinable?`: un-nested transactions, transactions within test
cases, and transactions in `console --sandbox`.

5 files changed, 51 insertions, 22 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index c777af342f..620d256d16 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -12,6 +12,15 @@
 
     *Sean Griffin*
 
+*   Tests now run after_commit callbacks. You no longer have to declare
+    `uses_transaction ‘test name’` to test the results of an after_commit.
+
+    after_commit callbacks run after committing a transaction whose parent
+    is not `joinable?`: un-nested transactions, transactions within test cases,
+    and transactions in `console --sandbox`.
+
+    *arthurnn*, *Ravil Bayramgalin*, *Matthew Draper*
+
 *   `nil` as a value for a binary column in a query no longer logs as
     "<NULL binary data>", and instead logs as just "nil".
 
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
index 7535e9147a..84e5386c20 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
@@ -142,7 +142,6 @@ module ActiveRecord
       def commit
         connection.commit_db_transaction
         super
-        commit_records
       end
     end
 
@@ -159,14 +158,22 @@ module ActiveRecord
           else
             SavepointTransaction.new(@connection, "active_record_#{@stack.size}", options)
           end
+
         @stack.push(transaction)
         transaction
       end
 
       def commit_transaction
-        transaction = @stack.pop
-        transaction.commit
-        transaction.records.each { |r| current_transaction.add_record(r) }
+        inner_transaction = @stack.pop
+        inner_transaction.commit
+
+        if current_transaction.joinable?
+          inner_transaction.records.each do |r|
+            current_transaction.add_record(r)
+          end
+        else
+          inner_transaction.commit_records
+        end
       end
 
       def rollback_transaction
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 21a45042fa..3bd7506bb5 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -1307,7 +1307,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_nothing_raised { topic.destroy }
   end
 
-  uses_transaction :test_dependence_with_transaction_support_on_failure
   def test_dependence_with_transaction_support_on_failure
     firm = companies(:first_firm)
     clients = firm.clients
diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb
index 670d94dc06..3ae4a6eade 100644
--- a/activerecord/test/cases/callbacks_test.rb
+++ b/activerecord/test/cases/callbacks_test.rb
@@ -288,7 +288,12 @@ class CallbacksTest < ActiveRecord::TestCase
       [ :after_save,                  :string ],
       [ :after_save,                  :proc   ],
       [ :after_save,                  :object ],
-      [ :after_save,                  :block  ]
+      [ :after_save,                  :block  ],
+      [ :after_commit,                :block  ],
+      [ :after_commit,                :object ],
+      [ :after_commit,                :proc   ],
+      [ :after_commit,                :string ],
+      [ :after_commit,                :method ]
     ], david.history
   end
 
@@ -357,7 +362,12 @@ class CallbacksTest < ActiveRecord::TestCase
       [ :after_save,                  :string ],
       [ :after_save,                  :proc   ],
       [ :after_save,                  :object ],
-      [ :after_save,                  :block  ]
+      [ :after_save,                  :block  ],
+      [ :after_commit,                :block  ],
+      [ :after_commit,                :object ],
+      [ :after_commit,                :proc   ],
+      [ :after_commit,                :string ],
+      [ :after_commit,                :method ]
     ], david.history
   end
 
@@ -408,7 +418,12 @@ class CallbacksTest < ActiveRecord::TestCase
       [ :after_destroy,               :string ],
       [ :after_destroy,               :proc   ],
       [ :after_destroy,               :object ],
-      [ :after_destroy,               :block  ]
+      [ :after_destroy,               :block  ],
+      [ :after_commit,                :block  ],
+      [ :after_commit,                :object ],
+      [ :after_commit,                :proc   ],
+      [ :after_commit,                :string ],
+      [ :after_commit,                :method ]
     ], david.history
   end
 
diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb
index 185fc22e98..9804707d00 100644
--- a/activerecord/test/cases/transaction_callbacks_test.rb
+++ b/activerecord/test/cases/transaction_callbacks_test.rb
@@ -4,7 +4,6 @@ require 'models/pet'
 require 'models/topic'
 
 class TransactionCallbacksTest < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
   fixtures :topics, :owners, :pets
 
   class ReplyWithCallbacks < ActiveRecord::Base
@@ -200,21 +199,21 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_call_after_rollback_when_commit_fails
-    @first.class.connection.singleton_class.send(:alias_method, :real_method_commit_db_transaction, :commit_db_transaction)
-    begin
-      @first.class.connection.singleton_class.class_eval do
-        def commit_db_transaction; raise "boom!"; end
+    @first.after_commit_block { |r| r.history << :after_commit }
+    @first.after_rollback_block { |r| r.history << :after_rollback }
+    
+    assert_raises RuntimeError do
+      @first.transaction do
+        tx = @first.class.connection.transaction_manager.current_transaction
+        def tx.commit
+          raise
+        end
+
+        @first.save
       end
-
-      @first.after_commit_block{|r| r.history << :after_commit}
-      @first.after_rollback_block{|r| r.history << :after_rollback}
-
-      assert !@first.save rescue nil
-      assert_equal [:after_rollback], @first.history
-    ensure
-      @first.class.connection.singleton_class.send(:remove_method, :commit_db_transaction)
-      @first.class.connection.singleton_class.send(:alias_method, :commit_db_transaction, :real_method_commit_db_transaction)
     end
+
+    assert_equal [:after_rollback], @first.history
   end
 
   def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint