Merge pull request rails#51230 from Shopify/deprecate-connection

Deprecate `ConnectionPool#connection`

Author: byroot

actioncable/test/subscription_adapter/postgresql_test.rb

@@ -23,7 +23,7 @@ def setup
     ActiveRecord::Base.establish_connection database_config
 
     begin
-      ActiveRecord::Base.connection.connect!
+      ActiveRecord::Base.lease_connection.connect!
     rescue
       @rx_adapter = @tx_adapter = nil
       skip "Couldn't connect to PostgreSQL: #{database_config.inspect}"
@@ -68,7 +68,7 @@ def active?
   def test_default_subscription_connection_identifier
     subscribe_as_queue("channel") { }
 
-    identifiers = ActiveRecord::Base.connection.exec_query("SELECT application_name FROM pg_stat_activity").rows
+    identifiers = ActiveRecord::Base.lease_connection.exec_query("SELECT application_name FROM pg_stat_activity").rows
     assert_includes identifiers, ["ActionCable-PID-#{$$}"]
   end
 
@@ -81,7 +81,7 @@ def test_custom_subscription_connection_identifier
 
     subscribe_as_queue("channel", adapter) { }
 
-    identifiers = ActiveRecord::Base.connection.exec_query("SELECT application_name FROM pg_stat_activity").rows
+    identifiers = ActiveRecord::Base.lease_connection.exec_query("SELECT application_name FROM pg_stat_activity").rows
     assert_includes identifiers, ["hello-world-42"]
   end
 end

actionmailbox/test/migrations_test.rb

@@ -8,7 +8,7 @@ class ActionMailbox::MigrationsTest < ActiveSupport::TestCase
     @original_verbose = ActiveRecord::Migration.verbose
     ActiveRecord::Migration.verbose = false
 
-    @connection = ActiveRecord::Base.connection
+    @connection = ActiveRecord::Base.lease_connection
     @original_options = Rails.configuration.generators.options.deep_dup
   end
 

actionview/test/active_record_unit.rb

@@ -44,7 +44,7 @@ def setup
 
     def reconnect
       return unless able_to_connect
-      ActiveRecord::Base.connection.reconnect!
+      ActiveRecord::Base.lease_connection.reconnect!
       load_schema
     end
 
@@ -56,9 +56,9 @@ def setup_connection
           options = defaults.merge adapter: adapter, timeout: 500
           ActiveRecord::Base.establish_connection(options)
           ActiveRecord::Base.configurations = { "sqlite3_ar_integration" => options }
-          ActiveRecord::Base.connection
+          ActiveRecord::Base.lease_connection
 
-          Object.const_set :QUOTED_TYPE, ActiveRecord::Base.connection.quote_column_name("type") unless Object.const_defined?(:QUOTED_TYPE)
+          Object.const_set :QUOTED_TYPE, ActiveRecord::Base.lease_connection.quote_column_name("type") unless Object.const_defined?(:QUOTED_TYPE)
         else
           raise "Can't setup connection since ActiveRecord isn't loaded."
         end
@@ -67,7 +67,7 @@ def setup_connection
       # Load actionpack sqlite3 tables
       def load_schema
         File.read(File.expand_path("fixtures/db_definitions/sqlite.sql", __dir__)).split(";").each do |sql|
-          ActiveRecord::Base.connection.execute(sql) unless sql.blank?
+          ActiveRecord::Base.lease_connection.execute(sql) unless sql.blank?
         end
       end
 
@@ -107,7 +107,7 @@ def run(*args)
   end
 
   def capture_sql
-    ActiveRecord::Base.connection.materialize_transactions
+    ActiveRecord::Base.lease_connection.materialize_transactions
     SQLCounter.clear_log
     yield
     SQLCounter.log.dup

activerecord/CHANGELOG.md

