From 91e4b65093f0c485507dadcb018c1f0d2661ba89 Mon Sep 17 00:00:00 2001 From: Arthur Neves Date: Mon, 28 Jul 2014 14:26:27 -0400 Subject: [PATCH 1/3] Remove being/number methods from transaction class --- .../abstract/transaction.rb | 35 +++++-------------- activerecord/test/cases/transactions_test.rb | 4 +-- 2 files changed, 10 insertions(+), 29 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 33cc22425d..1691daf861 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -8,7 +8,7 @@ module ActiveRecord def begin_transaction(options = {}) transaction_class = @stack.empty? ? RealTransaction : SavepointTransaction - transaction = transaction_class.new(@connection, current_transaction, options) + transaction = transaction_class.new(@connection, current_transaction, "active_record_#{@stack.size}", options) @stack.push(transaction) transaction @@ -102,14 +102,6 @@ module ActiveRecord end class ClosedTransaction < Transaction #:nodoc: - def number - 0 - end - - def begin(options = {}) - RealTransaction.new(connection, self, options) - end - def closed? true end @@ -144,14 +136,6 @@ module ActiveRecord @joinable end - def number - parent.number + 1 - end - - def begin(options = {}) - SavepointTransaction.new(connection, self, options) - end - def rollback perform_rollback parent @@ -202,8 +186,8 @@ module ActiveRecord end class RealTransaction < OpenTransaction #:nodoc: - def initialize(connection, parent, options = {}) - super + def initialize(connection, parent, _, options = {}) + super(connection, parent, options) if options[:isolation] connection.begin_isolated_db_transaction(options[:isolation]) @@ -226,26 +210,23 @@ module ActiveRecord class SavepointTransaction < OpenTransaction #:nodoc: attr_reader :savepoint_name - def initialize(connection, parent, options = {}) + def initialize(connection, parent, savepoint_name, options = {}) if options[:isolation] raise ActiveRecord::TransactionIsolationError, "cannot set transaction isolation in a nested transaction" end - super - - # Savepoint name only counts the Savepoint transactions, so we need to subtract 1 - @savepoint_name = "active_record_#{number - 1}" - connection.create_savepoint(@savepoint_name) + super(connection, parent, options) + connection.create_savepoint(@savepoint_name = savepoint_name) end def perform_rollback - connection.rollback_to_savepoint(@savepoint_name) + connection.rollback_to_savepoint(savepoint_name) rollback_records end def perform_commit @state.set_state(:committed) - connection.release_savepoint(@savepoint_name) + connection.release_savepoint(savepoint_name) end end end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index f28a7b00e2..e518033192 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -546,7 +546,7 @@ 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? @@ -560,7 +560,7 @@ 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? From dac9c92e3096ae196d7ea4b58e7141f4a36007ea Mon Sep 17 00:00:00 2001 From: Arthur Neves Date: Mon, 28 Jul 2014 13:07:47 -0400 Subject: [PATCH 2/3] Remove parent on Transaction object --- .../abstract/transaction.rb | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 1691daf861..17d57a7519 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -8,7 +8,7 @@ module ActiveRecord def begin_transaction(options = {}) transaction_class = @stack.empty? ? RealTransaction : SavepointTransaction - transaction = transaction_class.new(@connection, current_transaction, "active_record_#{@stack.size}", options) + transaction = transaction_class.new(@connection, "active_record_#{@stack.size}", options) @stack.push(transaction) transaction @@ -55,17 +55,13 @@ module ActiveRecord end class Transaction #:nodoc: - attr_reader :connection + attr_reader :connection, :state def initialize(connection) @connection = connection @state = TransactionState.new end - def state - @state - end - def savepoint_name nil end @@ -120,13 +116,12 @@ module ActiveRecord end class OpenTransaction < Transaction #:nodoc: - attr_reader :parent, :records + attr_reader :records attr_writer :joinable - def initialize(connection, parent, options = {}) + def initialize(connection, options = {}) super connection - @parent = parent @records = [] @joinable = options.fetch(:joinable, true) end @@ -138,12 +133,10 @@ module ActiveRecord def rollback perform_rollback - parent end def commit perform_commit - parent end def add_record(record) @@ -158,7 +151,7 @@ module ActiveRecord @state.set_state(:rolledback) records.uniq.each do |record| begin - record.rolledback!(parent.closed?) + record.rolledback!(self.is_a?(RealTransaction)) rescue => e record.logger.error(e) if record.respond_to?(:logger) && record.logger end @@ -186,8 +179,8 @@ module ActiveRecord end class RealTransaction < OpenTransaction #:nodoc: - def initialize(connection, parent, _, options = {}) - super(connection, parent, options) + def initialize(connection, _, options = {}) + super(connection, options) if options[:isolation] connection.begin_isolated_db_transaction(options[:isolation]) @@ -210,12 +203,12 @@ module ActiveRecord class SavepointTransaction < OpenTransaction #:nodoc: attr_reader :savepoint_name - def initialize(connection, parent, savepoint_name, options = {}) + def initialize(connection, savepoint_name, options = {}) if options[:isolation] raise ActiveRecord::TransactionIsolationError, "cannot set transaction isolation in a nested transaction" end - super(connection, parent, options) + super(connection, options) connection.create_savepoint(@savepoint_name = savepoint_name) end From 4140797967a8f50eccaa70614c01531dab6a90f4 Mon Sep 17 00:00:00 2001 From: Arthur Neves Date: Wed, 30 Jul 2014 14:32:56 -0400 Subject: [PATCH 3/3] Make ClosedTransaction a null object --- .../abstract/transaction.rb | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 17d57a7519..ff7f922d7f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -50,7 +50,7 @@ module ActiveRecord private def closed_transaction - @closed_transaction ||= ClosedTransaction.new(@connection) + @closed_transaction ||= ClosedTransaction.new end end @@ -98,21 +98,12 @@ module ActiveRecord end class ClosedTransaction < Transaction #:nodoc: - def closed? - true - end - - def open? - false - end - - def joinable? - false - end - + def initialize; super(nil); end + def closed?; true; end + def open?; false; end + def joinable?; false; end # This is a noop when there are no open transactions - def add_record(record) - end + def add_record(record); end end class OpenTransaction < Transaction #:nodoc: @@ -126,7 +117,6 @@ module ActiveRecord @joinable = options.fetch(:joinable, true) end - def joinable? @joinable end