Make the task preloading process configurable

In ordered to be displayed in the UI, tasks have to be loaded by the
time `MaintenanceTasks::Task.descendants` is called.
To achieve that, the `Task.load_constants` method was previously
implemented, and called from `Task.available_tasks`.

A few inconvenients of that method are:

- It could easily be abused and used in lower levels of the gem, where
  ideally it should ne be necessary to load all tasks to achieve the
  gem's goal. See for example my previous PR
  [Avoid loading all tasks to validate a
`Run#task_name`](#866).

- It can introduce unnecessary overhead. For example, displaying the
  CLI's basic help message with `maintenance_tasks help` will
  necessitate all tasks to be loaded (even though the help message does
  not display anything related to the available tasks), and this loading
  time will delay the appearance of that help message.
  Similarly, a single test case that want to test a single task's
  behaviour, while never displaying the whole list of available tasks,
  would be negatively impacted by a misuse of `Task.available_tasks`.

- It does not allow an application to define an alternative way to load
  all constants. As a matter of fact, the Shopify Core monolith
  monkey-patched that method to allow multiple "task modules" instead of
  a single one. With a configurable `task_loader`, monkey-patching would
  not be necessary. (Which is good. 🙈)

This PR introduces `MaintenanceTasks.task_loader`, a configuration that
allows applications to change the logic applied to ensure all task are
loaded before being displayed. As a bonus:

- it changes how the CLI generates the long description containing all
  task names, so that tasks get loaded at the last minute, only when
  necessary
- it gets rid of `Task.available_tasks` (its only use was for UI
  purposes, so I changed it to a direct call to `Task.descendants`)
- it introduces some tests of the loading behaviour

README.md

@@ -41,6 +41,10 @@ Please ensure your application is using
 information, please consult the [Rails guides on autoloading and reloading
 constants](https://guides.rubyonrails.org/autoloading_and_reloading_constants.html).
 
+You might be able to define a different
+[_task loader_](#customizing-the-way-the-gem-loads-all-tasks) that would allow
+you to circumvent this limitation.
+
 ## Usage
 
 The typical Maintenance Tasks workflow is as follows:
@@ -716,6 +720,35 @@ module Maintenance
 end
 ```
 
+#### Customizing the way the gem loads all tasks
+
+For the UI (Web and CLI) to be able to display all available tasks, their
+respective classes must have been previously loaded. This can be an issue
+for applications running without
+[eager loading](https://guides.rubyonrails.org/configuring.html#config-eager-load)
+(for example a default Rails application uses lazy loading in development
+mode).
+
+To work around that, this gem uses a task loader, which can be customized
+by changing the `MaintenanceTasks.task_loader` in the configuration:
+
+```rb
+# config/initializers/maintenance_tasks.rb
+
+MaintenanceTasks.task_loader = -> { DefaultTaskLoader.load_all }
+```
+
+By using a different proc, you can define a different way to preload all tasks.
+You could for example browse for tasks in a given directory:
+
+```rb
+# config/initializers/maintenance_tasks.rb
+
+MaintenanceTasks.task_loader = -> {
+  Dir[Rails.root.join("app", "tasks", "**", "*_task.rb")].each { |file| require file }
+}
+```
+
 #### Customizing the underlying job class
 
 `MaintenanceTasks.job` can be configured to define a Job class for your tasks to

app/models/maintenance_tasks/default_task_loader.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module MaintenanceTasks
+  # This class defines how the gem will find and load all tasks defined by the application.
+  # It can be overridden by the application to change the way tasks are loaded.
+  class DefaultTaskLoader
+    class << self
+      # Recursively browse the MaintenanceTasks.tasks_module namespace to load all defined tasks.
+      #
+      # @return [void]
+      def load_all
+        load_constants(MaintenanceTasks.tasks_module.safe_constantize)
+      end
+
+      private
+
+      def load_constants(namespace)
+        namespace.constants.each do |name|
+          object = namespace.const_get(name)
+          load_constants(object) if object.instance_of?(Module)
+        end
+      end
+    end
+  end
+end

app/models/maintenance_tasks/task.rb

@@ -41,15 +41,6 @@ def named(name)
         task
       end
 
-      # Returns a list of concrete classes that inherit from the Task
-      # superclass.
-      #
-      # @return [Array<Class>] the list of classes.
-      def available_tasks
-        load_constants
-        descendants
-      end
-
       # Make this Task a task that handles CSV.
       #
       # @param in_batches [Integer] optionally, supply a batch size if the CSV
@@ -168,21 +159,6 @@ def after_cancel(*filter_list, &block)
       def after_error(*filter_list, &block)
         set_callback(:error, :after, *filter_list, &block)
       end
-
-      private
-
-      def load_constants
-        namespace = MaintenanceTasks.tasks_module.safe_constantize
-        return unless namespace
-
-        load_const = lambda do |root|
-          root.constants.each do |name|
-            object = root.const_get(name)
-            load_const.call(object) if object.instance_of?(Module)
-          end
-        end
-        load_const.call(namespace)
-      end
     end
 
     # The contents of a CSV file to be processed by a Task.

app/models/maintenance_tasks/task_data_index.rb

@@ -25,7 +25,10 @@ class << self
       def available_tasks
         tasks = []
 
-        task_names = Task.available_tasks.map(&:name)
+        # Ensure all tasks are loaded before listing them.
+        MaintenanceTasks.task_loader.call
+
+        task_names = Task.descendants.map(&:name)
 
         active_runs = Run.with_attached_csv.active.where(task_name: task_names)
         active_runs.each do |run|

lib/maintenance_tasks.rb

@@ -21,6 +21,12 @@ module MaintenanceTasks
   #   @return [String] the name of the module.
   mattr_accessor :tasks_module, default: "Maintenance"
 
+  # @!atribute task_loader
+  #   @scope class
+  #
+  #   A Proc that loads all the Tasks to be used by the application.
+  mattr_accessor :task_loader, default: -> { DefaultTaskLoader.load_all }
+
   # @!attribute job
   #   @scope class
   #

lib/maintenance_tasks/cli.rb

@@ -16,15 +16,6 @@ def exit_on_failure?
 
     desc "perform [TASK NAME]", "Runs the given Maintenance Task"
 
-    long_desc <<-LONGDESC
-      `maintenance_tasks perform` will run the Maintenance Task specified by the
-      [TASK NAME] argument.
-
-      Available Tasks:
-
-      #{MaintenanceTasks::Task.available_tasks.join("\n\n")}
-    LONGDESC
-
     # Specify the CSV file to process for CSV Tasks
     desc = "Supply a CSV file to be processed by a CSV Task, "\
       "--csv path/to/csv/file.csv"
@@ -50,6 +41,24 @@ def perform(name)
       say_status(:error, error.message, :red)
     end
 
+    # `long_desc` only allows us to use a static string as "long description".
+    # By redefining the `#long_description` method on the "perform" Command
+    # object instead, we make it dynamic, thus delaying the task loading
+    # process until it's actually required.
+    commands["perform"].define_singleton_method(:long_description) do
+      # Ensure all tasks are loaded before listing them.
+      MaintenanceTasks.task_loader.call
+
+      <<~LONGDESC
+        `maintenance_tasks perform` will run the Maintenance Task specified by the
+        [TASK NAME] argument.
+
+        Available Tasks:
+
+        #{Task.descendants.map(&:name).sort.join("\n\n")}
+      LONGDESC
+    end
+
     private
 
     def csv_file

test/dummy/lib/not_autoloaded_task.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class NotAutoloadedTask < MaintenanceTasks::Task
+  def collection
+    [1, 2]
+  end
+
+  def process(_); end
+end

test/lib/maintenance_tasks/cli_test.rb

@@ -84,5 +84,52 @@ class CLITest < ActiveSupport::TestCase
         CLI.start(["perform", "MyParamsTask", "--arguments", "post_ids:1,2,3"])
       end
     end
+
+    test "`help perform` loads all tasks and displays them" do
+      # Ensure test tasks have been loaded
+      MaintenanceTasks.task_loader.call
+
+      dummy_loader = -> {}
+      MaintenanceTasks.expects(:task_loader).returns(dummy_loader).at_least_once
+      dummy_loader.expects(:call).at_least_once
+
+      expected_output = <<~TASKS
+        Available Tasks:
+
+        Maintenance::BatchImportPostsTask
+
+        Maintenance::CallbackTestTask
+
+        Maintenance::CancelledEnqueueTask
+
+        Maintenance::EnqueueErrorTask
+
+        Maintenance::ErrorTask
+
+        Maintenance::ImportPostsTask
+
+        Maintenance::Nested::NestedMore::NestedMoreTask
+
+        Maintenance::Nested::NestedTask
+
+        Maintenance::NoCollectionTask
+
+        Maintenance::ParamsTask
+
+        Maintenance::TestTask
+
+        Maintenance::UpdatePostsInBatchesTask
+
+        Maintenance::UpdatePostsModulePrependedTask
+
+        Maintenance::UpdatePostsTask
+
+        Maintenance::UpdatePostsThrottledTask
+      TASKS
+
+      assert_output(/#{expected_output.indent(2)}/) do
+        CLI.start(["help", "perform"])
+      end
+    end
   end
 end

test/models/maintenance_tasks/default_task_loader_test.rb

@@ -0,0 +1,40 @@
+
+# frozen_string_literal: true
+
+require "test_helper"
+
+module MaintenanceTasks
+  class DefaultTaskLoaderTest < ActiveSupport::TestCase
+    include ActiveSupport::Testing::Isolation
+
+    test ".load_all loads all autoloadable Task subclasses within the Maintenance namespace" do
+      [
+        Maintenance::BatchImportPostsTask,
+        Maintenance::CallbackTestTask,
+        Maintenance::CancelledEnqueueTask,
+        Maintenance::EnqueueErrorTask,
+        Maintenance::ErrorTask,
+        Maintenance::ImportPostsTask,
+        Maintenance::Nested,
+        Maintenance::Nested::NestedMore,
+        Maintenance::Nested::NestedMore::NestedMoreTask,
+        Maintenance::Nested::NestedTask,
+        Maintenance::NoCollectionTask,
+        Maintenance::ParamsTask,
+        Maintenance::TestTask,
+        Maintenance::UpdatePostsInBatchesTask,
+        Maintenance::UpdatePostsModulePrependedTask,
+        Maintenance::UpdatePostsTask,
+        Maintenance::UpdatePostsThrottledTask,
+      ].each do |constant|
+        constant
+          .module_parent
+          .expects(:const_get)
+          .with(constant.name.demodulize.to_sym)
+          .returns(constant)
+      end
+
+      DefaultTaskLoader.load_all
+    end
+  end
+end

test/models/maintenance_tasks/task_data_index_test.rb

@@ -27,6 +27,14 @@ class TaskDataIndexTest < ActiveSupport::TestCase
       assert_equal expected, TaskDataIndex.available_tasks.map(&:name)
     end
 
+    test ".available_tasks calls MaintenanceTasks.task_loader to load all tasks" do
+      dummy_loader = -> {}
+      MaintenanceTasks.expects(:task_loader).returns(dummy_loader)
+      dummy_loader.expects(:call)
+
+      TaskDataIndex.available_tasks
+    end
+
     test "#new sets last_run if one is passed as an argument" do
       run = Run.create!(task_name: "Maintenance::UpdatePostsTask")
       task_data = TaskDataIndex.new("Maintenance::UpdatePostsTask", run)

test/models/maintenance_tasks/task_test.rb

@@ -4,28 +4,6 @@
 
 module MaintenanceTasks
   class TaskTest < ActiveSupport::TestCase
-    test ".available_tasks returns list of tasks that inherit from the Task superclass" do
-      expected = [
-        "Maintenance::BatchImportPostsTask",
-        "Maintenance::CallbackTestTask",
-        "Maintenance::CancelledEnqueueTask",
-        "Maintenance::EnqueueErrorTask",
-        "Maintenance::ErrorTask",
-        "Maintenance::ImportPostsTask",
-        "Maintenance::Nested::NestedMore::NestedMoreTask",
-        "Maintenance::Nested::NestedTask",
-        "Maintenance::NoCollectionTask",
-        "Maintenance::ParamsTask",
-        "Maintenance::TestTask",
-        "Maintenance::UpdatePostsInBatchesTask",
-        "Maintenance::UpdatePostsModulePrependedTask",
-        "Maintenance::UpdatePostsTask",
-        "Maintenance::UpdatePostsThrottledTask",
-      ]
-      assert_equal expected,
-        MaintenanceTasks::Task.available_tasks.map(&:name).sort
-    end
-
     test ".named returns the task based on its name" do
       expected_task = Maintenance::UpdatePostsTask
       assert_equal expected_task, Task.named("Maintenance::UpdatePostsTask")