Move sprite hashing out of module import time

This speeds up application startup. The hash is now a query param, injected in the template. As this param is only needed for cache invalidation, it's optional. A helper method is provided to generate the URL, along with a template tag. This also migrates to an `lru_cache` over a global variable for simplicity. Fixes wagtail#11680
albinazs · Jul 8, 2024 · 35a197d · 35a197d
1 parent d00df53
commit 35a197d
Show file tree

Hide file tree

Showing 8 changed files with 65 additions and 68 deletions.
diff --git a/CHANGELOG.txt b/CHANGELOG.txt
@@ -57,6 +57,7 @@ Changelog
  * Maintenance: Remove unused `docs/autobuild.sh` script (Sævar Öfjörð Magnússon)
  * Maintenance: Replace `urlparse` with `urlsplit` to improve performance (Jake Howard)
  * Maintenance: Optimise embed finder lookups (Jake Howard)
+ * Maintenance: Improve performance of initial admin loading by moving sprite hashing out of module import time (Jake Howard)
 
 
 6.1.2 (30.05.2024)

diff --git a/docs/releases/6.2.md b/docs/releases/6.2.md
@@ -81,6 +81,7 @@ This feature was implemented by Albina Starykova, with support from the Wagtail
  * Remove unused `docs/autobuild.sh` script (Sævar Öfjörð Magnússon)
  * Replace `urlparse` with `urlsplit` to improve performance (Jake Howard)
  * Optimise embed finder lookups (Jake Howard)
+ * Improve performance of initial admin loading by moving sprite hashing out of module import time (Jake Howard)
 
 
 ## Upgrade considerations - changes affecting all projects

diff --git a/wagtail/admin/icons.py b/wagtail/admin/icons.py
@@ -0,0 +1,43 @@
+import hashlib
+import itertools
+import re
+from functools import lru_cache
+
+from django.conf import settings
+from django.template.loader import render_to_string
+from django.urls import reverse
+
+from wagtail import hooks
+
+icon_comment_pattern = re.compile(r"<!--.*?-->")
+
+
+@lru_cache(maxsize=None)
+def get_icons():
+    icon_hooks = hooks.get_hooks("register_icons")
+    all_icons = sorted(itertools.chain.from_iterable(hook([]) for hook in icon_hooks))
+    combined_icon_markup = ""
+    for icon in all_icons:
+        symbol = (
+            render_to_string(icon)
+            .replace('xmlns="http://www.w3.org/2000/svg"', "")
+            .replace("svg", "symbol")
+        )
+        symbol = icon_comment_pattern.sub("", symbol)
+        combined_icon_markup += symbol
+
+    return render_to_string(
+        "wagtailadmin/shared/icons.html", {"icons": combined_icon_markup}
+    )
+
+
+@lru_cache(maxsize=None)
+def get_icon_sprite_hash():
+    # SECRET_KEY is used to prevent exposing the Wagtail version
+    return hashlib.sha1(
+        (get_icons() + settings.SECRET_KEY).encode("utf-8")
+    ).hexdigest()[:8]
+
+
+def get_icon_sprite_url():
+    return reverse("wagtailadmin_sprite") + f"?h={get_icon_sprite_hash()}"
diff --git a/wagtail/admin/templates/wagtailadmin/skeleton.html b/wagtail/admin/templates/wagtailadmin/skeleton.html
@@ -19,7 +19,7 @@
     <body id="wagtail" class="{% classnames bodyclass sidebar_collapsed|yesno:"sidebar-collapsed," messages|yesno:"has-messages," %}" data-controller="w-init" data-w-init-ready-class="ready">
         <div data-sprite></div>
 
-        <script src="{% versioned_static 'wagtailadmin/js/icons.js' %}" data-icon-url="{% url 'wagtailadmin_sprite' %}"></script>
+        <script src="{% versioned_static 'wagtailadmin/js/icons.js' %}" data-icon-url="{% icon_sprite_url %}"></script>
 
         <noscript class="capabilitymessage">
             {% blocktrans trimmed %}

diff --git a/wagtail/admin/templatetags/wagtailadmin_tags.py b/wagtail/admin/templatetags/wagtailadmin_tags.py
@@ -29,6 +29,7 @@
 
 from wagtail import hooks
 from wagtail.admin.admin_url_finder import AdminURLFinder
+from wagtail.admin.icons import get_icon_sprite_url
 from wagtail.admin.localization import get_js_translation_strings
 from wagtail.admin.menu import admin_menu
 from wagtail.admin.search import admin_search_areas
@@ -1377,3 +1378,6 @@ def human_readable_date(date, description=None, placement="top"):
 # Shadow the laces `component` tag which was extracted from Wagtail. The shadowing
 # is useful to avoid having to update all the templates that use the `component` tag.
 register.tag("component", component)
