FIX: Don’t apply callbacks from disabled plugins (#35630) (#35703)

Flink · web-flow · commit 372f1f09009f · 2025-10-30T12:44:28.000+01:00
Currently, when a plugin is disabled, its various callbacks are still
taken into account. This can lead to performance issues.

This patch adds a check to most of the `register_*` methods a plugin can
use, so if the plugin is disabled, those callbacks won&rsquo;t be applied.
diff --git a/app/models/site.rb b/app/models/site.rb
@@ -25,8 +25,8 @@ def self.reset_preloaded_category_custom_fields
   cattr_accessor :markdown_additional_options
   self.markdown_additional_options = {}
 
-  def self.add_categories_callbacks(&block)
-    categories_callbacks << block
+  def self.add_categories_callbacks(enabled: -> { true }, &block)
+    categories_callbacks << { block:, enabled: }
   end
 
   def self.categories_callbacks
@@ -178,7 +178,10 @@ def categories
 
         categories.reject! { |c| c[:parent_category_id] && !by_id[c[:parent_category_id]] }
 
-        self.class.categories_callbacks.each { |callback| callback.call(categories, @guardian) }
+        self.class.categories_callbacks.each do |callback|
+          next unless callback[:enabled].call
+          callback[:block].call(categories, @guardian)
+        end
 
         categories
       end
diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb
@@ -21,14 +21,15 @@ def self.preload(topics, object)
     @preload.each { |preload| preload.call(topics, object) } if @preload
   end
 
-  def self.on_preload_user_ids(&blk)
-    (@preload_user_ids ||= Set.new) << blk
+  def self.on_preload_user_ids(enabled: -> { true }, &block)
+    (@preload_user_ids ||= Set.new) << { block:, enabled: }
   end
 
   def self.preload_user_ids(topics, user_ids, object)
     if @preload_user_ids
       @preload_user_ids.each do |preload_user_ids|
-        user_ids = preload_user_ids.call(topics, user_ids, object)
+        next unless preload_user_ids[:enabled].call
+        user_ids = preload_user_ids[:block].call(topics, user_ids, object)
       end
     end
     user_ids
diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb
@@ -293,31 +293,31 @@ def add_filter_custom_filter(name, &block)
   #     scope.where("word_count = 42")
   #   end
   def register_custom_filter_by_status(status, &block)
-    TopicsFilter.add_filter_by_status(status, &block)
+    TopicsFilter.add_filter_by_status(status, enabled: method(:enabled?), &block)
   end
 
   # Allows to define custom search order. Example usage:
   #   Search.advanced_order(:chars) do |posts|
   #     posts.reorder("(SELECT LENGTH(raw) FROM posts WHERE posts.topic_id = subquery.topic_id) DESC")
   #   end
   def register_search_advanced_order(trigger, &block)
-    Search.advanced_order(trigger, &block)
+    Search.advanced_order(trigger, enabled: method(:enabled?), &block)
   end
 
   # Allows to define custom search filters. Example usage:
   #   Search.advanced_filter(/^min_chars:(\d+)$/) do |posts, match|
   #     posts.where("(SELECT LENGTH(p2.raw) FROM posts p2 WHERE p2.id = posts.id) >= ?", match.to_i)
   #   end
   def register_search_advanced_filter(trigger, &block)
-    Search.advanced_filter(trigger, &block)
+    Search.advanced_filter(trigger, enabled: method(:enabled?), &block)
   end
 
   # Allows to define TopicView posts filters. Example usage:
   #   TopicView.advanced_filter do |posts, opts|
   #     posts.where(wiki: true)
   #   end
   def register_topic_view_posts_filter(trigger, &block)
-    TopicView.add_custom_filter(trigger, &block)
+    TopicView.add_custom_filter(trigger, enabled: method(:enabled?), &block)
   end
 
   # Allows to add more user IDs to the list of preloaded users. This can be
@@ -327,7 +327,7 @@ def register_topic_view_posts_filter(trigger, &block)
   #     user_ids << Discourse::SYSTEM_USER_ID
   #   end
   def register_topic_list_preload_user_ids(&block)
-    TopicList.on_preload_user_ids(&block)
+    TopicList.on_preload_user_ids(enabled: method(:enabled?), &block)
   end
 
   # Allow to eager load additional tables in Search. Useful to avoid N+1 performance problems.
@@ -338,7 +338,7 @@ def register_topic_list_preload_user_ids(&block)
   # OR
   #   register_search_topic_eager_load(%i(example_table))
   def register_search_topic_eager_load(tables = nil, &block)
-    Search.custom_topic_eager_load(tables, &block)
+    Search.custom_topic_eager_load(tables, enabled: method(:enabled?), &block)
   end
 
   # Request a new size for topic thumbnails
@@ -359,7 +359,7 @@ def register_topic_thumbnail_size(size)
   #     end
   #   end
   def register_site_categories_callback(&block)
-    Site.add_categories_callbacks(&block)
+    Site.add_categories_callbacks(enabled: method(:enabled?), &block)
   end
 
   def register_upload_unused(&block)
@@ -736,8 +736,7 @@ def register_html_builder(name, &block)
   end
 
   def register_email_poller(poller)
-    plugin = self
-    DiscoursePluginRegistry.register_mail_poller(poller) if plugin.enabled?
+    DiscoursePluginRegistry.register_mail_poller(poller) if enabled?
   end
 
   def register_asset(file, opts = nil)
diff --git a/lib/search.rb b/lib/search.rb
@@ -365,24 +365,24 @@ def execute(readonly_mode: Discourse.readonly_mode?)
     @results
   end
 
-  def self.advanced_order(trigger, &block)
-    advanced_orders[trigger] = block
+  def self.advanced_order(trigger, enabled: -> { true }, &block)
+    advanced_orders[trigger] = { block:, enabled: }
   end
 
   def self.advanced_orders
     @advanced_orders ||= {}
   end
 
-  def self.advanced_filter(trigger, name: nil, &block)
-    advanced_filters[trigger] = { block:, name: }
+  def self.advanced_filter(trigger, name: nil, enabled: -> { true }, &block)
+    advanced_filters[trigger] = { block:, name:, enabled: }
   end
 
   def self.advanced_filters
     @advanced_filters ||= {}
   end
 
-  def self.custom_topic_eager_load(tables = nil, &block)
-    (@custom_topic_eager_loads ||= []) << (tables || block)
+  def self.custom_topic_eager_load(tables = nil, enabled: -> { true }, &block)
+    (@custom_topic_eager_loads ||= []) << { tables:, block:, enabled: }
   end
 
   def self.custom_topic_eager_loads
@@ -880,8 +880,8 @@ def apply_order(
     end
 
     if @order
-      advanced_order = Search.advanced_orders&.fetch(@order, nil)
-      posts = advanced_order.call(posts) if advanced_order
+      advanced_order = Search.advanced_orders[@order]
+      posts = advanced_order[:block].call(posts) if advanced_order.try(:[], :enabled).try(:call)
     end
 
     posts
@@ -934,6 +934,7 @@ def process_advanced_search!(term)
         Search.advanced_filters.each do |matcher, options|
           block = options[:block]
           name = options[:name]
+          next unless options[:enabled].call
 
           case_insensitive_matcher =
             Regexp.new(matcher.source, matcher.options | Regexp::IGNORECASE)
@@ -1539,8 +1540,9 @@ def posts_eager_loads(query)
     topic_eager_loads << :tags if SiteSetting.tagging_enabled
 
     Search.custom_topic_eager_loads.each do |custom_loads|
+      next unless custom_loads[:enabled].call
       topic_eager_loads.concat(
-        custom_loads.is_a?(Array) ? custom_loads : custom_loads.call(search_pms: @search_pms).to_a,
+        custom_loads[:tables] || custom_loads[:block].call(search_pms: @search_pms).to_a,
       )
     end
 
diff --git a/lib/topic_view.rb b/lib/topic_view.rb
@@ -76,8 +76,8 @@ def self.allowed_post_custom_fields(user, topic)
     wpcf.flatten.uniq
   end
 
-  def self.add_custom_filter(key, &blk)
-    custom_filters[key] = blk
+  def self.add_custom_filter(key, enabled: -> { true }, &block)
+    custom_filters[key] = { block:, enabled: }
   end
 
   def self.custom_filters
@@ -1017,7 +1017,11 @@ def setup_filtered_posts
     end
 
     if @filter.present? && @filter.to_s != "summary" && TopicView.custom_filters[@filter].present?
-      @filtered_posts = TopicView.custom_filters[@filter].call(@filtered_posts, self)
+      @filtered_posts =
+        TopicView.custom_filters[@filter][:block].call(
+          @filtered_posts,
+          self,
+        ) if TopicView.custom_filters[@filter][:enabled].call
     end
 
     if @best.present?
diff --git a/lib/topics_filter.rb b/lib/topics_filter.rb
@@ -107,8 +107,8 @@ def filter_from_query_string(query_string)
     @scope
   end
 
-  def self.add_filter_by_status(status, &blk)
-    custom_status_filters[status] = blk
+  def self.add_filter_by_status(status, enabled: -> { true }, &block)
+    custom_status_filters[status] = { block:, enabled: }
   end
 
   def self.custom_status_filters
@@ -137,7 +137,7 @@ def filter_status(status:, category_id: nil)
       @scope = @scope.joins(:category).where("NOT categories.read_restricted")
     else
       if custom_filter = TopicsFilter.custom_status_filters[status]
-        @scope = custom_filter.call(@scope)
+        @scope = custom_filter[:block].call(@scope) if custom_filter[:enabled].call
       end
     end
 
diff --git a/plugins/discourse-topic-voting/plugin.rb b/plugins/discourse-topic-voting/plugin.rb
@@ -47,6 +47,8 @@ module ::DiscourseTopicVoting
 
   if TopicQuery.respond_to?(:results_filter_callbacks)
     TopicQuery.results_filter_callbacks << ->(_type, result, user, options) do
+      return result unless SiteSetting.topic_voting_enabled
+
       result = result.includes(:topic_vote_count)
 
       if user
diff --git a/spec/lib/plugin/instance_spec.rb b/spec/lib/plugin/instance_spec.rb
@@ -853,24 +853,60 @@ def scrub(reason, guardian)
   describe "#register_site_categories_callback" do
     fab!(:category)
 
-    it "adds a callback to the Site#categories" do
-      instance = Plugin::Instance.new
-
-      site_guardian = Guardian.new
+    let(:instance) { Plugin::Instance.new }
+    let(:site_guardian) { Guardian.new }
+    let(:site) { Site.new(site_guardian) }
 
+    before do
+      allow(instance).to receive(:enabled?).and_call_original
       instance.register_site_categories_callback do |categories, guardian|
         categories.each { |category| category[:test_field] = "test" }
 
         expect(guardian).to eq(site_guardian)
       end
+    end
 
-      site = Site.new(site_guardian)
-
-      expect(site.categories.first[:test_field]).to eq("test")
-    ensure
+    after do
       Site.clear_cache
       Site.categories_callbacks.clear
     end
+
+    it "adds a callback to the Site#categories" do
+      expect(site.categories.first[:test_field]).to eq("test")
+    end
+
+    context "when the plugin is disabled" do
+      before { allow(instance).to receive(:enabled?).and_return(false) }
+
+      it "does not run the callback" do
+        expect(site.categories.first[:test_field]).to be_blank
+      end
+    end
+  end
+
+  describe "#register_topic_list_preload_user_ids" do
+    subject(:preload_user_ids) { TopicList.preload_user_ids(stub, [], stub) }
+
+    let(:instance) { Plugin::Instance.new }
+
+    before do
+      allow(instance).to receive(:enabled?).and_call_original
+      instance.register_topic_list_preload_user_ids { |topics, user_ids, object| [1] }
+    end
+
+    after { TopicList.instance_variable_set(:@preload_user_ids, nil) }
+
+    it "adds a callback to TopicList.preload_user_ids" do
+      expect(preload_user_ids).to include(1)
+    end
+
+    context "when the plugin is disabled" do
+      before { allow(instance).to receive(:enabled?).and_return(false) }
+
+      it "does not run the callback" do
+        expect(preload_user_ids).to be_empty
+      end
+    end
   end
 
   describe "#register_notification_consolidation_plan" do
diff --git a/spec/lib/search_spec.rb b/spec/lib/search_spec.rb
diff --git a/spec/lib/topic_view_spec.rb b/spec/lib/topic_view_spec.rb
diff --git a/spec/lib/topics_filter_spec.rb b/spec/lib/topics_filter_spec.rb
