diff options
Diffstat (limited to 'activerecord')
15 files changed, 296 insertions, 206 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 11e4d34de2..32e3c7f5d8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -3,8 +3,7 @@ module ActiveRecord module DatabaseStatements def initialize super - @_current_transaction_records = [] - @transaction_joinable = nil + reset_transaction end # Converts an arel AST to SQL @@ -108,20 +107,6 @@ module ActiveRecord exec_delete(to_sql(arel, binds), name, binds) end - # Checks whether there is currently no transaction active. This is done - # by querying the database driver, and does not use the transaction - # house-keeping information recorded by #increment_open_transactions and - # friends. - # - # Returns true if there is no transaction active, false if there is a - # transaction active, and nil if this information is unknown. - # - # Not all adapters supports transaction state introspection. Currently, - # only the PostgreSQL adapter supports this. - def outside_transaction? - nil - end - # Returns +true+ when the connection adapter supports prepared statement # caching, otherwise returns +false+ def supports_statement_cache? @@ -173,76 +158,60 @@ module ActiveRecord def transaction(options = {}) options.assert_valid_keys :requires_new, :joinable - last_transaction_joinable = @transaction_joinable - @transaction_joinable = options.fetch(:joinable, true) - requires_new = options[:requires_new] || !last_transaction_joinable - transaction_open = false - - begin - if requires_new || open_transactions == 0 - if open_transactions == 0 - begin_db_transaction - elsif requires_new - create_savepoint - end - increment_open_transactions - transaction_open = true - @_current_transaction_records.push([]) - end + if !options[:requires_new] && current_transaction.joinable? yield - rescue Exception => database_transaction_rollback - if transaction_open && !outside_transaction? - transaction_open = false - txn = decrement_open_transactions - txn.aborted! - if open_transactions == 0 - rollback_db_transaction - rollback_transaction_records(true) - else - rollback_to_savepoint - rollback_transaction_records(false) - end - end - raise unless database_transaction_rollback.is_a?(ActiveRecord::Rollback) + else + within_new_transaction(options) { yield } end + rescue ActiveRecord::Rollback + # rollbacks are silently swallowed + end + + def within_new_transaction(options = {}) #:nodoc: + begin_transaction(options) + yield + rescue Exception => error + rollback_transaction + raise ensure - @transaction_joinable = last_transaction_joinable - - if outside_transaction? - @current_transaction = nil - elsif transaction_open - txn = decrement_open_transactions - txn.committed! - begin - if open_transactions == 0 - commit_db_transaction - commit_transaction_records - else - release_savepoint - save_point_records = @_current_transaction_records.pop - unless save_point_records.blank? - @_current_transaction_records.push([]) if @_current_transaction_records.empty? - @_current_transaction_records.last.concat(save_point_records) - end - end - rescue Exception - if open_transactions == 0 - rollback_db_transaction - rollback_transaction_records(true) - else - rollback_to_savepoint - rollback_transaction_records(false) - end - raise - end + begin + commit_transaction unless error + rescue Exception + rollback_transaction + raise end end + def current_transaction #:nodoc: + @transaction + end + + def transaction_open? + @transaction.open? + end + + def begin_transaction(options = {}) #:nodoc: + @transaction = @transaction.begin + @transaction.joinable = options.fetch(:joinable, true) + @transaction + end + + def commit_transaction #:nodoc: + @transaction = @transaction.commit + end + + def rollback_transaction #:nodoc: + @transaction = @transaction.rollback + end + + def reset_transaction #:nodoc: + @transaction = ClosedTransaction.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) - last_batch = @_current_transaction_records.last - last_batch << record if last_batch + @transaction.add_record(record) end # Begins the transaction (and turns off auto-committing). @@ -356,42 +325,6 @@ module ActiveRecord update_sql(sql, name) end - # Send a rollback message to all records after they have been rolled back. If rollback - # is false, only rollback records since the last save point. - def rollback_transaction_records(rollback) - if rollback - records = @_current_transaction_records.flatten - @_current_transaction_records.clear - else - records = @_current_transaction_records.pop - end - - unless records.blank? - records.uniq.each do |record| - begin - record.rolledback!(rollback) - rescue => e - record.logger.error(e) if record.respond_to?(:logger) && record.logger - end - end - end - end - - # Send a commit message to all records after they have been committed. - def commit_transaction_records - records = @_current_transaction_records.flatten - @_current_transaction_records.clear - unless records.blank? - records.uniq.each do |record| - begin - record.committed! - rescue => e - record.logger.error(e) if record.respond_to?(:logger) && record.logger - end - end - end - end - def sql_for_insert(sql, pk, id_value, sequence_name, binds) [sql, binds] end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb new file mode 100644 index 0000000000..2117eae5cb --- /dev/null +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -0,0 +1,156 @@ +module ActiveRecord + module ConnectionAdapters + class Transaction #:nodoc: + attr_reader :connection + + def initialize(connection) + @connection = connection + end + end + + class ClosedTransaction < Transaction #:nodoc: + def number + 0 + end + + def begin + RealTransaction.new(connection, self) + 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 + end + + class OpenTransaction < Transaction #:nodoc: + attr_reader :parent, :records + attr_writer :joinable + + def initialize(connection, parent) + super connection + + @parent = parent + @records = [] + @finishing = false + @joinable = true + end + + # This state is necesarry 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? + end + + def number + if finishing? + parent.number + else + parent.number + 1 + end + end + + def begin + if finishing? + parent.begin + else + SavepointTransaction.new(connection, self) + end + end + + def rollback + @finishing = true + perform_rollback + parent + end + + def commit + @finishing = true + perform_commit + parent + end + + def add_record(record) + records << record + end + + def rollback_records + records.uniq.each do |record| + begin + record.rolledback!(parent.closed?) + rescue => e + record.logger.error(e) if record.respond_to?(:logger) && record.logger + end + end + end + + def commit_records + records.uniq.each do |record| + begin + record.committed! + rescue => e + record.logger.error(e) if record.respond_to?(:logger) && record.logger + end + end + end + + def closed? + false + end + + def open? + true + end + end + + class RealTransaction < OpenTransaction #:nodoc: + def initialize(connection, parent) + super + connection.begin_db_transaction + end + + def perform_rollback + connection.rollback_db_transaction + rollback_records + end + + def perform_commit + connection.commit_db_transaction + commit_records + end + end + + class SavepointTransaction < OpenTransaction #:nodoc: + def initialize(connection, parent) + super + connection.create_savepoint + end + + def perform_rollback + connection.rollback_to_savepoint + rollback_records + end + + def perform_commit + connection.release_savepoint + records.each { |r| parent.add_record(r) } + end + 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 27700e4fd2..c37c9b1ae1 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -4,6 +4,7 @@ require 'bigdecimal/util' require 'active_support/core_ext/benchmark' require 'active_record/connection_adapters/schema_cache' require 'monitor' +require 'active_support/deprecation' module ActiveRecord module ConnectionAdapters # :nodoc: @@ -33,6 +34,12 @@ module ActiveRecord autoload :QueryCache end + autoload_at 'active_record/connection_adapters/abstract/transaction' do + autoload :ClosedTransaction + autoload :RealTransaction + autoload :SavepointTransaction + end + # Active Record supports multiple database systems. AbstractAdapter and # related classes form the abstraction layer which makes this possible. # An AbstractAdapter represents a connection to a database, and provides an @@ -62,14 +69,11 @@ module ActiveRecord def initialize(connection, logger = nil, pool = nil) #:nodoc: super() - @active = nil @connection = connection @in_use = false @instrumenter = ActiveSupport::Notifications.instrumenter @last_use = false @logger = logger - @open_transactions = 0 - @current_transaction = nil @pool = pool @query_cache = Hash.new { |h,sql| h[sql] = {} } @query_cache_enabled = false @@ -182,19 +186,21 @@ module ActiveRecord # checking whether the database is actually capable of responding, i.e. whether # the connection isn't stale. def active? - @active != false end # Disconnects from the database if already connected, and establishes a - # new connection with the database. + # new connection with the database. Implementors should call super if they + # override the default implementation. def reconnect! - @active = true + clear_cache! + reset_transaction end # Disconnects from the database if already connected. Otherwise, this # method does nothing. def disconnect! - @active = false + clear_cache! + reset_transaction end # Reset the state of this connection, directing the DBMS to clear @@ -238,33 +244,20 @@ module ActiveRecord end def open_transactions - count = 0 - txn = current_transaction - - while txn - count += 1 - txn = txn.next - end - - count + @transaction.number end - attr_reader :current_transaction - def increment_open_transactions - @current_transaction = Transaction.new(current_transaction) + ActiveSupport::Deprecation.warn "#increment_open_transactions is deprecated and has no effect" end def decrement_open_transactions - return unless current_transaction - - txn = current_transaction - @current_transaction = txn.next - txn + ActiveSupport::Deprecation.warn "#decrement_open_transactions is deprecated and has no effect" end def transaction_joinable=(joinable) - @transaction_joinable = joinable + ActiveSupport::Deprecation.warn "#transaction_joinable= is deprecated. Please pass the :joinable option to #begin_transaction instead." + @transaction.joinable = joinable end def create_savepoint diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 8fc172f6e8..328d080687 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -74,6 +74,7 @@ module ActiveRecord end def reconnect! + super disconnect! connect end @@ -82,6 +83,7 @@ module ActiveRecord # Disconnects from the database if already connected. # Otherwise, this method does nothing. def disconnect! + super unless @connection.nil? @connection.close @connection = nil diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index bb63fddf9b..0b936bbf39 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -189,14 +189,15 @@ module ActiveRecord end def reconnect! + super disconnect! - clear_cache! connect end # Disconnects from the database if already connected. Otherwise, this # method does nothing. def disconnect! + super @connection.close rescue nil end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index eb3084e066..c8437c18cc 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -1,3 +1,5 @@ +require 'active_support/deprecation' + module ActiveRecord module ConnectionAdapters class PostgreSQLAdapter < AbstractAdapter @@ -214,6 +216,10 @@ module ActiveRecord end def outside_transaction? + ActiveSupport::Deprecation.warn( + "#outside_transaction? is deprecated. This method was only really used " \ + "internally, but you can use #transaction_open? instead." + ) @connection.transaction_status == PGconn::PQTRANS_IDLE end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index d1751d70c6..e85e63d607 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -431,9 +431,8 @@ module ActiveRecord # Close then reopen the connection. def reconnect! - clear_cache! + super @connection.reset - @open_transactions = 0 configure_connection end @@ -445,7 +444,7 @@ module ActiveRecord # Disconnects from the database if already connected. Otherwise, this # method does nothing. def disconnect! - clear_cache! + super @connection.close rescue nil end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 4fe0013f0f..b6dd2e17f4 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -104,6 +104,8 @@ module ActiveRecord def initialize(connection, logger, config) super(connection, logger) + + @active = nil @statements = StatementPool.new(@connection, config.fetch(:statement_limit) { 1000 }) @config = config @@ -154,11 +156,15 @@ module ActiveRecord true end + def active? + @active != false + end + # Disconnects from the database if already connected. Otherwise, this # method does nothing. def disconnect! super - clear_cache! + @active = false @connection.close rescue nil end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index cf64985ddb..43ffca0227 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -387,7 +387,6 @@ module ActiveRecord @marked_for_destruction = false @new_record = true @mass_assignment_options = nil - @txn = nil @_start_transaction_state = {} end end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index b1db5f6f9f..60fc653735 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -843,9 +843,7 @@ module ActiveRecord end @fixture_connections = enlist_fixture_connections @fixture_connections.each do |connection| - connection.increment_open_transactions - connection.transaction_joinable = false - connection.begin_db_transaction + connection.begin_transaction joinable: false end # Load fixtures for every test. else @@ -868,10 +866,7 @@ module ActiveRecord # Rollback changes if a transaction is active. if run_in_transaction? @fixture_connections.each do |connection| - if connection.open_transactions != 0 - connection.rollback_db_transaction - connection.decrement_open_transactions - end + connection.rollback_transaction if connection.transaction_open? end @fixture_connections.clear end diff --git a/activerecord/lib/active_record/railties/console_sandbox.rb b/activerecord/lib/active_record/railties/console_sandbox.rb index 65a3d68619..90b462fad6 100644 --- a/activerecord/lib/active_record/railties/console_sandbox.rb +++ b/activerecord/lib/active_record/railties/console_sandbox.rb @@ -1,6 +1,4 @@ -ActiveRecord::Base.connection.increment_open_transactions ActiveRecord::Base.connection.begin_db_transaction at_exit do ActiveRecord::Base.connection.rollback_db_transaction - ActiveRecord::Base.connection.decrement_open_transactions end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index e008b32170..09318879d5 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -1,32 +1,6 @@ require 'thread' module ActiveRecord - class Transaction - attr_reader :next - - def initialize(txn = nil) - @next = txn - @committed = false - @aborted = false - end - - def committed! - @committed = true - end - - def aborted! - @aborted = true - end - - def committed? - @committed - end - - def aborted? - @aborted - end - end - # See ActiveRecord::Transactions::ClassMethods for documentation. module Transactions extend ActiveSupport::Concern @@ -333,11 +307,11 @@ module ActiveRecord def with_transaction_returning_status status = nil self.class.transaction do - @txn = self.class.connection.current_transaction add_to_transaction begin status = yield rescue ActiveRecord::Rollback + @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1 status = nil end @@ -353,17 +327,20 @@ module ActiveRecord @_start_transaction_state[:id] = id if has_attribute?(self.class.primary_key) @_start_transaction_state[:new_record] = @new_record @_start_transaction_state[:destroyed] = @destroyed + @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1 end # Clear the new record state and id of a record. def clear_transaction_record_state #:nodoc: - @_start_transaction_state.clear if @txn.committed? + @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1 + @_start_transaction_state.clear if @_start_transaction_state[:level] < 1 end # Restore the new record state and id of a record that was previously saved by a call to save_record_state. def restore_transaction_record_state(force = false) #:nodoc: unless @_start_transaction_state.empty? - if @txn.aborted? || force + @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1 + if @_start_transaction_state[:level] < 1 || force restore_state = @_start_transaction_state was_frozen = @attributes.frozen? @attributes = @attributes.dup if was_frozen diff --git a/activerecord/test/active_record/connection_adapters/fake_adapter.rb b/activerecord/test/active_record/connection_adapters/fake_adapter.rb index 1199be68eb..59324c4857 100644 --- a/activerecord/test/active_record/connection_adapters/fake_adapter.rb +++ b/activerecord/test/active_record/connection_adapters/fake_adapter.rb @@ -36,6 +36,10 @@ module ActiveRecord def columns(table_name) @columns[table_name] end + + def active? + true + end end end end diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 852fc0e26e..93b01a3934 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -160,4 +160,36 @@ module ActiveRecord end end end + + class AdapterTestWithoutTransaction < ActiveRecord::TestCase + self.use_transactional_fixtures = false + + def setup + @klass = Class.new(ActiveRecord::Base) + @klass.establish_connection 'arunit' + @connection = @klass.connection + end + + def teardown + @klass.remove_connection + end + + test "transaction state is reset after a reconnect" do + skip "in-memory db doesn't allow reconnect" if in_memory_db? + + @connection.begin_transaction + assert @connection.transaction_open? + @connection.reconnect! + assert !@connection.transaction_open? + end + + test "transaction state is reset after a disconnect" do + skip "in-memory db doesn't allow disconnect" if in_memory_db? + + @connection.begin_transaction + assert @connection.transaction_open? + @connection.disconnect! + assert !@connection.transaction_open? + end + end end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 0d0de455b3..0b5fda3817 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -36,6 +36,7 @@ class TransactionTest < ActiveRecord::TestCase end end + # FIXME: Get rid of this fucking global variable! def test_successful_with_return class << Topic.connection alias :real_commit_db_transaction :commit_db_transaction @@ -348,7 +349,6 @@ class TransactionTest < ActiveRecord::TestCase def test_rollback_when_commit_raises Topic.connection.expects(:begin_db_transaction) Topic.connection.expects(:commit_db_transaction).raises('OH NOES') - Topic.connection.expects(:outside_transaction?).returns(false) Topic.connection.expects(:rollback_db_transaction) assert_raise RuntimeError do @@ -397,31 +397,11 @@ class TransactionTest < ActiveRecord::TestCase if current_adapter?(:PostgreSQLAdapter) && defined?(PGconn::PQTRANS_IDLE) def test_outside_transaction_works - assert Topic.connection.outside_transaction? + assert assert_deprecated { Topic.connection.outside_transaction? } Topic.connection.begin_db_transaction - assert !Topic.connection.outside_transaction? + assert assert_deprecated { !Topic.connection.outside_transaction? } Topic.connection.rollback_db_transaction - assert Topic.connection.outside_transaction? - end - - def test_rollback_wont_be_executed_if_no_transaction_active - assert_raise RuntimeError do - Topic.transaction do - Topic.connection.rollback_db_transaction - Topic.connection.expects(:rollback_db_transaction).never - raise "Rails doesn't scale!" - end - end - end - - def test_open_transactions_count_is_reset_to_zero_if_no_transaction_active - Topic.transaction do - Topic.transaction do - Topic.connection.rollback_db_transaction - end - assert_equal 0, Topic.connection.open_transactions - end - assert_equal 0, Topic.connection.open_transactions + assert assert_deprecated { Topic.connection.outside_transaction? } end end @@ -580,5 +560,14 @@ if current_adapter?(:PostgreSQLAdapter) assert_equal original_salary, Developer.find(1).salary end + + test "#transaction_joinable= is deprecated" do + Developer.transaction do + conn = Developer.connection + assert conn.current_transaction.joinable? + assert_deprecated { conn.transaction_joinable = false } + assert !conn.current_transaction.joinable? + end + end end end |