Skip to content

Commit

Permalink
Refactor configs_for and friends
Browse files Browse the repository at this point in the history
Moves the configs_for and DatabaseConfig struct into it's own file. I
was considering doing this in a future refactoring but our set up forced
me to move it now. You see there are `mattr_accessor`'s on the Core
module that have default settings. For example the `schema_format`
defaults to Ruby. So if I call `configs_for` or any methods in the Core
module it will reset the `schema_format` to `:ruby`. By moving it to
it's own class we can keep the logic contained and avoid this
unfortunate issue.

The second change here does a double loop over the yaml files. Bear with
me...

Our tests dictate that we need to load an environment before our rake
tasks because we could have something in an environment that the
database.yml depends on. There are side-effects to this and I think
there's a deeper bug that needs to be fixed but that's for another
issue. The gist of the problem is when I was creating the dynamic rake
tasks if the yaml that that rake task is calling evaluates code (like
erb) that calls the environment configs the code will blow up because
the environment is not loaded yet.

To avoid this issue we added a new method that simply loads the yaml and
does not evaluate the erb or anything in it. We then use that yaml to
create the task name. Inside the task name we can then call
`load_config` and load the real config to actually call the code
internal to the task. I admit, this is gross, but refactoring can't all
be pretty all the time and I'm working hard with `@tenderlove` to
refactor much more of this code to get to a better place re connection
management and rake tasks.
  • Loading branch information
eileencodes committed Mar 21, 2018
1 parent bb9e554 commit 4e663c1
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 56 deletions.
1 change: 1 addition & 0 deletions activerecord/lib/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ module ActiveRecord
autoload :Core
autoload :ConnectionHandling
autoload :CounterCache
autoload :DatabaseConfigurations
autoload :DynamicMatchers
autoload :Enum
autoload :InternalMetadata
Expand Down
1 change: 1 addition & 0 deletions activerecord/lib/active_record/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ class Base
extend CollectionCacheKey

include Core
include DatabaseConfigurations
include Persistence
include ReadonlyAttributes
include ModelSchema
Expand Down
39 changes: 0 additions & 39 deletions activerecord/lib/active_record/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,45 +60,6 @@ def self.configurations
@@configurations
end

DatabaseConfig = Struct.new(:env_name, :spec_name, :config) # :nodoc

# Given an env, spec and config creates DatabaseConfig structs with
# each attribute set.
def self.walk_configs(env_name, spec_name, config) # :nodoc:
if config["database"] || env_name == "default"
DatabaseConfig.new(env_name, spec_name, config)
else
config.each_pair.map do |spec_name, sub_config|
walk_configs(env_name, spec_name, sub_config)
end
end
end

# Walks all the configs passed in and returns an array
# of DatabaseConfig structs for each configuration.
def self.db_configs(configs = configurations) # :nodoc:
configs.each_pair.flat_map do |env_name, config|
walk_configs(env_name, "primary", config)
end
end

# Collects the configs for the environment passed in.
#
# If a block is given returns the specification name and configuration
# otherwise returns an array of DatabaseConfig structs for the environment.
def self.configs_for(environment, configs = configurations, &blk) # :nodoc:
env_with_configs = db_configs(configs).select do |db_config|
db_config.env_name == environment
end

if block_given?
env_with_configs.each do |env_with_config|
yield env_with_config.spec_name, env_with_config.config
end
else
env_with_configs
end
end
##
# :singleton-method:
# Determines whether to use Time.utc (using :utc) or Time.local (using :local) when pulling
Expand Down
63 changes: 63 additions & 0 deletions activerecord/lib/active_record/database_configurations.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# frozen_string_literal: true

module ActiveRecord
module DatabaseConfigurations # :nodoc:
class DatabaseConfig
attr_reader :env_name, :spec_name, :config

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

# Selects the config for the specified environment and specification name
#
# For example if passed :development, and :animals it will select the database
# under the :development and :animals configuration level
def self.config_for_env_and_spec(environment, specification_name, configs = ActiveRecord::Base.configurations) # :nodoc:
configs_for(environment, configs).find do |db_config|
db_config.spec_name == specification_name
end
end

# Collects the configs for the environment passed in.
#
# If a block is given returns the specification name and configuration
# otherwise returns an array of DatabaseConfig structs for the environment.
def self.configs_for(env, configs = ActiveRecord::Base.configurations, &blk) # :nodoc:
env_with_configs = db_configs(configs).select do |db_config|
db_config.env_name == env
end

if block_given?
env_with_configs.each do |env_with_config|
yield env_with_config.spec_name, env_with_config.config
end
else
env_with_configs
end
end

# Given an env, spec and config creates DatabaseConfig structs with
# each attribute set.
def self.walk_configs(env_name, spec_name, config) # :nodoc:
if config["database"] || env_name == "default"
DatabaseConfig.new(env_name, spec_name, config)
else
config.each_pair.map do |spec_name, sub_config|
walk_configs(env_name, spec_name, sub_config)
end
end
end

# Walks all the configs passed in and returns an array
# of DatabaseConfig structs for each configuration.
def self.db_configs(configs = ActiveRecord::Base.configurations) # :nodoc:
configs.each_pair.flat_map do |env_name, config|
walk_configs(env_name, "primary", config)
end
end
end
end
32 changes: 16 additions & 16 deletions activerecord/lib/active_record/railties/databases.rake
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ db_namespace = namespace :db do
ActiveRecord::Tasks::DatabaseTasks.create_all
end

