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`.

3 files changed, 31 insertions, 18 deletions

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