mirrors
/
rails
mirror of https://github.com/rails/rails
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Remove owner_name 

We don't actually need this since the only reason it exists is to pass
the owning class name down to the `handler`. This removes a level of
indirection and an unnecessary accessor on db_config. db_config
shouldn't have to know what class owns it, so we can just remove this
and pass it to the handler.

The Symbol case is needed to preserve current behavior. This doesn't
need a changelog because it's changing un-released behavior.

Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
This commit is contained in:
eileencodes 2020-03-06 17:28:46 -05:00
parent 526dd6472b
commit 7400d195e2
No known key found for this signature in database
GPG Key ID: BA5C575120BBE8DF
6 changed files with 24 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
View File

@ -1039,8 +1039,10 @@ module ActiveRecord
end
alias :connection_pools :connection_pool_list
def establish_connection(config, pool_key = ActiveRecord::Base.default_pool_key)
pool_config = resolve_pool_config(config)
def establish_connection(config, pool_key = Base.default_pool_key, owner_name = Base.name)
owner_name = config.to_s if config.is_a?(Symbol)
pool_config = resolve_pool_config(config, owner_name)
db_config = pool_config.db_config
# Protects the connection named `ActiveRecord::Base` from being removed
@ -1182,10 +1184,8 @@ module ActiveRecord
# 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)
def resolve_pool_config(config, owner_name)
db_config = Base.configurations.resolve(config)
raise(AdapterNotSpecified, "database configuration does not specify adapter") unless db_config.adapter
@ -1214,9 +1214,7 @@ module ActiveRecord
raise AdapterNotFound, "database configuration specifies nonexistent #{db_config.adapter} adapter"
end
pool_name = db_config.owner_name || Base.name
db_config.owner_name = nil
ConnectionAdapters::PoolConfig.new(pool_name, db_config)
ConnectionAdapters::PoolConfig.new(owner_name, db_config)
end
end
end

25
activerecord/lib/active_record/connection_handling.rb
View File

@ -48,8 +48,8 @@ module ActiveRecord
# may be returned on an error.
def establish_connection(config_or_env = nil)
config_or_env ||= DEFAULT_ENV.call.to_sym
db_config = resolve_config_for_connection(config_or_env)
connection_handler.establish_connection(db_config, current_pool_key)
db_config, owner_name = resolve_config_for_connection(config_or_env)
connection_handler.establish_connection(db_config, current_pool_key, owner_name)
end
# Connects a model to the databases specified. The +database+ keyword
@ -86,18 +86,18 @@ module ActiveRecord
connections = []
database.each do |role, database_key|
db_config = resolve_config_for_connection(database_key)
db_config, owner_name = resolve_config_for_connection(database_key)
handler = lookup_connection_handler(role.to_sym)
connections << handler.establish_connection(db_config)
connections << handler.establish_connection(db_config, default_pool_key, owner_name)
end
shards.each do |pool_key, database_keys|
database_keys.each do |role, database_key|
db_config = resolve_config_for_connection(database_key)
db_config, owner_name = resolve_config_for_connection(database_key)
handler = lookup_connection_handler(role.to_sym)
connections << handler.establish_connection(db_config, pool_key.to_sym)
connections << handler.establish_connection(db_config, pool_key.to_sym, owner_name)
end
end
@ -151,10 +151,10 @@ module ActiveRecord
role = role.to_sym
end
db_config = resolve_config_for_connection(database)
db_config, owner_name = resolve_config_for_connection(database)
handler = lookup_connection_handler(role)
handler.establish_connection(db_config)
handler.establish_connection(db_config, default_pool_key, owner_name)
with_handler(role, &blk)
elsif shard
@ -282,12 +282,11 @@ module ActiveRecord
def resolve_config_for_connection(config_or_env)
raise "Anonymous class is not allowed." unless name
pool_name = primary_class? ? Base.name : name
self.connection_specification_name = pool_name
owner_name = primary_class? ? Base.name : name
self.connection_specification_name = owner_name
db_config = Base.configurations.resolve(config_or_env, pool_name)
db_config.owner_name = pool_name
db_config
db_config = Base.configurations.resolve(config_or_env)
[db_config, owner_name]
end
def with_handler(handler_key, &blk)

7
activerecord/lib/active_record/database_configurations.rb
View File

@ -133,12 +133,12 @@ module ActiveRecord
#
# DatabaseConfigurations.new({}).resolve("postgresql://localhost/foo")
# # => DatabaseConfigurations::UrlConfig.new(config: {"adapter" => "postgresql", "host" => "localhost", "database" => "foo"})
def resolve(config, pool_name = nil) # :nodoc:
def resolve(config) # :nodoc:
return config if DatabaseConfigurations::DatabaseConfig === config
case config
when Symbol
resolve_symbol_connection(config, pool_name)
resolve_symbol_connection(config)
when Hash, String
build_db_config_from_raw_config(default_env, "primary", config)
else
@ -184,9 +184,8 @@ module ActiveRecord
end
end
def resolve_symbol_connection(name, pool_name)
def resolve_symbol_connection(name)
if db_config = find_db_config(name)
db_config.owner_name = pool_name.to_s
db_config
else
raise AdapterNotSpecified, <<~MSG

