diff options
4 files changed, 101 insertions, 62 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index e8ce00d92b..98e96099cb 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -203,62 +203,30 @@ module ActiveRecord if options[:isolation] raise ActiveRecord::TransactionIsolationError, "cannot set isolation when joining a transaction" end - yield else - within_new_transaction(options) { yield } + transaction_manager.within_new_transaction(options) { yield } end rescue ActiveRecord::Rollback # rollbacks are silently swallowed end - def within_new_transaction(options = {}) #:nodoc: - transaction = begin_transaction(options) - yield - rescue Exception => error - rollback_transaction if transaction - raise - ensure - begin - commit_transaction unless error - rescue Exception - rollback_transaction - raise - end - end - - def open_transactions - @transaction.number - end + attr_reader :transaction_manager #:nodoc: - def current_transaction #:nodoc: - @transaction - end + delegate :within_new_transaction, :open_transactions, :current_transaction, :begin_transaction, :commit_transaction, :rollback_transaction, to: :transaction_manager def transaction_open? - @transaction.open? - end - - def begin_transaction(options = {}) #:nodoc: - @transaction = @transaction.begin(options) - end - - def commit_transaction #:nodoc: - @transaction = @transaction.commit - end - - def rollback_transaction #:nodoc: - @transaction = @transaction.rollback + current_transaction.open? end def reset_transaction #:nodoc: - @transaction = ClosedTransaction.new(self) + @transaction_manager = TransactionManager.new(self) end # Register a record with the current transaction so that its after_commit and after_rollback callbacks # can be called. def add_transaction_record(record) - @transaction.add_record(record) + current_transaction.add_record(record) end # Begins the transaction (and turns off auto-committing). diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index bc4884b538..54f873a2a2 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -1,5 +1,63 @@ module ActiveRecord module ConnectionAdapters + class TransactionManager #:nodoc: + def initialize(connection) + @stack = [] + @connection = connection + end + + def begin_transaction(options = {}) + transaction = + if @stack.empty? + RealTransaction.new(@connection, current_transaction, options) + else + SavepointTransaction.new(@connection, current_transaction, options) + end + + @stack.push(transaction) + transaction + end + + def commit_transaction + @stack.pop.commit + end + + def rollback_transaction + @stack.pop.rollback + end + + def within_new_transaction(options = {}) + transaction = begin_transaction options + yield + rescue Exception => error + transaction.rollback if transaction + raise + ensure + begin + transaction.commit unless error + rescue Exception + transaction.rollback + raise + ensure + @stack.pop if transaction + end + end + + def open_transactions + @stack.size + end + + def current_transaction + @stack.last || closed_transaction + end + + private + + def closed_transaction + @closed_transaction ||= ClosedTransaction.new(@connection) + end + end + class Transaction #:nodoc: attr_reader :connection @@ -11,6 +69,10 @@ module ActiveRecord def state @state end + + def savepoint_name + nil + end end class TransactionState @@ -78,45 +140,28 @@ module ActiveRecord @parent = parent @records = [] - @finishing = false @joinable = options.fetch(:joinable, true) end - # This state is necessary so that we correctly handle stuff that might - # happen in a commit/rollback. But it's kinda distasteful. Maybe we can - # find a better way to structure it in the future. - def finishing? - @finishing - end def joinable? - @joinable && !finishing? + @joinable end def number - if finishing? - parent.number - else - parent.number + 1 - end + parent.number + 1 end def begin(options = {}) - if finishing? - parent.begin - else - SavepointTransaction.new(connection, self, options) - end + SavepointTransaction.new(connection, self, options) end def rollback - @finishing = true perform_rollback parent end def commit - @finishing = true perform_commit parent end @@ -183,24 +228,29 @@ module ActiveRecord end class SavepointTransaction < OpenTransaction #:nodoc: + attr_reader :savepoint_name + def initialize(connection, parent, options = {}) if options[:isolation] raise ActiveRecord::TransactionIsolationError, "cannot set transaction isolation in a nested transaction" end super - connection.create_savepoint + + # Savepoint name only counts the Savepoint transactions, so we need to subtract 1 + @savepoint_name = "active_record_#{number - 1}" + connection.create_savepoint(@savepoint_name) end def perform_rollback - connection.rollback_to_savepoint + connection.rollback_to_savepoint(@savepoint_name) rollback_records end def perform_commit @state.set_state(:committed) @state.parent = parent.state - connection.release_savepoint + connection.release_savepoint(@savepoint_name) end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index f8c054eb69..99c728814a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -45,6 +45,7 @@ module ActiveRecord end autoload_at 'active_record/connection_adapters/abstract/transaction' do + autoload :TransactionManager autoload :ClosedTransaction autoload :RealTransaction autoload :SavepointTransaction @@ -357,7 +358,7 @@ module ActiveRecord end def current_savepoint_name - "active_record_#{open_transactions}" + current_transaction.savepoint_name end # Check the connection back in to the connection pool diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index de1f624191..f28a7b00e2 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -424,6 +424,26 @@ class TransactionTest < ActiveRecord::TestCase end end + def test_savepoints_name + Topic.transaction do + assert_nil Topic.connection.current_savepoint_name + assert_nil Topic.connection.current_transaction.savepoint_name + + Topic.transaction(requires_new: true) do + assert_equal "active_record_1", Topic.connection.current_savepoint_name + assert_equal "active_record_1", Topic.connection.current_transaction.savepoint_name + + Topic.transaction(requires_new: true) do + assert_equal "active_record_2", Topic.connection.current_savepoint_name + assert_equal "active_record_2", Topic.connection.current_transaction.savepoint_name + end + + assert_equal "active_record_1", Topic.connection.current_savepoint_name + assert_equal "active_record_1", Topic.connection.current_transaction.savepoint_name + end + end + end + def test_rollback_when_commit_raises Topic.connection.expects(:begin_db_transaction) Topic.connection.expects(:commit_db_transaction).raises('OH NOES') |