Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Part 3: Multi-db Improvements, identifying replica configurations #33770

Merged
merged 3 commits into from Sep 1, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Convert configs_for to kwargs, add include_replicas
Changes the `configs_for` method from using traditional arguments to
using kwargs. This is so I can add the `include_replicas` kwarg without
having to always include `env_name` and `spec_name` in the method call.

`include_replicas` defaults to false because everywhere internally in
Rails we don't want replicas. `configs_for` is for iterating over
configurations to create / run rake tasks, so we really don't ever need
replicas in that case.
  • Loading branch information
eileencodes committed Aug 31, 2018
commit a572d2283b05c4c05c220ff520732fc570beb2ae
31 changes: 24 additions & 7 deletions activerecord/lib/active_record/database_configurations.rb
Expand Up @@ -16,17 +16,34 @@ def initialize(configurations = {})
end

# Collects the configs for the environment and optionally the specification
# name passed in.
# name passed in. To include replica configurations pass `include_replicas: true`.
#
# If a spec name is provided a single DatabaseConfiguration object will be
# returned, otherwise an array of DatabaseConfiguration objects will be
# returned that corresponds with the environment requested.
def configs_for(env = nil, spec = nil, &blk)
configs = env_with_configs(env)
# 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 (ie primary, animals, etc). Defailts
# to nil.
# <tt>include_replicas:</tt> Determines whether to include replicas in the
# the returned list. Most of the time we're only iterating over the write
# connection (ie 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, &blk)
configs = env_with_configs(env_name)

unless include_replicas
configs = configs.select do |db_config|
!db_config.replica?
end
end

if spec
if spec_name
configs.find do |db_config|
db_config.spec_name == spec
db_config.spec_name == spec_name
end
else
configs
Expand Down Expand Up @@ -157,7 +174,7 @@ def method_missing(method, *args, &blk)
when :each, :first
configurations.send(method, *args, &blk)
when :fetch
configs_for(args.first)
configs_for(env_name: args.first)
when :values
configurations.map(&:config)
else
Expand Down
Expand Up @@ -32,6 +32,9 @@ def initialize(env_name, spec_name, config)
@config = config
end

# Determines whether a database configuration is for a replica / readonly
# connection. If the `replica` key is present in the config, `replica?` will
# return true.
def replica?
config["replica"]
end
Expand Down
Expand Up @@ -41,6 +41,9 @@ def url_config? # :nodoc:
true
end

# Determines whether a database configuration is for a replica / readonly
# connection. If the `replica` key is present in the config, `replica?` will
# return true.
def replica?
config["replica"]
end
Expand Down
18 changes: 9 additions & 9 deletions activerecord/lib/active_record/railties/databases.rake
Expand Up @@ -26,7 +26,7 @@ db_namespace = namespace :db do
ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
desc "Create #{spec_name} database for current environment"
task spec_name => :load_config do
db_config = ActiveRecord::Base.configurations.configs_for(Rails.env, spec_name)
db_config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, spec_name: spec_name)
ActiveRecord::Tasks::DatabaseTasks.create(db_config.config)
end
end
Expand All @@ -45,7 +45,7 @@ db_namespace = namespace :db do
ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
desc "Drop #{spec_name} database for current environment"
task spec_name => [:load_config, :check_protected_environments] do
db_config = ActiveRecord::Base.configurations.configs_for(Rails.env, spec_name)
db_config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, spec_name: spec_name)
ActiveRecord::Tasks::DatabaseTasks.drop(db_config.config)
end
end
Expand Down Expand Up @@ -73,7 +73,7 @@ db_namespace = namespace :db do

