1 files changed, 146 insertions, 18 deletions

diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index f28a7b00e2..ec5bdfd725 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -2,17 +2,18 @@ require "cases/helper"
 require 'models/topic'
 require 'models/reply'
 require 'models/developer'
+require 'models/computer'
 require 'models/book'
 require 'models/author'
 require 'models/post'
 require 'models/movie'
 
 class TransactionTest < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_tests = false
   fixtures :topics, :developers, :authors, :posts
 
   def setup
-    @first, @second = Topic.find(1, 2).sort_by { |t| t.id }
+    @first, @second = Topic.find(1, 2).sort_by(&:id)
   end
 
   def test_persisted_in_a_model_with_custom_primary_key_after_failed_save
@@ -80,6 +81,30 @@ class TransactionTest < ActiveRecord::TestCase
     end
   end
 
+  def test_number_of_transactions_in_commit
+    num = nil
+
+    Topic.connection.class_eval do
+      alias :real_commit_db_transaction :commit_db_transaction
+      define_method(:commit_db_transaction) do
+        num = transaction_manager.open_transactions
+        real_commit_db_transaction
+      end
+    end
+
+    Topic.transaction do
+      @first.approved  = true
+      @first.save!
+    end
+
+    assert_equal 0, num
+  ensure
+    Topic.connection.class_eval do
+      remove_method :commit_db_transaction
+      alias :commit_db_transaction :real_commit_db_transaction rescue nil
+    end
+  end
+
   def test_successful_with_instance_method
     @first.transaction do
       @first.approved  = true
@@ -150,13 +175,20 @@ class TransactionTest < ActiveRecord::TestCase
     assert topic.new_record?, "#{topic.inspect} should be new record"
   end
 
+  def test_transaction_state_is_cleared_when_record_is_persisted
+    author = Author.create! name: 'foo'
+    author.name = nil
+    assert_not author.save
+    assert_not author.new_record?
+  end
+
   def test_update_should_rollback_on_failure
     author = Author.find(1)
     posts_count = author.posts.size
     assert posts_count > 0
     status = author.update(name: nil, post_ids: [])
     assert !status
-    assert_equal posts_count, author.posts(true).size
+    assert_equal posts_count, author.posts.reload.size
   end
 
   def test_update_should_rollback_on_failure!
@@ -166,7 +198,17 @@ class TransactionTest < ActiveRecord::TestCase
     assert_raise(ActiveRecord::RecordInvalid) do
       author.update!(name: nil, post_ids: [])
     end
-    assert_equal posts_count, author.posts(true).size
+    assert_equal posts_count, author.posts.reload.size
+  end
+
+  def test_cancellation_from_returning_false_in_before_filter
+    def @first.before_save_for_transaction
+      false
+    end
+
+    assert_deprecated do
+      @first.save
+    end
   end
 
   def test_cancellation_from_before_destroy_rollbacks_in_destroy
@@ -445,13 +487,17 @@ class TransactionTest < ActiveRecord::TestCase
   end
 
   def test_rollback_when_commit_raises
-    Topic.connection.expects(:begin_db_transaction)
-    Topic.connection.expects(:commit_db_transaction).raises('OH NOES')
-    Topic.connection.expects(:rollback_db_transaction)
+    assert_called(Topic.connection, :begin_db_transaction) do
+      Topic.connection.stub(:commit_db_transaction, ->{ raise('OH NOES') }) do
+        assert_called(Topic.connection, :rollback_db_transaction) do
 
-    assert_raise RuntimeError do
-      Topic.transaction do
-        # do nothing
+          e = assert_raise RuntimeError do
+            Topic.transaction do
+              # do nothing
+            end
+          end
+          assert_equal 'OH NOES', e.message
+        end
       end
     end
   end
@@ -468,6 +514,34 @@ class TransactionTest < ActiveRecord::TestCase
     assert topic.frozen?, 'not frozen'
   end
 
+  def test_rollback_when_thread_killed
+    return if in_memory_db?
+
+    queue = Queue.new
+    thread = Thread.new do
+      Topic.transaction do
+        @first.approved  = true
+        @second.approved = false
+        @first.save
+
+        queue.push nil
+        sleep
+
+        @second.save
+      end
+    end
+
+    queue.pop
+    thread.kill
+    thread.join
+
+    assert @first.approved?, "First should still be changed in the objects"
+    assert !@second.approved?, "Second should still be changed in the objects"
+
+    assert !Topic.find(1).approved?, "First shouldn't have been approved"
+    assert Topic.find(2).approved?, "Second should still be approved"
+  end
+
   def test_restore_active_record_state_for_all_records_in_a_transaction
     topic_without_callbacks = Class.new(ActiveRecord::Base) do
       self.table_name = 'topics'
