ActiveRecord::Base.remove_connection explicitly closes database connections and doesn't corrupt the connection cache. Introducing the disconnect! instance method for the PostgreSQL, MySQL, and SQL Server adapters; implementations for the others are welcome. References #3591.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3674 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

7 files changed, 45 insertions, 14 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 5b5beab6eb..957f13b7f9 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* ActiveRecord::Base.remove_connection explicitly closes database connections and doesn't corrupt the connection cache. Introducing the disconnect! instance method for the PostgreSQL, MySQL, and SQL Server adapters; implementations for the others are welcome.  #3591 [Simon Stapleton, Tom Ward]
+
 * Added support for nested scopes #3407 [anna@wota.jp]. Examples:
 
     Developer.with_scope(:find => { :conditions => "salary > 10000", :limit => 10 }) do
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
index 84dde1b14f..a73ce02104 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb
@@ -130,12 +130,13 @@ module ActiveRecord
     # can be used as argument for establish_connection, for easy
     # re-establishing of the connection.
     def self.remove_connection(klass=self)
-      conn = @@defined_connections[klass.name]
-      @@defined_connections.delete(klass.name)
-      @@connection_cache[Thread.current.object_id].delete(klass.name)
-      active_connections.delete(klass.name)
-      @connection = nil
-      conn.config if conn
+      spec = @@defined_connections[klass.name]
+      konn = active_connections[klass.name]
+      @@defined_connections.delete_if { |key, value| value == spec }
+      @@connection_cache[Thread.current.object_id].delete_if { |key, value| value == konn }
+      active_connections.delete_if { |key, value| value == konn }
+      konn.disconnect! if konn
+      spec.config if spec
     end
 
     # Set the connection for the class.
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index b01ef89dfc..8187112b64 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -63,11 +63,17 @@ module ActiveRecord
 
       # Is this connection active and ready to perform queries?
       def active?
-        true
+        @active != false
       end
 
       # Close this connection and open a new one in its place.
       def reconnect!
+        @active = true
+      end
+
+      # Close this connection
+      def disconnect!
+        @active = false
       end
 
 
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 6c768cb9f6..dd7331c57d 100755
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -160,9 +160,13 @@ module ActiveRecord
       end
 
       def reconnect!
-        @connection.close rescue nil
+        disconnect!
         connect
       end
+      
+      def disconnect!
+        @connection.close rescue nil
+      end
 
 
       # DATABASE STATEMENTS ======================================
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index a5bef4138d..86b89c6c1c 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -75,6 +75,11 @@ module ActiveRecord
           configure_connection
         end
       end
+      
+      def disconnect!
+        # Both postgres and postgres-pr respond to :close
+        @connection.close rescue nil
+      end
 
       def native_database_types
         {
diff --git a/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
index 708ec086fd..be06887fa0 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb
@@ -216,12 +216,18 @@ module ActiveRecord
 
       # Reconnects to the database, returns false if no connection could be made.
       def reconnect!
-        @connection.disconnect rescue nil
+        disconnect!
         @connection = DBI.connect(*@connection_options)
       rescue DBI::DatabaseError => e
         @logger.warn "#{adapter_name} reconnection failed: #{e.message}" if @logger
         false
       end
+      
+      # Disconnects from the database
+      
+      def disconnect!
+        @connection.disconnect rescue nil
+      end
 
       def select_all(sql, name = nil)
         select(sql, name)
diff --git a/activerecord/test/unconnected_test.rb b/activerecord/test/unconnected_test.rb
index df656ec773..2d06410dd8 100755
--- a/activerecord/test/unconnected_test.rb
+++ b/activerecord/test/unconnected_test.rb
@@ -7,19 +7,26 @@ class TestUnconnectedAdaptor < Test::Unit::TestCase
   self.use_transactional_fixtures = false
 
   def setup
-    @connection = ActiveRecord::Base.remove_connection
+    @underlying = ActiveRecord::Base.connection
+    @specification = ActiveRecord::Base.remove_connection
   end
 
   def teardown
-    ActiveRecord::Base.establish_connection(@connection)
+    @underlying = nil
+    ActiveRecord::Base.establish_connection(@specification)
   end
 
-  def test_unconnected
+  def test_connection_no_longer_established
     assert_raise(ActiveRecord::ConnectionNotEstablished) do
-      TestRecord.find(1)   
+      TestRecord.find(1)
     end
+
     assert_raise(ActiveRecord::ConnectionNotEstablished) do
-      TestRecord.new.save   
+      TestRecord.new.save
     end
   end
+
+  def test_underlying_adapter_no_longer_active
+    assert !@underlying.active?, "Removed adapter should no longer be active"
+  end
 end