PERF: Cache all extra-locale bundles and use CDN (#31445)

Code/translations for the admin panel and wizard are not considered
sensitive, so there's no need for access control checks here. Once
they're removed, we can cache in NGINX and the CDN, and thereby improve
server and client-load performance.

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

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}";

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