From 795e6d72a4a0665997fa72fafd5d7bf71a8a4e4c Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Thu, 30 Mar 2023 14:39:55 +1100
Subject: [PATCH] FEATURE: modifier API for plugins (#20887)

Introduces a new API for plugin data modification without class-based extension overhead.

This commit introduces a new API that allows plugins to modify data in cases where they return different data rather than additional data, as is common with filtered_registers in DiscoursePluginRegistry. This API removes the need for defining class-based extension points.

When a plugin registers a modifier, it will automatically be called if the plugin is enabled. The core will then modify the parameter sent to it using the block registered by the plugin:

```ruby
DiscoursePluginRegistry.register_modifier(plugin_instance, :magic_sum_modifier) { |a, b| a + b }
sum = DiscoursePluginRegistry.apply_modifier(:magic_sum_filter, 1, 2)
expect(sum).to eq(3)
```

Key features of these modifiers:

- Operate in a stack (first registered, first called)
- Automatically disabled when the plugin is disabled
- Pass the cumulative result of all block invocations to the caller
---
 lib/discourse_plugin_registry.rb           | 31 +++++++++++++
 lib/plugin/instance.rb                     |  4 ++
 spec/lib/discourse_plugin_registry_spec.rb | 51 ++++++++++++++++++++++
 spec/lib/plugin/instance_spec.rb           | 13 ++++++
 4 files changed, 99 insertions(+)

diff --git a/lib/discourse_plugin_registry.rb b/lib/discourse_plugin_registry.rb
index d727ab2e625..138b124bfd8 100644
--- a/lib/discourse_plugin_registry.rb
+++ b/lib/discourse_plugin_registry.rb
@@ -246,8 +246,39 @@ class DiscoursePluginRegistry
     asset
   end
 
+  def self.clear_modifiers!
+    @modifiers = nil
+  end
+
+  def self.register_modifier(plugin_instance, name, &blk)
+    @modifiers ||= {}
+    modifiers = @modifiers[name] ||= []
+    modifiers << [plugin_instance, blk]
+  end
+
+  def self.apply_modifier(name, arg, *more_args)
+    return arg if !@modifiers
+
+    registered_modifiers = @modifiers[name]
+    return arg if !registered_modifiers
+
+    # iterate as fast as possible to minimize cost (avoiding each)
+    # also erases one stack frame
+    length = registered_modifiers.length
+    index = 0
+    while index < length
+      plugin_instance, block = registered_modifiers[index]
+      arg = block.call(arg, *more_args) if plugin_instance.enabled?
+
+      index += 1
+    end
+
+    arg
+  end
+
   def self.reset!
     @@register_names.each { |name| instance_variable_set(:"@#{name}", nil) }
+    clear_modifiers!
   end
 
   def self.reset_register!(register_name)
diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb
index ce8ce5e8b11..60eeb014e06 100644
--- a/lib/plugin/instance.rb
+++ b/lib/plugin/instance.rb
@@ -134,6 +134,10 @@ class Plugin::Instance
     end
   end
 
+  def register_modifier(modifier_name, &blk)
+    DiscoursePluginRegistry.register_modifier(self, modifier_name, &blk)
+  end
+
   # Applies to all sites in a multisite environment. Ignores plugin.enabled?
   def add_report(name, &block)
     reloadable_patch { |plugin| Report.add_report(name, &block) }
diff --git a/spec/lib/discourse_plugin_registry_spec.rb b/spec/lib/discourse_plugin_registry_spec.rb
index c74db5e14fc..b37bbd2bf6b 100644
--- a/spec/lib/discourse_plugin_registry_spec.rb
+++ b/spec/lib/discourse_plugin_registry_spec.rb
@@ -258,4 +258,55 @@ RSpec.describe DiscoursePluginRegistry do
       )
     end
   end
+
+  context "with filters" do
+    after { DiscoursePluginRegistry.clear_modifiers! }
+
+    class TestFilterPlugInstance < Plugin::Instance
+      def enabled?
+        !@disabled
+      end
+
+      def enabled=(value)
+        @disabled = !value
+      end
+    end
+
+    let(:plugin_instance) { TestFilterPlugInstance.new }
+    let(:plugin_instance2) { TestFilterPlugInstance.new }
+
+    it "handles modifiers with multiple parameters" do
+      DiscoursePluginRegistry.register_modifier(plugin_instance, :magic_sum_modifier) do |a, b|
+        a + b
+      end
+
+      sum = DiscoursePluginRegistry.apply_modifier(:magic_sum_modifier, 1, 2)
+      expect(sum).to eq(3)
+    end
+
+    it "handles modifier stacking" do
+      # first in, first called
+      DiscoursePluginRegistry.register_modifier(plugin_instance, :stacking) { |x| x == 1 ? 2 : 1 }
+      DiscoursePluginRegistry.register_modifier(plugin_instance2, :stacking) { |x| x + 1 }
+
+      expect(DiscoursePluginRegistry.apply_modifier(:stacking, 1)).to eq(3)
+    end
+
+    it "handles disabled plugins" do
+      plugin_instance.enabled = false
+      DiscoursePluginRegistry.register_modifier(plugin_instance, :magic_sum_modifier) do |a, b|
+        a + b
+      end
+
+      sum = DiscoursePluginRegistry.apply_modifier(:magic_sum_modifier, 1, 2)
+      expect(sum).to eq(1)
+    end
+
+    it "can handle arity mismatch" do
+      DiscoursePluginRegistry.register_modifier(plugin_instance, :magic_sum_modifier) { 42 }
+
+      sum = DiscoursePluginRegistry.apply_modifier(:magic_sum_modifier, 1, 2)
+      expect(sum).to eq(42)
+    end
+  end
 end
diff --git a/spec/lib/plugin/instance_spec.rb b/spec/lib/plugin/instance_spec.rb
index 4f69c994f39..750226c8acf 100644
--- a/spec/lib/plugin/instance_spec.rb
+++ b/spec/lib/plugin/instance_spec.rb
@@ -827,4 +827,17 @@ RSpec.describe Plugin::Instance do
       expect(callback_called).to eq(false)
     end
   end
+
+  describe "#register_modifier" do
+    let(:plugin) { Plugin::Instance.new }
+
+    after { DiscoursePluginRegistry.clear_modifiers! }
+
+    it "allows modifier registration" do
+      plugin.register_modifier(:magic_sum_modifier) { |a, b| a + b }
+
+      sum = DiscoursePluginRegistry.apply_modifier(:magic_sum_modifier, 1, 2)
+      expect(sum).to eq(3)
+    end
+  end
 end