Add support for setting the schema/structure dump filepath in the config

Previously Rails would generate your schema/structure dump based on the
database config name. As noted on the forum in
https://discuss.rubyonrails.org/t/horizontal-sharding-schema-management/78621/6
this is problematic if you want to your shards to share the same schema
dump file.

This is also useful for applications that might have already written a
custom solution for setting the schema dump filename and want to use
something upstream. We allow setting the path for the schema cache, why
not the schema dump too?

To do this I deprecated the old schema_file database task in favor for a
new one that passes the `db_config`. The code to derive the filename
from the db_config has been duplicated there and will be the correct way
to determine filename in the future. Since these values are set on the
config they should be accessible in the config as well.

I did not need to deprecate the behavior of schema_dump because it has
not been included in any release yet (other than the alpha). However the
behavior is the same for false/nil.

Closes rails#43173
Supercedes rails#43240

Co-authored-by: Ryan Kerr <[email protected]>

Author: eileencodes

activerecord/CHANGELOG.md

@@ -1,3 +1,22 @@
+*   Add support for setting the filename of the schema or structure dump in the database config.
+
+    Applications may now set their the filename or path of the schema / structure dump file in their database configuration.
+
+
+    ```yaml
+    production:
+      primary:
+        database: my_db
+        schema_dump: my_schema_dump_filename.rb
+      animals:
+        database: animals_db
+        schema_dump: false
+    ```
+
+    The filename set in `schema_dump` will be used by the application. If set to `false` the schema will not be dumped. The database tasks are responsible for adding the database directory to the filename. If a full path is provided, the Rails tasks will use that instead of `ActiveRecord::DatabaseTasks.db_dir`.
+
+    *Eileen M. Uchitelle*, *Ryan Kerr*
+
 *   Add `ActiveRecord::Base.prohibit_shard_swapping` to prevent attempts to change the shard within a block.
 
     *John Crepezzi*, *Eileen M. Uchitelle*

activerecord/lib/active_record/database_configurations/hash_config.rb

@@ -121,14 +121,39 @@ def primary? # :nodoc:
         Base.configurations.primary?(name)
       end
 
-      # Determines whether to dump the schema for a database.
-      def schema_dump
-        configuration_hash.fetch(:schema_dump, true)
+      # Determines whether to dump the schema/structure files and the
+      # filename that should be used.
+      #
+      # If +configuration_hash[:schema_dump]+ is set to +false+ or +nil+
+      # the schema will not be dumped.
+      #
+      # If the config option is set that will be used. Otherwise Rails
+      # will generate the filename from the database config name.
+      def schema_dump(format = ActiveRecord.schema_format)
+        if configuration_hash.key?(:schema_dump)
+          if config = configuration_hash[:schema_dump]
+            config
+          end
+        elsif primary?
+          schema_file_type(format)
+        else
+          "#{name}_#{schema_file_type(format)}"
+        end
       end
 
       def database_tasks? # :nodoc:
         !replica? && !!configuration_hash.fetch(:database_tasks, true)
       end
+
+      private
+        def schema_file_type(format)
+          case format
+          when :ruby
+            "schema.rb"
+          when :sql
+            "structure.sql"
+          end
+        end
     end
   end
 end

activerecord/lib/active_record/tasks/database_tasks.rb

@@ -216,10 +216,9 @@ def prepare_all
             dump_schema(db_config, ActiveRecord.schema_format)
           end
         rescue ActiveRecord::NoDatabaseError
-          config_name = db_config.name
-          create_current(db_config.env_name, config_name)
+          create_current(db_config.env_name, db_config.name)
 
