Skip to content

Commit

Permalink
Deprecate delegation to connection handler from Base
Browse files Browse the repository at this point in the history
In a multi-db world, delegating from `Base` to the handler doesn't make
much sense. Applications should know when they are dealing with a single
connection (Base.connection) or the handler which deals with multiple
connections. Delegating to the connection handler from Base breaks this
contract and makes behavior confusing. I think eventually a new object
will replace the handler altogether but for now I'd like to just
separate concerns to avoid confusion.
  • Loading branch information
eileencodes committed Oct 19, 2022
1 parent cf2be23 commit a93d8fe
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 19 deletions.
6 changes: 6 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
* Deprecate delegation from `Base` to `connection_handler`.

Calling `Base.clear_all_connections!`, `Base.clear_active_connections!`, `Base.clear_reloadable_connections!` and `Base.flush_idle_connections!` is deprecated. Please call these methods on the connection handler directly. In future Rails versions, the delegation from `Base` to the `connection_handler` will be removed.

*Eileen M. Uchitelle*

* Allow ActiveRecord::QueryMethods#reselect to receive hash values, similar to ActiveRecord::QueryMethods#select

*Sampat Badhe*
Expand Down
Expand Up @@ -64,7 +64,7 @@ def async_executor; end
# as with Active Record 2.1 and
# earlier (pre-connection-pooling). Eventually, when you're done with
# the connection(s) and wish it to be returned to the pool, you call
# {ActiveRecord::Base.clear_active_connections!}[rdoc-ref:ConnectionAdapters::ConnectionHandler#clear_active_connections!].
# {ActiveRecord::Base.connection_handler.clear_active_connections!}[rdoc-ref:ConnectionAdapters::ConnectionHandler#clear_active_connections!].
# This will be the default behavior for Active Record when used in conjunction with
# Action Pack's request handling cycle.
# 2. Manually check out a connection from the pool with
Expand Down
29 changes: 27 additions & 2 deletions activerecord/lib/active_record/connection_handling.rb
Expand Up @@ -308,10 +308,35 @@ def clear_cache! # :nodoc:
connection.schema_cache.clear!
end

delegate :clear_active_connections!, :clear_reloadable_connections!,
:clear_all_connections!, :flush_idle_connections!, to: :connection_handler
def clear_active_connections!(role = nil)
deprecation_for_delegation(__method__)
connection_handler.clear_active_connections!(role)
end

def clear_reloadable_connections!(role = nil)
deprecation_for_delegation(__method__)
connection_handler.clear_reloadable_connections!(role)
end

def clear_all_connections!(role = nil)
deprecation_for_delegation(__method__)
connection_handler.clear_all_connections!(role)
end

def flush_idle_connections!(role = nil)
deprecation_for_delegation(__method__)
connection_handler.flush_idle_connections!(role)
end

private
def deprecation_for_delegation(method)
ActiveSupport::Deprecation.warn(<<-MSG.squish)
Calling `ActiveRecord::Base.#{method} is deprecated. Please
call the method directly on the connection handler; for
example: `ActiveRecord::Base.connection_handler.#{method}`.
MSG
end

def resolve_config_for_connection(config_or_env)
raise "Anonymous class is not allowed." unless name

Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/errors.rb
Expand Up @@ -93,7 +93,7 @@ def username_error(username)
# Raised when a pool was unable to get ahold of all its connections
# to perform a "group" action such as
# {ActiveRecord::Base.connection_pool.disconnect!}[rdoc-ref:ConnectionAdapters::ConnectionPool#disconnect!]
# or {ActiveRecord::Base.clear_reloadable_connections!}[rdoc-ref:ConnectionAdapters::ConnectionHandler#clear_reloadable_connections!].
# or {ActiveRecord::Base.connection_handler.clear_reloadable_connections!}[rdoc-ref:ConnectionAdapters::ConnectionHandler#clear_reloadable_connections!].
class ExclusiveConnectionTimeoutError < ConnectionTimeoutError
end

Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/migration.rb
Expand Up @@ -646,7 +646,7 @@ def load_schema_if_pending!
# Roundtrip to Rake to allow plugins to hook into database initialization.
root = defined?(ENGINE_ROOT) ? ENGINE_ROOT : Rails.root
FileUtils.cd(root) do
Base.clear_all_connections!
Base.connection_handler.clear_all_connections!
system("bin/rails db:test:prepare")
end
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/railtie.rb
Expand Up @@ -297,7 +297,7 @@ class Railtie < Rails::Railtie # :nodoc:
ActiveSupport::Reloader.before_class_unload do
if ActiveRecord::Base.connected?
ActiveRecord::Base.clear_cache!
ActiveRecord::Base.clear_reloadable_connections!(:all)
ActiveRecord::Base.connection_handler.clear_reloadable_connections!(:all)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/test_fixtures.rb
Expand Up @@ -162,7 +162,7 @@ def teardown_fixtures
ActiveRecord::FixtureSet.reset_cache
end

