mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Remove ConnectionAdapters::Resolver in favor of DatabaseConfigurations
We have these two objects, `ConnectionAdapters::Resolver` and `DatabaseConfiguratons` that implement a lot of the same logic. One of them is used for configurations defined in `config/database.yml` and the other is used when passing raw configurations `String` or `Hash` objects into methods like `establish_connection`. Over time these two have diverged a bit. In the interest of less code complexity, and more consistency for users this commit brings them back together. * Remove `Resolver` altogether and replace its primary method with `DatabaseConfigurations#resolve`. * Move `resolve_pool_config` over to the `ConnectionPool` alongside the code that uses it.
This commit is contained in:
parent
30349de203
commit
8d5a4ff6a7
9 changed files with 124 additions and 192 deletions
|
@ -1,3 +1,7 @@
|
|||
* Calling methods like `establish_connection` with a `Hash` which is invalid (eg: no `adapter`) will now raise an error the same way as connections defined in `config/database.yml`.
|
||||
|
||||
*John Crepezzi*
|
||||
|
||||
* Specifying `implicit_order_column` now subsorts the records by primary key if available to ensure deterministic results.
|
||||
|
||||
*Paweł Urbanek*
|
||||
|
|
|
@ -1040,8 +1040,7 @@ module ActiveRecord
|
|||
alias :connection_pools :connection_pool_list
|
||||
|
||||
def establish_connection(config, pool_key = :default)
|
||||
resolver = Resolver.new(Base.configurations)
|
||||
pool_config = resolver.resolve_pool_config(config)
|
||||
pool_config = resolve_pool_config(config)
|
||||
db_config = pool_config.db_config
|
||||
|
||||
remove_connection(pool_config.connection_specification_name, pool_key)
|
||||
|
@ -1146,6 +1145,51 @@ module ActiveRecord
|
|||
|
||||
private
|
||||
attr_reader :owner_to_pool_manager
|
||||
|
||||
# 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" } }
|
||||
# pool_config = Base.configurations.resolve_pool_config(:production)
|
||||
# pool_config.db_config.configuration_hash
|
||||
# # => { host: "localhost", database: "foo", adapter: "sqlite3" }
|
||||
#
|
||||
def resolve_pool_config(config)
|
||||
pool_name = config if config.is_a?(Symbol)
|
||||
|
||||
db_config = Base.configurations.resolve(config, pool_name)
|
||||
|
||||
raise(AdapterNotSpecified, "database configuration does not specify adapter") unless db_config.adapter
|
||||
|
||||
# Require the adapter itself and give useful feedback about
|
||||
# 1. Missing adapter gems and
|
||||
# 2. Adapter gems' missing dependencies.
|
||||
path_to_adapter = "active_record/connection_adapters/#{db_config.adapter}_adapter"
|
||||
begin
|
||||
require path_to_adapter
|
||||
rescue LoadError => e
|
||||
# We couldn't require the adapter itself. Raise an exception that
|
||||
# points out config typos and missing gems.
|
||||
if e.path == path_to_adapter
|
||||
# We can assume that a non-builtin adapter was specified, so it's
|
||||
# either misspelled or missing from Gemfile.
|
||||
raise LoadError, "Could not load the '#{db_config.adapter}' 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.", e.backtrace
|
||||
|
||||
# Bubbled up from the adapter require. Prefix the exception message
|
||||
# with some guidance about how to address it and reraise.
|
||||
else
|
||||
raise LoadError, "Error loading the '#{db_config.adapter}' Active Record adapter. Missing a gem it depends on? #{e.message}", e.backtrace
|
||||
end
|
||||
end
|
||||
|
||||
unless ActiveRecord::Base.respond_to?(db_config.adapter_method)
|
||||
raise AdapterNotFound, "database configuration specifies nonexistent #{db_config.adapter} adapter"
|
||||
end
|
||||
|
||||
ConnectionAdapters::PoolConfig.new(db_config.configuration_hash.delete(:name) || "primary", db_config)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,158 +0,0 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module ActiveRecord
|
||||
module ConnectionAdapters
|
||||
# Builds a PoolConfig from user input.
|
||||
class Resolver # :nodoc:
|
||||
attr_reader :configurations
|
||||
|
||||
# Accepts a list of db config objects.
|
||||
def initialize(configurations)
|
||||
@configurations = configurations
|
||||
end
|
||||
|
||||
# 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" } }
|
||||
# pool_config = Resolver.new(config).resolve_pool_config(:production)
|
||||
# pool_config.db_config.configuration_hash
|
||||
# # => { host: "localhost", database: "foo", adapter: "sqlite3" }
|
||||
#
|
||||
def resolve_pool_config(config)
|
||||
pool_name = config if config.is_a?(Symbol)
|
||||
|
||||
db_config = resolve(config, pool_name)
|
||||
|
||||
raise(AdapterNotSpecified, "database configuration does not specify adapter") unless db_config.adapter
|
||||
|
||||
# Require the adapter itself and give useful feedback about
|
||||
# 1. Missing adapter gems and
|
||||
# 2. Adapter gems' missing dependencies.
|
||||
path_to_adapter = "active_record/connection_adapters/#{db_config.adapter}_adapter"
|
||||
begin
|
||||
require path_to_adapter
|
||||
rescue LoadError => e
|
||||
# We couldn't require the adapter itself. Raise an exception that
|
||||
# points out config typos and missing gems.
|
||||
if e.path == path_to_adapter
|
||||
# We can assume that a non-builtin adapter was specified, so it's
|
||||
# either misspelled or missing from Gemfile.
|
||||
raise LoadError, "Could not load the '#{db_config.adapter}' 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.", e.backtrace
|
||||
|
||||
# Bubbled up from the adapter require. Prefix the exception message
|
||||
# with some guidance about how to address it and reraise.
|
||||
else
|
||||
raise LoadError, "Error loading the '#{db_config.adapter}' Active Record adapter. Missing a gem it depends on? #{e.message}", e.backtrace
|
||||
end
|
||||
end
|
||||
|
||||
unless ActiveRecord::Base.respond_to?(db_config.adapter_method)
|
||||
raise AdapterNotFound, "database configuration specifies nonexistent #{db_config.adapter} adapter"
|
||||
end
|
||||
|
||||
PoolConfig.new(db_config.configuration_hash.delete(:name) || "primary", db_config)
|
||||
end
|
||||
|
||||
# Returns fully resolved connection, accepts hash, string or symbol.
|
||||
# Always returns a DatabaseConfiguration::DatabaseConfig
|
||||
#
|
||||
# == Examples
|
||||
#
|
||||
# Symbol representing current environment.
|
||||
#
|
||||
# Resolver.new("production" => {}).resolve(:production)
|
||||
# # => DatabaseConfigurations::HashConfig.new(env_name: "production", config: {})
|
||||
#
|
||||
# One layer deep hash of connection values.
|
||||
#
|
||||
# Resolver.new({}).resolve("adapter" => "sqlite3")
|
||||
# # => DatabaseConfigurations::HashConfig.new(config: {"adapter" => "sqlite3"})
|
||||
#
|
||||
# Connection URL.
|
||||
#
|
||||
# Resolver.new({}).resolve("postgresql://localhost/foo")
|
||||
# # => DatabaseConfigurations::UrlConfig.new(config: {"adapter" => "postgresql", "host" => "localhost", "database" => "foo"})
|
||||
#
|
||||
def resolve(config_or_env, pool_name = nil)
|
||||
env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_s
|
||||
|
||||
case config_or_env
|
||||
when Symbol
|
||||
resolve_symbol_connection(config_or_env, pool_name)
|
||||
when String
|
||||
DatabaseConfigurations::UrlConfig.new(env, "primary", config_or_env)
|
||||
when Hash
|
||||
resolve_hash_configuration(env, config_or_env.symbolize_keys)
|
||||
when DatabaseConfigurations::DatabaseConfig
|
||||
config_or_env
|
||||
else
|
||||
raise TypeError, "Invalid type for configuration. Expected Symbol, String, or Hash. Got #{config_or_env.inspect}"
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
# Resolve a hash to a valid configuration object. This method will
|
||||
# either return a HashConfig, or a UrlConfig if the passed Hash
|
||||
# contains a `:url` key.
|
||||
def resolve_hash_configuration(env, config)
|
||||
if config.has_key?(:url)
|
||||
url = config[:url]
|
||||
config_without_url = config.dup
|
||||
config_without_url.delete :url
|
||||
DatabaseConfigurations::UrlConfig.new(env, "primary", url, config)
|
||||
else
|
||||
DatabaseConfigurations::HashConfig.new(env, "primary", config)
|
||||
end
|
||||
end
|
||||
|
||||
# Takes the environment such as +:production+ or +:development+ and a
|
||||
# pool name the corresponds to the name given by the connection pool
|
||||
# to the connection. That pool name is merged into the hash with the
|
||||
# name key.
|
||||
#
|
||||
# This requires that the @configurations was initialized with a key that
|
||||
# matches.
|
||||
#
|
||||
# configurations = #<ActiveRecord::DatabaseConfigurations:0x00007fd9fdace3e0
|
||||
# @configurations=[
|
||||
# #<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd9fdace250
|
||||
# @env_name="production", @spec_name="primary", @config={database: "my_db"}>
|
||||
# ]>
|
||||
#
|
||||
# Resolver.new(configurations).resolve_symbol_connection(:production, "primary")
|
||||
# # => DatabaseConfigurations::HashConfig(config: database: "my_db", env_name: "production", spec_name: "primary")
|
||||
def resolve_symbol_connection(env_name, pool_name)
|
||||
db_config = configurations.find_db_config(env_name)
|
||||
|
||||
if db_config
|
||||
config = db_config.configuration_hash.merge(name: pool_name.to_s)
|
||||
DatabaseConfigurations::HashConfig.new(db_config.env_name, db_config.spec_name, config)
|
||||
else
|
||||
raise AdapterNotSpecified, <<~MSG
|
||||
The `#{env_name}` database is not configured for the `#{ActiveRecord::ConnectionHandling::DEFAULT_ENV.call}` environment.
|
||||
|
||||
Available databases configurations are:
|
||||
|
||||
#{build_configuration_sentence}
|
||||
MSG
|
||||
end
|
||||
end
|
||||
|
||||
def build_configuration_sentence # :nodoc:
|
||||
configs = configurations.configs_for(include_replicas: true)
|
||||
|
||||
configs.group_by(&:env_name).map do |env, config|
|
||||
namespaces = config.map(&:spec_name)
|
||||
if namespaces.size > 1
|
||||
"#{env}: #{namespaces.join(", ")}"
|
||||
else
|
||||
env
|
||||
end
|
||||
end.join("\n")
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -256,9 +256,7 @@ module ActiveRecord
|
|||
pool_name = primary_class? ? "primary" : name
|
||||
self.connection_specification_name = pool_name
|
||||
|
||||
resolver = ConnectionAdapters::Resolver.new(Base.configurations)
|
||||
|
||||
db_config = resolver.resolve(config_or_env, pool_name)
|
||||
db_config = Base.configurations.resolve(config_or_env, pool_name)
|
||||
db_config.configuration_hash[:name] = pool_name
|
||||
db_config
|
||||
end
|
||||
|
|
|
@ -106,6 +106,39 @@ module ActiveRecord
|
|||
[config.env_name, config.configuration_hash]
|
||||
end
|
||||
|
||||
# Returns fully resolved connection, accepts hash, string or symbol.
|
||||
# Always returns a DatabaseConfiguration::DatabaseConfig
|
||||
#
|
||||
# == Examples
|
||||
#
|
||||
# Symbol representing current environment.
|
||||
#
|
||||
# DatabaseConfigurations.new("production" => {}).resolve(:production)
|
||||
# # => DatabaseConfigurations::HashConfig.new(env_name: "production", config: {})
|
||||
#
|
||||
# One layer deep hash of connection values.
|
||||
#
|
||||
# DatabaseConfigurations.new({}).resolve("adapter" => "sqlite3")
|
||||
# # => DatabaseConfigurations::HashConfig.new(config: {"adapter" => "sqlite3"})
|
||||
#
|
||||
# Connection URL.
|
||||
#
|
||||
# DatabaseConfigurations.new({}).resolve("postgresql://localhost/foo")
|
||||
# # => DatabaseConfigurations::UrlConfig.new(config: {"adapter" => "postgresql", "host" => "localhost", "database" => "foo"})
|
||||
def resolve(config, pool_name = nil) # :nodoc:
|
||||
return config if DatabaseConfigurations::DatabaseConfig === config
|
||||
|
||||
case config
|
||||
when Symbol
|
||||
resolve_symbol_connection(config, pool_name)
|
||||
when Hash, String
|
||||
env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_s
|
||||
build_db_config_from_raw_config(env, "primary", config)
|
||||
else
|
||||
raise TypeError, "Invalid type for configuration. Expected Symbol, String, or Hash. Got #{config.inspect}"
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
def env_with_configs(env = nil)
|
||||
if env
|
||||
|
@ -142,6 +175,36 @@ module ActiveRecord
|
|||
end
|
||||
end
|
||||
|
||||
def resolve_symbol_connection(env_name, pool_name)
|
||||
db_config = find_db_config(env_name)
|
||||
|
||||
if db_config
|
||||
config = db_config.configuration_hash.merge(name: pool_name.to_s)
|
||||
DatabaseConfigurations::HashConfig.new(db_config.env_name, db_config.spec_name, config)
|
||||
else
|
||||
raise AdapterNotSpecified, <<~MSG
|
||||
The `#{env_name}` database is not configured for the `#{ActiveRecord::ConnectionHandling::DEFAULT_ENV.call}` environment.
|
||||
|
||||
Available databases configurations are:
|
||||
|
||||
#{build_configuration_sentence}
|
||||
MSG
|
||||
end
|
||||
end
|
||||
|
||||
def build_configuration_sentence
|
||||
configs = configs_for(include_replicas: true)
|
||||
|
||||
configs.group_by(&:env_name).map do |env, config|
|
||||
namespaces = config.map(&:spec_name)
|
||||
if namespaces.size > 1
|
||||
"#{env}: #{namespaces.join(", ")}"
|
||||
else
|
||||
env
|
||||
end
|
||||
end.join("\n")
|
||||
end
|
||||
|
||||
def build_db_config_from_raw_config(env_name, spec_name, config)
|
||||
case config
|
||||
when String
|
||||
|
|
|
@ -456,8 +456,7 @@ module ActiveRecord
|
|||
|
||||
private
|
||||
def resolve_configuration(configuration)
|
||||
resolver = ConnectionAdapters::Resolver.new(ActiveRecord::Base.configurations)
|
||||
resolver.resolve(configuration)
|
||||
Base.configurations.resolve(configuration)
|
||||
end
|
||||
|
||||
def verbose?
|
||||
|
|
|
@ -31,8 +31,7 @@ module ActiveRecord
|
|||
old_config = ActiveRecord::Base.configurations
|
||||
config = { "readonly" => { "adapter" => "sqlite3", "pool" => "5" } }
|
||||
ActiveRecord::Base.configurations = config
|
||||
resolver = ConnectionAdapters::Resolver.new(ActiveRecord::Base.configurations)
|
||||
config_hash = resolver.resolve(config["readonly"], "readonly").configuration_hash
|
||||
config_hash = ActiveRecord::Base.configurations.resolve(config["readonly"], "readonly").configuration_hash
|
||||
config_hash[:name] = "readonly"
|
||||
@handler.establish_connection(config_hash)
|
||||
|
||||
|
|
|
@ -24,8 +24,7 @@ module ActiveRecord
|
|||
|
||||
def resolve_spec(spec, config)
|
||||
configs = ActiveRecord::DatabaseConfigurations.new(config)
|
||||
resolver = ConnectionAdapters::Resolver.new(configs)
|
||||
resolver.resolve(spec, spec).configuration_hash
|
||||
configs.resolve(spec, spec).configuration_hash
|
||||
end
|
||||
|
||||
def test_invalid_string_config
|
||||
|
|
|
@ -8,19 +8,12 @@ module ActiveRecord
|
|||
class ResolverTest < ActiveRecord::TestCase
|
||||
def resolve(pool_config, config = {})
|
||||
configs = ActiveRecord::DatabaseConfigurations.new(config)
|
||||
resolver = ConnectionAdapters::Resolver.new(configs)
|
||||
resolver.resolve(pool_config, pool_config).configuration_hash
|
||||
end
|
||||
|
||||
def resolve_pool_config(pool_config, config = {})
|
||||
configs = ActiveRecord::DatabaseConfigurations.new(config)
|
||||
resolver = ConnectionAdapters::Resolver.new(configs)
|
||||
resolver.resolve_pool_config(pool_config)
|
||||
configs.resolve(pool_config, pool_config).configuration_hash
|
||||
end
|
||||
|
||||
def test_url_invalid_adapter
|
||||
error = assert_raises(LoadError) do
|
||||
resolve_pool_config "ridiculous://foo?encoding=utf8"
|
||||
Base.connection_handler.establish_connection "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 +21,7 @@ module ActiveRecord
|
|||
|
||||
def test_error_if_no_adapter_method
|
||||
error = assert_raises(AdapterNotFound) do
|
||||
resolve_pool_config "abstract://foo?encoding=utf8"
|
||||
Base.connection_handler.establish_connection "abstract://foo?encoding=utf8"
|
||||
end
|
||||
|
||||
assert_match "database configuration specifies nonexistent abstract adapter", error.message
|
||||
|
@ -79,8 +72,9 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
def test_url_missing_scheme
|
||||
pool_config = resolve "foo"
|
||||
assert_equal({ database: "foo" }, pool_config)
|
||||
assert_raises ActiveRecord::DatabaseConfigurations::InvalidConfigurationError do
|
||||
resolve "foo"
|
||||
end
|
||||
end
|
||||
|
||||
def test_url_host_db
|
||||
|
@ -140,19 +134,9 @@ module ActiveRecord
|
|||
}, pool_config)
|
||||
end
|
||||
|
||||
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_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_pool_config_with_invalid_type
|
||||
assert_raises TypeError do
|
||||
resolve_pool_config(Object.new)
|
||||
Base.connection_handler.establish_connection(Object.new)
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in a new issue