Add ActiveRecord::Tasks::AbstractTasks for subclassing per adapter behavior

The main motivation was to be able to override
`check_current_protected_environment!` in the case of SQLite3 which
raises an error on read-only databases in Linux.

Author: zzak

activerecord/lib/active_record.rb

@@ -174,7 +174,8 @@ module Tasks
     extend ActiveSupport::Autoload
 
     autoload :DatabaseTasks
-    autoload :MySQLDatabaseTasks,  "active_record/tasks/mysql_database_tasks"
+    autoload :AbstractTasks, "active_record/tasks/abstract_tasks"
+    autoload :MySQLDatabaseTasks, "active_record/tasks/mysql_database_tasks"
     autoload :PostgreSQLDatabaseTasks, "active_record/tasks/postgresql_database_tasks"
     autoload :SQLiteDatabaseTasks, "active_record/tasks/sqlite_database_tasks"
   end

activerecord/lib/active_record/tasks/abstract_tasks.rb

@@ -0,0 +1,76 @@
+# frozen_string_literal: true
+
+module ActiveRecord
+  module Tasks # :nodoc:
+    class AbstractTasks # :nodoc:
+      def self.using_database_configurations?
+        true
+      end
+
+      def initialize(db_config)
+        @db_config = db_config
+        @configuration_hash = db_config.configuration_hash
+      end
+
+      def charset
+        connection.encoding
+      end
+
+      def collation
+        connection.collation
+      end
+
+      def check_current_protected_environment!(db_config, migration_class)
+        with_temporary_pool(db_config, migration_class) do |pool|
+          migration_context = pool.migration_context
+          current = migration_context.current_environment
+          stored  = migration_context.last_stored_environment
+
+          if migration_context.protected_environment?
+            raise ActiveRecord::ProtectedEnvironmentError.new(stored)
+          end
+
+          if stored && stored != current
+            raise ActiveRecord::EnvironmentMismatchError.new(current: current, stored: stored)
+          end
+        rescue ActiveRecord::NoDatabaseError
+        end
+      end
+
+      private
+        attr_reader :db_config, :configuration_hash
+
+        def connection
+          ActiveRecord::Base.lease_connection
+        end
+
+        def establish_connection(config = db_config)
+          ActiveRecord::Base.establish_connection(config)
+        end
+
+        def configuration_hash_without_database
+          configuration_hash.merge(database: nil)
+        end
+
+        def run_cmd(cmd, *args, **opts)
+          fail run_cmd_error(cmd, args) unless Kernel.system(cmd, *args, opts)
+        end
+
+        def run_cmd_error(cmd, args)
+          msg = +"failed to execute:\n"
+          msg << "#{cmd} #{args.join(' ')}\n\n"
+          msg << "Please check the output above for any errors and make sure that `#{cmd}` is installed in your PATH and has proper permissions.\n\n"
+          msg
+        end
+
+        def with_temporary_pool(db_config, migration_class, clobber: false)
+          original_db_config = migration_class.connection_db_config
+          pool = migration_class.connection_handler.establish_connection(db_config, clobber: clobber)
+
+          yield pool
+        ensure
+          migration_class.connection_handler.establish_connection(original_db_config, clobber: clobber)
+        end
+    end
+  end
+end

activerecord/lib/active_record/tasks/database_tasks.rb

@@ -66,7 +66,7 @@ def check_protected_environments!(environment = env)
         return if ENV["DISABLE_DATABASE_ENVIRONMENT_CHECK"]
 
         configs_for(env_name: environment).each do |db_config|
-          check_current_protected_environment!(db_config)
+          database_adapter_for(db_config).check_current_protected_environment!(db_config, migration_class)
         end
       end
 
@@ -640,23 +640,6 @@ def structure_load_flags_for(adapter)
           end
         end
 
