DEV: Refactor ApplicationHelper#server_plugin_outlet take 2 (#36418)

Take 1 reverted in
https://github.com/discourse/discourse/commit/dc97318ad12c16d28d2e80b944278aaed2c10d24
as it ended up causing a memory leak.
This commit is contained in:
Alan Guo Xiang Tan
2025-12-03 15:55:39 +08:00
committed by GitHub
parent 46b9a5903e
commit 6a4ea0a7b9
8 changed files with 58 additions and 55 deletions
+30 -28
View File
@@ -516,40 +516,42 @@ module ApplicationHelper
CategoryBadge.html_for(category, opts).html_safe
end
def self.all_connectors
@all_connectors = Dir.glob("plugins/*/app/views/connectors/**/*.html.erb")
SERVER_PLUGIN_OUTLET_PLUGINS_PREFIXES = [Rails.root.join("plugins/").to_s]
private_constant :SERVER_PLUGIN_OUTLET_PLUGINS_PREFIXES
if Rails.env.test?
SERVER_PLUGIN_OUTLET_PLUGINS_PREFIXES << Rails.root.join("spec/fixtures/plugins/").to_s
end
PLUGIN_OUTLET_TEMPLATE_CACHE = Concurrent::Map.new
SERVER_PLUGIN_OUTLET_CONNECTOR_TEMPLATES =
SERVER_PLUGIN_OUTLET_PLUGINS_PREFIXES.each_with_object({}) do |plugins_prefix, connectors|
Dir
.glob("#{plugins_prefix}*/app/views/connectors/**/*.html.erb")
.each do |template_path|
template_path =~ Regexp.new("/connectors/(.*)/.*\.html\.erb$")
outlet_name = Regexp.last_match(1)
connectors[outlet_name] ||= []
connectors[outlet_name] << begin
ActionView::Template.new(
File.read(template_path),
"discourse_plugin_outlet__#{name}",
ActionView::Template.handler_for_extension("erb"),
locals: [],
format: :html,
virtual_path: template_path,
)
end
end
end
private_constant :SERVER_PLUGIN_OUTLET_CONNECTOR_TEMPLATES
def server_plugin_outlet(name, locals: {})
return "" if !GlobalSetting.load_plugins?
return "" if !SERVER_PLUGIN_OUTLET_CONNECTOR_TEMPLATES.key?(name)
matcher = Regexp.new("/connectors/#{name}/.*\.html\.erb$")
erbs = ApplicationHelper.all_connectors.select { |c| c =~ matcher }
return "" if erbs.blank?
erbs
.map do |erb|
cache_key = [erb, locals.keys.sort]
template =
PLUGIN_OUTLET_TEMPLATE_CACHE.compute_if_absent(cache_key) do
source = File.read(erb)
handler = ActionView::Template.handler_for_extension("erb")
ActionView::Template.new(
source,
"discourse_plugin_outlet__#{name}",
handler,
locals: locals.keys,
format: :html,
virtual_path: erb,
)
end
render template: template, locals: locals
end
SERVER_PLUGIN_OUTLET_CONNECTOR_TEMPLATES[name]
.map { |template| render template:, locals: }
.join
.html_safe
end
-5
View File
@@ -1,5 +0,0 @@
# frozen_string_literal: true
if Rails.env.development?
Rails.application.reloader.to_prepare { ApplicationHelper::PLUGIN_OUTLET_TEMPLATE_CACHE.clear }
end
@@ -0,0 +1 @@
Fixture from my_plugin template 1: <%= @topic_view.topic.title %>
@@ -0,0 +1 @@
Fixture from my_plugin template 2: <%= @topic_view.topic.title %>
@@ -0,0 +1 @@
Fixture from my_plugin_2 template 1: <%= @topic_view.topic.title %>
@@ -0,0 +1 @@
Fixture from my_plugin_2 template 2: <%= @topic_view.topic.title %>
@@ -0,0 +1 @@
Fixture from my_plugin_3 template 1: <%= @topic_view.topic.title %>
+23 -22
View File
@@ -40,34 +40,35 @@ RSpec.describe TopicsController do
describe "topic_header plugin outlet" do
fab!(:another_topic) { Fabricate(:topic, title: "Another topic by me") }
let(:tmp_dir) { Dir.mktmpdir }
let(:template_dir) do
path = File.join(tmp_dir, "connectors", "topic_header")
FileUtils.mkdir_p(path)
path
end
let(:template_file) do
file = Tempfile.new(%w[test_template .html.erb], template_dir)
file.write("Topic title from outlet: <%= @topic_view.topic.title %>")
file.close
file
end
before { global_setting(:load_plugins?, true) }
before do
global_setting(:load_plugins?, true)
ApplicationHelper.stubs(:all_connectors).returns([template_file.path])
end
after { FileUtils.remove_entry(tmp_dir) if tmp_dir }
it "doesn't leak state between requests" do
it "renders the connector templates from multiple plugins" do
get "/t/#{topic.slug}/#{topic.id}"
expect(response.status).to eq(200)
expect(response.body).to include("Topic title from outlet: #{topic.title}")
expect(response.body).to include("Fixture from my_plugin template 1: #{topic.title}")
expect(response.body).to include("Fixture from my_plugin template 2: #{topic.title}")
expect(response.body).to include("Fixture from my_plugin_2 template 1: #{topic.title}")
expect(response.body).to include("Fixture from my_plugin_2 template 2: #{topic.title}")
expect(response.body).not_to include("Fixture from my_plugin_3 template 1: #{topic.title}")
get "/t/#{another_topic.slug}/#{another_topic.id}"
expect(response.status).to eq(200)
expect(response.body).to include("Topic title from outlet: #{another_topic.title}")
expect(response.body).to include("Fixture from my_plugin template 1: #{another_topic.title}")
expect(response.body).to include("Fixture from my_plugin template 2: #{another_topic.title}")
expect(response.body).to include(
"Fixture from my_plugin_2 template 1: #{another_topic.title}",
)
expect(response.body).to include(
"Fixture from my_plugin_2 template 2: #{another_topic.title}",
)
expect(response.body).not_to include(
"Fixture from my_plugin_3 template 1: #{another_topic.title}",
)
end
end