Skip to content

Commit

Permalink
Deprecate spec_name and use name for configurations
Browse files Browse the repository at this point in the history
I have so. many. regrets. about using `spec_name` for database
configurations and now I'm finally putting this mistake to an end.

Back when I started multi-db work I assumed that eventually
`connection_specification_name` (sometimes called `spec_name`) and
`spec_name` for configurations would one day be the same thing. After
2 years I no longer believe they will ever be the same thing.

This PR deprecates `spec_name` on database configurations in favor of
`name`. It's the same behavior, just a better name, or at least a
less confusing name.

`connection_specification_name` refers to the parent class name (ie
ActiveRecord::Base, AnimalsBase, etc) that holds the connection for it's
models. In some places like ConnectionHandler it shortens this to
`spec_name`, hence the major confusion.

Recently I've been working with some new folks on database stuff and
connection management and realize how confusing it was to explain that
`db_config.spec_name` was not `spec_name` and
`connection_specification_name`. Worse than that one is a symbole while
the other is a class name. This was made even more complicated by the
fact that `ActiveRecord::Base` used `primary` as the
`connection_specification_name` until #38190.

After spending 2 years with connection management I don't believe that
we can ever use the symbols from the database configs as a way to
connect the database without the class name being _somewhere_ because
a db_config does not know who it's owner class is until it's been
connected and a model has no idea what db_config belongs to it until
it's connected. The model is the only way to tie a primary/writer config
to a replica/reader config. This could change in the future but I don't
see value in adding a class name to the db_configs before connection or
telling a model what config belongs to it before connection. That would
probably break a lot of application assumptions. If we do ever end up in
that world, we can use name, because tbh `spec_name` and
`connection_specification_name` were always confusing to me.
  • Loading branch information
eileencodes committed Feb 24, 2020
1 parent a5cda04 commit 79ce7d9
Show file tree
Hide file tree
Showing 34 changed files with 293 additions and 244 deletions.
20 changes: 20 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,23 @@
* Deprecate `spec_name` in favor of `name` on database configurations

The accessors for `spec_name` on `configs_for` and `DatabaseConfig` are deprecated. Please use `name` instead.

Deprecated behavior:

```ruby
db_config = ActiveRecord::Base.configs_for(env_name: "development", spec_name: "primary")
db_config.spec_name
```

New behavior:

```ruby
db_config = ActiveRecord::Base.configs_for(env_name: "development", name: "primary")
db_config.name
```

*Eileen M. Uchitelle*

* Add additional database-specific rake tasks for multi-database users

Previously, `rails db:create`, `rails db:drop`, and `rails db:migrate` were the only rails tasks that could operate on a single
Expand Down
Expand Up @@ -130,12 +130,12 @@ def migration_context # :nodoc:
def schema_migration # :nodoc:
@schema_migration ||= begin
conn = self
spec_name = conn.pool.db_config.spec_name
name = "#{spec_name}::SchemaMigration"
name = conn.pool.db_config.name
schema_migration_name = "#{name}::SchemaMigration"

Class.new(ActiveRecord::SchemaMigration) do
define_singleton_method(:name) { name }
define_singleton_method(:to_s) { name }
define_singleton_method(:name) { schema_migration_name }
define_singleton_method(:to_s) { schema_migration_name }

connection_handler.connection_pool_names.each do |pool_name|
if conn.pool == connection_handler.retrieve_connection_pool(pool_name)
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/connection_handling.rb
Expand Up @@ -203,7 +203,7 @@ def connection_config
#
# ActiveRecord::Base.connection_db_config
# #<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbded10 @env_name="development",
# @spec_name="primary", @config={pool: 5, timeout: 5000, database: "db/development.sqlite3", adapter: "sqlite3"}>
# @name="primary", @config={pool: 5, timeout: 5000, database: "db/development.sqlite3", adapter: "sqlite3"}>
#
# Use only for reading.
def connection_db_config
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/core.rb
Expand Up @@ -43,9 +43,9 @@ module Core
#
# #<ActiveRecord::DatabaseConfigurations:0x00007fd1acbdf800 @configurations=[
# #<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbded10 @env_name="development",
# @spec_name="primary", @config={adapter: "sqlite3", database: "db/development.sqlite3"}>,
# @name="primary", @config={adapter: "sqlite3", database: "db/development.sqlite3"}>,
# #<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbdea90 @env_name="production",
# @spec_name="primary", @config={adapter: "sqlite3", database: "db/production.sqlite3"}>
# @name="primary", @config={adapter: "sqlite3", database: "db/production.sqlite3"}>
# ]>
def self.configurations=(config)
@@configurations = ActiveRecord::DatabaseConfigurations.new(config)
Expand Down
67 changes: 36 additions & 31 deletions activerecord/lib/active_record/database_configurations.rb
Expand Up @@ -22,23 +22,28 @@ def initialize(configurations = {})
# Collects the configs for the environment and optionally the specification
# name passed in. To include replica configurations pass <tt>include_replicas: true</tt>.
#
# If a spec name is provided a single DatabaseConfig object will be
# If a name is provided a single DatabaseConfig object will be
# returned, otherwise an array of DatabaseConfig objects will be
# returned that corresponds with the environment and type requested.
#
# ==== Options
#
# * <tt>env_name:</tt> The environment name. Defaults to +nil+ which will collect
# configs for all environments.
# * <tt>spec_name:</tt> The specification name (i.e. primary, animals, etc.). Defaults
# * <tt>name:</tt> The db name name (i.e. primary, animals, etc.). Defaults
# to +nil+. If no +env_name+ is specified the config for the default env and the
# passed +spec_name+ will be returned.
# passed +name+ will be returned.
# * <tt>include_replicas:</tt> Determines whether to include replicas in
# the returned list. Most of the time we're only iterating over the write
# connection (i.e. migrations don't need to run for the write and read connection).
# Defaults to +false+.
def configs_for(env_name: nil, spec_name: nil, include_replicas: false)
env_name ||= default_env if spec_name
def configs_for(env_name: nil, spec_name: nil, name: nil, include_replicas: false)
if spec_name
name = spec_name
ActiveSupport::Deprecation.warn("The kwarg `spec_name` is deprecated in favor of `name`. `spec_name` will be removed in Rails 6.2")
end

