#195: cache each language's site root paths individually

instead of evicting the cache when language changes.
Patch `Page._get_site_root_paths()` and `Page.get_url_parts()` on Wagtail<1.11
Update `test_root_page_slug` to cover more scenarios

wagtail_modeltranslation/patch_wagtailadmin.py

@@ -5,20 +5,18 @@ import types
 
 from django.core.cache import cache
 from django.core.exceptions import ValidationError
+from django.core.urlresolvers import reverse
 from django.db import transaction, connection
 from django.db.models import Q, Value
 from django.db.models.functions import Concat, Substr
 from django.http import Http404
 from django.utils.translation import trans_real
 from django.utils.translation import ugettext_lazy as _
-from functools import wraps
 from modeltranslation import settings as mt_settings
 from modeltranslation.translator import translator, NotRegistered
 from modeltranslation.utils import build_localized_fieldname, get_language
 from wagtail.contrib.settings.models import BaseSetting
 from wagtail.contrib.settings.views import get_setting_edit_handler
-from wagtail_modeltranslation.settings import CUSTOM_SIMPLE_PANELS, CUSTOM_COMPOSED_PANELS
-from wagtail_modeltranslation.utils import compare_class_tree_depth
 try:
     from wagtail.contrib.routable_page.models import RoutablePageMixin
     from wagtail.admin.edit_handlers import FieldPanel, \
@@ -41,6 +39,15 @@ except ImportError:
     from wagtail.wagtailsearch.index import SearchField
     from wagtail.wagtailsnippets.models import get_snippet_models
     from wagtail.wagtailsnippets.views.snippets import SNIPPET_EDIT_HANDLERS
+try:
+    from wagtail.core.utils import WAGTAIL_APPEND_SLASH
+except ImportError:
+    try:
+        from wagtail.wagtailcore.utils import WAGTAIL_APPEND_SLASH
+    except ImportError:
+        WAGTAIL_APPEND_SLASH = True  # Wagtail<1.5
+from wagtail_modeltranslation.settings import CUSTOM_SIMPLE_PANELS, CUSTOM_COMPOSED_PANELS
+from wagtail_modeltranslation.utils import compare_class_tree_depth
 
 logger = logging.getLogger('wagtail.core')
 
@@ -114,11 +121,10 @@ class WagtailTranslator(object):
         # OVERRIDE PAGE METHODS
         model.set_url_path = _new_set_url_path
         model.route = _new_route
-        try:
-            model._get_site_root_paths = _get_site_root_paths_decorator(model._get_site_root_paths)
-        except AttributeError:
-            model.get_url_parts = _get_site_root_paths_decorator(model.get_url_parts)
         model._update_descendant_url_paths = _new_update_descendant_url_paths
+        if not hasattr(model, '_get_site_root_paths'):
+            model.get_url_parts = _new_get_url_parts  # Wagtail<1.11
+        model._get_site_root_paths = _new_get_site_root_paths
         _patch_clean(model)
 
         if not model.save.__name__.startswith('localized'):
@@ -392,6 +398,57 @@ def _localized_update_descendant_url_paths(page, old_url_path, new_url_path, lan
                 Substr(localized_url_path, len(old_url_path) + 1))}))
 
 
+def _localized_site_get_site_root_paths():
+    """
+    Localized version of ``Site.get_site_root_paths()``
+    """
+    current_language = get_language()
+    cache_key = 'wagtail_site_root_paths_{}'.format(current_language)
+    result = cache.get(cache_key)
+
+    if result is None:
+        result = [
+            (site.id, site.root_page.url_path, site.root_url)
+            for site in Site.objects.select_related('root_page').order_by('-root_page__url_path')
+        ]
+        cache.set(cache_key, result, 3600)
+
+    return result
+
+
+def _new_get_site_root_paths(self, request=None):
+    """
+    Return localized site_root_paths, using the cached copy on the
+    request object if available and if language is the same.
+    """
+    # if we have a request, use that to cache site_root_paths; otherwise, use self
+    current_language = get_language()
+    cache_object = request if request else self
+    if not hasattr(cache_object, '_wagtail_cached_site_root_paths_language') or \
+            cache_object._wagtail_cached_site_root_paths_language != current_language:
+        cache_object._wagtail_cached_site_root_paths_language = current_language
+        cache_object._wagtail_cached_site_root_paths = _localized_site_get_site_root_paths()
+
+    return cache_object._wagtail_cached_site_root_paths
+
+
+def _new_get_url_parts(self, request=None):
+    """
+    For Wagtail<1.11 ``Page.get_url_parts()`` is patched so it uses ``self._get_site_root_paths(request)``
+    """
+    for (site_id, root_path, root_url) in self._get_site_root_paths(request):
+        if self.url_path.startswith(root_path):
+            page_path = reverse('wagtail_serve', args=(self.url_path[len(root_path):],))
+
+            # Remove the trailing slash from the URL reverse generates if
+            # WAGTAIL_APPEND_SLASH is False and we're not trying to serve
+            # the root path
+            if not WAGTAIL_APPEND_SLASH and page_path != '/':
+                page_path = page_path.rstrip('/')
+
+            return (site_id, root_url, page_path)
+
+
 def _update_translation_descendant_url_paths(old_record, page):
     # update children paths, must be done for all languages to ensure fallbacks are applied
     languages_changed = []
