Make DatabaseTasks adapters use DatabaseConfig objects

We are moving to using `DatabaseConfig` objects everywhere inside of
Rails instead of passing around Hash objects.

This PR moves to using `DatabaseConfig` objects inside of the individual
database tasks adapters. In the interest of not breaking existing
adapters, we've introduced an upgrade path by which adapters can get a
hold of database configuration objects instead of Hashes by implementing
a method `self.using_database_configurations?`
Co-authored-by: Neileencodes <eileencodes@gmail.com>

activerecord/lib/active_record/tasks/database_tasks.rb

@@ -125,7 +125,7 @@ def current_config(options = {})
 
       def create(configuration, *arguments)
         db_config = resolve_configuration(configuration)
-        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).create
+        database_adapter_for(db_config, *arguments).create
         $stdout.puts "Created database '#{db_config.database}'" if verbose?
       rescue DatabaseAlreadyExists
         $stderr.puts "Database '#{db_config.database}' already exists" if verbose?
@@ -189,7 +189,7 @@ def create_current(environment = env, spec_name = nil)
 
       def drop(configuration, *arguments)
         db_config = resolve_configuration(configuration)
-        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).drop
+        database_adapter_for(db_config, *arguments).drop
         $stdout.puts "Dropped database '#{db_config.database}'" if verbose?
       rescue ActiveRecord::NoDatabaseError
         $stderr.puts "Database '#{db_config.database}' does not exist"
@@ -274,7 +274,7 @@ def charset_current(env_name = env, spec_name = spec)
 
       def charset(configuration, *arguments)
         db_config = resolve_configuration(configuration)
-        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).charset
+        database_adapter_for(db_config, *arguments).charset
       end
 
       def collation_current(env_name = env, spec_name = spec)
@@ -284,12 +284,12 @@ def collation_current(env_name = env, spec_name = spec)
 
       def collation(configuration, *arguments)
         db_config = resolve_configuration(configuration)
-        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).collation
+        database_adapter_for(db_config, *arguments).collation
       end
 
       def purge(configuration)
         db_config = resolve_configuration(configuration)
-        class_for_adapter(db_config.adapter).new(db_config.configuration_hash).purge
+        database_adapter_for(db_config).purge
       end
 
       def purge_all
@@ -304,13 +304,13 @@ def purge_current(environment = env)
       def structure_dump(configuration, *arguments)
         db_config = resolve_configuration(configuration)
         filename = arguments.delete_at(0)
-        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).structure_dump(filename, structure_dump_flags)
+        database_adapter_for(db_config, *arguments).structure_dump(filename, structure_dump_flags)
       end
 
       def structure_load(configuration, *arguments)
         db_config = resolve_configuration(configuration)
         filename = arguments.delete_at(0)
-        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).structure_load(filename, structure_load_flags)
+        database_adapter_for(db_config, *arguments).structure_load(filename, structure_load_flags)
       end
 
       def load_schema(db_config, format = ActiveRecord::Base.schema_format, file = nil) # :nodoc:
@@ -470,6 +470,17 @@ def verbose?
           ENV["VERBOSE"] ? ENV["VERBOSE"] != "false" : true
         end
 
+        # Create a new instance for the specified db configuration object
+        # For classes that have been converted to use db_config objects, pass a
+        # `DatabaseConfig`, otherwise pass a `Hash`
+        def database_adapter_for(db_config, *arguments)
+          klass = class_for_adapter(db_config.adapter)
+          converted = klass.respond_to?(:using_database_configurations?) && klass.using_database_configurations?
+
+          config = converted ? db_config : db_config.configuration_hash
+          klass.new(config, *arguments)
+        end
+
         def class_for_adapter(adapter)
           _key, task = @tasks.each_pair.detect { |pattern, _task| adapter[pattern] }
           unless task

activerecord/lib/active_record/tasks/mysql_database_tasks.rb

@@ -7,24 +7,29 @@ class MySQLDatabaseTasks # :nodoc:
 
       delegate :connection, :establish_connection, to: ActiveRecord::Base
 
-      def initialize(configuration)
-        @configuration = configuration
+      def self.using_database_configurations?
+        true
+      end
+
+      def initialize(db_config)
+        @db_config = db_config
+        @configuration_hash = db_config.configuration_hash
       end
 
       def create
-        establish_connection configuration_without_database
-        connection.create_database configuration[:database], creation_options
-        establish_connection configuration
+        establish_connection(configuration_hash_without_database)
+        connection.create_database(db_config.database, creation_options)
+        establish_connection(db_config)
       end
 
       def drop
-        establish_connection configuration
-        connection.drop_database configuration[:database]
+        establish_connection(db_config)
+        connection.drop_database(db_config.database)
       end
 
       def purge