-        def check_current_protected_environment!(db_config)
-          with_temporary_pool(db_config) do |pool|
-            migration_context = pool.migration_context
-            current = migration_context.current_environment
-            stored  = migration_context.last_stored_environment
-
-            if migration_context.protected_environment?
-              raise ActiveRecord::ProtectedEnvironmentError.new(stored)
-            end
-
-            if stored && stored != current
-              raise ActiveRecord::EnvironmentMismatchError.new(current: current, stored: stored)
-            end
-          rescue ActiveRecord::NoDatabaseError
-          end
-        end
-
         def initialize_database(db_config)
           with_temporary_pool(db_config) do
             begin

activerecord/lib/active_record/tasks/mysql_database_tasks.rb

@@ -2,16 +2,7 @@
 
 module ActiveRecord
   module Tasks # :nodoc:
-    class MySQLDatabaseTasks # :nodoc:
-      def self.using_database_configurations?
-        true
-      end
-
-      def initialize(db_config)
-        @db_config = db_config
-        @configuration_hash = db_config.configuration_hash
-      end
-
+    class MySQLDatabaseTasks < AbstractTasks # :nodoc:
       def create
         establish_connection(configuration_hash_without_database)
         connection.create_database(db_config.database, creation_options)
@@ -33,10 +24,6 @@ def charset
         connection.charset
       end
 
-      def collation
-        connection.collation
-      end
-
       def structure_dump(filename, extra_flags)
         args = prepare_command_options
         args.concat(["--result-file", "#{filename}"])
@@ -53,7 +40,7 @@ def structure_dump(filename, extra_flags)
         args.concat([db_config.database.to_s])
         args.unshift(*extra_flags) if extra_flags
 
-        run_cmd("mysqldump", args, "dumping")
+        run_cmd("mysqldump", *args)
       end
 
       def structure_load(filename, extra_flags)
@@ -62,24 +49,10 @@ def structure_load(filename, extra_flags)
         args.concat(["--database", db_config.database.to_s])
         args.unshift(*extra_flags) if extra_flags
 
-        run_cmd("mysql", args, "loading")
+        run_cmd("mysql", *args)
       end
 
       private
-        attr_reader :db_config, :configuration_hash
-
-        def connection
-          ActiveRecord::Base.lease_connection
-        end
-
-        def establish_connection(config = db_config)
-          ActiveRecord::Base.establish_connection(config)
-        end
-
-        def configuration_hash_without_database
-          configuration_hash.merge(database: nil)
-        end
-
         def creation_options
           Hash.new.tap do |options|
             options[:charset]     = configuration_hash[:encoding]   if configuration_hash.include?(:encoding)
@@ -105,16 +78,6 @@ def prepare_command_options
 
           args
         end
-
-        def run_cmd(cmd, args, action)
-          fail run_cmd_error(cmd, args, action) unless Kernel.system(cmd, *args)
-        end
-
-        def run_cmd_error(cmd, args, action)
-          msg = +"failed to execute: `#{cmd}`\n"
-          msg << "Please check the output above for any errors and make sure that `#{cmd}` is installed in your PATH and has proper permissions.\n\n"
-          msg
-        end
     end
   end
 end

activerecord/lib/active_record/tasks/postgresql_database_tasks.rb

@@ -4,20 +4,11 @@
 
 module ActiveRecord
   module Tasks # :nodoc:
-    class PostgreSQLDatabaseTasks # :nodoc:
+    class PostgreSQLDatabaseTasks < AbstractTasks # :nodoc:
       DEFAULT_ENCODING = ENV["CHARSET"] || "utf8"
       ON_ERROR_STOP_1 = "ON_ERROR_STOP=1"
       SQL_COMMENT_BEGIN = "--"
 
-      def self.using_database_configurations?
-        true
-      end
-
-      def initialize(db_config)
-        @db_config = db_config
-        @configuration_hash = db_config.configuration_hash
-      end
-
       def create(connection_already_established = false)
         establish_connection(public_schema_config) unless connection_already_established
         connection.create_database(db_config.database, configuration_hash.merge(encoding: encoding))
@@ -29,14 +20,6 @@ def drop
         connection.drop_database(db_config.database)
       end
 
-      def charset
-        connection.encoding
-      end
-
-      def collation
-        connection.collation
-      end
-
       def purge
         ActiveRecord::Base.connection_handler.clear_active_connections!(:all)
         drop
