Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary or incorrect calls to connection_handler #45592

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,6 @@ def retrieve_connection(connection_name, role: ActiveRecord::Base.current_role,
unless pool
if shard != ActiveRecord::Base.default_shard
message = "No connection pool for '#{connection_name}' found for the '#{shard}' shard."
elsif ActiveRecord::Base.connection_handler != ActiveRecord::Base.default_connection_handler
message = "No connection pool for '#{connection_name}' found for the '#{ActiveRecord::Base.current_role}' role."
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer use multiple handlers to control the pools so we don't need this error anymore.

elsif role != ActiveRecord::Base.default_role
message = "No connection pool for '#{connection_name}' found for the '#{role}' role."
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase

def setup
@conn = ActiveRecord::Base.connection
@connection_handler = ActiveRecord::Base.connection_handler
end

def test_connection_error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ class PostgreSQLAdapterTest < ActiveRecord::PostgreSQLTestCase

def setup
@connection = ActiveRecord::Base.connection
@connection_handler = ActiveRecord::Base.connection_handler
end

def test_connection_error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ def setup
@conn = Base.sqlite3_connection database: ":memory:",
adapter: "sqlite3",
timeout: 100

@connection_handler = ActiveRecord::Base.connection_handler
end

def test_bad_connection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,6 @@ class ConnectionHandlersShardingDbTest < ActiveRecord::TestCase

fixtures :people

def setup
@handler = ConnectionHandler.new
db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary")
@rw_pool = @handler.establish_connection(db_config)
@ro_pool = @handler.establish_connection(db_config, role: :reading)
end

def teardown
clean_up_connection_handler
end
Expand All @@ -28,7 +21,7 @@ def test_establishing_a_connection_in_connected_to_block_uses_current_role_and_s
ActiveRecord::Base.establish_connection(db_config)
assert_nothing_raised { Person.first }

assert_equal [:default, :shard_one], ActiveRecord::Base.connection_handler.send(:connection_name_to_pool_manager).fetch("ActiveRecord::Base").instance_variable_get(:@role_to_shard_mapping).values.flat_map(&:keys).uniq
assert_equal [:default, :shard_one], ActiveRecord::Base.connection_handler.send(:get_pool_manager, "ActiveRecord::Base").shard_names
end
end

Expand Down Expand Up @@ -230,11 +223,8 @@ def test_connects_to_raises_with_a_shard_and_database_key
end

def test_retrieve_connection_pool_with_invalid_shard
assert_not_nil @handler.retrieve_connection_pool("ActiveRecord::Base")
assert_nil @handler.retrieve_connection_pool("ActiveRecord::Base", shard: :foo)

assert_not_nil @handler.retrieve_connection_pool("ActiveRecord::Base", role: :reading)
assert_nil @handler.retrieve_connection_pool("ActiveRecord::Base", role: :reading, shard: :foo)
assert_not_nil ActiveRecord::Base.connection_handler.retrieve_connection_pool("ActiveRecord::Base")
assert_nil ActiveRecord::Base.connection_handler.retrieve_connection_pool("ActiveRecord::Base", shard: :foo)
end

def test_calling_connected_to_on_a_non_existent_shard_raises
Expand Down
21 changes: 10 additions & 11 deletions activerecord/test/cases/query_cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,9 @@ def test_query_cache_is_applied_to_all_connections
end

mw = middleware { |env|
rw_conn = ActiveRecord::Base.connection_handler.connection_pool_list(:writing).first.connection
assert_predicate rw_conn, :query_cache_enabled

ro_conn = ActiveRecord::Base.connection_handler.connection_pool_list(:reading).first.connection
assert_predicate ActiveRecord::Base.connection, :query_cache_enabled
assert_predicate ro_conn, :query_cache_enabled
ActiveRecord::Base.connection_handler.all_connection_pools.each do |pool|
assert_predicate pool.connection, :query_cache_enabled
end
}

mw.call({})
Expand Down Expand Up @@ -439,14 +436,16 @@ def test_cache_is_available_when_connection_is_connected

def test_cache_is_available_when_using_a_not_connected_connection
skip "In-Memory DB can't test for using a not connected connection" if in_memory_db?
db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary").dup
new_conn = ActiveRecord::Base.connection_handler.establish_connection(db_config)
db_config = ActiveRecord::Base.connection_db_config
original_connection = ActiveRecord::Base.remove_connection

ActiveRecord::Base.establish_connection(db_config)
assert_not_predicate Task, :connected?

Task.cache do
assert_queries(1) { Task.find(1); Task.find(1) }
ensure
ActiveRecord::Base.connection_handler.remove_connection_pool(new_conn)
ActiveRecord::Base.establish_connection(original_connection)
end
end

Expand Down Expand Up @@ -559,7 +558,7 @@ def test_query_caching_is_local_to_the_current_thread

def test_query_cache_is_enabled_on_all_connection_pools
middleware {
ActiveRecord::Base.connection_handler.connection_pool_list.each do |pool|
ActiveRecord::Base.connection_handler.all_connection_pools.each do |pool|
assert pool.query_cache_enabled
assert pool.connection.query_cache_enabled
end
Expand Down Expand Up @@ -618,7 +617,7 @@ def test_clear_query_cache_is_called_on_all_connections

private
def with_temporary_connection_pool(&block)
pool_config = ActiveRecord::Base.connection_handler.send(:connection_name_to_pool_manager).fetch("ActiveRecord::Base").get_pool_config(ActiveRecord.writing_role, :default)
pool_config = ActiveRecord::Base.connection.pool.pool_config
new_pool = ActiveRecord::ConnectionAdapters::ConnectionPool.new(pool_config)

pool_config.stub(:pool, new_pool, &block)
Expand Down
8 changes: 5 additions & 3 deletions activerecord/test/cases/relation/load_async_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,6 @@ def setup
@old_config = ActiveRecord.async_query_executor
ActiveRecord.async_query_executor = :multi_thread_pool

handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new
config_hash1 = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary").configuration_hash
new_config1 = config_hash1.merge(min_threads: 0, max_threads: 10)
db_config1 = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", new_config1)
Expand All @@ -363,8 +362,11 @@ def setup
new_config2 = config_hash2.merge(min_threads: 0, max_threads: 10)
db_config2 = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit2", "primary", new_config2)

handler.establish_connection(db_config1)
handler.establish_connection(db_config2, owner_name: ARUnit2Model)
ActiveRecord::Base.establish_connection(db_config1)
ARUnit2Model.establish_connection(db_config2)
ensure
ActiveRecord::Base.establish_connection(:arunit)
ARUnit2Model.establish_connection(:arunit2)
end

def teardown
Expand Down