Merge pull request rails#41859 from eileencodes/deprecate-legacy-connection-handling

Deprecate legacy_connection_handling

Author: eileencodes

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Deprecate `legacy_connection_handling`.
+
+    *Eileen M. Uchitelle*
+
 *   Add attribute encryption support.
 
     Encrypted attributes are declared at the model level. These

activerecord/lib/active_record/core.rb

@@ -216,15 +216,26 @@ def self.connection_handler=(handler)
       end
 
       def self.connection_handlers
-        unless legacy_connection_handling
+        if legacy_connection_handling
+        else
           raise NotImplementedError, "The new connection handling does not support accessing multiple connection handlers."
         end
 
         @@connection_handlers ||= {}
       end
 
       def self.connection_handlers=(handlers)
-        unless legacy_connection_handling
+        if legacy_connection_handling
+          ActiveSupport::Deprecation.warn(<<~MSG)
+            Using legacy connection handling is deprecated. Please set
+            `legacy_connection_handling` to `false` in your application.
+
+            The new connection handling does not support `connection_handlers`
+            getter and setter.
+
+            Read more about how to migrate at:
+          MSG
+        else
           raise NotImplementedError, "The new connection handling does not setting support multiple connection handlers."
         end
 

activerecord/test/cases/connection_adapters/legacy_connection_handlers_multi_db_test.rb

@@ -13,7 +13,9 @@ class LegacyConnectionHandlersMultiDbTest < ActiveRecord::TestCase
       def setup
         @old_value = ActiveRecord::Base.legacy_connection_handling
         ActiveRecord::Base.legacy_connection_handling = true
-        ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler }
+        assert_deprecated do
+          ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler }
+        end
 
         @handlers = { writing: ConnectionHandler.new, reading: ConnectionHandler.new }
         @rw_handler = @handlers[:writing]
@@ -379,7 +381,9 @@ def test_retrieve_connection_pool_with_invalid_id
       end
 
       def test_connection_handlers_are_per_thread_and_not_per_fiber
-        ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler, reading: ActiveRecord::ConnectionAdapters::ConnectionHandler.new }
+        assert_deprecated do
+          ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler, reading: ActiveRecord::ConnectionAdapters::ConnectionHandler.new }
+        end
 
         reading_handler = ActiveRecord::Base.connection_handlers[:reading]
 
@@ -392,7 +396,9 @@ def test_connection_handlers_are_per_thread_and_not_per_fiber
       end
 
       def test_connection_handlers_swapping_connections_in_fiber
-        ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler, reading: ActiveRecord::ConnectionAdapters::ConnectionHandler.new }
+        assert_deprecated do
+          ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler, reading: ActiveRecord::ConnectionAdapters::ConnectionHandler.new }
+        end
 
         reading_handler = ActiveRecord::Base.connection_handlers[:reading]
 

activerecord/test/cases/connection_adapters/legacy_connection_handlers_sharding_db_test.rb

@@ -13,7 +13,9 @@ class LegacyConnectionHandlersShardingDbTest < ActiveRecord::TestCase
       def setup
         @legacy_setting = ActiveRecord::Base.legacy_connection_handling
         ActiveRecord::Base.legacy_connection_handling = true
-        ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler }
+        assert_deprecated do
+          ActiveRecord::Base.connection_handlers = { writing: ActiveRecord::Base.default_connection_handler }
+        end
 
         @handlers = { writing: ConnectionHandler.new, reading: ConnectionHandler.new }
         @rw_handler = @handlers[:writing]

activerecord/test/cases/fixtures_test.rb

@@ -1523,7 +1523,9 @@ def setup
 
       handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new
       handler.establish_connection(db_config)
-      ActiveRecord::Base.connection_handlers = {}
+      assert_deprecated do
+        ActiveRecord::Base.connection_handlers = {}
+      end
       ActiveRecord::Base.connection_handler = handler
       ActiveRecord::Base.connects_to(database: { writing: :default, reading: :readonly })
 

activerecord/test/cases/helper.rb

@@ -151,7 +151,9 @@ def disable_extension!(extension, connection)
 
 def clean_up_legacy_connection_handlers
   handler = ActiveRecord::Base.default_connection_handler
-  ActiveRecord::Base.connection_handlers = {}
+  assert_deprecated do
+    ActiveRecord::Base.connection_handlers = {}
+  end
 
   handler.connection_pool_names.each do |name|
     next if ["ActiveRecord::Base", "ARUnit2Model", "Contact", "ContactSti"].include?(name)

activerecord/test/cases/query_cache_test.rb

@@ -78,10 +78,12 @@ def test_query_cache_is_applied_to_legacy_connections_in_all_handlers
     old_value = ActiveRecord::Base.legacy_connection_handling
     ActiveRecord::Base.legacy_connection_handling = true
 
