Make sure get_site_root_paths returns a list of SiteRootPath objects (#8819)

Fixes #8062

CHANGELOG.txt

@@ -9,6 +9,7 @@ Changelog
  * Migrate `.button-longrunning` behaviour to a Stimulus controller with support for custom label element & duration (Loveth Omokaro)
  * Fix: Ensure `label_format` on StructBlock gracefully handles missing variables (Aadi jindal)
  * Fix: Adopt a no-JavaScript and more accessible solution for the 'Reset to default' switch to Gravatar when editing user profile (Loveth Omokaro)
+ * Fix: Ensure `Site.get_site_root_paths` works on cache backends that do not preserve Python objects (Jaap Roes)
  * Docs: Add code block to make it easier to understand contribution docs (Suyash Singh)
  * Docs: Add new "Icons" page for icons customisation and reuse across the admin interface (Coen van der Kamp)
  * Maintenance: Removed features deprecated in Wagtail 3.0 and 4.0 (Matt Westcott)

docs/releases/5.0.md

@@ -23,6 +23,7 @@ depth: 1
 
  * Ensure `label_format` on StructBlock gracefully handles missing variables (Aadi jindal)
  * Adopt a no-JavaScript and more accessible solution for the 'Reset to default' switch to Gravatar when editing user profile (Loveth Omokaro)
+ * Ensure `Site.get_site_root_paths` works on cache backends that do not preserve Python objects (Jaap Roes)
 
 ### Documentation
 

wagtail/models/__init__.py

@@ -22,7 +22,6 @@ from django.contrib.auth.models import Group
 from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
 from django.contrib.contenttypes.models import ContentType
 from django.core import checks
-from django.core.cache import cache
 from django.core.exceptions import (
     ImproperlyConfigured,
     PermissionDenied,
@@ -1414,7 +1413,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
         # Note: New translations of existing site roots are considered site roots as well, so we must
         # always check if this page is a site root, even if it's new.
         if self.is_site_root():
-            cache.delete("wagtail_site_root_paths")
+            Site.clear_site_root_paths_cache()
 
         # Log
         if is_new:

wagtail/models/sites.py

@@ -71,6 +71,10 @@ class SiteManager(models.Manager):
 
 SiteRootPath = namedtuple("SiteRootPath", "site_id root_path root_url language_code")
 
+SITE_ROOT_PATHS_CACHE_KEY = "wagtail_site_root_paths"
+# Increase the cache version whenever the structure SiteRootPath tuple changes
+SITE_ROOT_PATHS_CACHE_VERSION = 2
+
 
 class Site(models.Model):
     hostname = models.CharField(
@@ -209,13 +213,11 @@ class Site(models.Model):
         - `root_url` - The scheme/domain name of the site (for example 'https://www.example.com/')
         - `language_code` - The language code of the site (for example 'en')
         """
-        result = cache.get("wagtail_site_root_paths")
+        result = cache.get(
+            SITE_ROOT_PATHS_CACHE_KEY, version=SITE_ROOT_PATHS_CACHE_VERSION
+        )
 
-        # Wagtail 2.11 changed the way site root paths were stored. This can cause an upgraded 2.11
-        # site to break when loading cached site root paths that were cached with 2.10.2 or older
-        # versions of Wagtail. The line below checks if the any of the cached site urls is consistent
-        # with an older version of Wagtail and invalidates the cache.
-        if result is None or any(len(site_record) == 3 for site_record in result):
+        if result is None:
             result = []
 
             for site in Site.objects.select_related(
@@ -245,6 +247,20 @@ class Site(models.Model):
                         )
                     )
 
-            cache.set("wagtail_site_root_paths", result, 3600)
+            cache.set(
+                SITE_ROOT_PATHS_CACHE_KEY,
+                result,
+                3600,
+                version=SITE_ROOT_PATHS_CACHE_VERSION,
+            )
+
+        else:
+            # Convert the cache result to a list of SiteRootPath tuples, as some
+            # cache backends (e.g. Redis) don't support named tuples.
+            result = [SiteRootPath(*result) for result in result]
 
         return result
+
+    @staticmethod
+    def clear_site_root_paths_cache():
+        cache.delete(SITE_ROOT_PATHS_CACHE_KEY, version=SITE_ROOT_PATHS_CACHE_VERSION)

wagtail/signal_handlers.py

@@ -2,7 +2,6 @@ import logging
 from contextlib import contextmanager
 
 from asgiref.local import Local
-from django.core.cache import cache
 from django.db import transaction
 from django.db.models.signals import (
     post_delete,
@@ -21,11 +20,11 @@ logger = logging.getLogger("wagtail")
 
 # Clear the wagtail_site_root_paths from the cache whenever Site records are updated.
 def post_save_site_signal_handler(instance, update_fields=None, **kwargs):
-    cache.delete("wagtail_site_root_paths")
+    Site.clear_site_root_paths_cache()
 
 
 def post_delete_site_signal_handler(instance, **kwargs):
-    cache.delete("wagtail_site_root_paths")
+    Site.clear_site_root_paths_cache()
 
 
 def pre_delete_page_unpublish(sender, instance, **kwargs):

wagtail/tests/test_page_model.py

@@ -738,9 +738,7 @@ class TestServeView(TestCase):
         # Explicitly clear the cache of site root paths. Normally this would be kept
         # in sync by the Site.save logic, but this is bypassed when the database is
         # rolled back between tests using transactions.
-        from django.core.cache import cache
-
-        cache.delete("wagtail_site_root_paths")
+        Site.clear_site_root_paths_cache()
 
         # also need to clear urlresolver caches before/after tests, because we override
         # ROOT_URLCONF in some tests here

wagtail/tests/tests.py

@@ -1,3 +1,5 @@
+import json
+
 from django import template
 from django.core.cache import cache
 from django.http import HttpRequest
@@ -8,6 +10,10 @@ from django.utils.safestring import SafeString
 
 from wagtail.coreutils import get_dummy_request, resolve_model_string
 from wagtail.models import Locale, Page, Site, SiteRootPath
+from wagtail.models.sites import (
+    SITE_ROOT_PATHS_CACHE_KEY,
+    SITE_ROOT_PATHS_CACHE_VERSION,
+)
 from wagtail.templatetags.wagtailcore_tags import richtext, slugurl
 from wagtail.test.testapp.models import SimplePage
 
@@ -268,6 +274,11 @@ class TestWagtailSiteTag(TestCase):
 class TestSiteRootPathsCache(TestCase):
     fixtures = ["test.json"]
 
+    def get_cached_site_root_paths(self):
+        return cache.get(
+            SITE_ROOT_PATHS_CACHE_KEY, version=SITE_ROOT_PATHS_CACHE_VERSION
+        )
+
     def test_cache(self):
         """
         This tests that the cache is populated when building URLs
@@ -280,7 +291,7 @@ class TestSiteRootPathsCache(TestCase):
 
         # Check that the cache has been set correctly
         self.assertEqual(
-            cache.get("wagtail_site_root_paths"),
+            self.get_cached_site_root_paths(),
             [
                 SiteRootPath(
                     site_id=1,
@@ -291,6 +302,36 @@ class TestSiteRootPathsCache(TestCase):
             ],
         )
 
+    def test_cache_backend_uses_json_serialization(self):
+        """
+        This tests that, even if the cache backend uses JSON serialization,
+        get_site_root_paths() returns a list of SiteRootPath objects.
+        """
+        result = Site.get_site_root_paths()
+
+        self.assertEqual(
+            result,
+            [
+                SiteRootPath(
+                    site_id=1,
+                    root_path="/home/",
+                    root_url="http://localhost",
+                    language_code="en",
+                )
+            ],
+        )
+
+        # Go through JSON (de)serialisation to check that the result is
+        # still a list of named tuples.
+        cache.set(
+            SITE_ROOT_PATHS_CACHE_KEY,
+            json.loads(json.dumps(result)),
+            version=SITE_ROOT_PATHS_CACHE_VERSION,
+        )
+
+        result = Site.get_site_root_paths()
+        self.assertIsInstance(result[0], SiteRootPath)
+
     def test_cache_clears_when_site_saved(self):
         """
         This tests that the cache is cleared whenever a site is saved
@@ -302,13 +343,23 @@ class TestSiteRootPathsCache(TestCase):
         _ = homepage.url  # noqa
 
         # Check that the cache has been set
-        self.assertTrue(cache.get("wagtail_site_root_paths"))
+        self.assertEqual(
+            self.get_cached_site_root_paths(),
+            [
+                SiteRootPath(
+                    site_id=1,
+                    root_path="/home/",
+                    root_url="http://localhost",
+                    language_code="en",
+                )
+            ],
+        )
 
         # Save the site
         Site.objects.get(is_default_site=True).save()
 
         # Check that the cache has been cleared
-        self.assertFalse(cache.get("wagtail_site_root_paths"))
+        self.assertIsNone(self.get_cached_site_root_paths())
 
     def test_cache_clears_when_site_deleted(self):
         """
@@ -321,13 +372,23 @@ class TestSiteRootPathsCache(TestCase):
         _ = homepage.url  # noqa
 
         # Check that the cache has been set
-        self.assertTrue(cache.get("wagtail_site_root_paths"))
+        self.assertEqual(
+            self.get_cached_site_root_paths(),
+            [
+                SiteRootPath(
+                    site_id=1,
+                    root_path="/home/",
+                    root_url="http://localhost",
+                    language_code="en",
+                )
+            ],
+        )
 
         # Delete the site
         Site.objects.get(is_default_site=True).delete()
 
         # Check that the cache has been cleared
-        self.assertFalse(cache.get("wagtail_site_root_paths"))
+        self.assertIsNone(self.get_cached_site_root_paths())
 
     def test_cache_clears_when_site_root_moves(self):
         """