desc "Migrate the database (options: VERSION=x, VERBOSE=false, SCOPE=blog)."
task migrate: :load_config do
ActiveRecord::Base.configurations.configs_for(Rails.env).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
ActiveRecord::Base.establish_connection(db_config.config)
ActiveRecord::Tasks::DatabaseTasks.migrate
end
Expand All @@ -99,7 +99,7 @@ db_namespace = namespace :db do
ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
desc "Migrate #{spec_name} database for current environment"
task spec_name => :load_config do
db_config = ActiveRecord::Base.configurations.configs_for(Rails.env, spec_name)
db_config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, spec_name: spec_name)
ActiveRecord::Base.establish_connection(db_config.config)
ActiveRecord::Tasks::DatabaseTasks.migrate
end
Expand Down Expand Up @@ -274,7 +274,7 @@ db_namespace = namespace :db do
desc "Creates a db/schema.rb file that is portable against any DB supported by Active Record"
task dump: :load_config do
require "active_record/schema_dumper"
ActiveRecord::Base.configurations.configs_for(Rails.env).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(db_config.spec_name, :ruby)
File.open(filename, "w:utf-8") do |file|
ActiveRecord::Base.establish_connection(db_config.config)
Expand Down Expand Up @@ -313,7 +313,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.configurations.configs_for(Rails.env).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
ActiveRecord::Base.establish_connection(db_config.config)
filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(db_config.spec_name, :sql)
ActiveRecord::Tasks::DatabaseTasks.structure_dump(db_config.config, filename)
Expand Down Expand Up @@ -354,7 +354,7 @@ db_namespace = namespace :db do
begin
should_reconnect = ActiveRecord::Base.connection_pool.active_connection?
ActiveRecord::Schema.verbose = false
ActiveRecord::Base.configurations.configs_for("test").each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: "test").each do |db_config|
filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(db_config.spec_name, :ruby)
ActiveRecord::Tasks::DatabaseTasks.load_schema(db_config.config, :ruby, filename, "test")
end
Expand All @@ -367,15 +367,15 @@ db_namespace = namespace :db do

# desc "Recreate the test database from an existent structure.sql file"
task load_structure: %w(db:test:purge) do
ActiveRecord::Base.configurations.configs_for("test").each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: "test").each do |db_config|
filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(db_config.spec_name, :sql)
ActiveRecord::Tasks::DatabaseTasks.load_schema(db_config.config, :sql, filename, "test")
end
end

# desc "Empty the test database"
task purge: %w(load_config check_protected_environments) do
ActiveRecord::Base.configurations.configs_for("test").each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: "test").each do |db_config|
ActiveRecord::Tasks::DatabaseTasks.purge(db_config.config)
end
end
Expand Down
10 changes: 5 additions & 5 deletions activerecord/lib/active_record/tasks/database_tasks.rb
Expand Up @@ -117,7 +117,7 @@ def current_config(options = {})
if options.has_key?(:config)
@current_config = options[:config]
else
@current_config ||= ActiveRecord::Base.configurations.configs_for(options[:env], options[:spec]).config
@current_config ||= ActiveRecord::Base.configurations.configs_for(env_name: options[:env], spec_name: options[:spec]).config
end
end

Expand All @@ -143,7 +143,7 @@ def create_all

def for_each
databases = Rails.application.config.database_configuration
database_configs = ActiveRecord::DatabaseConfigurations.new(databases).configs_for(Rails.env)
database_configs = ActiveRecord::DatabaseConfigurations.new(databases).configs_for(env_name: Rails.env)

# if this is a single database application we don't want tasks for each primary database
return if database_configs.count == 1
Expand Down Expand Up @@ -208,7 +208,7 @@ def target_version
end

def charset_current(environment = env, specification_name = spec)
charset ActiveRecord::Base.configurations.configs_for(environment, specification_name).config
charset ActiveRecord::Base.configurations.configs_for(env_name: environment, spec_name: specification_name).config
end

def charset(*arguments)
Expand All @@ -217,7 +217,7 @@ def charset(*arguments)
end

def collation_current(environment = env, specification_name = spec)
collation ActiveRecord::Base.configurations.configs_for(environment, specification_name).config
collation ActiveRecord::Base.configurations.configs_for(env_name: environment, spec_name: specification_name).config
end

def collation(*arguments)
Expand Down Expand Up @@ -351,7 +351,7 @@ def each_current_configuration(environment)
environments << "test" if environment == "development"

environments.each do |env|
ActiveRecord::Base.configurations.configs_for(env).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: env).each do |db_config|
yield db_config.config, db_config.spec_name, env
end
end
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/test_databases.rb
Expand Up @@ -15,7 +15,7 @@ module TestDatabases # :nodoc:
def self.create_and_load_schema(i, env_name:)
old, ENV["VERBOSE"] = ENV["VERBOSE"], "false"