-          if File.exist?(dump_filename(config_name))
+          if File.exist?(schema_dump_path(db_config))
             load_schema(
               db_config,
               ActiveRecord.schema_format,
@@ -378,7 +377,7 @@ def structure_load(configuration, *arguments)
       end
 
       def load_schema(db_config, format = ActiveRecord.schema_format, file = nil) # :nodoc:
-        file ||= dump_filename(db_config.name, format)
+        file ||= schema_dump_path(db_config, format)
 
         verbose_was, Migration.verbose = Migration.verbose, verbose? && ENV["VERBOSE"]
         check_schema_file(file)
@@ -406,9 +405,7 @@ def schema_up_to_date?(configuration, format = ActiveRecord.schema_format, file
           ActiveSupport::Deprecation.warn("`environment` and `name` will be removed as parameters in 7.0.0, you may now pass an ActiveRecord::DatabaseConfigurations::DatabaseConfig as `configuration` instead.")
         end
 
-        name ||= db_config.name
-
-        file ||= dump_filename(name, format)
+        file ||= schema_dump_path(db_config)
 
         return true unless File.exist?(file)
 
@@ -421,7 +418,7 @@ def schema_up_to_date?(configuration, format = ActiveRecord.schema_format, file
       end
 
       def reconstruct_from_schema(db_config, format = ActiveRecord.schema_format, file = nil) # :nodoc:
-        file ||= dump_filename(db_config.name, format)
+        file ||= schema_dump_path(db_config, format)
 
         check_schema_file(file)
 
@@ -440,7 +437,7 @@ def reconstruct_from_schema(db_config, format = ActiveRecord.schema_format, file
 
       def dump_schema(db_config, format = ActiveRecord.schema_format) # :nodoc:
         require "active_record/schema_dumper"
-        filename = dump_filename(db_config.name, format)
+        filename = schema_dump_path(db_config, format)
         connection = ActiveRecord::Base.connection
 
         FileUtils.mkdir_p(db_dir)
@@ -475,6 +472,8 @@ def schema_file_type(format = ActiveRecord.schema_format)
       end
 
       def dump_filename(db_config_name, format = ActiveRecord.schema_format)
+        ActiveSupport::Deprecation.warn("#dump_filename is deprecated. Please call `schema_dump_path` or call `schema_dump` on the `db_config` directly.")
+
         filename = if ActiveRecord::Base.configurations.primary?(db_config_name)
           schema_file_type(format)
         else
@@ -484,6 +483,19 @@ def dump_filename(db_config_name, format = ActiveRecord.schema_format)
         ENV["SCHEMA"] || File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, filename)
       end
 
+      def schema_dump_path(db_config, format = ActiveRecord.schema_format)
+        return ENV["SCHEMA"] if ENV["SCHEMA"]
+
+        filename = db_config.schema_dump(format)
+        return unless filename
+
+        if File.dirname(filename) == ActiveRecord::Tasks::DatabaseTasks.db_dir
+          filename
+        else
+          File.join(ActiveRecord::Tasks::DatabaseTasks.db_dir, filename)
+        end
+      end
+
       def cache_dump_filename(db_config_name, schema_cache_path: nil)
         filename = if ActiveRecord::Base.configurations.primary?(db_config_name)
           "schema_cache.yml"

activerecord/test/cases/database_configurations/hash_config_test.rb

@@ -105,12 +105,12 @@ def test_idle_timeout_nil_when_less_than_or_equal_to_zero
 
       def test_default_schema_dump_value
         config = HashConfig.new("default_env", "primary", {})
-        assert_equal true, config.schema_dump
+        assert_equal "schema.rb", config.schema_dump
       end
 
-      def test_schema_dump_value_set_to_true
-        config = HashConfig.new("default_env", "primary", { schema_dump: true })
-        assert_equal true, config.schema_dump
+      def test_schema_dump_value_set_to_filename
+        config = HashConfig.new("default_env", "primary", { schema_dump: "my_schema.rb" })
+        assert_equal "my_schema.rb", config.schema_dump
       end
 
       def test_schema_dump_value_set_to_nil
@@ -120,7 +120,7 @@ def test_schema_dump_value_set_to_nil
 
       def test_schema_dump_value_set_to_false
         config = HashConfig.new("default_env", "primary", { schema_dump: false })
-        assert_equal false, config.schema_dump
+        assert_nil config.schema_dump
       end
 
       def test_database_tasks_defaults_to_true

activerecord/test/cases/tasks/database_tasks_test.rb

@@ -298,7 +298,27 @@ class DatabaseTasksDumpSchemaTest < ActiveRecord::TestCase
     def test_ensure_db_dir
       Dir.mktmpdir do |dir|
         ActiveRecord::Tasks::DatabaseTasks.stub(:db_dir, dir) do
-          db_config = OpenStruct.new(name: "fake_db_config")
+          updated_hash = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary").configuration_hash.merge(schema_dump: "fake_db_config_schema.rb")
+          db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", updated_hash)
+          path = "#{dir}/fake_db_config_schema.rb"
+
+          FileUtils.rm_rf(dir)
+          assert_not File.file?(path)
+
+          ActiveRecord::Tasks::DatabaseTasks.dump_schema(db_config)
+
+          assert File.file?(path)
+        end
+      end
+    ensure
+      ActiveRecord::Base.clear_cache!
+    end
+
+    def test_db_dir_ignored_if_included_in_schema_dump
+      Dir.mktmpdir do |dir|
+        ActiveRecord::Tasks::DatabaseTasks.stub(:db_dir, dir) do
+          updated_hash = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary").configuration_hash.merge(schema_dump: "#{dir}/fake_db_config_schema.rb")
+          db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", updated_hash)
           path = "#{dir}/fake_db_config_schema.rb"
 
           FileUtils.rm_rf(dir)
@@ -1598,7 +1618,17 @@ def test_check_schema_file_defaults
     def test_check_dump_filename_defaults
       ActiveRecord::Tasks::DatabaseTasks.stub(:db_dir, "/tmp") do
         with_stubbed_configurations do
-          assert_equal "/tmp/schema.rb", ActiveRecord::Tasks::DatabaseTasks.dump_filename(config_for("development", "primary").name)
+          assert_equal "/tmp/schema.rb", ActiveRecord::Tasks::DatabaseTasks.schema_dump_path(config_for("development", "primary"))
+        end
+      end
+    end
+
+    def test_dump_filename_is_deprecated
+      ActiveRecord::Tasks::DatabaseTasks.stub(:db_dir, "/tmp") do
+        with_stubbed_configurations do
+          assert_deprecated do
+            assert_equal "/tmp/schema.rb", ActiveRecord::Tasks::DatabaseTasks.dump_filename(config_for("development", "primary").name)
+          end
         end
       end
     end
@@ -1608,7 +1638,7 @@ def test_check_dump_filename_with_schema_env
       ENV["SCHEMA"] = "schema_path"
       ActiveRecord::Tasks::DatabaseTasks.stub(:db_dir, "/tmp") do
         with_stubbed_configurations do
-          assert_equal "schema_path", ActiveRecord::Tasks::DatabaseTasks.dump_filename(config_for("development", "primary").name)
+          assert_equal "schema_path", ActiveRecord::Tasks::DatabaseTasks.schema_dump_path(config_for("development", "primary"))
         end
       end
     ensure
@@ -1619,7 +1649,7 @@ def test_check_dump_filename_with_schema_env
       define_method("test_check_dump_filename_for_#{fmt}_format") do
         ActiveRecord::Tasks::DatabaseTasks.stub(:db_dir, "/tmp") do
           with_stubbed_configurations do
-            assert_equal "/tmp/#{filename}", ActiveRecord::Tasks::DatabaseTasks.dump_filename(config_for("development", "primary").name, fmt)
+            assert_equal "/tmp/#{filename}", ActiveRecord::Tasks::DatabaseTasks.schema_dump_path(config_for("development", "primary"), fmt)
           end
         end
       end
@@ -1631,7 +1661,18 @@ def test_check_dump_filename_defaults_for_non_primary_databases
           "development" => { "primary" => { "database" => "dev-db" }, "secondary" => { "database" => "secondary-dev-db" } },
         }
         with_stubbed_configurations(configurations) do
-          assert_equal "/tmp/secondary_schema.rb", ActiveRecord::Tasks::DatabaseTasks.dump_filename(config_for("development", "secondary").name)
+          assert_equal "/tmp/secondary_schema.rb", ActiveRecord::Tasks::DatabaseTasks.schema_dump_path(config_for("development", "secondary"))
+        end
+      end
+    end
+
+    def test_setting_schema_dump_to_nil
+      ActiveRecord::Tasks::DatabaseTasks.stub(:db_dir, "/tmp") do
+        configurations = {
+          "development" => { "primary" => { "database" => "dev-db", "schema_dump" => false } },
+        }
+        with_stubbed_configurations(configurations) do
+          assert_nil ActiveRecord::Tasks::DatabaseTasks.schema_dump_path(config_for("development", "primary"))
         end
       end
     end
@@ -1644,7 +1685,7 @@ def test_check_dump_filename_with_schema_env_with_non_primary_databases
           "development" => { "primary" => { "database" => "dev-db" }, "secondary" => { "database" => "secondary-dev-db" } },
         }
         with_stubbed_configurations(configurations) do
-          assert_equal "schema_path", ActiveRecord::Tasks::DatabaseTasks.dump_filename(config_for("development", "secondary").name)
+          assert_equal "schema_path", ActiveRecord::Tasks::DatabaseTasks.schema_dump_path(config_for("development", "secondary"))
         end
       end
     ensure
@@ -1658,7 +1699,7 @@ def test_check_dump_filename_with_schema_env_with_non_primary_databases
             "development" => { "primary" => { "database" => "dev-db" }, "secondary" => { "database" => "secondary-dev-db" } },
           }
           with_stubbed_configurations(configurations) do