-        establish_connection configuration
-        connection.recreate_database configuration[:database], creation_options
+        establish_connection(db_config)
+        connection.recreate_database(db_config.database, creation_options)
       end
 
       def charset
@@ -44,10 +49,10 @@ def structure_dump(filename, extra_flags)
 
         ignore_tables = ActiveRecord::SchemaDumper.ignore_tables
         if ignore_tables.any?
-          args += ignore_tables.map { |table| "--ignore-table=#{configuration[:database]}.#{table}" }
+          args += ignore_tables.map { |table| "--ignore-table=#{db_config.database}.#{table}" }
         end
 
-        args.concat(["#{configuration[:database]}"])
+        args.concat([db_config.database.to_s])
         args.unshift(*extra_flags) if extra_flags
 
         run_cmd("mysqldump", args, "dumping")
@@ -56,23 +61,23 @@ def structure_dump(filename, extra_flags)
       def structure_load(filename, extra_flags)
         args = prepare_command_options
         args.concat(["--execute", %{SET FOREIGN_KEY_CHECKS = 0; SOURCE #{filename}; SET FOREIGN_KEY_CHECKS = 1}])
-        args.concat(["--database", "#{configuration[:database]}"])
+        args.concat(["--database", db_config.database.to_s])
         args.unshift(*extra_flags) if extra_flags
 
         run_cmd("mysql", args, "loading")
       end
 
       private
-        attr_reader :configuration
+        attr_reader :db_config, :configuration_hash
 
-        def configuration_without_database
-          configuration.merge(database: nil)
+        def configuration_hash_without_database
+          configuration_hash.merge(database: nil)
         end
 
         def creation_options
           Hash.new.tap do |options|
-            options[:charset]     = configuration[:encoding]   if configuration.include? :encoding
-            options[:collation]   = configuration[:collation]  if configuration.include? :collation
+            options[:charset]     = configuration_hash[:encoding]   if configuration_hash.include?(:encoding)
+            options[:collation]   = configuration_hash[:collation]  if configuration_hash.include?(:collation)
           end
         end
 
@@ -89,7 +94,7 @@ def prepare_command_options
             sslcapath: "--ssl-capath",
             sslcipher: "--ssl-cipher",
             sslkey:    "--ssl-key"
-          }.map { |opt, arg| "#{arg}=#{configuration[opt]}" if configuration[opt] }.compact
+          }.map { |opt, arg| "#{arg}=#{configuration_hash[opt]}" if configuration_hash[opt] }.compact
 
           args
         end

activerecord/lib/active_record/tasks/postgresql_database_tasks.rb

@@ -12,20 +12,24 @@ class PostgreSQLDatabaseTasks # :nodoc:
       delegate :connection, :establish_connection, :clear_active_connections!,
         to: ActiveRecord::Base
 
-      def initialize(configuration)
-        @configuration = configuration
+      def self.using_database_configurations?
+        true
+      end
+
+      def initialize(db_config)
+        @db_config = db_config
+        @configuration_hash = db_config.configuration_hash
       end
 
       def create(master_established = false)
         establish_master_connection unless master_established
-        connection.create_database configuration[:database],
-          configuration.merge(encoding: encoding)
-        establish_connection configuration
+        connection.create_database(db_config.database, configuration_hash.merge(encoding: encoding))
+        establish_connection(db_config)
       end
 
       def drop
         establish_master_connection
-        connection.drop_database configuration[:database]
+        connection.drop_database(db_config.database)
       end
 
       def charset
@@ -48,7 +52,7 @@ def structure_dump(filename, extra_flags)
         search_path = \
           case ActiveRecord::Base.dump_schemas
           when :schema_search_path
-            configuration[:schema_search_path]
+            configuration_hash[:schema_search_path]
           when :all
             nil
           when String
@@ -68,7 +72,7 @@ def structure_dump(filename, extra_flags)
           args += ignore_tables.flat_map { |table| ["-T", table] }
         end
 
-        args << configuration[:database]
+        args << db_config.database
         run_cmd("pg_dump", args, "dumping")
         remove_sql_header_comments(filename)
         File.open(filename, "a") { |f| f << "SET search_path TO #{connection.schema_search_path};\n\n" }
@@ -78,29 +82,29 @@ def structure_load(filename, extra_flags)
         set_psql_env
         args = ["-v", ON_ERROR_STOP_1, "-q", "-X", "-f", filename]
         args.concat(Array(extra_flags)) if extra_flags
-        args << configuration[:database]
+        args << db_config.database
         run_cmd("psql", args, "loading")
       end
 
       private
-        attr_reader :configuration
+        attr_reader :db_config, :configuration_hash
 
         def encoding
-          configuration[:encoding] || DEFAULT_ENCODING
+          configuration_hash[:encoding] || DEFAULT_ENCODING
         end
 
         def establish_master_connection
-          establish_connection configuration.merge(
+          establish_connection configuration_hash.merge(
             database: "postgres",
             schema_search_path: "public"
           )
         end
 
         def set_psql_env
-          ENV["PGHOST"]     = configuration[:host]          if configuration[:host]
-          ENV["PGPORT"]     = configuration[:port].to_s     if configuration[:port]
-          ENV["PGPASSWORD"] = configuration[:password].to_s if configuration[:password]
-          ENV["PGUSER"]     = configuration[:username].to_s if configuration[:username]
+          ENV["PGHOST"]     = configuration_hash[:host]          if configuration_hash[:host]
+          ENV["PGPORT"]     = configuration_hash[:port].to_s     if configuration_hash[:port]
+          ENV["PGPASSWORD"] = configuration_hash[:password].to_s if configuration_hash[:password]
+          ENV["PGUSER"]     = configuration_hash[:username].to_s if configuration_hash[:username]
         end
 
         def run_cmd(cmd, args, action)

activerecord/lib/active_record/tasks/sqlite_database_tasks.rb

@@ -5,20 +5,25 @@ module Tasks # :nodoc:
     class SQLiteDatabaseTasks # :nodoc:
       delegate :connection, :establish_connection, to: ActiveRecord::Base
 
-      def initialize(configuration, root = ActiveRecord::Tasks::DatabaseTasks.root)
-        @configuration, @root = configuration, root
+      def self.using_database_configurations?
+        true
+      end
+
+      def initialize(db_config, root = ActiveRecord::Tasks::DatabaseTasks.root)
+        @db_config = db_config
+        @root = root
       end
 
       def create
-        raise DatabaseAlreadyExists if File.exist?(configuration[:database])
+        raise DatabaseAlreadyExists if File.exist?(db_config.database)
 
-        establish_connection configuration
+        establish_connection(db_config)
         connection
       end
 
       def drop
         require "pathname"
-        path = Pathname.new configuration[:database]
+        path = Pathname.new(db_config.database)
         file = path.absolute? ? path.to_s : File.join(root, path)
 
         FileUtils.rm(file)
@@ -40,7 +45,7 @@ def charset
       def structure_dump(filename, extra_flags)
         args = []
         args.concat(Array(extra_flags)) if extra_flags
-        args << configuration[:database]
+        args << db_config.database
 
         ignore_tables = ActiveRecord::SchemaDumper.ignore_tables
         if ignore_tables.any?
@@ -53,13 +58,12 @@ def structure_dump(filename, extra_flags)
       end
 
       def structure_load(filename, extra_flags)
-        dbfile = configuration[:database]
         flags = extra_flags.join(" ") if extra_flags
-        `sqlite3 #{flags} #{dbfile} < "#{filename}"`
+        `sqlite3 #{flags} #{db_config.database} < "#{filename}"`
       end
 
       private
-        attr_reader :configuration, :root
+        attr_reader :db_config, :root
 
         def run_cmd(cmd, args, out)
           fail run_cmd_error(cmd, args) unless Kernel.system(cmd, *args, out: out)

activerecord/test/cases/tasks/mysql_rake_test.rb

@@ -24,16 +24,18 @@ def teardown
       end
 
       def test_establishes_connection_without_database
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("default_env", "primary", @configuration)
+
         ActiveRecord::Base.stub(:connection, @connection) do
           assert_called_with(
             ActiveRecord::Base,
             :establish_connection,
             [
-              [ adapter: "mysql2", database: nil ],
-              [ adapter: "mysql2", database: "my-app-db" ],
+              [adapter: "mysql2", database: nil],
+              [db_config]
             ]
           ) do
-            ActiveRecord::Tasks::DatabaseTasks.create @configuration
+            ActiveRecord::Tasks::DatabaseTasks.create(db_config)
           end
         end
       end
@@ -67,16 +69,18 @@ def test_creates_database_with_given_collation
       end
 
       def test_establishes_connection_to_database
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("default_env", "primary", @configuration)
+
         ActiveRecord::Base.stub(:connection, @connection) do
           assert_called_with(
             ActiveRecord::Base,
             :establish_connection,
             [
               [adapter: "mysql2", database: nil],
-              [@configuration.symbolize_keys]
+              [db_config]
             ]
           ) do
-            ActiveRecord::Tasks::DatabaseTasks.create @configuration
+            ActiveRecord::Tasks::DatabaseTasks.create(db_config)
           end
         end
       end
@@ -154,13 +158,15 @@ def teardown
       end
 
       def test_establishes_connection_to_mysql_database
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("default_env", "primary", @configuration)
+
         ActiveRecord::Base.stub(:connection, @connection) do
           assert_called_with(
             ActiveRecord::Base,
             :establish_connection,
-            [@configuration.symbolize_keys]
+            [db_config]
           ) do
-            ActiveRecord::Tasks::DatabaseTasks.drop @configuration
+            ActiveRecord::Tasks::DatabaseTasks.drop(db_config)
           end
         end
       end
@@ -201,13 +207,15 @@ def setup
       end
 
       def test_establishes_connection_to_the_appropriate_database
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("default_env", "primary", @configuration)
+
         ActiveRecord::Base.stub(:connection, @connection) do
           assert_called_with(
             ActiveRecord::Base,
             :establish_connection,
-            [@configuration.symbolize_keys]
+            [db_config]
           ) do
-            ActiveRecord::Tasks::DatabaseTasks.purge @configuration
+            ActiveRecord::Tasks::DatabaseTasks.purge(db_config)
           end
         end
       end

activerecord/test/cases/tasks/postgresql_rake_test.rb

@@ -21,6 +21,8 @@ def teardown
       end
 
       def test_establishes_connection_to_postgresql_database
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("default_env", "primary", @configuration)
+
         ActiveRecord::Base.stub(:connection, @connection) do
           assert_called_with(
             ActiveRecord::Base,
@@ -31,13 +33,10 @@ def test_establishes_connection_to_postgresql_database
                 database: "postgres",
                 schema_search_path: "public"
               ],
-              [
-                adapter: "postgresql",
-                database: "my-app-db"
-              ]
+              [db_config]
             ]
           ) do