ActiveRecord::Base.configurations.configs_for(env_name).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: env_name).each do |db_config|
db_config.config["database"] += "-#{i}"
ActiveRecord::Tasks::DatabaseTasks.create(db_config.config)
ActiveRecord::Tasks::DatabaseTasks.load_schema(db_config.config, ActiveRecord::Base.schema_format, nil, env_name, db_config.spec_name)
Expand All @@ -28,7 +28,7 @@ def self.create_and_load_schema(i, env_name:)
def self.drop(env_name:)
old, ENV["VERBOSE"] = ENV["VERBOSE"], "false"

ActiveRecord::Base.configurations.configs_for(env_name).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: env_name).each do |db_config|
ActiveRecord::Tasks::DatabaseTasks.drop(db_config.config)
end
ensure
Expand Down
17 changes: 10 additions & 7 deletions railties/test/application/rake/multi_dbs_test.rb
Expand Up @@ -16,21 +16,24 @@ def teardown
teardown_app
end

def db_create_and_drop(namespace, expected_database, environment_loaded: true)
def db_create_and_drop(namespace, expected_database)
Dir.chdir(app_path) do
output = rails("db:create")
assert_match(/Created database/, output)
assert_match_namespace(namespace, output)
assert_no_match(/already exists/, output)
assert File.exist?(expected_database)


output = rails("db:drop")
assert_match(/Dropped database/, output)
assert_match_namespace(namespace, output)
assert_no_match(/does not exist/, output)
assert_not File.exist?(expected_database)
end
end

def db_create_and_drop_namespace(namespace, expected_database, environment_loaded: true)
def db_create_and_drop_namespace(namespace, expected_database)
Dir.chdir(app_path) do
output = rails("db:create:#{namespace}")
assert_match(/Created database/, output)
Expand Down Expand Up @@ -127,35 +130,35 @@ class AnimalsBase < ActiveRecord::Base

test "db:create and db:drop works on all databases for env" do
require "#{app_path}/config/environment"
ActiveRecord::Base.configurations.configs_for(Rails.env).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
db_create_and_drop db_config.spec_name, db_config.config["database"]
end
end

test "db:create:namespace and db:drop:namespace works on specified databases" do
require "#{app_path}/config/environment"
ActiveRecord::Base.configurations.configs_for(Rails.env).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
db_create_and_drop_namespace db_config.spec_name, db_config.config["database"]
end
end

test "db:migrate and db:schema:dump and db:schema:load works on all databases" do
require "#{app_path}/config/environment"
ActiveRecord::Base.configurations.configs_for(Rails.env).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
db_migrate_and_schema_dump_and_load db_config.spec_name, db_config.config["database"], "schema"
end
end

test "db:migrate and db:structure:dump and db:structure:load works on all databases" do
require "#{app_path}/config/environment"
ActiveRecord::Base.configurations.configs_for(Rails.env).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
db_migrate_and_schema_dump_and_load db_config.spec_name, db_config.config["database"], "structure"
end
end

test "db:migrate:namespace works" do
require "#{app_path}/config/environment"
ActiveRecord::Base.configurations.configs_for(Rails.env).each do |db_config|
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
db_migrate_namespaced db_config.spec_name, db_config.config["database"]
end
end
Expand Down
27 changes: 27 additions & 0 deletions railties/test/isolation/abstract_unit.rb
Expand Up @@ -124,26 +124,53 @@ def build_app(options = {})
primary:
<<: *default
database: db/development.sqlite3
primary_readonly:
<<: *default
database: db/development.sqlite3
replica: true
animals:
<<: *default
database: db/development_animals.sqlite3
migrations_paths: db/animals_migrate
animals_readonly:
<<: *default
database: db/development_animals.sqlite3
migrations_paths: db/animals_migrate
replica: true
test:
primary:
<<: *default
database: db/test.sqlite3
primary_readonly:
<<: *default
database: db/test.sqlite3
replica: true
animals:
<<: *default
database: db/test_animals.sqlite3
migrations_paths: db/animals_migrate
animals_readonly:
<<: *default
database: db/test_animals.sqlite3
migrations_paths: db/animals_migrate
replica: true
production:
primary:
<<: *default
database: db/production.sqlite3
primary_readonly:
<<: *default
database: db/production.sqlite3
replica: true
animals:
<<: *default
database: db/production_animals.sqlite3
migrations_paths: db/animals_migrate
animals_readonly:
<<: *default
database: db/production_animals.sqlite3
migrations_paths: db/animals_migrate
readonly: true
YAML
end
else
Expand Down