-            assert_equal "/tmp/secondary_#{filename}", ActiveRecord::Tasks::DatabaseTasks.dump_filename(config_for("development", "secondary").name, fmt)
+            assert_equal "/tmp/secondary_#{filename}", ActiveRecord::Tasks::DatabaseTasks.schema_dump_path(config_for("development", "secondary"), fmt)
           end
         end
       end

railties/test/application/rake/migrations_test.rb

@@ -425,21 +425,21 @@ class TwoMigration < ActiveRecord::Migration::Current
         assert_match(/up\s+002\s+Two migration/, output)
       end
 
-      test "schema generation when dump_schema_after_migration and schema_dump are true" do
+      test "schema generation when dump_schema_after_migration and schema_dump are set" do
         add_to_config("config.active_record.dump_schema_after_migration = true")
 
         app_file "config/database.yml", <<~EOS
           development:
             adapter: sqlite3
             database: 'dev_db'
-            schema_dump: true
+            schema_dump: "schema_file.rb"
         EOS
 
         Dir.chdir(app_path) do
           rails "generate", "model", "book", "title:string"
           rails "db:migrate"
 
-          assert File.exist?("db/schema.rb"), "should dump schema when configured to"
+          assert File.exist?("db/schema_file.rb"), "should dump schema when configured to"
         end
       end