+
+
+register.simple_tag(get_icon_sprite_url, name="icon_sprite_url")
diff --git a/wagtail/admin/tests/test_icon_sprite.py b/wagtail/admin/tests/test_icon_sprite.py
@@ -1,26 +1,17 @@
-import re
+from django.test import SimpleTestCase
 
-from django.test import TestCase
-from django.urls import reverse
+from wagtail.admin.icons import get_icon_sprite_hash, get_icon_sprite_url
 
-from wagtail.admin.urls import get_sprite_hash, sprite_hash
 
+class TestIconSpriteView(SimpleTestCase):
+    def test_content_type(self):
+        response = self.client.get(get_icon_sprite_url())
+        self.assertEqual(
+            response.headers["Content-Type"], "image/svg+xml; charset=utf-8"
+        )
+        self.assertEqual(response.wsgi_request.GET["h"], get_icon_sprite_hash())
 
-class TestIconSprite(TestCase):
-    def test_get_sprite_hash(self):
-        result = get_sprite_hash()
-        self.assertTrue(bool(re.match(r"^[a-z0-9]{8}$", result)))
-
-    def test_hash_var(self):
-        self.assertIsInstance(sprite_hash, str)
-        self.assertEqual(len(sprite_hash), 8)
-
-    def test_url(self):
-        url = reverse("wagtailadmin_sprite")
-        self.assertEqual(url[:14], "/admin/sprite-")
 
-    def test_view(self):
-        response = self.client.get(reverse("wagtailadmin_sprite"))
-        self.assertIn(
-            "Content-Type: text/html; charset=utf-8", str(response.serialize_headers())
-        )
+class TestIconSpriteHash(SimpleTestCase):
+    def test_hash(self):
+        self.assertEqual(len(get_icon_sprite_hash()), 8)
diff --git a/wagtail/admin/urls/__init__.py b/wagtail/admin/urls/__init__.py
@@ -1,5 +1,4 @@
 import functools
-import hashlib
 
 from django.conf import settings
 from django.http import Http404
@@ -132,23 +131,10 @@
 # Add "wagtailadmin.access_admin" permission check
 urlpatterns = decorate_urlpatterns(urlpatterns, require_admin_access)
 
-sprite_hash = None
-
-
-def get_sprite_hash():
-    global sprite_hash
-    if not sprite_hash:
-        content = str(home.sprite(None).content, "utf-8")
-        # SECRET_KEY is used to prevent exposing the Wagtail version
-        sprite_hash = hashlib.sha1(
-            (content + settings.SECRET_KEY).encode("utf-8")
-        ).hexdigest()[:8]
-    return sprite_hash
-
 
 # These url patterns do not require an authenticated admin user
 urlpatterns += [
-    path(f"sprite-{get_sprite_hash()}/", home.sprite, name="wagtailadmin_sprite"),
+    path("sprite/", home.sprite, name="wagtailadmin_sprite"),
     path("login/", account.LoginView.as_view(), name="wagtailadmin_login"),
     # Password reset
     path("password_reset/", include(wagtailadmin_password_reset_urls)),

diff --git a/wagtail/admin/views/home.py b/wagtail/admin/views/home.py
@@ -1,5 +1,3 @@
-import itertools
-import re
 from typing import Any, Mapping, Union
 
 from django.conf import settings
@@ -9,11 +7,11 @@
 from django.db.models.functions import Cast
 from django.forms import Media
 from django.http import Http404, HttpResponse
-from django.template.loader import render_to_string
 from django.utils.translation import gettext_lazy
 from django.views.generic.base import TemplateView
 
 from wagtail import hooks
+from wagtail.admin.icons import get_icons
 from wagtail.admin.navigation import get_site_for_user
 from wagtail.admin.site_summary import SiteSummaryPanel
 from wagtail.admin.ui.components import Component
@@ -350,32 +348,5 @@ def default(request):
     raise Http404
 
 
-icon_comment_pattern = re.compile(r"<!--.*?-->")
-_icons_html = None
-
-
-def icons():
-    global _icons_html
-    if _icons_html is None:
-        icon_hooks = hooks.get_hooks("register_icons")
-        all_icons = sorted(
-            itertools.chain.from_iterable(hook([]) for hook in icon_hooks)
-        )
-        combined_icon_markup = ""
-        for icon in all_icons:
-            symbol = (
-                render_to_string(icon)
-                .replace('xmlns="http://www.w3.org/2000/svg"', "")
-                .replace("svg", "symbol")
-            )
-            symbol = icon_comment_pattern.sub("", symbol)
-            combined_icon_markup += symbol
-
-        _icons_html = render_to_string(
-            "wagtailadmin/shared/icons.html", {"icons": combined_icon_markup}
-        )
-    return _icons_html
-
-
 def sprite(request):
-    return HttpResponse(icons())
+    return HttpResponse(get_icons(), content_type="image/svg+xml; charset=utf-8")