DEV: early hints around_action -> after_action (#26423)

Using around_action means `add_early_hint_header` is in the stack for every request, and gets included in the backtrace of any errors.

We can manage with an after_action instead, which avoids adding to the stack depth (and avoids people blaming me for unrelated application errors 😉)
This commit is contained in:
David Taylor 2024-04-04 14:37:44 +01:00 committed by GitHub
parent 2995da2625
commit 39ae85b0e7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 12 additions and 27 deletions

View File

@ -53,7 +53,7 @@ class ApplicationController < ActionController::Base
after_action :add_noindex_header_to_non_canonical, if: :spa_boot_request?
after_action :set_cross_origin_opener_policy_header, if: :spa_boot_request?
after_action :clean_xml, if: :is_feed_response?
around_action :add_early_hint_header, if: -> { spa_boot_request? }
after_action :add_early_hint_header, if: -> { spa_boot_request? }
HONEYPOT_KEY ||= "HONEYPOT_KEY"
CHALLENGE_KEY ||= "CHALLENGE_KEY"
@ -1100,11 +1100,7 @@ class ApplicationController < ActionController::Base
# to cache a response header from the app and use that to send an Early Hint response to future clients.
# See 'early_hint_header_mode' and 'early_hint_header_name' Global Setting descriptions for more info.
def add_early_hint_header
return yield if GlobalSetting.early_hint_header_mode.nil?
@asset_preload_links = []
yield
return if GlobalSetting.early_hint_header_mode.nil?
links = []

View File

@ -158,9 +158,10 @@ module ApplicationHelper
end
def add_resource_preload_list(resource_url, type)
if !@asset_preload_links.nil?
@asset_preload_links << %Q(<#{resource_url}>; rel="preload"; as="#{type}")
end
links =
controller.instance_variable_get(:@asset_preload_links) ||
controller.instance_variable_set(:@asset_preload_links, [])
links << %Q(<#{resource_url}>; rel="preload"; as="#{type}")
end
def discourse_csrf_tags

View File

@ -139,43 +139,31 @@ RSpec.describe ApplicationHelper do
end
describe "add_resource_preload_list" do
it "adds resources to the preload list when it's available" do
@asset_preload_links = []
it "adds resources to the preload list" do
add_resource_preload_list("/assets/start-discourse.js", "script")
add_resource_preload_list("/assets/discourse.css", "style")
expect(@asset_preload_links.size).to eq(2)
end
it "doesn't add resources to the preload list when it's not available" do
@asset_preload_links = nil
add_resource_preload_list("/assets/start-discourse.js", "script")
add_resource_preload_list("/assets/discourse.css", "style")
expect(@asset_preload_links).to eq(nil)
expect(controller.instance_variable_get(:@asset_preload_links).size).to eq(2)
end
it "adds resources to the preload list when preload_script is called" do
@asset_preload_links = []
helper.preload_script("start-discourse")
expect(@asset_preload_links.size).to eq(1)
expect(controller.instance_variable_get(:@asset_preload_links).size).to eq(1)
end
it "adds resources to the preload list when discourse_stylesheet_link_tag is called" do
@asset_preload_links = []
helper.discourse_stylesheet_link_tag(:desktop)
expect(@asset_preload_links.size).to eq(1)
expect(controller.instance_variable_get(:@asset_preload_links).size).to eq(1)
end
it "adds resources as the correct type" do
@asset_preload_links = []
helper.discourse_stylesheet_link_tag(:desktop)
helper.preload_script("start-discourse")
expect(@asset_preload_links[0]).to match(/as="style"/)
expect(@asset_preload_links[1]).to match(/as="script"/)
expect(controller.instance_variable_get(:@asset_preload_links)[0]).to match(/as="style"/)
expect(controller.instance_variable_get(:@asset_preload_links)[1]).to match(/as="script"/)
end
end