-            ActiveRecord::Tasks::DatabaseTasks.create @configuration
+            ActiveRecord::Tasks::DatabaseTasks.create(db_config)
           end
         end
       end
@@ -88,6 +87,8 @@ def test_creates_database_with_given_collation_and_ctype
       end
 
       def test_establishes_connection_to_new_database
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("default_env", "primary", @configuration)
+
         ActiveRecord::Base.stub(:connection, @connection) do
           assert_called_with(
             ActiveRecord::Base,
@@ -98,12 +99,10 @@ def test_establishes_connection_to_new_database
                 database: "postgres",
                 schema_search_path: "public"
               ],
-              [
-                @configuration.symbolize_keys
-              ]
+              [db_config]
             ]
           ) do
-            ActiveRecord::Tasks::DatabaseTasks.create @configuration
+            ActiveRecord::Tasks::DatabaseTasks.create(db_config)
           end
         end
       end
@@ -232,6 +231,8 @@ def test_clears_active_connections
       end
 
       def test_establishes_connection_to_postgresql_database
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("default_env", "primary", @configuration)
+
         with_stubbed_connection do
           assert_called_with(
             ActiveRecord::Base,
@@ -242,13 +243,10 @@ def test_establishes_connection_to_postgresql_database
                 database: "postgres",
                 schema_search_path: "public"
               ],