ActiveRecord::Base.clear_active_connections!(:all)
ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
end

def enlist_fixture_connections
Expand Down
Expand Up @@ -26,7 +26,7 @@ class Sample < ActiveRecord::Base
end

teardown do
ActiveRecord::Base.clear_active_connections!(:all)
ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
ActiveRecord::Base.connection.drop_table "samples", if_exists: true

Thread.abort_on_exception = @abort
Expand Down
Expand Up @@ -217,7 +217,7 @@ def with_warning_suppression
ActiveRecord::Base.connection.client_min_messages = "error"
yield
ensure
ActiveRecord::Base.clear_active_connections!(:all)
ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
ActiveRecord::Base.connection.client_min_messages = log_level
end

Expand Down
Expand Up @@ -205,7 +205,7 @@ def with_warning_suppression
ActiveRecord::Base.connection.client_min_messages = "error"
yield
ensure
ActiveRecord::Base.clear_active_connections!(:all)
ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
ActiveRecord::Base.connection.client_min_messages = log_level
end
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/hot_compatibility_test.rb
Expand Up @@ -136,7 +136,7 @@ def with_two_connections
ActiveRecord::Base.connection_pool.checkin ddl_connection
end
ensure
ActiveRecord::Base.clear_all_connections!(:all)
ActiveRecord::Base.connection_handler.clear_all_connections!(:all)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/pooled_connections_test.rb
Expand Up @@ -13,7 +13,7 @@ def setup
end

teardown do
ActiveRecord::Base.clear_all_connections!(:all)
ActiveRecord::Base.connection_handler.clear_all_connections!(:all)
ActiveRecord::Base.establish_connection(@connection)
@per_test_teardown.each(&:call)
end
Expand Down
10 changes: 5 additions & 5 deletions activerecord/test/cases/query_cache_test.rb
Expand Up @@ -178,7 +178,7 @@ def test_query_cache_across_threads
assert_cache :dirty

thread_1_connection = ActiveRecord::Base.connection
ActiveRecord::Base.clear_active_connections!(:all)
ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
assert_cache :off, thread_1_connection

started = Concurrent::Event.new
Expand All @@ -200,7 +200,7 @@ def test_query_cache_across_threads
started.set
checked.wait

ActiveRecord::Base.clear_active_connections!(:all)
ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
}.call({})
}

Expand Down Expand Up @@ -522,7 +522,7 @@ def test_query_cached_even_when_types_are_reset
end

def test_query_cache_does_not_establish_connection_if_unconnected
ActiveRecord::Base.clear_active_connections!(:all)
ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
assert_not ActiveRecord::Base.connection_handler.active_connections?(:all) # Double check they are cleared

middleware {
Expand All @@ -533,7 +533,7 @@ def test_query_cache_does_not_establish_connection_if_unconnected
end

def test_query_cache_is_enabled_on_connections_established_after_middleware_runs
ActiveRecord::Base.clear_active_connections!(:all)
ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
assert_not ActiveRecord::Base.connection_handler.active_connections?(:all) # Double check they are cleared

middleware {
Expand All @@ -543,7 +543,7 @@ def test_query_cache_is_enabled_on_connections_established_after_middleware_runs
end

def test_query_caching_is_local_to_the_current_thread
ActiveRecord::Base.clear_active_connections!(:all)
ActiveRecord::Base.connection_handler.clear_active_connections!(:all)

middleware {
assert ActiveRecord::Base.connection_pool.query_cache_enabled
Expand Down
4 changes: 2 additions & 2 deletions activerecord/test/cases/transactions_test.rb
Expand Up @@ -51,7 +51,7 @@ def test_rollback_dirty_changes_even_with_raise_during_rollback_removes_from_poo
assert_not connection.active?
assert_not Topic.connection_pool.connections.include?(connection)
ensure
ActiveRecord::Base.clear_all_connections!(:all)
ActiveRecord::Base.connection_handler.clear_all_connections!(:all)
end

def test_rollback_dirty_changes_even_with_raise_during_rollback_doesnt_commit_transaction
Expand All @@ -77,7 +77,7 @@ def test_rollback_dirty_changes_even_with_raise_during_rollback_doesnt_commit_tr

assert_equal "The Fifth Topic of the day", topic.reload.title
ensure
ActiveRecord::Base.clear_all_connections!(:all)
ActiveRecord::Base.connection_handler.clear_all_connections!(:all)
end
end

Expand Down

0 comments on commit a93d8fe

Please sign in to comment.