Merge pull request rails#43530 from eileencodes/support-setting-schema-path-in-config

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

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