diff --git a/app/controllers/extra_locales_controller.rb b/app/controllers/extra_locales_controller.rb index 1b5fc059514..e1e49fb5b7d 100644 --- a/app/controllers/extra_locales_controller.rb +++ b/app/controllers/extra_locales_controller.rb @@ -12,30 +12,36 @@ class ExtraLocalesController < ApplicationController OVERRIDES_BUNDLE = "overrides" MD5_HASH_LENGTH = 32 MF_BUNDLE = "mf" - BUNDLES = [OVERRIDES_BUNDLE, MF_BUNDLE] + ADMIN_BUNDLE = "admin" + WIZARD_BUNDLE = "wizard" + + SITE_SPECIFIC_BUNDLES = [OVERRIDES_BUNDLE, MF_BUNDLE] + SHARED_BUNDLES = [ADMIN_BUNDLE, WIZARD_BUNDLE] class << self def js_digests - @js_digests ||= {} + @js_digests ||= { site_specific: {}, shared: {} } end def bundle_js_hash(bundle) bundle_key = "#{bundle}_#{I18n.locale}" - if bundle.in?(BUNDLES) + if bundle.in?(SITE_SPECIFIC_BUNDLES) site = RailsMultisite::ConnectionManagement.current_db - js_digests[site] ||= {} - js_digests[site][bundle_key] ||= begin + js_digests[:site_specific][site] ||= {} + js_digests[:site_specific][site][bundle_key] ||= begin js = bundle_js(bundle) js.present? ? Digest::MD5.hexdigest(js) : nil end + elsif bundle.in?(SHARED_BUNDLES) + js_digests[:shared][bundle_key] ||= Digest::MD5.hexdigest(bundle_js(bundle)) else - js_digests[bundle_key] ||= Digest::MD5.hexdigest(bundle_js(bundle)) + raise "Unknown bundle: #{bundle}" end end def url(bundle) - "#{Discourse.base_path}/extra-locales/#{bundle}?v=#{bundle_js_hash(bundle)}" + "#{GlobalSetting.cdn_url}#{Discourse.base_path}/extra-locales/#{bundle}?v=#{bundle_js_hash(bundle)}" end def client_overrides_exist? @@ -63,7 +69,7 @@ class ExtraLocalesController < ApplicationController def clear_cache! site = RailsMultisite::ConnectionManagement.current_db - js_digests.delete(site) + js_digests[:site_specific].delete(site) end end @@ -85,7 +91,6 @@ class ExtraLocalesController < ApplicationController private def valid_bundle?(bundle) - return true if bundle.in?(BUNDLES) - bundle =~ /\A(admin|wizard)\z/ && (current_user&.staff? || Rails.env.local?) + bundle.in?(SITE_SPECIFIC_BUNDLES) || bundle.in?(SHARED_BUNDLES) end end diff --git a/config/nginx.sample.conf b/config/nginx.sample.conf index be7d1fa04f8..cda41bb3a9e 100644 --- a/config/nginx.sample.conf +++ b/config/nginx.sample.conf @@ -232,7 +232,7 @@ server { # This big block is needed so we can selectively enable # acceleration for backups, avatars, sprites and so on. # see note about repetition above - location ~ ^/(svg-sprite/|letter_avatar/|letter_avatar_proxy/|user_avatar|highlight-js|stylesheets|theme-javascripts|favicon/proxied|service-worker|extra-locales/(mf|overrides)) { + location ~ ^/(svg-sprite/|letter_avatar/|letter_avatar_proxy/|user_avatar|highlight-js|stylesheets|theme-javascripts|favicon/proxied|service-worker|extra-locales/) { proxy_set_header Host $http_host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Request-Start "t=${msec}"; diff --git a/spec/requests/extra_locales_controller_spec.rb b/spec/requests/extra_locales_controller_spec.rb index be4c910b2ee..098aa59256a 100644 --- a/spec/requests/extra_locales_controller_spec.rb +++ b/spec/requests/extra_locales_controller_spec.rb @@ -14,15 +14,6 @@ RSpec.describe ExtraLocalesController do expect(response.status).to eq(403) end - it "requires staff access in production" do - Rails.env.stubs(:local?).returns(false) - get "/extra-locales/admin" - expect(response.status).to eq(403) - - get "/extra-locales/wizard" - expect(response.status).to eq(403) - end - it "requires a valid version" do get "/extra-locales/overrides", params: { v: "a" } expect(response.status).to eq(400) @@ -31,54 +22,49 @@ RSpec.describe ExtraLocalesController do expect(response.status).to eq(400) end - context "when logged in as a moderator" do - let(:moderator) { Fabricate(:moderator) } - before { sign_in(moderator) } + it "caches for 1 year if version is provided and it matches current hash" do + get "/extra-locales/admin", params: { v: ExtraLocalesController.bundle_js_hash("admin") } + expect(response.status).to eq(200) + expect(response.headers["Cache-Control"]).to eq("max-age=31556952, public, immutable") + end - it "caches for 1 year if version is provided and it matches current hash" do - get "/extra-locales/admin", params: { v: ExtraLocalesController.bundle_js_hash("admin") } - expect(response.status).to eq(200) - expect(response.headers["Cache-Control"]).to eq("max-age=31556952, public, immutable") - end + it "does not cache at all if version is invalid" do + get "/extra-locales/admin", params: { v: "a" * 32 } + expect(response.status).to eq(200) + expect(response.headers["Cache-Control"]).not_to include("max-age", "public", "immutable") + end - it "does not cache at all if version is invalid" do - get "/extra-locales/admin", params: { v: "a" * 32 } - expect(response.status).to eq(200) - expect(response.headers["Cache-Control"]).not_to include("max-age", "public", "immutable") - end + it "doesn’t generate the bundle twice" do + described_class.expects(:bundle_js).returns("JS").once + get "/extra-locales/admin", params: { v: "a" * 32 } + end - it "doesn’t generate the bundle twice" do - described_class.expects(:bundle_js).returns("JS").once - get "/extra-locales/admin", params: { v: "a" * 32 } - end - - context "with plugin" do - before do - JsLocaleHelper.clear_cache! - JsLocaleHelper - .expects(:plugin_translations) - .with(any_of("en", "en_GB")) - .returns( - "admin_js" => { - "admin" => { - "site_settings" => { - "categories" => { - "github_badges" => "GitHub Badges", - }, + context "with plugin" do + before do + JsLocaleHelper.clear_cache! + JsLocaleHelper + .expects(:plugin_translations) + .with(any_of("en", "en_GB")) + .returns( + "admin_js" => { + "admin" => { + "site_settings" => { + "categories" => { + "github_badges" => "GitHub Badges", }, }, }, - ) - .at_least_once - end + }, + ) + .at_least_once + end - after { JsLocaleHelper.clear_cache! } + after { JsLocaleHelper.clear_cache! } - it "includes plugin translations" do - get "/extra-locales/admin" - expect(response.status).to eq(200) - expect(response.body.include?("github_badges")).to eq(true) - end + it "includes plugin translations" do + get "/extra-locales/admin" + expect(response.status).to eq(200) + expect(response.body.include?("github_badges")).to eq(true) end end @@ -251,4 +237,22 @@ RSpec.describe ExtraLocalesController do ) end end + + describe ".url" do + it "works" do + expect(ExtraLocalesController.url("admin")).to start_with("/extra-locales/admin?v=") + end + + it "includes subfolder path" do + set_subfolder "/forum" + expect(ExtraLocalesController.url("admin")).to start_with("/forum/extra-locales/admin?v=") + end + + it "includes CDN" do + set_cdn_url "https://cdn.example.com" + expect(ExtraLocalesController.url("admin")).to start_with( + "https://cdn.example.com/extra-locales/admin?v=", + ) + end + end end