Skip to content

Commit

Permalink
Deprecate ConnectionPool#connection
Browse files Browse the repository at this point in the history
Replaced by `#lease_connection` to better reflect what it does.

`ActiveRecord::Base#connection` is deprecated in the same way
but without a removal timeline nor a deprecation warning.

Inside the Active Record test suite, we do remove `Base.connection`
to ensure it's not used internally.

Some callsites have been converted to use `with_connection`,
some other have been more simply migrated to `lease_connection`
and will serve as a list of callsites to convert for
#50793
  • Loading branch information
byroot committed Mar 1, 2024
1 parent 75e3407 commit 9f6fa2b
Show file tree
Hide file tree
Showing 277 changed files with 1,467 additions and 1,390 deletions.
6 changes: 3 additions & 3 deletions actioncable/test/subscription_adapter/postgresql_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -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

Expand All @@ -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
2 changes: 1 addition & 1 deletion actionmailbox/test/migrations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 5 additions & 5 deletions actionview/test/active_record_unit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions activerecord/examples/performance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 9 additions & 7 deletions activerecord/lib/active_record/associations/association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ def truncate_tables(*table_names) # :nodoc:
# #transaction will raise exceptions when it tries to release the
# already-automatically-released savepoints:
#
# Model.connection.transaction do # BEGIN
# Model.connection.transaction(requires_new: true) do # CREATE SAVEPOINT active_record_1
# Model.connection.create_table(...)
# Model.lease_connection.transaction do # BEGIN
# Model.lease_connection.transaction(requires_new: true) do # CREATE SAVEPOINT active_record_1
# Model.lease_connection.create_table(...)
# # active_record_1 now automatically released
# end # RELEASE SAVEPOINT active_record_1 <--- BOOM! database error!
# end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module ConnectionAdapters # :nodoc:
# and +:limit+ options, etc.
#
# All the concrete database adapters follow the interface laid down in this class.
# {ActiveRecord::Base.connection}[rdoc-ref:ConnectionHandling#connection] returns an AbstractAdapter object, which
# {ActiveRecord::Base.lease_connection}[rdoc-ref:ConnectionHandling#connection] returns an AbstractAdapter object, which
# you can use.
#
# Most of the methods in the adapter are useful during migrations. Most
Expand Down
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,8 @@ def type_caster # :nodoc:
end

def cached_find_by_statement(key, &block) # :nodoc:
cache = @find_by_statement_cache[connection.prepared_statements]
cache.compute_if_absent(key) { StatementCache.create(connection, &block) }
cache = @find_by_statement_cache[lease_connection.prepared_statements]
cache.compute_if_absent(key) { StatementCache.create(lease_connection, &block) }
end

private
Expand Down Expand Up @@ -431,7 +431,7 @@ def cached_find_by(keys, values)
}

begin
statement.execute(values.flatten, connection).first
statement.execute(values.flatten, lease_connection).first
rescue TypeError
raise ActiveRecord::StatementInvalid
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def initialize(message = nil, connection_pool: nil)
end

# Raised when connection to the database could not been established (for example when
# {ActiveRecord::Base.connection=}[rdoc-ref:ConnectionHandling#connection]
# {ActiveRecord::Base.lease_connection=}[rdoc-ref:ConnectionHandling#connection]
# is given a +nil+ object).
class ConnectionNotEstablished < AdapterError
def initialize(message = nil, connection_pool: nil)
Expand Down
25 changes: 13 additions & 12 deletions activerecord/lib/active_record/explain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@ def collecting_queries_for_explain # :nodoc:
# Makes the adapter execute EXPLAIN for the tuples of queries and bindings.
# Returns a formatted string ready to be logged.
def exec_explain(queries, options = []) # :nodoc:
str = queries.map do |sql, binds|
msg = +"#{build_explain_clause(options)} #{sql}"
unless binds.empty?
msg << " "
msg << binds.map { |attr| render_bind(attr) }.inspect
end
msg << "\n"
msg << connection.explain(sql, binds, options)
end.join("\n")

str = with_connection do |c|
queries.map do |sql, binds|
msg = +"#{build_explain_clause(c, options)} #{sql}"
unless binds.empty?
msg << " "
msg << binds.map { |attr| render_bind(c, attr) }.inspect
end
msg << "\n"
msg << c.explain(sql, binds, options)
end.join("\n")
end
# Overriding inspect to be more human readable, especially in the console.
def str.inspect
self
Expand All @@ -36,7 +37,7 @@ def str.inspect
end

private
def render_bind(attr)
def render_bind(connection, attr)
if ActiveModel::Attribute === attr
value = if attr.type.binary? && attr.value
"<#{attr.value_for_database.to_s.bytesize} bytes of binary data>"
Expand All @@ -51,7 +52,7 @@ def render_bind(attr)
[attr&.name, value]
end

def build_explain_clause(options = [])
def build_explain_clause(connection, options = [])
if connection.respond_to?(:build_explain_clause, true)
connection.build_explain_clause(options)
else
Expand Down
4 changes: 3 additions & 1 deletion activerecord/lib/active_record/future_result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ def execute_or_wait
start = Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_millisecond)
@mutex.synchronize do
if pending?
execute_query(@pool.connection)
@pool.with_connection do |connection|
execute_query(connection)
end
else
@lock_wait = (Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_millisecond) - start)
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/insert_all.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class InsertAll # :nodoc:
attr_reader :on_duplicate, :update_only, :returning, :unique_by, :update_sql

def initialize(model, inserts, on_duplicate:, update_only: nil, returning: nil, unique_by: nil, record_timestamps: nil)
@model, @connection, @inserts = model, model.connection, inserts.map(&:stringify_keys)
@model, @connection, @inserts = model, model.lease_connection, inserts.map(&:stringify_keys)
@on_duplicate, @update_only, @returning, @unique_by = on_duplicate, update_only, returning, unique_by
@record_timestamps = record_timestamps.nil? ? model.record_timestamps : record_timestamps

Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def collection_cache_key(collection = all, timestamp_column = :updated_at) # :no
def can_use_fast_cache_version?(timestamp)
timestamp.is_a?(String) &&
cache_timestamp_format == :usec &&
self.class.connection.default_timezone == :utc &&
self.class.lease_connection.default_timezone == :utc &&
!updated_at_came_from_user?
end

Expand Down

0 comments on commit 9f6fa2b

Please sign in to comment.