DEV: Remove Zeitwerk inflection monkey patch.

There isn't a good reason we need to patch the inflector.

Co-authored-by: Loïc Guitaut <loic@discourse.org>

app/jobs/onceoff/onceoff.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require_relative '../base.rb'
-
 class Jobs::Onceoff < ::Jobs::Base
   sidekiq_options retry: false
 

config/application.rb

@@ -55,8 +55,6 @@ require 'pry-rails' if Rails.env.development?
 
 require 'discourse_fonts'
 
-require_relative '../lib/zeitwerk_config.rb'
-
 if defined?(Bundler)
   bundler_groups = [:default]
 
@@ -69,6 +67,8 @@ if defined?(Bundler)
   Bundler.require(*bundler_groups)
 end
 
+require_relative '../lib/require_dependency_backward_compatibility'
+
 module Discourse
   class Application < Rails::Application
 
@@ -110,9 +110,6 @@ module Discourse
     config.autoloader = :zeitwerk
 
     # Custom directories with classes and modules you want to be autoloadable.
-    config.autoload_paths += Dir["#{config.root}/app"]
-    config.autoload_paths += Dir["#{config.root}/app/jobs"]
-    config.autoload_paths += Dir["#{config.root}/app/serializers"]
     config.autoload_paths += Dir["#{config.root}/lib"]
     config.autoload_paths += Dir["#{config.root}/lib/common_passwords"]
     config.autoload_paths += Dir["#{config.root}/lib/highlight_js"]

config/initializers/000-zeitwerk.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+# This custom inflector is needed because of our jobs directory structure.
+# Ideally, we should not prefix our jobs with a `Jobs` namespace but instead
+# have a `Job` suffix to follow the Rails conventions on naming.
+class DiscourseInflector < Zeitwerk::Inflector
+  def camelize(basename, abspath)
+    return basename.camelize if abspath.ends_with?("onceoff.rb")
+    return 'Jobs' if abspath.ends_with?("jobs/base.rb")
+    super
+  end
+end
+
+Rails.autoloaders.each do |autoloader|
+  autoloader.inflector = DiscourseInflector.new
+
+  # We have filenames that do not follow Zeitwerk's camelization convention. Maintain an inflections for these files
+  # for now until we decide to fix them one day.
+  autoloader.inflector.inflect(
+    'canonical_url' => 'CanonicalURL',
+    'clean_up_unmatched_ips' => 'CleanUpUnmatchedIPs',
+    'homepage_constraint' => 'HomePageConstraint',
+    'ip_addr' => 'IPAddr',
+    'onpdiff' => 'ONPDiff',
+    'pop3_polling_enabled_setting_validator' => 'POP3PollingEnabledSettingValidator',
+    'version' => 'Discourse',
+    'onceoff' => 'Jobs',
+    'regular' => 'Jobs',
+    'scheduled' => 'Jobs',
+  )
+end

config/initializers/100-sidekiq.rb

@@ -1,126 +1,119 @@
 # frozen_string_literal: true
 
-# Ensure that scheduled jobs are loaded before mini_scheduler is configured.
-if Rails.env == "development"
-  require "jobs/base"
+Rails.application.reloader.to_prepare do
+  require "sidekiq/pausable"
 
-  Dir.glob("#{Rails.root}/app/jobs/scheduled/*.rb") do |f|
-    require(f)
-  end
-end
-
-require "sidekiq/pausable"
-
-Sidekiq.configure_client do |config|
-  config.redis = Discourse.sidekiq_redis_config
-end
-
-Sidekiq.configure_server do |config|
-  config.redis = Discourse.sidekiq_redis_config
-
-  config.server_middleware do |chain|
-    chain.add Sidekiq::Pausable
-  end
-end
-
-MiniScheduler.configure do |config|
-
-  config.redis = Discourse.redis
-
-  config.job_exception_handler do |ex, context|
-    Discourse.handle_job_exception(ex, context)
+  Sidekiq.configure_client do |config|
+    config.redis = Discourse.sidekiq_redis_config
   end
 
-  config.job_ran do |stat|
-    DiscourseEvent.trigger(:scheduled_job_ran, stat)
+  Sidekiq.configure_server do |config|
+    config.redis = Discourse.sidekiq_redis_config
+
+    config.server_middleware do |chain|
+      chain.add Sidekiq::Pausable
+    end
   end
 