@@ -506,6 +580,39 @@ class TransactionTest < ActiveRecord::TestCase
     assert !@second.destroyed?, 'not destroyed'
   end
 
+  def test_restore_frozen_state_after_double_destroy
+    topic = Topic.create
+    reply = topic.replies.create
+
+    Topic.transaction do
+      topic.destroy # calls #destroy on reply (since dependent: destroy)
+      reply.destroy
+
+      raise ActiveRecord::Rollback
+    end
+
+    assert_not reply.frozen?
+    assert_not topic.frozen?
+  end
+
+  def test_rollback_of_frozen_records
+    topic = Topic.create.freeze
+    Topic.transaction do
+      topic.destroy
+      raise ActiveRecord::Rollback
+    end
+    assert topic.frozen?, 'frozen'
+  end
+
+  def test_rollback_for_freshly_persisted_records
+    topic = Topic.create
+    Topic.transaction do
+      topic.destroy
+      raise ActiveRecord::Rollback
+    end
+    assert topic.persisted?, 'persisted'
+  end
+
   def test_sqlite_add_column_in_transaction
     return true unless current_adapter?(:SQLite3Adapter)
 
@@ -546,13 +653,13 @@ class TransactionTest < ActiveRecord::TestCase
 
   def test_transactions_state_from_rollback
     connection = Topic.connection
-    transaction = ActiveRecord::ConnectionAdapters::ClosedTransaction.new(connection).begin
+    transaction = ActiveRecord::ConnectionAdapters::TransactionManager.new(connection).begin_transaction
 
     assert transaction.open?
     assert !transaction.state.rolledback?
     assert !transaction.state.committed?
 
-    transaction.perform_rollback
+    transaction.rollback
 
     assert transaction.state.rolledback?
     assert !transaction.state.committed?
@@ -560,18 +667,39 @@ class TransactionTest < ActiveRecord::TestCase
 
   def test_transactions_state_from_commit
     connection = Topic.connection
-    transaction = ActiveRecord::ConnectionAdapters::ClosedTransaction.new(connection).begin
+    transaction = ActiveRecord::ConnectionAdapters::TransactionManager.new(connection).begin_transaction
 
     assert transaction.open?
     assert !transaction.state.rolledback?
     assert !transaction.state.committed?
 
-    transaction.perform_commit
+    transaction.commit
 
     assert !transaction.state.rolledback?
     assert transaction.state.committed?
   end
 
+  def test_transaction_rollback_with_primarykeyless_tables
+    connection = ActiveRecord::Base.connection
+    connection.create_table(:transaction_without_primary_keys, force: true, id: false) do |t|
+       t.integer :thing_id
+    end
+
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = 'transaction_without_primary_keys'
+      after_commit { } # necessary to trigger the has_transactional_callbacks branch
+    end
+
+    assert_no_difference(-> { klass.count }) do
+      ActiveRecord::Base.transaction do
+        klass.create!
+        raise ActiveRecord::Rollback
+      end
+    end
+  ensure
+    connection.drop_table 'transaction_without_primary_keys', if_exists: true
+  end
+
   private
 
   %w(validation save destroy).each do |filter|
@@ -579,14 +707,14 @@ class TransactionTest < ActiveRecord::TestCase
       meta = class << topic; self; end
       meta.send("define_method", "before_#{filter}_for_transaction") do
         Book.create
-        false
+        throw(:abort)
       end
     end
   end
 end
 
 class TransactionsWithTransactionalFixturesTest < ActiveRecord::TestCase
-  self.use_transactional_fixtures = true
+  self.use_transactional_tests = true
   fixtures :topics
 
   def test_automatic_savepoint_in_outer_transaction
@@ -643,7 +771,7 @@ if current_adapter?(:PostgreSQLAdapter)
           end
         end
 
-        threads.each { |t| t.join }
+        threads.each(&:join)
       end
     end
 
@@ -691,7 +819,7 @@ if current_adapter?(:PostgreSQLAdapter)
           Developer.connection.close
         end
 
-        threads.each { |t| t.join }
+        threads.each(&:join)
       end
 
       assert_equal original_salary, Developer.find(1).salary