From 61951427903dbc0d92f6106ec5874025e2185056 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 14 Sep 2012 22:33:44 +0100 Subject: [PATCH] Remove our use of #outside_transaction? This method was first seen in 045713ee240fff815edb5962b25d668512649478, and subsequently reimplemented in fb2325e35855d62abd2c76ce03feaa3ca7992e4f. According to @jeremy, this is okay to remove. He thinks it was added because at the time we didn't have much transaction state to keep track of, and he viewed it as a hack for us to track it internally, thinking it was better to ask the connection for the transaction state. Over the years we have added more and more state to track, a lot of which is impossible to ask the connection for. So it seems that this is just a relic of the passed and we will just track the state internally only. --- .../abstract/database_statements.rb | 40 ++++--------------- .../postgresql/database_statements.rb | 6 +++ activerecord/test/cases/transactions_test.rb | 32 ++------------- 3 files changed, 17 insertions(+), 61 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 0589e6b447..c8c3fcb22a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -107,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,7 +159,7 @@ module ActiveRecord options.assert_valid_keys :requires_new, :joinable if !options[:requires_new] && current_transaction.joinable? - within_existing_transaction { yield } + yield else within_new_transaction(options) { yield } end @@ -185,27 +171,17 @@ module ActiveRecord begin_transaction(options) yield rescue Exception => error - rollback_transaction unless outside_transaction? + rollback_transaction raise ensure - if outside_transaction? - reset_transaction - else - begin - commit_transaction unless error - rescue Exception => e - rollback_transaction - raise - end + begin + commit_transaction unless error + rescue Exception => e + rollback_transaction + raise end end - def within_existing_transaction #:nodoc: - yield - ensure - reset_transaction if outside_transaction? - end - def current_transaction #:nodoc: @transaction end @@ -228,7 +204,7 @@ module ActiveRecord @transaction = @transaction.rollback end - def reset_transaction + def reset_transaction #:nodoc: @transaction = ClosedTransaction.new(self) 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/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index e618c0984e..0b5fda3817 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -349,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 @@ -398,36 +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 - - Topic.transaction do - Topic.connection.rollback_db_transaction - end - assert_equal 0, Topic.connection.open_transactions + assert assert_deprecated { Topic.connection.outside_transaction? } end end