@@ -1,3 +1,20 @@
+*   Deprecate `ActiveRecord::Base.connection` in favor of `.lease_connection`
+
+    The method has been renamed as `lease_connection` to better reflect that the returned
+    connection will be held for the duration of the request or job.
+
+    This deprecation is a soft deprecation, no warnings will be issued and there is no
+    current plan to remove the method.
+
+    *Jean Boussier*
+
+*   Deprecate `ActiveRecord::ConnectionAdapters::ConnectionPool#connection`
+
+    The method has been renamed as `lease_connection` to better reflect that the returned
+    connection will be held for the duration of the request or job.
+
+    *Jean Boussier*
+
 *   Expose a generic fixture accessor for fixture names that may conflict with Minitest
 
     ```ruby

activerecord/examples/performance.rb

@@ -176,10 +176,10 @@ def self.email
   end
 
   x.report "Model.log" do
-    Exhibit.connection.send(:log, "hello", "world") { }
+    Exhibit.lease_connection.send(:log, "hello", "world") { }
   end
 
   x.report "AR.execute(query)" do
-    ActiveRecord::Base.connection.execute("SELECT * FROM exhibits WHERE id = #{(rand * 1000 + 1).to_i}")
+    ActiveRecord::Base.lease_connection.execute("SELECT * FROM exhibits WHERE id = #{(rand * 1000 + 1).to_i}")
   end
 end

activerecord/lib/active_record/associations/association.rb

@@ -63,7 +63,7 @@ def reset_negative_cache # :nodoc:
       # Reloads the \target and returns +self+ on success.
       # The QueryCache is cleared if +force+ is true.
       def reload(force = false)
-        klass.connection.clear_query_cache if force && klass
+        klass.connection_pool.clear_query_cache if force && klass
         reset
         reset_scope
         load_target
@@ -231,12 +231,14 @@ def find_target
           end
 
           binds = AssociationScope.get_bind_values(owner, reflection.chain)
-          sc.execute(binds, klass.connection) do |record|
-            set_inverse_instance(record)
-            if owner.strict_loading_n_plus_one_only? && reflection.macro == :has_many
-              record.strict_loading!
-            else
-              record.strict_loading!(false, mode: owner.strict_loading_mode)
+          klass.with_connection do |c|
+            sc.execute(binds, c) do |record|
+              set_inverse_instance(record)
+              if owner.strict_loading_n_plus_one_only? && reflection.macro == :has_many
+                record.strict_loading!
+              else
+                record.strict_loading!(false, mode: owner.strict_loading_mode)
+              end
             end
           end
         end

activerecord/lib/active_record/associations/join_dependency/join_association.rb

@@ -91,7 +91,7 @@ def strict_loading?
           def append_constraints(join, constraints)
             if join.is_a?(Arel::Nodes::StringJoin)
               join_string = Arel::Nodes::And.new(constraints.unshift join.left)
-              join.left = Arel.sql(base_klass.connection.visitor.compile(join_string))
+              join.left = Arel.sql(base_klass.lease_connection.visitor.compile(join_string))
             else
               right = join.right
               right.expr = Arel::Nodes::And.new(constraints.unshift right.expr)

activerecord/lib/active_record/base.rb

@@ -233,7 +233,7 @@ module ActiveRecord # :nodoc:
   #
   # Connections are usually created through
   # {ActiveRecord::Base.establish_connection}[rdoc-ref:ConnectionHandling#establish_connection] and retrieved
-  # by ActiveRecord::Base.connection. All classes inheriting from ActiveRecord::Base will use this
+  # by ActiveRecord::Base.lease_connection. All classes inheriting from ActiveRecord::Base will use this
   # connection. But you can also set a class-specific connection. For example, if Course is an
   # ActiveRecord::Base, but resides in a different database, you can just say <tt>Course.establish_connection</tt>
   # and Course and all of its subclasses will use this connection instead.

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

@@ -190,7 +190,7 @@ def flush_idle_connections!(role = nil)
       # for (not necessarily the current class).
       def retrieve_connection(connection_name, role: ActiveRecord::Base.current_role, shard: ActiveRecord::Base.current_shard) # :nodoc:
         pool = retrieve_connection_pool(connection_name, role: role, shard: shard, strict: true)
-        pool.connection
+        pool.lease_connection
       end
 
       # Returns true if a connection that's accessible to this class has

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

@@ -69,7 +69,7 @@ def db_config
     # Connections can be obtained and used from a connection pool in several
     # ways:
     #
-    # 1. Simply use {ActiveRecord::Base.connection}[rdoc-ref:ConnectionHandling.connection].
+    # 1. Simply use {ActiveRecord::Base.lease_connection}[rdoc-ref:ConnectionHandling.connection].
     #    When you're done with the connection(s) and wish it to be returned to the pool, you call
     #    {ActiveRecord::Base.connection_handler.clear_active_connections!}[rdoc-ref:ConnectionAdapters::ConnectionHandler#clear_active_connections!].
     #    This is the default behavior for Active Record when used in conjunction with
@@ -139,46 +139,46 @@ def clear(connection)
         end
       end
 
-      if ObjectSpace.const_defined?(:WeakKeyMap) # RUBY_VERSION >= 3.3
-        WeakKeyMap = ::ObjectSpace::WeakKeyMap # :nodoc:
-      else
-        class WeakKeyMap # :nodoc:
-          def initialize
-            @map = ObjectSpace::WeakMap.new
-            @values = nil
-            @size = 0
-          end
-
-          alias_method :clear, :initialize
+      class LeaseRegistry # :nodoc:
+        if ObjectSpace.const_defined?(:WeakKeyMap) # RUBY_VERSION >= 3.3
+          WeakKeyMap = ::ObjectSpace::WeakKeyMap # :nodoc:
+        else
+          class WeakKeyMap # :nodoc:
+            def initialize
+              @map = ObjectSpace::WeakMap.new
+              @values = nil
+              @size = 0
+            end
 
-          def [](key)
-            prune if @map.size != @size
-            @map[key]
-          end
+            alias_method :clear, :initialize
 
-          def []=(key, value)
-            @map[key] = value
-            prune if @map.size != @size
-            value
-          end
+            def [](key)
+              prune if @map.size != @size
+              @map[key]
+            end
 
-          def delete(key)
-            if value = self[key]
-              self[key] = nil
-              prune
+            def []=(key, value)
+              @map[key] = value
+              prune if @map.size != @size
+              value
             end
-            value
-          end
 
-          private
-            def prune(force = false)
-              @values = @map.values
-              @size = @map.size
+            def delete(key)
+              if value = self[key]
+                self[key] = nil
+                prune
+              end
+              value
             end
+
+            private
+              def prune(force = false)
+                @values = @map.values
+                @size = @map.size
+              end
+          end
         end
-      end
 
-      class LeaseRegistry # :nodoc:
         def initialize
           @mutex = Mutex.new
           @map = WeakKeyMap.new
@@ -293,7 +293,13 @@ def lease_connection
         lease.connection ||= checkout
       end
 
-      alias_method :connection, :lease_connection # TODO: deprecate
+      def connection
+        ActiveRecord.deprecator.warn(<<~MSG)
+          ConnectionPoool#connection is deprecated and will be removed
+          in Rails 7.3. Use #lease_connection instead
+        MSG
+        lease_connection
+      end
 
       def pin_connection!(lock_thread) # :nodoc:
         raise "There is already a pinned connection" if @pinned_connection