env_name ||= default_env if name
configs = env_with_configs(env_name)

unless include_replicas
Expand All @@ -47,9 +52,9 @@ def configs_for(env_name: nil, spec_name: nil, include_replicas: false)
end
end

if spec_name
if name
configs.find do |db_config|
db_config.spec_name == spec_name
db_config.name == name
end
else
configs
Expand All @@ -76,7 +81,7 @@ def default_hash(env = default_env)
def find_db_config(env)
configurations.find do |db_config|
db_config.env_name == env.to_s ||
(db_config.for_current_env? && db_config.spec_name == env.to_s)
(db_config.for_current_env? && db_config.name == env.to_s)
end
end

Expand All @@ -86,7 +91,7 @@ def to_h
memo.merge(db_config.env_name => db_config.configuration_hash.stringify_keys)
end
end
deprecate to_h: "You can use `ActiveRecord::Base.configurations.configs_for(env_name: 'env', spec_name: 'primary').configuration_hash` to get the configuration hashes."
deprecate to_h: "You can use `ActiveRecord::Base.configurations.configs_for(env_name: 'env', name: 'primary').configuration_hash` to get the configuration hashes."

# Checks if the application's configurations are empty.
#
Expand Down Expand Up @@ -174,8 +179,8 @@ def build_configs(configs)
end

def walk_configs(env_name, config)
config.map do |spec_name, sub_config|
build_db_config_from_raw_config(env_name, spec_name.to_s, sub_config)
config.map do |name, sub_config|
build_db_config_from_raw_config(env_name, name.to_s, sub_config)
end
end

Expand All @@ -184,7 +189,7 @@ def resolve_symbol_connection(env_name, pool_name)

if db_config
config = db_config.configuration_hash.dup
db_config = DatabaseConfigurations::HashConfig.new(db_config.env_name, db_config.spec_name, config)
db_config = DatabaseConfigurations::HashConfig.new(db_config.env_name, db_config.name, config)
db_config.owner_name = pool_name.to_s
db_config
else
Expand All @@ -202,68 +207,68 @@ 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(", ")}"
names = config.map(&:name)
if names.size > 1
"#{env}: #{names.join(", ")}"
else
env
end
end.join("\n")
end

def build_db_config_from_raw_config(env_name, spec_name, config)
def build_db_config_from_raw_config(env_name, name, config)
case config
when String
build_db_config_from_string(env_name, spec_name, config)
build_db_config_from_string(env_name, name, config)
when Hash
build_db_config_from_hash(env_name, spec_name, config.symbolize_keys)
build_db_config_from_hash(env_name, name, config.symbolize_keys)
else
raise InvalidConfigurationError, "'{ #{env_name} => #{config} }' is not a valid configuration. Expected '#{config}' to be a URL string or a Hash."
end
end

def build_db_config_from_string(env_name, spec_name, config)
def build_db_config_from_string(env_name, name, config)
url = config
uri = URI.parse(url)
if uri.scheme
UrlConfig.new(env_name, spec_name, url)
UrlConfig.new(env_name, name, url)
else
raise InvalidConfigurationError, "'{ #{env_name} => #{config} }' is not a valid configuration. Expected '#{config}' to be a URL string or a Hash."
end
end