-              [
-                adapter: "postgresql",
-                database: "my-app-db"
-              ]
+              [db_config]
             ]
           ) do
-            ActiveRecord::Tasks::DatabaseTasks.purge @configuration
+            ActiveRecord::Tasks::DatabaseTasks.purge(db_config)
           end
         end
       end
@@ -278,6 +276,8 @@ def test_creates_database
       end
 
       def test_establishes_connection
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("default_env", "primary", @configuration)
+
         with_stubbed_connection do
           assert_called_with(
             ActiveRecord::Base,
@@ -288,12 +288,10 @@ def test_establishes_connection
                 database: "postgres",
                 schema_search_path: "public"
               ],
-              [
-                @configuration.symbolize_keys
-              ]
+              [db_config]
             ]
           ) do
-            ActiveRecord::Tasks::DatabaseTasks.purge @configuration
+            ActiveRecord::Tasks::DatabaseTasks.purge(db_config)
           end
         end
       end

activerecord/test/cases/tasks/sqlite_rake_test.rb

@@ -54,9 +54,12 @@ def test_db_create_with_file_does_nothing
       end
 
       def test_db_create_establishes_a_connection
-        assert_called_with(ActiveRecord::Base, :establish_connection, [@configuration.symbolize_keys]) do
+        calls = []
+        ActiveRecord::Base.stub(:establish_connection, proc { |*args| calls << args }) do
           ActiveRecord::Tasks::DatabaseTasks.create @configuration, "/rails/root"
         end
+
+        assert_equal [@configuration.symbolize_keys], calls.map { |c| c.first.configuration_hash }
       end
 
       def test_db_create_with_error_prints_message