From 4e663c1e8d1e643783f157f904b5276e62f65589 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Tue, 20 Mar 2018 13:47:34 -0400 Subject: Refactor configs_for and friends 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. --- activerecord/lib/active_record.rb | 1 + activerecord/lib/active_record/base.rb | 1 + activerecord/lib/active_record/core.rb | 39 -------------- .../lib/active_record/database_configurations.rb | 63 ++++++++++++++++++++++ .../lib/active_record/railties/databases.rake | 32 +++++------ .../lib/active_record/tasks/database_tasks.rb | 9 +++- 6 files changed, 89 insertions(+), 56 deletions(-) create mode 100644 activerecord/lib/active_record/database_configurations.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index d43378c64f..d198466dbf 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -40,6 +40,7 @@ module ActiveRecord autoload :Core autoload :ConnectionHandling autoload :CounterCache + autoload :DatabaseConfigurations autoload :DynamicMatchers autoload :Enum autoload :InternalMetadata diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index cc99401390..7ab9160265 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -290,6 +290,7 @@ module ActiveRecord #:nodoc: extend CollectionCacheKey include Core + include DatabaseConfigurations include Persistence include ReadonlyAttributes include ModelSchema diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index b6b4c76569..e1a0b2ecf8 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -60,45 +60,6 @@ module ActiveRecord @@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 diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb new file mode 100644 index 0000000000..86624a41c9 --- /dev/null +++ b/activerecord/lib/active_record/database_configurations.rb @@ -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 diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index cd0c4616b8..76dbcafffb 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -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 @@ -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 @@ -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 @@ -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 @@ -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) @@ -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 diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index e2c292ee56..5787660148 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -134,6 +134,13 @@ module ActiveRecord 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 @@ -327,7 +334,7 @@ module ActiveRecord 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 -- cgit v1.2.3