FIX: migrations-tooling CLI didn't work anymore (#29777)

The previous approach of splitting Thor commands into multiple files caused problems when the same method name was used in multiple commands.

This also loads the Rails environment only for commands that need it. That makes the CLI boot faster for most commands or when the help should be shown. That's also why we can't use `Rails.root` in the CLI.

migrations/bin/cli

@@ -1,23 +1,49 @@
 #!/usr/bin/env ruby
 # frozen_string_literal: true
 
-require "thor"
 require_relative "../lib/migrations"
 
+require "colored2"
+require "thor"
+
 module Migrations
-  load_rails_environment
   configure_zeitwerk
   enable_i18n
 
-  class CommandLineInterface < Thor
-    include ::Migrations::CLI::ConvertCommand
-    include ::Migrations::CLI::ImportCommand
-    include ::Migrations::CLI::UploadCommand
+  module CLI
+    class Application < Thor
+      desc "convert [FROM]", "Convert a file"
+      option :settings, type: :string, desc: "Path of settings file", banner: "path"
+      option :reset, type: :boolean, desc: "Reset database before converting data"
+      def convert(converter_type)
+        ::Migrations::CLI::ConvertCommand.new(converter_type, options).execute
+      end
 
-    def self.exit_on_failure?
-      true
+      desc "import", "Import a file"
+      def import
+        ::Migrations::CLI::ImportCommand.new(options).execute
+      end
+
+      desc "upload", "Upload media uploads"
+      option :settings,
+             type: :string,
+             desc: "Uploads settings file path",
+             default: "./migrations/config/upload.yml",
+             aliases: "-s",
+             banner: "path"
+      option :fix_missing, type: :boolean, desc: "Fix missing uploads"
+      option :optimize, type: :boolean, desc: "Optimize uploads"
+      def upload
+        ::Migrations::CLI::UploadCommand.new(options).execute
+      end
+
+      def self.exit_on_failure?
+        true
+      end
     end
   end
-
-  Dir.chdir(Rails.root) { CommandLineInterface.start } # rubocop:disable Discourse/NoChdir
 end
+
+# rubocop:disable Discourse/NoChdir
+Dir.chdir(File.expand_path("../..", __dir__)) { ::Migrations::CLI::Application.start }
+# rubocop:enable Discourse/NoChdir

migrations/lib/cli/convert_command.rb

@@ -1,52 +1,48 @@
 # frozen_string_literal: true
 
-module Migrations::CLI::ConvertCommand
-  def self.included(thor)
-    thor.class_eval do
-      desc "convert [FROM]", "Convert a file"
-      option :settings, type: :string, desc: "Path of settings file", banner: "path"
-      option :reset, type: :boolean, desc: "Reset database before converting data"
-      def convert(converter_type)
-        converter_type = converter_type.downcase
-        validate_converter_type!(converter_type)
+module Migrations::CLI
+  class ConvertCommand
+    def initialize(converter_type, options)
+      @converter_type = converter_type.downcase
+      @options = options
+    end
 
-        settings = load_settings(converter_type)
+    def execute
+      validate_converter_type!
+      settings = load_settings
 
-        ::Migrations::Database.reset!(settings[:intermediate_db][:path]) if options[:reset]
+      ::Migrations::Database.reset!(settings[:intermediate_db][:path]) if @options[:reset]
 
-        converter = "migrations/converters/#{converter_type}/converter".camelize.constantize
-        converter.new(settings).run
-      end
+      converter = "migrations/converters/#{@converter_type}/converter".camelize.constantize
+      converter.new(settings).run
+    end
 
-      private
+    private
 
-      def validate_converter_type!(type)
-        converter_names = ::Migrations::Converters.names
+    def validate_converter_type!
+      converter_names = ::Migrations::Converters.names
 
-        raise Thor::Error, <<~MSG if !converter_names.include?(type)
-            Unknown converter name: #{type}
-            Valid names are: #{converter_names.join(", ")}
-          MSG
-      end
+      raise Thor::Error, <<~MSG if !converter_names.include?(@converter_type)
+        Unknown converter name: #{@converter_type}
+        Valid names are: #{converter_names.join(", ")}
+      MSG
+    end
 
-      def validate_settings_path!(settings_path)
-        if !File.exist?(settings_path)
-          raise Thor::Error, "Settings file not found: #{settings_path}"
-        end
-      end
+    def validate_settings_path!(settings_path)
+      raise Thor::Error, "Settings file not found: #{settings_path}" if !File.exist?(settings_path)
+    end
 
-      def load_settings(converter_type)
-        settings_path = calculate_settings_path(converter_type)
-        validate_settings_path!(settings_path)
+    def load_settings
+      settings_path = calculate_settings_path
+      validate_settings_path!(settings_path)
 
-        YAML.safe_load(File.read(settings_path), symbolize_names: true)
-      end
+      YAML.safe_load(File.read(settings_path), symbolize_names: true)
+    end
 