-    ActiveRecord::Base.connection_handlers = {
-      writing: ActiveRecord::Base.default_connection_handler,
-      reading: ActiveRecord::ConnectionAdapters::ConnectionHandler.new
-    }
+    assert_deprecated do
+      ActiveRecord::Base.connection_handlers = {
+        writing: ActiveRecord::Base.default_connection_handler,
+        reading: ActiveRecord::ConnectionAdapters::ConnectionHandler.new
+      }
+    end
 
     ActiveRecord::Base.connected_to(role: :reading) do
       db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary")
@@ -126,10 +128,13 @@ def test_query_cache_is_applied_to_all_connections
     def test_query_cache_with_multiple_handlers_and_forked_processes_legacy_handling
       old_value = ActiveRecord::Base.legacy_connection_handling
       ActiveRecord::Base.legacy_connection_handling = true
-      ActiveRecord::Base.connection_handlers = {
-        writing: ActiveRecord::Base.default_connection_handler,
-        reading: ActiveRecord::ConnectionAdapters::ConnectionHandler.new
-      }
+
+      assert_deprecated do
+        ActiveRecord::Base.connection_handlers = {
+          writing: ActiveRecord::Base.default_connection_handler,
+          reading: ActiveRecord::ConnectionAdapters::ConnectionHandler.new
+        }
+      end
 
       ActiveRecord::Base.connected_to(role: :reading) do
         db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary")
@@ -663,10 +668,12 @@ def test_clear_query_cache_is_called_on_all_legacy_connections
     old_value = ActiveRecord::Base.legacy_connection_handling
     ActiveRecord::Base.legacy_connection_handling = true
 
-    ActiveRecord::Base.connection_handlers = {
-      writing: ActiveRecord::Base.default_connection_handler,
-      reading: ActiveRecord::ConnectionAdapters::ConnectionHandler.new
-    }
+    assert_deprecated do
+      ActiveRecord::Base.connection_handlers = {
+        writing: ActiveRecord::Base.default_connection_handler,
+        reading: ActiveRecord::ConnectionAdapters::ConnectionHandler.new
+      }
+    end
 
     ActiveRecord::Base.connected_to(role: :reading) do
       db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary")

activerecord/test/cases/test_fixtures_test.rb

@@ -48,7 +48,9 @@ def test_run_successfully
 
       old_handler = ActiveRecord::Base.connection_handler
       ActiveRecord::Base.connection_handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new
-      ActiveRecord::Base.connection_handlers = {}
+      assert_deprecated do
+        ActiveRecord::Base.connection_handlers = {}
+      end
       ActiveRecord::Base.establish_connection(:arunit)
 
       test_result = klass.new("test_run_successfully").run

guides/source/active_record_multiple_databases.md

@@ -10,6 +10,7 @@ After reading this guide you will know:
 * How to set up your application for multiple databases.
 * How automatic connection switching works.
 * How to use horizontal sharding for multiple databases.
+* How to migrate from `legacy_connection_handling` to the new connection handling.
 * What features are supported and what's still a work in progress.
 
 --------------------------------------------------------------------------------
@@ -393,13 +394,37 @@ ActiveRecord::Base.connected_to(role: :reading, shard: :shard_one) do
 end
 ```
 
+## Migrate to the new connection handling
+
+In Rails 6.1+, Active Record provides a new internal API for connection management.
+In most cases applications will not need to make any changes except to opt-in to the
+new behavior (if upgrading from 6.0 and below) by setting
+`config.active_record.legacy_connection_handling = false`. If you have a single database
+application, no other changes will be required. If you have a multiple database application
+the following changes are required if you application is using these methods:
+
+* `connection_handlers` and `connection_handlers=` no longer works in the new connection
+handling. If you were calling a method on one of the connection handlers, for example,
+`connection_handlers[:reading].retrieve_connection_pool("ActiveRecord::Base")`
+you will now need to update that call to be
+`connection_handlers.retrieve_connection_pool("ActiveRecord::Base", role: :reading)`.
+* Calls to `ActiveRecord::Base.connection_handler.prevent_writes` will need to be updated
+to `ActiveRecord::Base.connection.preventing_writes?`.
+* If you need all the pools, including writing and reading, a new method has been provided on
+the handler. Call `connection_handler.all_connection_pools` to use this. In most cases though
+you'll want writing or reading pools with `connection_handler.connection_pool_list(:writing)` or
+`connection_handler.connection_pool_list(:reading)`.
+* If you turn off `legacy_connection_handling` in your application, any method that's unsupported
+will raise an error (ie `connection_handlers=`).
+
 ## Granular Database Connection Switching
 
 In Rails 6.1 it's possible to switch connections for one database instead of
 all databases globally. To use this feature you must first set
 `config.active_record.legacy_connection_handling` to `false` in your application
 configuration. The majority of applications should not need to make any other
-changes since the public APIs have the same behavior.
+changes since the public APIs have the same behavior. See the above section for
+how to enable and migrate away from `legacy_connection_handling`.
 
 With `legacy_connection_handling` set to `false`, any abstract connection class
 will be able to switch connections without affecting other connections. This