@@ -72,7 +55,7 @@ def structure_dump(filename, extra_flags)
         end
 
         args << db_config.database
-        run_cmd("pg_dump", args, "dumping")
+        run_cmd("pg_dump", *args)
         remove_sql_header_comments(filename)
         File.open(filename, "a") { |f| f << "SET search_path TO #{connection.schema_search_path};\n\n" }
       end
@@ -82,20 +65,10 @@ def structure_load(filename, extra_flags)
         args.concat(Array(extra_flags)) if extra_flags
         args.concat(["--file", filename])
         args << db_config.database
-        run_cmd("psql", args, "loading")
+        run_cmd("psql", *args)
       end
 
       private
-        attr_reader :db_config, :configuration_hash
-
-        def connection
-          ActiveRecord::Base.lease_connection
-        end
-
-        def establish_connection(config = db_config)
-          ActiveRecord::Base.establish_connection(config)
-        end
-
         def encoding
           configuration_hash[:encoding] || DEFAULT_ENCODING
         end
@@ -117,15 +90,8 @@ def psql_env
           end
         end
 
-        def run_cmd(cmd, args, action)
-          fail run_cmd_error(cmd, args, action) unless Kernel.system(psql_env, cmd, *args)
-        end
-
-        def run_cmd_error(cmd, args, action)
-          msg = +"failed to execute:\n"
-          msg << "#{cmd} #{args.join(' ')}\n\n"
-          msg << "Please check the output above for any errors and make sure that `#{cmd}` is installed in your PATH and has proper permissions.\n\n"
-          msg
+        def run_cmd(cmd, *args, **opts)
+          fail run_cmd_error(cmd, args) unless Kernel.system(psql_env, cmd, *args, **opts)
         end
 
         def remove_sql_header_comments(filename)

activerecord/lib/active_record/tasks/sqlite_database_tasks.rb

@@ -2,11 +2,7 @@
 
 module ActiveRecord
   module Tasks # :nodoc:
-    class SQLiteDatabaseTasks # :nodoc:
-      def self.using_database_configurations?
-        true
-      end
-
+    class SQLiteDatabaseTasks < AbstractTasks # :nodoc:
       def initialize(db_config, root = ActiveRecord::Tasks::DatabaseTasks.root)
         @db_config = db_config
         @root = root
@@ -37,10 +33,6 @@ def purge
         connection.reconnect!
       end
 
-      def charset
-        connection.encoding
-      end
-
       def structure_dump(filename, extra_flags)
         args = []
         args.concat(Array(extra_flags)) if extra_flags
@@ -54,36 +46,32 @@ def structure_dump(filename, extra_flags)
         else
           args << ".schema --nosys"
         end
-        run_cmd("sqlite3", args, filename)
+
+        run_cmd("sqlite3", *args, out: filename)
       end
 
       def structure_load(filename, extra_flags)
         flags = extra_flags.join(" ") if extra_flags
         `sqlite3 #{flags} #{db_config.database} < "#{filename}"`
       end
 
-      private
-        attr_reader :db_config, :root
-
-        def connection
-          ActiveRecord::Base.lease_connection
+      def check_current_protected_environment!(db_config, migration_class)
+        super
+      rescue ActiveRecord::StatementInvalid => e
+        case e.cause
+        when SQLite3::ReadOnlyException
+        else
+          raise e
         end
+      end
+
+      private
+        attr_reader :root
 
         def establish_connection(config = db_config)
           ActiveRecord::Base.establish_connection(config)
           connection.connect!
         end
-
-        def run_cmd(cmd, args, out)
-          fail run_cmd_error(cmd, args) unless Kernel.system(cmd, *args, out: out)
-        end
-
-        def run_cmd_error(cmd, args)
-          msg = +"failed to execute:\n"
-          msg << "#{cmd} #{args.join(' ')}\n\n"
-          msg << "Please check the output above for any errors and make sure that `#{cmd}` is installed in your PATH and has proper permissions.\n\n"
-          msg
-        end
     end
   end
 end