Merge pull request #38258 from eileencodes/deprecate-remove-connection

Deprecate `#remove_connection` in favor of `#remove_connection_pool`

activerecord/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Deprecate `#remove_connection` in favor of `#remove_connection_pool` when called on the handler.
+
+    `#remove_connection` is deprecated in order to support returning a `DatabaseConfig` object instead of a `Hash`. Use `#remove_connection_pool`, `#remove_connection` will be removed in 6.2.
+
+    *Eileen M. Uchitelle*, *John Crepezzi*
+
 *   Deprecate `#default_hash` and it's alias `#[]` on database configurations
 
     Applications should use `configs_for`. `#default_hash` and `#[]` will be removed in 6.2.

activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb

@@ -1046,7 +1046,7 @@ module ActiveRecord
         # Protects the connection named `ActiveRecord::Base` from being removed
         # if the user calls `establish_connection :primary`.
         if owner_to_pool_manager.key?(pool_config.connection_specification_name)
-          remove_connection(pool_config.connection_specification_name, pool_key)
+          remove_connection_pool(pool_config.connection_specification_name, pool_key)
         end
 
         message_bus = ActiveSupport::Notifications.instrumenter
@@ -1100,7 +1100,7 @@ module ActiveRecord
       # active or defined connection: if it is the latter, it will be
       # opened and set as the active connection for the class it was defined
       # for (not necessarily the current class).
-      def retrieve_connection(spec_name) #:nodoc:
+      def retrieve_connection(spec_name) # :nodoc:
         pool = retrieve_connection_pool(spec_name)
 
         unless pool
@@ -1127,12 +1127,17 @@ module ActiveRecord
       # can be used as an argument for #establish_connection, for easily
       # re-establishing the connection.
       def remove_connection(owner, pool_key = :default)
+        remove_connection_pool(owner, pool_key)&.configuration_hash
+      end
+      deprecate remove_connection: "Use #remove_connection_pool, which now returns a DatabaseConfig object instead of a Hash"
+
+      def remove_connection_pool(owner, pool_key = :default)
         if pool_manager = get_pool_manager(owner)
           pool_config = pool_manager.remove_pool_config(pool_key)
 
           if pool_config
             pool_config.disconnect!
-            pool_config.db_config.configuration_hash
+            pool_config.db_config
           end
         end
       end

activerecord/lib/active_record/connection_handling.rb

@@ -232,7 +232,7 @@ module ActiveRecord
         self.connection_specification_name = nil
       end
 
-      connection_handler.remove_connection(name)
+      connection_handler.remove_connection_pool(name)
     end
 
     def clear_cache! # :nodoc:

activerecord/test/cases/connection_adapters/connection_handler_test.rb

@@ -39,7 +39,7 @@ module ActiveRecord
         assert_not_nil @handler.retrieve_connection_pool("readonly")
       ensure
         ActiveRecord::Base.configurations = old_config
-        @handler.remove_connection("readonly")
+        @handler.remove_connection_pool("readonly")
       end
 
       def test_establish_connection_using_3_levels_config
@@ -85,7 +85,7 @@ module ActiveRecord
 
           assert_not_deprecated do
             @handler.retrieve_connection("primary")
-            @handler.remove_connection("primary")
+            @handler.remove_connection_pool("primary")
           end
         ensure
           ActiveRecord::Base.configurations = old_config
@@ -99,7 +99,7 @@ module ActiveRecord
           ActiveRecord::Base.establish_connection(:primary)
 
           assert_deprecated { @handler.retrieve_connection("primary") }
-          assert_deprecated { @handler.remove_connection("primary") }
+          assert_deprecated { @handler.remove_connection_pool("primary") }
         ensure
           ActiveRecord::Base.configurations = old_config
           ActiveRecord::Base.establish_connection(:arunit)
@@ -152,6 +152,18 @@ module ActiveRecord
           ActiveRecord::Base.establish_connection(:arunit)
           FileUtils.rm_rf "db"
         end
+
+        def test_remove_connection_is_deprecated
+          expected = @handler.retrieve_connection_pool(@owner_name).db_config.configuration_hash
+
+          config_hash = assert_deprecated do
+            @handler.remove_connection(@owner_name)
+          end
+
+          assert_equal expected, config_hash
+        ensure
+          ActiveRecord::Base.establish_connection(:arunit)
+        end
       end
 
       def test_establish_connection_using_two_level_configurations

activerecord/test/cases/connection_adapters/connection_handlers_multi_pool_config_test.rb

@@ -62,7 +62,7 @@ module ActiveRecord
           @writing_handler.establish_connection(:primary, :pool_config_two)
 
           # remove default
-          @writing_handler.remove_connection("primary")
+          @writing_handler.remove_connection_pool("primary")
 
           assert_nil @writing_handler.retrieve_connection_pool("primary")
           assert_not_nil @writing_handler.retrieve_connection_pool("primary", :pool_config_two)

activerecord/test/cases/defaults_test.rb

@@ -154,7 +154,8 @@ if current_adapter?(:Mysql2Adapter)
 
     def using_strict(strict)
       connection = ActiveRecord::Base.remove_connection
-      ActiveRecord::Base.establish_connection connection.merge(strict: strict)
+      conn_hash = connection.configuration_hash
+      ActiveRecord::Base.establish_connection conn_hash.merge(strict: strict)
       yield
     ensure
       ActiveRecord::Base.remove_connection

activerecord/test/cases/pooled_connections_test.rb

@@ -9,7 +9,7 @@ class PooledConnectionsTest < ActiveRecord::TestCase
 
   def setup
     @per_test_teardown = []
-    @connection = ActiveRecord::Base.remove_connection
+    @connection = ActiveRecord::Base.remove_connection.configuration_hash
   end
 
   teardown do

activerecord/test/cases/query_cache_test.rb

@@ -454,7 +454,7 @@ class QueryCacheTest < ActiveRecord::TestCase
       Task.cache do
         assert_queries(1) { Task.find(1); Task.find(1) }
       ensure
-        ActiveRecord::Base.connection_handler.remove_connection(db_config.owner_name)
+        ActiveRecord::Base.connection_handler.remove_connection_pool(db_config.owner_name)
       end
     end
   end

activerecord/test/support/connection_helper.rb

@@ -3,7 +3,7 @@
 module ConnectionHelper
   def run_without_connection
     original_connection = ActiveRecord::Base.remove_connection
-    yield original_connection
+    yield original_connection.configuration_hash
   ensure
     ActiveRecord::Base.establish_connection(original_connection)
   end