-      def calculate_settings_path(converter_type)
-        settings_path =
-          options[:settings] || ::Migrations::Converters.default_settings_path(converter_type)
-        File.expand_path(settings_path, Dir.pwd)
-      end
+    def calculate_settings_path
+      settings_path =
+        @options[:settings] || ::Migrations::Converters.default_settings_path(@converter_type)
+      File.expand_path(settings_path, Dir.pwd)
     end
   end
 end

migrations/lib/cli/import_command.rb

@@ -1,15 +1,18 @@
 # frozen_string_literal: true
 
-module Migrations::CLI::ImportCommand
-  def self.included(thor)
-    thor.class_eval do
-      desc "import", "Import a file"
-      def import
-        require "extralite"
+require "extralite"
 
-        puts "Importing into Discourse #{Discourse::VERSION::STRING}"
-        puts "Extralite SQLite version: #{Extralite.sqlite3_version}"
-      end
+module Migrations::CLI
+  class ImportCommand
+    def initialize(options)
+      @options = options
+    end
+
+    def execute
+      ::Migrations.load_rails_environment
+
+      puts "Importing into Discourse #{Discourse::VERSION::STRING}"
+      puts "Extralite SQLite version: #{Extralite.sqlite3_version}"
     end
   end
 end

migrations/lib/cli/upload_command.rb

@@ -1,49 +1,40 @@
 # frozen_string_literal: true
 
-module Migrations::CLI::UploadCommand
-  def self.included(thor)
-    thor.class_eval do
-      desc "upload", "Upload media uploads"
-      option :settings,
-             type: :string,
-             desc: "Uploads settings file path",
-             default: "./migrations/config/upload.yml",
-             aliases: "-s",
-             banner: "path"
-      option :fix_missing, type: :boolean, desc: "Fix missing uploads"
-      option :optimize, type: :boolean, desc: "Optimize uploads"
-      def upload
-        puts "Starting uploads..."
+module Migrations::CLI
+  class UploadCommand
+    def initialize(options)
+      @options = options
+    end
 
-        validate_settings_file!
-        settings = load_settings
+    def execute
+      puts "Starting uploads..."
 
-        ::Migrations::Uploader::Uploads.perform!(settings)
+      validate_settings_file!
+      settings = load_settings
 
-        puts ""
-      end
+      ::Migrations::Uploader::Uploads.perform!(settings)
 
-      private
+      puts ""
+    end
 
-      def load_settings
-        settings = ::Migrations::SettingsParser.parse!(options.settings)
-        merge_settings_from_cli_args!(settings)
+    private
 
-        settings
-      end
+    def load_settings
+      settings = ::Migrations::SettingsParser.parse!(@options.settings)
+      merge_settings_from_cli_args!(@options, settings)
 
-      def merge_settings_from_cli_args!(settings)
-        settings[:fix_missing] = options.fix_missing if options.fix_missing.present?
-        settings[:create_optimized_images] = options.optimize if options.optimize.present?
-      end
+      settings
+    end
 
-      def validate_settings_file!
-        path = options.settings
+    def merge_settings_from_cli_args!(settings)
+      settings[:fix_missing] = options.fix_missing if @options.fix_missing.present?
+      settings[:create_optimized_images] = options.optimize if @options.optimize.present?
+    end
 
-        if !File.exist?(path)
-          raise ::Migrations::NoSettingsFound, "Settings file not found: #{path}"
-        end
-      end
+    def validate_settings_file!
+      path = @options.settings
+
+      raise ::Migrations::NoSettingsFound, "Settings file not found: #{path}" if !File.exist?(path)
     end
   end
 end

migrations/lib/migrations.rb

@@ -19,7 +19,7 @@ module Migrations
 
   def self.load_rails_environment(quiet: false)
     message = "Loading Rails environment ..."
-    print message unless quiet
+    print message if !quiet
 
     rails_root = File.expand_path("../..", __dir__)
     # rubocop:disable Discourse/NoChdir
@@ -33,9 +33,11 @@ module Migrations
     end
     # rubocop:enable Discourse/NoChdir
 
-    print "\r"
-    print " " * message.length
-    print "\r"
+    if !quiet
+      print "\r"
+      print " " * message.length
+      print "\r"
+    end
   end
 
   def self.configure_zeitwerk
@@ -49,7 +51,7 @@ module Migrations
     loader.push_dir(File.join(::Migrations.root_path, "lib"), namespace: ::Migrations)
     loader.push_dir(File.join(::Migrations.root_path, "lib", "common"), namespace: ::Migrations)
 
-    # All sub-directories of a converter should have the same namespace.
+    # All subdirectories of a converter should have the same namespace.
     # Unfortunately `loader.collapse` doesn't work recursively.
     Converters.all.each do |name, converter_path|
       module_name = name.camelize.to_sym