-  config.skip_schedule { Sidekiq.paused? }
+  MiniScheduler.configure do |config|
+
+    config.redis = Discourse.redis
+
+    config.job_exception_handler do |ex, context|
+      Discourse.handle_job_exception(ex, context)
+    end
+
+    config.job_ran do |stat|
+      DiscourseEvent.trigger(:scheduled_job_ran, stat)
+    end
+
+    config.skip_schedule { Sidekiq.paused? }
+
+    config.before_sidekiq_web_request do
+      RailsMultisite::ConnectionManagement.establish_connection(
+        db: RailsMultisite::ConnectionManagement::DEFAULT
+      )
+    end
 
-  config.before_sidekiq_web_request do
-    RailsMultisite::ConnectionManagement.establish_connection(
-      db: RailsMultisite::ConnectionManagement::DEFAULT
-    )
   end
 
-end
+  if Sidekiq.server?
 
-if Sidekiq.server?
+    module Sidekiq
+      class CLI
+        private
 
-  module Sidekiq
-    class CLI
-      private
+        def print_banner
+          # banner takes up too much space
+        end
+      end
+    end
 
-      def print_banner
-        # banner takes up too much space
+    # defer queue should simply run in sidekiq
+    Scheduler::Defer.async = false
+
+    Rails.application.config.after_initialize do
+      # warm up AR
+      RailsMultisite::ConnectionManagement.safe_each_connection do
+        (ActiveRecord::Base.connection.tables - %w[schema_migrations versions]).each do |table|
+          table.classify.constantize.first rescue nil
+        end
+      end
+
+      scheduler_hostname = ENV["UNICORN_SCHEDULER_HOSTNAME"]
+
+      if !scheduler_hostname || scheduler_hostname.split(',').include?(Discourse.os_hostname)
+        begin
+          MiniScheduler.start(workers: GlobalSetting.mini_scheduler_workers)
+        rescue MiniScheduler::DistributedMutex::Timeout
+          sleep 5
+          retry
+        end
       end
     end
   end
 
-  # defer queue should simply run in sidekiq
-  Scheduler::Defer.async = false
+  # Sidekiq#logger= applies patches to whichever logger we pass it.
+  # Therefore something like Sidekiq.logger = Rails.logger will break
+  # all logging in the application.
+  #
+  # Instead, this patch adds a dedicated logger instance and patches
+  # the #add method to forward messages to Rails.logger.
+  Sidekiq.logger = Logger.new(nil)
+  Sidekiq.logger.define_singleton_method(:add) do |severity, message = nil, progname = nil, &blk|
+    Rails.logger.add(severity, message, progname, &blk)
+  end
 
-  Rails.application.config.after_initialize do
-    # warm up AR
-    RailsMultisite::ConnectionManagement.safe_each_connection do
-      (ActiveRecord::Base.connection.tables - %w[schema_migrations versions]).each do |table|
-        table.classify.constantize.first rescue nil
+  class SidekiqLogsterReporter < Sidekiq::ExceptionHandler::Logger
+    def call(ex, context = {})
+
+      return if Jobs::HandledExceptionWrapper === ex
+      Discourse.reset_active_record_cache_if_needed(ex)
+
+      # Pass context to Logster
+      fake_env = {}
+      context.each do |key, value|
+        Logster.add_to_env(fake_env, key, value)
       end
-    end
 
-    scheduler_hostname = ENV["UNICORN_SCHEDULER_HOSTNAME"]
-
-    if !scheduler_hostname || scheduler_hostname.split(',').include?(Discourse.os_hostname)
-      begin
-        MiniScheduler.start(workers: GlobalSetting.mini_scheduler_workers)
-      rescue MiniScheduler::DistributedMutex::Timeout
-        sleep 5
-        retry
+      text = "Job exception: #{ex}\n"
+      if ex.backtrace
+        Logster.add_to_env(fake_env, :backtrace, ex.backtrace)
       end
+
+      Logster.add_to_env(fake_env, :current_hostname, Discourse.current_hostname)
+
+      Thread.current[Logster::Logger::LOGSTER_ENV] = fake_env
+      Logster.logger.error(text)
+    rescue => e
+      Logster.logger.fatal("Failed to log exception #{ex} #{hash}\nReason: #{e.class} #{e}\n#{e.backtrace.join("\n")}")
+    ensure
+      Thread.current[Logster::Logger::LOGSTER_ENV] = nil
     end
   end
+
+  Sidekiq.error_handlers.clear
+  Sidekiq.error_handlers << SidekiqLogsterReporter.new
+
+  Sidekiq.strict_args!
 end
