Merge pull request #33770 from eileencodes/multi-db-improvements-part-3

Part 3: Multi-db Improvements, identifying replica configurations

10 files changed, 99 insertions, 30 deletions

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
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:
+    #
+    # <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
@@ -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/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..55b6ef65c4 100644
--- a/activerecord/lib/active_record/database_configurations/hash_config.rb
+++ b/activerecord/lib/active_record/database_configurations/hash_config.rb
@@ -31,6 +31,13 @@ module ActiveRecord
         super(env_name, spec_name)
         @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
     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..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,13 @@ 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
+
       private
         def build_config(original_config, url)
           if /^jdbc:/.match?(url)
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 0540311714..96d21b3ae2 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