def build_db_config_from_hash(env_name, spec_name, config)
def build_db_config_from_hash(env_name, name, config)
if config.has_key?(:url)
url = config[:url]
config_without_url = config.dup
config_without_url.delete :url

UrlConfig.new(env_name, spec_name, url, config_without_url)
UrlConfig.new(env_name, name, url, config_without_url)
else
HashConfig.new(env_name, spec_name, config)
HashConfig.new(env_name, name, config)
end
end

def merge_db_environment_variables(current_env, configs)
configs.map do |config|
next config if config.is_a?(UrlConfig) || config.env_name != current_env

url_config = environment_url_config(current_env, config.spec_name, config.configuration_hash)
url_config = environment_url_config(current_env, config.name, config.configuration_hash)
url_config || config
end
end

def environment_url_config(env, spec_name, config)
url = environment_value_for(spec_name)
def environment_url_config(env, name, config)
url = environment_value_for(name)
return unless url

UrlConfig.new(env, spec_name, url, config)
UrlConfig.new(env, name, url, config)
end

def environment_value_for(spec_name)
spec_env_key = "#{spec_name.upcase}_DATABASE_URL"
url = ENV[spec_env_key]
url ||= ENV["DATABASE_URL"] if spec_name == "primary"
def environment_value_for(name)
name_env_key = "#{name.upcase}_DATABASE_URL"
url = ENV[name_env_key]
url ||= ENV["DATABASE_URL"] if name == "primary"
url
end

Expand Down
Expand Up @@ -6,12 +6,15 @@ class DatabaseConfigurations
# UrlConfig respectively. It will never return a DatabaseConfig object,
# as this is the parent class for the types of database configuration objects.
class DatabaseConfig # :nodoc:
attr_reader :env_name, :spec_name
attr_reader :env_name, :name, :spec_name
deprecate :spec_name, "spec_name accessors are deprecated and will be removed in Rails 6.2, please use name instead."

attr_accessor :owner_name

def initialize(env_name, spec_name)
def initialize(env_name, name)
@env_name = env_name
@spec_name = spec_name
@name = name
@spec_name = name
end

def config
Expand Down
Expand Up @@ -12,21 +12,21 @@ class DatabaseConfigurations
# Becomes:
#
# #<ActiveRecord::DatabaseConfigurations::HashConfig:0x00007fd1acbded10
# @env_name="development", @spec_name="primary", @config={database: "db_name"}>
# @env_name="development", @name="primary", @config={database: "db_name"}>
#
# ==== Options
#
# * <tt>:env_name</tt> - The Rails environment, i.e. "development".
# * <tt>:spec_name</tt> - The specification name. In a standard two-tier
# * <tt>:name</tt> - The specification name. In a standard two-tier
# database configuration this will default to "primary". In a multiple
# database three-tier database configuration this corresponds to the name
# used in the second tier, for example "primary_readonly".
# * <tt>:config</tt> - The config hash. This is the hash that contains the
# database adapter, name, and other important information for database
# connections.
class HashConfig < DatabaseConfig
def initialize(env_name, spec_name, config)
super(env_name, spec_name)
def initialize(env_name, name, config)
super(env_name, name)
@config = config.symbolize_keys
end

Expand Down
Expand Up @@ -13,14 +13,14 @@ class DatabaseConfigurations
# Becomes:
#
# #<ActiveRecord::DatabaseConfigurations::UrlConfig:0x00007fdc3238f340
# @env_name="default_env", @spec_name="primary",
# @env_name="default_env", @name="primary",
# @config={adapter: "postgresql", database: "foo", host: "localhost"},
# @url="postgres://localhost/foo">
#
# ==== Options
#
# * <tt>:env_name</tt> - The Rails environment, ie "development".
# * <tt>:spec_name</tt> - The specification name. In a standard two-tier
# * <tt>:name</tt> - The specification name. In a standard two-tier
# database configuration this will default to "primary". In a multiple
# database three-tier database configuration this corresponds to the name
# used in the second tier, for example "primary_readonly".
Expand All @@ -31,8 +31,8 @@ class DatabaseConfigurations
class UrlConfig < HashConfig
attr_reader :url

def initialize(env_name, spec_name, url, config = {})
super(env_name, spec_name, config)
def initialize(env_name, name, url, config = {})
super(env_name, name, config)

@url = url
@config.merge!(build_url_hash)
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/railtie.rb
Expand Up @@ -131,7 +131,7 @@ class Railtie < Rails::Railtie # :nodoc:
ActiveSupport.on_load(:active_record) do
db_config = ActiveRecord::Base.configurations.configs_for(
env_name: Rails.env,
spec_name: "primary",
name: "primary",
)
filename = ActiveRecord::Tasks::DatabaseTasks.cache_dump_filename(
"primary",
Expand Down

0 comments on commit 79ce7d9

Please sign in to comment.