databases = Rails.application.config.database_configuration
ActiveRecord::Base.configs_for(Rails.env, databases) do |spec_name, config|
ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
desc "Create #{spec_name} database for current environment"
task spec_name do
ActiveRecord::Tasks::DatabaseTasks.create(config)
task spec_name => :load_config do
db_config = ActiveRecord::DatabaseConfigurations.config_for_env_and_spec(Rails.env, spec_name)
ActiveRecord::Tasks::DatabaseTasks.create(db_config.config)
end
end
end
Expand All @@ -42,11 +42,11 @@ db_namespace = namespace :db do
ActiveRecord::Tasks::DatabaseTasks.drop_all
end

databases = Rails.application.config.database_configuration
ActiveRecord::Base.configs_for(Rails.env, databases) do |spec_name, config|
ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
desc "Drop #{spec_name} database for current environment"
task spec_name => :check_protected_environments do
ActiveRecord::Tasks::DatabaseTasks.drop(config)
task spec_name => [:load_config, :check_protected_environments] do
db_config = ActiveRecord::DatabaseConfigurations.config_for_env_and_spec(Rails.env, spec_name)
ActiveRecord::Tasks::DatabaseTasks.drop(db_config.config)
end
end
end
Expand All @@ -73,11 +73,11 @@ db_namespace = namespace :db do

desc "Migrate the database (options: VERSION=x, VERBOSE=false, SCOPE=blog)."
task migrate: :load_config do
ActiveRecord::Base.configs_for(Rails.env) do |spec_name, config|
ActiveRecord::DatabaseConfigurations.configs_for(Rails.env) do |spec_name, config|
ActiveRecord::Base.establish_connection(config)
ActiveRecord::Tasks::DatabaseTasks.migrate
db_namespace["_dump"].invoke
end
db_namespace["_dump"].invoke
end

# IMPORTANT: This task won't dump the schema if ActiveRecord::Base.dump_schema_after_migration is set to false
Expand All @@ -96,11 +96,11 @@ db_namespace = namespace :db do
end

namespace :migrate do
databases = Rails.application.config.database_configuration
ActiveRecord::Base.configs_for(Rails.env, databases) do |spec_name, config|
ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
desc "Migrate #{spec_name} database for current environment"
task spec_name do
ActiveRecord::Base.establish_connection(config)
task spec_name => :load_config do
db_config = ActiveRecord::DatabaseConfigurations.config_for_env_and_spec(Rails.env, spec_name)
ActiveRecord::Base.establish_connection(db_config.config)
ActiveRecord::Tasks::DatabaseTasks.migrate
end
end
Expand Down Expand Up @@ -275,7 +275,7 @@ db_namespace = namespace :db do
task dump: :load_config do
require "active_record/schema_dumper"

ActiveRecord::Base.configs_for(Rails.env) do |spec_name, config|
ActiveRecord::DatabaseConfigurations.configs_for(Rails.env) do |spec_name, config|
filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(spec_name, :ruby)
File.open(filename, "w:utf-8") do |file|
ActiveRecord::Base.establish_connection(config)
Expand Down Expand Up @@ -314,7 +314,7 @@ db_namespace = namespace :db do
namespace :structure do
desc "Dumps the database structure to db/structure.sql. Specify another file with SCHEMA=db/my_structure.sql"
task dump: :load_config do
ActiveRecord::Base.configs_for(Rails.env) do |spec_name, config|
ActiveRecord::DatabaseConfigurations.configs_for(Rails.env) do |spec_name, config|
ActiveRecord::Base.establish_connection(config)
filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(spec_name, :sql)
current_config = ActiveRecord::Tasks::DatabaseTasks.current_config
Expand Down
9 changes: 8 additions & 1 deletion activerecord/lib/active_record/tasks/database_tasks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@ def create_all
end
end

def for_each
databases = Rails.application.config.load_database_yaml
ActiveRecord::DatabaseConfigurations.configs_for(Rails.env, databases) do |spec_name, _|
yield spec_name
end
end

def create_current(environment = env)
each_current_configuration(environment) { |configuration|
create configuration
Expand Down Expand Up @@ -327,7 +334,7 @@ def each_current_configuration(environment)
environments << "test" if environment == "development"

environments.each do |env|
ActiveRecord::Base.configs_for(env) do |spec_name, configuration|
ActiveRecord::DatabaseConfigurations.configs_for(env) do |spec_name, configuration|
yield configuration, spec_name, env
end
end
Expand Down
12 changes: 12 additions & 0 deletions railties/lib/rails/application/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,18 @@ def paths
end
end

# Loads the database YAML without evaluating ERB. People seem to
# write ERB that makes the database configuration depend on
# Rails configuration. But we want Rails configuration (specifically
# `rake` and `rails` tasks) to be generated based on information in
# the database yaml, so we need a method that loads the database
# yaml *without* the context of the Rails application.
def load_database_yaml # :nodoc:
path = paths["config/database"].existent.first
return {} unless path
YAML.load_file(path.to_s)
end

# Loads and returns the entire raw configuration of database from
# values stored in <tt>config/database.yml</tt>.
def database_configuration
Expand Down

0 comments on commit 4e663c1

Please sign in to comment.