-
-# Sidekiq#logger= applies patches to whichever logger we pass it.
-# Therefore something like Sidekiq.logger = Rails.logger will break
-# all logging in the application.
-#
-# Instead, this patch adds a dedicated logger instance and patches
-# the #add method to forward messages to Rails.logger.
-Sidekiq.logger = Logger.new(nil)
-Sidekiq.logger.define_singleton_method(:add) do |severity, message = nil, progname = nil, &blk|
-  Rails.logger.add(severity, message, progname, &blk)
-end
-
-class SidekiqLogsterReporter < Sidekiq::ExceptionHandler::Logger
-  def call(ex, context = {})
-
-    return if Jobs::HandledExceptionWrapper === ex
-    Discourse.reset_active_record_cache_if_needed(ex)
-
-    # Pass context to Logster
-    fake_env = {}
-    context.each do |key, value|
-      Logster.add_to_env(fake_env, key, value)
-    end
-
-    text = "Job exception: #{ex}\n"
-    if ex.backtrace
-      Logster.add_to_env(fake_env, :backtrace, ex.backtrace)
-    end
-
-    Logster.add_to_env(fake_env, :current_hostname, Discourse.current_hostname)
-
-    Thread.current[Logster::Logger::LOGSTER_ENV] = fake_env
-    Logster.logger.error(text)
-  rescue => e
-    Logster.logger.fatal("Failed to log exception #{ex} #{hash}\nReason: #{e.class} #{e}\n#{e.backtrace.join("\n")}")
-  ensure
-    Thread.current[Logster::Logger::LOGSTER_ENV] = nil
-  end
-end
-
-Sidekiq.error_handlers.clear
-Sidekiq.error_handlers << SidekiqLogsterReporter.new
-
-Sidekiq.strict_args!

lib/require_dependency_backward_compatibility.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+# Patch `require_dependency` to maintain backward compatibility with some
+# plugins. Calls to `require_dependency` are deprecated and we should remove
+# them whenever possible.
+#
+# Here we do nothing if `jobs/base` is required since all our jobs are
+# autoloaded through Zeitwerk. Requiring explicitly `jobs/base` actually breaks
+# the app with the “new” autoloader.
+# `lib` should not appear in a path that is required but we had probably a bug
+# at some point regarding this matter so we need to maintain compatibility with
+# some plugins that rely on this.
+module RequireDependencyBackwardCompatibility
+  def require_dependency(filename)
+    name = filename.to_s
+    return if name == 'jobs/base'
+    return super(name.sub(/^lib\//, '')) if name.start_with?('lib/')
+    super
+  end
+
+  ActiveSupport::Dependencies::ZeitwerkIntegration::RequireDependency.prepend(self)
+end

lib/zeitwerk_config.rb

@@ -1,22 +0,0 @@
-# frozen_string_literal: true
-
-module ActiveSupport::Dependencies::ZeitwerkIntegration::Inflector
-  CUSTOM_PATHS = {
-    'canonical_url' => 'CanonicalURL',
-    'clean_up_unmatched_ips' => 'CleanUpUnmatchedIPs',
-    'homepage_constraint' => 'HomePageConstraint',
-    'ip_addr' => 'IPAddr',
-    'onpdiff' => 'ONPDiff',
-    'onceoff' => 'Jobs',
-    'pop3_polling_enabled_setting_validator' => 'POP3PollingEnabledSettingValidator',
-    'regular' => 'Jobs',
-    'scheduled' => 'Jobs',
-    'version' => 'Discourse',
-  }
-
-  def self.camelize(basename, abspath)
-    return basename.camelize if abspath.ends_with?("onceoff.rb")
-    return 'Jobs' if abspath.ends_with?("jobs/base.rb")
-    CUSTOM_PATHS.fetch(basename, basename.camelize)
-  end
-end

spec/jobs/create_linked_topic_spec.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require 'jobs/regular/create_linked_topic'
-
 describe Jobs::CreateLinkedTopic do
 
   it "returns when the post cannot be found" do

spec/jobs/feature_topic_users_spec.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require 'jobs/regular/process_post'
-
 describe Jobs::FeatureTopicUsers do
 
   it "raises an error without a topic_id" do

spec/jobs/process_post_spec.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require 'jobs/regular/process_post'
-
 describe Jobs::ProcessPost do
 
   it "returns when the post cannot be found" do

spec/jobs/pull_hotlinked_images_spec.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require 'jobs/regular/pull_hotlinked_images'
-
 describe Jobs::PullHotlinkedImages do
 
   let(:image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat1.png" }

spec/jobs/send_system_message_spec.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require 'jobs/regular/send_system_message'
-
 describe Jobs::SendSystemMessage do
 
   it "raises an error without a user_id" do