From 6d81eab13d65b7239444c9d0bfb14518755ddbc9 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Tue, 5 Nov 2019 17:05:54 -0500 Subject: [PATCH] Rename the classes This commit renames `RoleManager` -> `PoolManager` and `Role` -> `PoolConfig`. Once we introduced the previous commit, and looking at the existing code, it's clearer that `Role` and `RoleManager` are not the right names for these. Since this PR moves away from swapping the connection handler concepts around and the role concept will continue existing on the handler level, we need to rename this. A `PoolConfig` holds a `connection_specification_name` (we may rename this down the road), a `db_config`, a `schema_cache`, and a `pool`. It does feel like `pool` could eventually hold all of these things instead of having a `PoolConfig` object. This would remove one level of the object graph and reduce complexity. For now I'm leaving this object to keep the change churn low and will revisit later. Co-authored-by: John Crepezzi --- .../lib/active_record/connection_adapters.rb | 4 +- .../abstract/connection_pool.rb | 66 +++++++-------- .../{role.rb => pool_config.rb} | 4 +- .../connection_adapters/pool_manager.rb | 27 ++++++ .../connection_adapters/resolver.rb | 12 +-- .../connection_adapters/role_manager.rb | 27 ------ .../lib/active_record/test_fixtures.rb | 8 +- .../adapter_leasing_test.rb | 4 +- ...ection_handlers_multi_pool_config_test.rb} | 17 ++-- .../test/cases/connection_pool_test.rb | 24 +++--- activerecord/test/cases/disconnected_test.rb | 4 +- activerecord/test/cases/fixtures_test.rb | 6 +- .../resolver_test.rb | 82 +++++++++---------- activerecord/test/cases/query_cache_test.rb | 6 +- activerecord/test/cases/reaper_test.rb | 38 ++++----- activerecord/test/cases/unconnected_test.rb | 2 +- 16 files changed, 166 insertions(+), 165 deletions(-) rename activerecord/lib/active_record/connection_adapters/{role.rb => pool_config.rb} (94%) create mode 100644 activerecord/lib/active_record/connection_adapters/pool_manager.rb delete mode 100644 activerecord/lib/active_record/connection_adapters/role_manager.rb rename activerecord/test/cases/connection_adapters/{connection_handlers_multi_role_test.rb => connection_handlers_multi_pool_config_test.rb} (86%) rename activerecord/test/cases/{connection_specification => pool_config}/resolver_test.rb (57%) diff --git a/activerecord/lib/active_record/connection_adapters.rb b/activerecord/lib/active_record/connection_adapters.rb index d7626150c9..ed3790d2ec 100644 --- a/activerecord/lib/active_record/connection_adapters.rb +++ b/activerecord/lib/active_record/connection_adapters.rb @@ -9,8 +9,8 @@ module ActiveRecord end autoload :Column - autoload :Role - autoload :RoleManager + autoload :PoolConfig + autoload :PoolManager autoload :Resolver autoload_at "active_record/connection_adapters/abstract/schema_definitions" do diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 3b5c23d2fb..c9fd12cd8b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -370,21 +370,21 @@ module ActiveRecord include ConnectionAdapters::AbstractPool attr_accessor :automatic_reconnect, :checkout_timeout - attr_reader :db_config, :size, :reaper, :role + attr_reader :db_config, :size, :reaper, :pool_config - delegate :schema_cache, :schema_cache=, to: :role + delegate :schema_cache, :schema_cache=, to: :pool_config - # Creates a new ConnectionPool object. +role+ is a Role + # Creates a new ConnectionPool object. +pool_config+ is a PoolConfig # object which describes database connection information (e.g. adapter, # host name, username, password, etc), as well as the maximum size for # this ConnectionPool. # # The default ConnectionPool maximum size is 5. - def initialize(role) + def initialize(pool_config) super() - @role = role - @db_config = role.db_config + @pool_config = pool_config + @db_config = pool_config.db_config @checkout_timeout = db_config.checkout_timeout @idle_timeout = db_config.idle_timeout @@ -1003,8 +1003,8 @@ module ActiveRecord private_constant :FINALIZER def initialize - # These caches are keyed by role.connection_specification_name (Role#connection_specification_name). - @owner_to_role_manager = Concurrent::Map.new(initial_capacity: 2) + # These caches are keyed by pool_config.connection_specification_name (PoolConfig#connection_specification_name). + @owner_to_pool_manager = Concurrent::Map.new(initial_capacity: 2) # Backup finalizer: if the forked child skipped Kernel#fork the early discard has not occurred ObjectSpace.define_finalizer self, FINALIZER @@ -1031,36 +1031,36 @@ module ActiveRecord end def connection_pool_names # :nodoc: - owner_to_role_manager.keys + owner_to_pool_manager.keys end def connection_pool_list - owner_to_role_manager.values.compact.flat_map { |rc| rc.roles.map(&:pool) } + owner_to_pool_manager.values.compact.flat_map { |m| m.pool_configs.map(&:pool) } end alias :connection_pools :connection_pool_list - def establish_connection(config, role_name = :default) + def establish_connection(config, pool_key = :default) resolver = Resolver.new(Base.configurations) - role = resolver.resolve_role(config) - db_config = role.db_config + pool_config = resolver.resolve_pool_config(config) + db_config = pool_config.db_config - remove_connection(role.connection_specification_name, role_name) + remove_connection(pool_config.connection_specification_name, pool_key) message_bus = ActiveSupport::Notifications.instrumenter payload = { connection_id: object_id } - if role - payload[:spec_name] = role.connection_specification_name + if pool_config + payload[:spec_name] = pool_config.connection_specification_name payload[:config] = db_config.configuration_hash end - owner_to_role_manager[role.connection_specification_name] ||= RoleManager.new - role_manager = owner_to_role_manager[role.connection_specification_name] - role_manager.set_role(role_name, role) + owner_to_pool_manager[pool_config.connection_specification_name] ||= PoolManager.new + pool_manager = owner_to_pool_manager[pool_config.connection_specification_name] + pool_manager.set_pool_config(pool_key, pool_config) message_bus.instrument("!connection.active_record", payload) do - role.pool + pool_config.pool end end @@ -1116,8 +1116,8 @@ module ActiveRecord # Returns true if a connection that's accessible to this class has # already been opened. - def connected?(spec_name, role_name = :default) - pool = retrieve_connection_pool(spec_name, role_name) + def connected?(spec_name, pool_key = :default) + pool = retrieve_connection_pool(spec_name, pool_key) pool && pool.connected? end @@ -1125,27 +1125,27 @@ module ActiveRecord # connection and the defined connection (if they exist). The result # can be used as an argument for #establish_connection, for easily # re-establishing the connection. - def remove_connection(spec_name, role_name = :default) - if role_manager = owner_to_role_manager[spec_name] - role = role_manager.remove_role(role_name) + def remove_connection(spec_name, pool_key = :default) + if pool_manager = owner_to_pool_manager[spec_name] + pool_config = pool_manager.remove_pool_config(pool_key) - if role - role.disconnect! - role.db_config.configuration_hash + if pool_config + pool_config.disconnect! + pool_config.db_config.configuration_hash end end end - # Retrieving the connection pool happens a lot, so we cache it in @owner_to_role_manager. + # Retrieving the connection pool happens a lot, so we cache it in @owner_to_pool_manager. # This makes retrieving the connection pool O(1) once the process is warm. # When a connection is established or removed, we invalidate the cache. - def retrieve_connection_pool(spec_name, role_name = :default) - role = owner_to_role_manager[spec_name]&.get_role(role_name) - role&.pool + def retrieve_connection_pool(spec_name, pool_key = :default) + pool_config = owner_to_pool_manager[spec_name]&.get_pool_config(pool_key) + pool_config&.pool end private - attr_reader :owner_to_role_manager + attr_reader :owner_to_pool_manager end end end diff --git a/activerecord/lib/active_record/connection_adapters/role.rb b/activerecord/lib/active_record/connection_adapters/pool_config.rb similarity index 94% rename from activerecord/lib/active_record/connection_adapters/role.rb rename to activerecord/lib/active_record/connection_adapters/pool_config.rb index db9d3e98ee..74588409c5 100644 --- a/activerecord/lib/active_record/connection_adapters/role.rb +++ b/activerecord/lib/active_record/connection_adapters/pool_config.rb @@ -2,7 +2,7 @@ module ActiveRecord module ConnectionAdapters - class Role # :nodoc: + class PoolConfig # :nodoc: include Mutex_m attr_reader :db_config, :connection_specification_name @@ -60,4 +60,4 @@ module ActiveRecord end end -ActiveSupport::ForkTracker.after_fork { ActiveRecord::ConnectionAdapters::Role.discard_pools! } +ActiveSupport::ForkTracker.after_fork { ActiveRecord::ConnectionAdapters::PoolConfig.discard_pools! } diff --git a/activerecord/lib/active_record/connection_adapters/pool_manager.rb b/activerecord/lib/active_record/connection_adapters/pool_manager.rb new file mode 100644 index 0000000000..bd62b0970d --- /dev/null +++ b/activerecord/lib/active_record/connection_adapters/pool_manager.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module ActiveRecord + module ConnectionAdapters + class PoolManager # :nodoc: + def initialize + @name_to_pool_config = {} + end + + def pool_configs + @name_to_pool_config.values + end + + def remove_pool_config(key) + @name_to_pool_config.delete(key) + end + + def get_pool_config(key) + @name_to_pool_config[key] + end + + def set_pool_config(key, pool_config) + @name_to_pool_config[key] = pool_config + end + end + end +end diff --git a/activerecord/lib/active_record/connection_adapters/resolver.rb b/activerecord/lib/active_record/connection_adapters/resolver.rb index 89d5d50869..18700f7fcc 100644 --- a/activerecord/lib/active_record/connection_adapters/resolver.rb +++ b/activerecord/lib/active_record/connection_adapters/resolver.rb @@ -2,7 +2,7 @@ module ActiveRecord module ConnectionAdapters - # Builds a Role from user input. + # Builds a PoolConfig from user input. class Resolver # :nodoc: attr_reader :configurations @@ -11,17 +11,17 @@ module ActiveRecord @configurations = configurations end - # Returns an instance of Role for a given adapter. + # Returns an instance of PoolConfig for a given adapter. # Accepts a hash one layer deep that contains all connection information. # # == Example # # config = { "production" => { "host" => "localhost", "database" => "foo", "adapter" => "sqlite3" } } - # role = Resolver.new(config).resolve_role(:production) - # role.db_config.configuration_hash + # pool_config = Resolver.new(config).resolve_pool_config(:production) + # pool_config.db_config.configuration_hash # # => { host: "localhost", database: "foo", adapter: "sqlite3" } # - def resolve_role(config) + def resolve_pool_config(config) pool_name = config if config.is_a?(Symbol) db_config = resolve(config, pool_name) @@ -53,7 +53,7 @@ module ActiveRecord raise AdapterNotFound, "database configuration specifies nonexistent #{db_config.adapter} adapter" end - Role.new(db_config.configuration_hash.delete(:name) || "primary", db_config) + PoolConfig.new(db_config.configuration_hash.delete(:name) || "primary", db_config) end # Returns fully resolved connection, accepts hash, string or symbol. diff --git a/activerecord/lib/active_record/connection_adapters/role_manager.rb b/activerecord/lib/active_record/connection_adapters/role_manager.rb deleted file mode 100644 index 3837cdbb0e..0000000000 --- a/activerecord/lib/active_record/connection_adapters/role_manager.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecord - module ConnectionAdapters - class RoleManager # :nodoc: - def initialize - @name_to_role = {} - end - - def roles - @name_to_role.values - end - - def remove_role(name) - @name_to_role.delete(name) - end - - def get_role(name) - @name_to_role[name] - end - - def set_role(name, role) - @name_to_role[name] = role - end - end - end -end diff --git a/activerecord/lib/active_record/test_fixtures.rb b/activerecord/lib/active_record/test_fixtures.rb index 30d3804ce3..665fd98c24 100644 --- a/activerecord/lib/active_record/test_fixtures.rb +++ b/activerecord/lib/active_record/test_fixtures.rb @@ -192,11 +192,11 @@ module ActiveRecord ActiveRecord::Base.connection_handlers.values.each do |handler| if handler != writing_handler handler.connection_pool_names.each do |name| - writing_role_manager = writing_handler.send(:owner_to_role_manager)[name] - writing_role = writing_role_manager.get_role(:default) + writing_pool_manager = writing_handler.send(:owner_to_pool_manager)[name] + writing_pool_config = writing_pool_manager.get_pool_config(:default) - role_manager = handler.send(:owner_to_role_manager)[name] - role_manager.set_role(:default, writing_role) + pool_manager = handler.send(:owner_to_pool_manager)[name] + pool_manager.set_pool_config(:default, writing_pool_config) end end end diff --git a/activerecord/test/cases/connection_adapters/adapter_leasing_test.rb b/activerecord/test/cases/connection_adapters/adapter_leasing_test.rb index 12a49513be..093d244791 100644 --- a/activerecord/test/cases/connection_adapters/adapter_leasing_test.rb +++ b/activerecord/test/cases/connection_adapters/adapter_leasing_test.rb @@ -40,8 +40,8 @@ module ActiveRecord def test_close db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("test", "primary", {}) - role = ActiveRecord::ConnectionAdapters::Role.new("primary", db_config) - pool = Pool.new(role) + pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new("primary", db_config) + pool = Pool.new(pool_config) pool.insert_connection_for_test! @adapter @adapter.pool = pool diff --git a/activerecord/test/cases/connection_adapters/connection_handlers_multi_role_test.rb b/activerecord/test/cases/connection_adapters/connection_handlers_multi_pool_config_test.rb similarity index 86% rename from activerecord/test/cases/connection_adapters/connection_handlers_multi_role_test.rb rename to activerecord/test/cases/connection_adapters/connection_handlers_multi_pool_config_test.rb index 292472fc36..d75045c7e7 100644 --- a/activerecord/test/cases/connection_adapters/connection_handlers_multi_role_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handlers_multi_pool_config_test.rb @@ -5,7 +5,7 @@ require "models/person" module ActiveRecord module ConnectionAdapters - class ConnectionHandlersMultiRoleTest < ActiveRecord::TestCase + class ConnectionHandlersMultiPoolConfigTest < ActiveRecord::TestCase self.use_transactional_tests = false fixtures :people @@ -22,7 +22,7 @@ module ActiveRecord end unless in_memory_db? - def test_establish_connection_with_roles + def test_establish_connection_with_pool_configs previous_env, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], "default_env" config = { @@ -34,10 +34,10 @@ module ActiveRecord @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config @writing_handler.establish_connection(:primary) - @writing_handler.establish_connection(:primary, :role_two) + @writing_handler.establish_connection(:primary, :pool_config_two) default_pool = @writing_handler.retrieve_connection_pool("primary", :default) - other_pool = @writing_handler.retrieve_connection_pool("primary", :role_two) + other_pool = @writing_handler.retrieve_connection_pool("primary", :pool_config_two) assert_not_nil default_pool assert_not_equal default_pool, other_pool @@ -62,13 +62,13 @@ module ActiveRecord @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config @writing_handler.establish_connection(:primary) - @writing_handler.establish_connection(:primary, :role_two) + @writing_handler.establish_connection(:primary, :pool_config_two) # remove default @writing_handler.remove_connection("primary") assert_nil @writing_handler.retrieve_connection_pool("primary") - assert_not_nil @writing_handler.retrieve_connection_pool("primary", :role_two) + assert_not_nil @writing_handler.retrieve_connection_pool("primary", :pool_config_two) ensure ActiveRecord::Base.configurations = @prev_configs ActiveRecord::Base.establish_connection(:arunit) @@ -87,18 +87,19 @@ module ActiveRecord @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config @writing_handler.establish_connection(:primary) - @writing_handler.establish_connection(:primary, :role_two) + @writing_handler.establish_connection(:primary, :pool_config_two) # connect to default @writing_handler.connection_pool_list.first.checkout assert @writing_handler.connected?("primary") assert @writing_handler.connected?("primary", :default) - assert_not @writing_handler.connected?("primary", :role_two) + assert_not @writing_handler.connected?("primary", :pool_config_two) ensure ActiveRecord::Base.configurations = @prev_configs ActiveRecord::Base.establish_connection(:arunit) ENV["RAILS_ENV"] = previous_env + FileUtils.rm_rf "db" end end end diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 8a6e64a60c..5b0302afb3 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -13,8 +13,8 @@ module ActiveRecord # Keep a duplicate pool so we do not bother others @db_config = ActiveRecord::Base.connection_pool.db_config - @role = ActiveRecord::ConnectionAdapters::Role.new("primary", @db_config) - @pool = ConnectionPool.new(@role) + @pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new("primary", @db_config) + @pool = ConnectionPool.new(@pool_config) if in_memory_db? # Separate connections to an in-memory database create an entirely new database, @@ -201,8 +201,8 @@ module ActiveRecord def test_idle_timeout_configuration @pool.disconnect! @db_config.configuration_hash.merge!(idle_timeout: "0.02") - role = ActiveRecord::ConnectionAdapters::Role.new("primary", @db_config) - @pool = ConnectionPool.new(role) + pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new("primary", @db_config) + @pool = ConnectionPool.new(pool_config) idle_conn = @pool.checkout @pool.checkin(idle_conn) @@ -226,8 +226,8 @@ module ActiveRecord def test_disable_flush @pool.disconnect! @db_config.configuration_hash.merge!(idle_timeout: -5) - role = ActiveRecord::ConnectionAdapters::Role.new("primary", @db_config) - @pool = ConnectionPool.new(role) + pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new("primary", @db_config) + @pool = ConnectionPool.new(pool_config) idle_conn = @pool.checkout @pool.checkin(idle_conn) @@ -317,7 +317,7 @@ module ActiveRecord end def test_checkout_behaviour - pool = ConnectionPool.new(@role) + pool = ConnectionPool.new(@pool_config) main_connection = pool.connection assert_not_nil main_connection threads = [] @@ -450,7 +450,7 @@ module ActiveRecord end def test_automatic_reconnect_restores_after_disconnect - pool = ConnectionPool.new(@role) + pool = ConnectionPool.new(@pool_config) assert pool.automatic_reconnect assert pool.connection @@ -459,7 +459,7 @@ module ActiveRecord end def test_automatic_reconnect_can_be_disabled - pool = ConnectionPool.new(@role) + pool = ConnectionPool.new(@pool_config) pool.disconnect! pool.automatic_reconnect = false @@ -723,10 +723,10 @@ module ActiveRecord old_config = @db_config.configuration_hash db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", old_config.dup) - db_config.configuration_hash[:pool] = 1 # this is safe to do, because .dupped Role also auto-dups its config + db_config.configuration_hash[:pool] = 1 # this is safe to do, because .dupped PoolConfig also auto-dups its config - role = ActiveRecord::ConnectionAdapters::Role.new("primary", db_config) - yield(pool = ConnectionPool.new(role)) + pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new("primary", db_config) + yield(pool = ConnectionPool.new(pool_config)) ensure pool.disconnect! if pool end diff --git a/activerecord/test/cases/disconnected_test.rb b/activerecord/test/cases/disconnected_test.rb index da90526425..b7237d11fc 100644 --- a/activerecord/test/cases/disconnected_test.rb +++ b/activerecord/test/cases/disconnected_test.rb @@ -14,8 +14,8 @@ class TestDisconnectedAdapter < ActiveRecord::TestCase teardown do return if in_memory_db? - role = ActiveRecord::Base.connection_config - ActiveRecord::Base.establish_connection(role) + config = ActiveRecord::Base.connection_config + ActiveRecord::Base.establish_connection(config) end unless in_memory_db? diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 9c0e9e5ea6..ff939ad1ac 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -1414,10 +1414,10 @@ class MultipleDatabaseFixturesTest < ActiveRecord::TestCase private def with_temporary_connection_pool - role = ActiveRecord::Base.connection_handler.send(:owner_to_role_manager).fetch("primary").get_role(:default) - new_pool = ActiveRecord::ConnectionAdapters::ConnectionPool.new(role) + pool_config = ActiveRecord::Base.connection_handler.send(:owner_to_pool_manager).fetch("primary").get_pool_config(:default) + new_pool = ActiveRecord::ConnectionAdapters::ConnectionPool.new(pool_config) - role.stub(:pool, new_pool) do + pool_config.stub(:pool, new_pool) do yield end end diff --git a/activerecord/test/cases/connection_specification/resolver_test.rb b/activerecord/test/cases/pool_config/resolver_test.rb similarity index 57% rename from activerecord/test/cases/connection_specification/resolver_test.rb rename to activerecord/test/cases/pool_config/resolver_test.rb index 28da6aa928..b45e2a13bd 100644 --- a/activerecord/test/cases/connection_specification/resolver_test.rb +++ b/activerecord/test/cases/pool_config/resolver_test.rb @@ -4,23 +4,23 @@ require "cases/helper" module ActiveRecord module ConnectionAdapters - class Role + class PoolConfig class ResolverTest < ActiveRecord::TestCase - def resolve(role, config = {}) + def resolve(pool_config, config = {}) configs = ActiveRecord::DatabaseConfigurations.new(config) resolver = ConnectionAdapters::Resolver.new(configs) - resolver.resolve(role, role).configuration_hash + resolver.resolve(pool_config, pool_config).configuration_hash end - def resolve_role(role, config = {}) + def resolve_pool_config(pool_config, config = {}) configs = ActiveRecord::DatabaseConfigurations.new(config) resolver = ConnectionAdapters::Resolver.new(configs) - resolver.resolve_role(role) + resolver.resolve_pool_config(pool_config) end def test_url_invalid_adapter error = assert_raises(LoadError) do - resolve_role "ridiculous://foo?encoding=utf8" + resolve_pool_config "ridiculous://foo?encoding=utf8" end assert_match "Could not load the 'ridiculous' Active Record adapter. Ensure that the adapter is spelled correctly in config/database.yml and that you've added the necessary adapter gem to your Gemfile.", error.message @@ -28,7 +28,7 @@ module ActiveRecord def test_error_if_no_adapter_method error = assert_raises(AdapterNotFound) do - resolve_role "abstract://foo?encoding=utf8" + resolve_pool_config "abstract://foo?encoding=utf8" end assert_match "database configuration specifies nonexistent abstract adapter", error.message @@ -38,121 +38,121 @@ module ActiveRecord # checks that the adapter file can be required in. def test_url_from_environment - role = resolve :production, "production" => "abstract://foo?encoding=utf8" + pool_config = resolve :production, "production" => "abstract://foo?encoding=utf8" assert_equal({ adapter: "abstract", host: "foo", encoding: "utf8", name: "production" - }, role) + }, pool_config) end def test_url_sub_key - role = resolve :production, "production" => { "url" => "abstract://foo?encoding=utf8" } + pool_config = resolve :production, "production" => { "url" => "abstract://foo?encoding=utf8" } assert_equal({ adapter: "abstract", host: "foo", encoding: "utf8", name: "production" - }, role) + }, pool_config) end def test_url_sub_key_merges_correctly hash = { "url" => "abstract://foo?encoding=utf8&", "adapter" => "sqlite3", "host" => "bar", "pool" => "3" } - role = resolve :production, "production" => hash + pool_config = resolve :production, "production" => hash assert_equal({ adapter: "abstract", host: "foo", encoding: "utf8", pool: "3", name: "production" - }, role) + }, pool_config) end def test_url_host_no_db - role = resolve "abstract://foo?encoding=utf8" + pool_config = resolve "abstract://foo?encoding=utf8" assert_equal({ adapter: "abstract", host: "foo", encoding: "utf8" - }, role) + }, pool_config) end def test_url_missing_scheme - role = resolve "foo" - assert_equal({ database: "foo" }, role) + pool_config = resolve "foo" + assert_equal({ database: "foo" }, pool_config) end def test_url_host_db - role = resolve "abstract://foo/bar?encoding=utf8" + pool_config = resolve "abstract://foo/bar?encoding=utf8" assert_equal({ adapter: "abstract", database: "bar", host: "foo", encoding: "utf8" - }, role) + }, pool_config) end def test_url_port - role = resolve "abstract://foo:123?encoding=utf8" + pool_config = resolve "abstract://foo:123?encoding=utf8" assert_equal({ adapter: "abstract", port: 123, host: "foo", encoding: "utf8" - }, role) + }, pool_config) end def test_encoded_password password = "am@z1ng_p@ssw0rd#!" encoded_password = URI.encode_www_form_component(password) - role = resolve "abstract://foo:#{encoded_password}@localhost/bar" - assert_equal password, role[:password] + pool_config = resolve "abstract://foo:#{encoded_password}@localhost/bar" + assert_equal password, pool_config[:password] end def test_url_with_authority_for_sqlite3 - role = resolve "sqlite3:///foo_test" - assert_equal("/foo_test", role[:database]) + pool_config = resolve "sqlite3:///foo_test" + assert_equal("/foo_test", pool_config[:database]) end def test_url_absolute_path_for_sqlite3 - role = resolve "sqlite3:/foo_test" - assert_equal("/foo_test", role[:database]) + pool_config = resolve "sqlite3:/foo_test" + assert_equal("/foo_test", pool_config[:database]) end def test_url_relative_path_for_sqlite3 - role = resolve "sqlite3:foo_test" - assert_equal("foo_test", role[:database]) + pool_config = resolve "sqlite3:foo_test" + assert_equal("foo_test", pool_config[:database]) end def test_url_memory_db_for_sqlite3 - role = resolve "sqlite3::memory:" - assert_equal(":memory:", role[:database]) + pool_config = resolve "sqlite3::memory:" + assert_equal(":memory:", pool_config[:database]) end def test_url_sub_key_for_sqlite3 - role = resolve :production, "production" => { "url" => "sqlite3:foo?encoding=utf8" } + pool_config = resolve :production, "production" => { "url" => "sqlite3:foo?encoding=utf8" } assert_equal({ adapter: "sqlite3", database: "foo", encoding: "utf8", name: "production" - }, role) + }, pool_config) end - def test_role_connection_specification_name_on_key_lookup - role = resolve_role(:readonly, "readonly" => { "adapter" => "sqlite3" }) - assert_equal "readonly", role.connection_specification_name + def test_pool_config_connection_specification_name_on_key_lookup + pool_config = resolve_pool_config(:readonly, "readonly" => { "adapter" => "sqlite3" }) + assert_equal "readonly", pool_config.connection_specification_name end - def test_role_connection_specification_name_with_inline_config - role = resolve_role("adapter" => "sqlite3") - assert_equal "primary", role.connection_specification_name, "should default to primary id" + def test_pool_config_connection_specification_name_with_inline_config + pool_config = resolve_pool_config("adapter" => "sqlite3") + assert_equal "primary", pool_config.connection_specification_name, "should default to primary id" end - def test_role_with_invalid_type + def test_pool_config_with_invalid_type assert_raises TypeError do - resolve_role(Object.new) + resolve_pool_config(Object.new) end end end diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index e35bc7010b..6e90124b81 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -555,10 +555,10 @@ class QueryCacheTest < ActiveRecord::TestCase private def with_temporary_connection_pool - role = ActiveRecord::Base.connection_handler.send(:owner_to_role_manager).fetch("primary").get_role(:default) - new_pool = ActiveRecord::ConnectionAdapters::ConnectionPool.new(role) + pool_config = ActiveRecord::Base.connection_handler.send(:owner_to_pool_manager).fetch("primary").get_pool_config(:default) + new_pool = ActiveRecord::ConnectionAdapters::ConnectionPool.new(pool_config) - role.stub(:pool, new_pool) do + pool_config.stub(:pool, new_pool) do yield end end diff --git a/activerecord/test/cases/reaper_test.rb b/activerecord/test/cases/reaper_test.rb index 8eab4d5807..dd9fc57299 100644 --- a/activerecord/test/cases/reaper_test.rb +++ b/activerecord/test/cases/reaper_test.rb @@ -59,8 +59,8 @@ module ActiveRecord def test_pool_has_reaper config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", spec_name: "primary") - role = Role.new("primary", config) - pool = ConnectionPool.new(role) + pool_config = PoolConfig.new("primary", config) + pool = ConnectionPool.new(pool_config) assert pool.reaper ensure @@ -68,10 +68,10 @@ module ActiveRecord end def test_reaping_frequency_configuration - role = duplicated_role - role.db_config.configuration_hash[:reaping_frequency] = "10.01" + pool_config = duplicated_pool_config + pool_config.db_config.configuration_hash[:reaping_frequency] = "10.01" - pool = ConnectionPool.new(role) + pool = ConnectionPool.new(pool_config) assert_equal 10.01, pool.reaper.frequency ensure @@ -79,10 +79,10 @@ module ActiveRecord end def test_connection_pool_starts_reaper - role = duplicated_role - role.db_config.configuration_hash[:reaping_frequency] = "0.0001" + pool_config = duplicated_pool_config + pool_config.db_config.configuration_hash[:reaping_frequency] = "0.0001" - pool = ConnectionPool.new(role) + pool = ConnectionPool.new(pool_config) conn, child = new_conn_in_thread(pool) @@ -97,11 +97,11 @@ module ActiveRecord end def test_reaper_works_after_pool_discard - role = duplicated_role - role.db_config.configuration_hash[:reaping_frequency] = "0.0001" + pool_config = duplicated_pool_config + pool_config.db_config.configuration_hash[:reaping_frequency] = "0.0001" 2.times do - pool = ConnectionPool.new(role) + pool = ConnectionPool.new(pool_config) conn, child = new_conn_in_thread(pool) @@ -119,8 +119,8 @@ module ActiveRecord # This doesn't test the reaper directly, but we want to test the action # it would take on a discarded pool def test_reap_flush_on_discarded_pool - role = duplicated_role - pool = ConnectionPool.new(role) + pool_config = duplicated_pool_config + pool = ConnectionPool.new(pool_config) pool.discard! pool.reap @@ -128,14 +128,14 @@ module ActiveRecord end def test_connection_pool_starts_reaper_in_fork - role = duplicated_role - role.db_config.configuration_hash[:reaping_frequency] = "0.0001" + pool_config = duplicated_pool_config + pool_config.db_config.configuration_hash[:reaping_frequency] = "0.0001" - pool = ConnectionPool.new(role) + pool = ConnectionPool.new(pool_config) pool.checkout pid = fork do - pool = ConnectionPool.new(role) + pool = ConnectionPool.new(pool_config) conn, child = new_conn_in_thread(pool) child.terminate @@ -173,10 +173,10 @@ module ActiveRecord end private - def duplicated_role + def duplicated_pool_config old_config = ActiveRecord::Base.connection_pool.db_config.configuration_hash db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", old_config.dup) - Role.new("primary", db_config) + PoolConfig.new("primary", db_config) end def new_conn_in_thread(pool) diff --git a/activerecord/test/cases/unconnected_test.rb b/activerecord/test/cases/unconnected_test.rb index a53d0ec258..f9b3d79709 100644 --- a/activerecord/test/cases/unconnected_test.rb +++ b/activerecord/test/cases/unconnected_test.rb @@ -13,7 +13,7 @@ class TestUnconnectedAdapter < ActiveRecord::TestCase @specification = ActiveRecord::Base.remove_connection # Clear out connection info from other pids (like a fork parent) too - ActiveRecord::ConnectionAdapters::Role.discard_pools! + ActiveRecord::ConnectionAdapters::PoolConfig.discard_pools! end teardown do