From 2fd997c111b3d2921d011d8023314dbfc2dec317 Mon Sep 17 00:00:00 2001 From: Eileen Uchitelle Date: Thu, 30 Aug 2018 16:49:13 -0400 Subject: Add config option for `replica`. This allows the user to add `replica: true` to the database config to signify the connection should be treated as readonly. This will be useful so we can ignore structure dumps or migrations (or creating / deleting etc) the readonly connection for the databases. These are paired with a write database which is where the create/drop/migrate should be run. This allows us to ask the connection if it's for a replica readonly db or a primary write db. --- .../lib/active_record/connection_adapters/abstract_adapter.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 8999d3232a..a62651daff 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -127,6 +127,10 @@ module ActiveRecord ) end + def replica? + @config[:replica] || false + end + def migrations_paths # :nodoc: @config[:migrations_paths] || Migrator.migrations_paths end -- cgit v1.2.3 From 8737dffce73a37dff073dfe0af8931efd08632c5 Mon Sep 17 00:00:00 2001 From: Eileen Uchitelle Date: Fri, 31 Aug 2018 10:53:20 -0400 Subject: Add replica? check to DatabaseConfig Checks if the config has a "replica" key, if so the configuration is for a replica database. This is useful for excluding replicas from the configurations list when creating the rake tasks or running rake tasks. For example, we don't want to create the primary and primary_readonly. They're the same database. --- .../lib/active_record/database_configurations/database_config.rb | 4 ++++ activerecord/lib/active_record/database_configurations/hash_config.rb | 4 ++++ activerecord/lib/active_record/database_configurations/url_config.rb | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/activerecord/lib/active_record/database_configurations/database_config.rb b/activerecord/lib/active_record/database_configurations/database_config.rb index 4a58115cd5..6250827b34 100644 --- a/activerecord/lib/active_record/database_configurations/database_config.rb +++ b/activerecord/lib/active_record/database_configurations/database_config.rb @@ -13,6 +13,10 @@ module ActiveRecord @spec_name = spec_name end + def replica? + raise NotImplementedError + end + def url_config? false end diff --git a/activerecord/lib/active_record/database_configurations/hash_config.rb b/activerecord/lib/active_record/database_configurations/hash_config.rb index 2ee218c730..249107ebeb 100644 --- a/activerecord/lib/active_record/database_configurations/hash_config.rb +++ b/activerecord/lib/active_record/database_configurations/hash_config.rb @@ -31,6 +31,10 @@ module ActiveRecord super(env_name, spec_name) @config = config end + + def replica? + config["replica"] + end end end end diff --git a/activerecord/lib/active_record/database_configurations/url_config.rb b/activerecord/lib/active_record/database_configurations/url_config.rb index c3d9798c37..181f04c3fd 100644 --- a/activerecord/lib/active_record/database_configurations/url_config.rb +++ b/activerecord/lib/active_record/database_configurations/url_config.rb @@ -41,6 +41,10 @@ module ActiveRecord true end + def replica? + config["replica"] + end + private def build_config(original_config, url) if /^jdbc:/.match?(url) -- cgit v1.2.3 From a572d2283b05c4c05c220ff520732fc570beb2ae Mon Sep 17 00:00:00 2001 From: Eileen Uchitelle Date: Fri, 31 Aug 2018 10:55:37 -0400 Subject: 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. --- .../lib/active_record/database_configurations.rb | 31 +++++++++++++++++----- .../database_configurations/hash_config.rb | 3 +++ .../database_configurations/url_config.rb | 3 +++ .../lib/active_record/railties/databases.rake | 18 ++++++------- .../lib/active_record/tasks/database_tasks.rb | 10 +++---- activerecord/lib/active_record/test_databases.rb | 4 +-- railties/test/application/rake/multi_dbs_test.rb | 17 +++++++----- railties/test/isolation/abstract_unit.rb | 27 +++++++++++++++++++ 8 files changed, 83 insertions(+), 30 deletions(-) diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index 14b7cb040f..2ec69e682a 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -16,17 +16,34 @@ module ActiveRecord 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: + # + # env_name: The environment name. Defaults to nil which will collect + # configs for all environments. + # spec_name: The specification name (ie primary, animals, etc). Defailts + # to nil. + # include_replicas: 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 @@ -157,7 +174,7 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/database_configurations/hash_config.rb b/activerecord/lib/active_record/database_configurations/hash_config.rb index 249107ebeb..55b6ef65c4 100644 --- a/activerecord/lib/active_record/database_configurations/hash_config.rb +++ b/activerecord/lib/active_record/database_configurations/hash_config.rb @@ -32,6 +32,9 @@ module ActiveRecord @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 diff --git a/activerecord/lib/active_record/database_configurations/url_config.rb b/activerecord/lib/active_record/database_configurations/url_config.rb index 181f04c3fd..ad4df3d3f2 100644 --- a/activerecord/lib/active_record/database_configurations/url_config.rb +++ b/activerecord/lib/active_record/database_configurations/url_config.rb @@ -41,6 +41,9 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 15b0459422..be5ef350a9 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -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 @@ -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 @@ -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 @@ -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 @@ -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) @@ -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) @@ -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 @@ -367,7 +367,7 @@ 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 @@ -375,7 +375,7 @@ db_namespace = namespace :db do # 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 diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 3a1791f9c7..16e66982e5 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -117,7 +117,7 @@ module ActiveRecord 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 @@ -143,7 +143,7 @@ module ActiveRecord 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 @@ -208,7 +208,7 @@ module ActiveRecord 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) @@ -217,7 +217,7 @@ module ActiveRecord 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) @@ -351,7 +351,7 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/test_databases.rb b/activerecord/lib/active_record/test_databases.rb index 16113eb04e..999830ba79 100644 --- a/activerecord/lib/active_record/test_databases.rb +++ b/activerecord/lib/active_record/test_databases.rb @@ -15,7 +15,7 @@ module ActiveRecord 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) @@ -28,7 +28,7 @@ module ActiveRecord 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 diff --git a/railties/test/application/rake/multi_dbs_test.rb b/railties/test/application/rake/multi_dbs_test.rb index f86bb9641f..bc6708c89e 100644 --- a/railties/test/application/rake/multi_dbs_test.rb +++ b/railties/test/application/rake/multi_dbs_test.rb @@ -16,21 +16,24 @@ module ApplicationTests 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) @@ -127,35 +130,35 @@ EOS 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 diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 516c457e48..8c5a344ba3 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -124,26 +124,53 @@ module TestHelpers 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 -- cgit v1.2.3