@@ -433,27 +490,6 @@ def _update_untranslated_descendants_url_paths(page, languages_changed):
         child.save(update_fields=update_fields)  # this will trigger any required saves downstream
 
 
-def _get_site_root_paths_decorator(func):
-    """
-    Resets cached site_root_paths if language is different than cached value
-    """
-    @wraps(func)
-    def wrapper(self, *args, **kwargs):
-        current_language = get_language()
-        request = args[0] if len(args) > 0 else None
-        cache_object = request if request else self
-        try:
-            if cache_object._wagtail_cached_site_root_paths_language != current_language:
-                cache.delete('wagtail_site_root_paths')
-                del cache_object._wagtail_cached_site_root_paths
-                cache_object._wagtail_cached_site_root_paths_language = current_language
-        except AttributeError:
-            cache_object._wagtail_cached_site_root_paths_language = current_language
-
-        return func(self, *args, **kwargs)
-    return wrapper
-
-
 class LocalizedSaveDescriptor(object):
     def __init__(self, f):
         self.func = f
@@ -498,6 +534,11 @@ class LocalizedSaveDescriptor(object):
         if change_descendant_url_path:
             _update_translation_descendant_url_paths(old_record, instance)
 
+        # Check if this is a root page of any sites and clear the 'wagtail_site_root_paths_XX' key if so
+        if Site.objects.filter(root_page=instance).exists():
+            for language in mt_settings.AVAILABLE_LANGUAGES:
+                cache.delete('wagtail_site_root_paths_{}'.format(language))
+
         return result
 
     def __get__(self, instance, owner=None):

wagtail_modeltranslation/tests/tests.py

@@ -124,7 +124,8 @@ class WagtailModeltranslationTransactionTestBase(TransactionTestCase):
         trans_real.activate('de')
 
         # ensure we have a fresh site cache
-        cache.delete('wagtail_site_root_paths')
+        for language in mt_settings.AVAILABLE_LANGUAGES:
+            cache.delete('wagtail_site_root_paths_{}'.format(language))
 
     def tearDown(self):
         trans_real.activate(self._old_language)
@@ -850,24 +851,45 @@ class WagtailModeltranslationTest(WagtailModeltranslationTestBase):
                     'model': models.TestSlugPage2,
                     'kwargs': {'title': 'child2 URL', 'slug': 'url-de-02'},
                 },
+                'child3': {
+                    'model': models.TestSlugPage2,
+                    'kwargs': {'title': 'child3 URL', 'slug': 'url-de-03'},
+                },
             },
         }
         page_factory.create_page_tree(site_pages)
+        request = HttpRequest()
 
+        site_root_page = site_pages['instance']
         wagtail_page_01 = site_pages['children']['child1']['instance']
         wagtail_page_02 = site_pages['children']['child2']['instance']
+        wagtail_page_03 = site_pages['children']['child3']['instance']
 
         self.assertEqual(wagtail_page_01.url, '/de/url-de-01/')
         self.assertEqual(wagtail_page_01.url_path, '/root-de/url-de-01/')
-        self.assertEqual(wagtail_page_02.url, '/de/url-de-02/')
-        self.assertEqual(wagtail_page_02.url_path, '/root-de/url-de-02/')
+        if VERSION >= (1, 11):
+            self.assertEqual(wagtail_page_02.get_url(request=request), '/de/url-de-02/')  # with request
 
         trans_real.activate('en')
 
         self.assertEqual(wagtail_page_01.url, '/en/url-en-01/')
         self.assertEqual(wagtail_page_01.url_path, '/root-en/url-en-01/')
-        self.assertEqual(wagtail_page_02.url, '/en/url-de-02/')
-        self.assertEqual(wagtail_page_02.url_path, '/root-en/url-de-02/')
+        if VERSION >= (1, 11):
+            self.assertEqual(wagtail_page_02.get_url(request=request), '/en/url-de-02/')
+
+        trans_real.activate('de')
+
+        # new request after changing language
+        self.assertEqual(wagtail_page_03.url, '/de/url-de-03/')
+        if VERSION >= (1, 11):
+            self.assertEqual(wagtail_page_01.get_url(request=HttpRequest()), '/de/url-de-01/')
+
+        # URL should not be broken after updating the root_page (ensure the cache is evicted)
+        self.assertEqual(wagtail_page_01.url, '/de/url-de-01/')
+        site_root_page.slug = 'new-root-de'
+        site_root_page.save()
+        wagtail_page_01_new = site_root_page.get_children().get(id=wagtail_page_01.id)
+        self.assertEqual(wagtail_page_01_new.url, '/de/url-de-01/')
 
     def test_set_translation_url_paths_command(self):
         """