From fc83920383b5f839cd6badbc5c962be9383fe150 Mon Sep 17 00:00:00 2001
From: Nicholas Seckar <nseckar@gmail.com>
Date: Wed, 28 Sep 2005 18:53:22 +0000
Subject: [PATCH] Fix transactions so that calling return while inside a
 transaction will not leave an open transaction on the connection.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2398 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
---
 activerecord/CHANGELOG                        |  2 ++
 .../abstract/database_statements.rb           | 11 ++++---
 activerecord/test/transactions_test.rb        | 33 ++++++++++++++++++-
 3 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 2006978b14c63..a98479bbd242a 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fix transactions so that calling return while inside a transaction will not leave an open transaction on the connection. [Nicholas Seckar]
+
 * Use foreign_key inflection uniformly.  #2156 [Blair Zajac <blair@orcaware.com>]
 
 * model.association.clear should destroy associated objects if :dependent => true instead of nullifying their foreign keys.  #2221 [joergd@pobox.com, ObieFernandez <obiefernandez@gmail.com>]
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 809424c0787d9..febc7029223be 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -39,17 +39,20 @@ def delete(sql, name = nil) end
 
       # Wrap a block in a transaction.  Returns result of block.
       def transaction(start_db_transaction = true)
+        needs_commit = false
         begin
           if block_given?
             begin_db_transaction if start_db_transaction
-            result = yield
-            commit_db_transaction if start_db_transaction
-            result
+            needs_commit = start_db_transaction
+            yield
           end
         rescue Exception => database_transaction_rollback
           rollback_db_transaction if start_db_transaction
+          needs_commit = false
           raise
         end
+      ensure
+        commit_db_transaction if needs_commit
       end
 
       # Begins the transaction (and turns off auto-committing).
@@ -81,4 +84,4 @@ def add_limit_offset!(sql, options)
       end
     end
   end
-end
\ No newline at end of file
+end
diff --git a/activerecord/test/transactions_test.rb b/activerecord/test/transactions_test.rb
index ea3b6ec5c5e4a..42ed57fad5c89 100644
--- a/activerecord/test/transactions_test.rb
+++ b/activerecord/test/transactions_test.rb
@@ -25,6 +25,37 @@ def test_successful
     assert !Topic.find(2).approved?, "Second should have been unapproved"
   end
 
+  def transaction_with_return
+    Topic.transaction do
+      @first.approved  = 1
+      @second.approved = 0
+      @first.save
+      @second.save
+      return
+    end
+  end
+
+  def test_successful_with_return
+    class << Topic.connection
+      alias :real_commit_db_transaction :commit_db_transaction
+      def commit_db_transaction
+        $committed = true
+        :real_commit_db_transaction
+      end
+    end
+
+    $committed = false
+    transaction_with_return
+    assert $committed
+
+    assert Topic.find(1).approved?, "First should have been approved"
+    assert !Topic.find(2).approved?, "Second should have been unapproved"
+  ensure
+    class << Topic.connection
+      alias :commit_db_transaction :real_commit_db_transaction rescue nil
+    end
+  end
+
   def test_successful_with_instance_method
     @first.transaction do
       @first.approved  = 1
@@ -36,7 +67,7 @@ def test_successful_with_instance_method
     assert Topic.find(1).approved?, "First should have been approved"
     assert !Topic.find(2).approved?, "Second should have been unapproved"
   end
- 
+
   def test_failing_on_exception
     begin
       Topic.transaction do