14
activerecord/test/cases/connection_adapters/connection_handler_test.rb
View File

@ -28,20 +28,6 @@ module ActiveRecord
ENV["RACK_ENV"] = original_rack_env
end
def test_establish_connection_uses_config_hash_with_name
old_config = ActiveRecord::Base.configurations
config = { "readonly" => { "adapter" => "sqlite3", "pool" => "5" } }
ActiveRecord::Base.configurations = config
db_config = ActiveRecord::Base.configurations.resolve(config["readonly"], "readonly")
db_config.owner_name = "readonly"
@handler.establish_connection(db_config)
assert_not_nil @handler.retrieve_connection_pool("readonly")
ensure
ActiveRecord::Base.configurations = old_config
@handler.remove_connection_pool("readonly")
end
def test_establish_connection_using_3_levels_config
previous_env, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], "default_env"

11
activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb
View File

@ -24,7 +24,7 @@ module ActiveRecord
def resolve_db_config(spec, config)
configs = ActiveRecord::DatabaseConfigurations.new(config)
configs.resolve(spec, spec)
configs.resolve(spec)
end
def test_invalid_string_config
@ -50,7 +50,6 @@ module ActiveRecord
expected = { adapter: "postgresql", database: "foo", host: "localhost" }
assert_equal expected, actual.configuration_hash
assert_equal "default_env", actual.owner_name
end
def test_resolver_with_database_uri_and_current_env_symbol_key_and_rails_env
@ -62,7 +61,6 @@ module ActiveRecord
expected = { adapter: "postgresql", database: "foo", host: "localhost" }
assert_equal expected, actual.configuration_hash
assert_equal "foo", actual.owner_name
end
def test_resolver_with_nil_database_url_and_current_env
@ -72,7 +70,6 @@ module ActiveRecord
expected_config = { adapter: "postgres", url: nil }
assert_equal expected_config, actual.configuration_hash
assert_equal "foo", actual.owner_name
end
def test_resolver_with_database_uri_and_current_env_symbol_key_and_rack_env
@ -84,7 +81,6 @@ module ActiveRecord
expected = { adapter: "postgresql", database: "foo", host: "localhost" }
assert_equal expected, actual.configuration_hash
assert_equal "foo", actual.owner_name
end
def test_resolver_with_database_uri_and_known_key
@ -94,7 +90,6 @@ module ActiveRecord
expected = { adapter: "not_postgres", database: "not_foo", host: "localhost" }
assert_equal expected, actual.configuration_hash
assert_equal "production", actual.owner_name
end
def test_resolver_with_database_uri_and_multiple_envs
@ -106,7 +101,6 @@ module ActiveRecord
expected = { adapter: "postgresql", database: "foo_test", host: "localhost" }
assert_equal expected, actual.configuration_hash
assert_equal "test", actual.owner_name
end
def test_resolver_with_database_uri_and_unknown_symbol_key
@ -152,7 +146,6 @@ module ActiveRecord
expected = { adapter: "ibm_db", database: "foo", host: "localhost" }
assert_equal expected, actual.configuration_hash
assert_equal "default_env", actual.owner_name
end
def test_string_connection
@ -417,7 +410,6 @@ module ActiveRecord
actual = resolve_db_config(:production, config)
assert_equal config["production"].symbolize_keys, actual.configuration_hash
assert_equal "production", actual.owner_name
actual = resolve_db_config(:default_env, config)
@ -426,7 +418,6 @@ module ActiveRecord
database: "foo",
adapter: "postgresql",
}, actual.configuration_hash)
assert_equal "default_env", actual.owner_name
end
end
end

6
activerecord/test/cases/database_configurations/resolver_test.rb
View File

@ -8,7 +8,7 @@ module ActiveRecord
class ResolverTest < ActiveRecord::TestCase
def resolve_db_config(pool_config, config = {})
configs = ActiveRecord::DatabaseConfigurations.new(config)
configs.resolve(pool_config, pool_config)
configs.resolve(pool_config)
end
def test_url_invalid_adapter
@ -38,7 +38,6 @@ module ActiveRecord
host: "foo",
encoding: "utf8"
}, pool_config.configuration_hash)
assert_equal "production", pool_config.owner_name
end
def test_url_sub_key
@ -49,7 +48,6 @@ module ActiveRecord
host: "foo",
encoding: "utf8"
}, pool_config.configuration_hash)
assert_equal "production", pool_config.owner_name
end
def test_url_sub_key_merges_correctly
@ -62,7 +60,6 @@ module ActiveRecord
encoding: "utf8",
pool: "3"
}, pool_config.configuration_hash)
assert_equal "production", pool_config.owner_name
end
def test_url_host_no_db
@ -140,7 +137,6 @@ module ActiveRecord
database: "foo",
encoding: "utf8"
}, pool_config.configuration_hash)
assert_equal "production", pool_config.owner_name
end
def test_pool_config_with_invalid_type