Merge pull request #197 from dmarcelino/root_page_slug

_get_site_root_paths: clear cache when changing languages

.gitignore

@@ -35,6 +35,9 @@ _build
 # GitEye / Eclipse project file
 /.project
 
+# PyCharm
+/.idea
+
 # vim
 *~
 *.swp

wagtail_modeltranslation/patch_wagtailadmin.py

@@ -3,7 +3,9 @@
 import logging
 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
@@ -15,13 +17,11 @@
 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, \
         MultiFieldPanel, FieldRowPanel, InlinePanel, StreamFieldPanel, RichTextFieldPanel
-    from wagtail.core.models import Page
+    from wagtail.core.models import Page, Site
     from wagtail.core.fields import StreamField, StreamValue
     from wagtail.core.url_routing import RouteResult
     from wagtail.images.edit_handlers import ImageChooserPanel
@@ -32,13 +32,22 @@
     from wagtail.contrib.wagtailroutablepage.models import RoutablePageMixin
     from wagtail.wagtailadmin.edit_handlers import FieldPanel, \
         MultiFieldPanel, FieldRowPanel, InlinePanel, StreamFieldPanel, RichTextFieldPanel
-    from wagtail.wagtailcore.models import Page
+    from wagtail.wagtailcore.models import Page, Site
     from wagtail.wagtailcore.fields import StreamField, StreamValue
     from wagtail.wagtailcore.url_routing import RouteResult
     from wagtail.wagtailimages.edit_handlers import ImageChooserPanel
     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')
 
@@ -113,6 +122,9 @@ def _patch_page_models(self, model):
         model.set_url_path = _new_set_url_path
         model.route = _new_route
         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'):
@@ -386,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 = []
@@ -471,6 +534,11 @@ def __call__(self, instance, *args, **kwargs):
         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

@@ -3,6 +3,7 @@
 
 import django
 from django.apps import apps as django_apps
+from django.core.cache import cache
 from django.core.exceptions import ValidationError
 from django.core.management import call_command
 from django.http import HttpRequest
@@ -122,6 +123,10 @@ def setUp(self):
         self._old_language = get_language()
         trans_real.activate('de')
 
+        # ensure we have a fresh site cache
+        for language in mt_settings.AVAILABLE_LANGUAGES:
+            cache.delete('wagtail_site_root_paths_{}'.format(language))
+
     def tearDown(self):
         trans_real.activate(self._old_language)
 
@@ -643,12 +648,12 @@ def test_set_url_path_non_translated_descendants(self):
 
         # re-fetch to pick up latest from DB
         grandchild1 = models.TestSlugPage1.objects.get(slug_de='grandchild1-untranslated')
-        self.assertEqual(grandchild1.url_path_de, '/child-untranslated/grandchild1-untranslated/')
+        self.assertEqual(grandchild1.url_path_de, '/root-untranslated/child-untranslated/grandchild1-untranslated/')
         self.assertEqual(grandchild1.slug_en, None)
         self.assertEqual(grandchild1.url_path_en, None)
         grandgrandchild = models.TestSlugPage1.objects.get(slug_de='grandgrandchild-untranslated')
         self.assertEqual(grandgrandchild.url_path_de,
-                         '/child-untranslated/grandchild1-untranslated/grandgrandchild-untranslated/')
+                         '/root-untranslated/child-untranslated/grandchild1-untranslated/grandgrandchild-untranslated/')
         self.assertEqual(grandgrandchild.slug_en, None)
         self.assertEqual(grandgrandchild.url_path_en, None)
 
@@ -658,18 +663,18 @@ def test_set_url_path_non_translated_descendants(self):
         child.slug_en = 'child-translated'
         child.save()
 
-        self.assertEqual(child.url_path_de, '/child-untranslated/')
-        self.assertEqual(child.url_path_en, '/child-translated/')
+        self.assertEqual(child.url_path_de, '/root-untranslated/child-untranslated/')
+        self.assertEqual(child.url_path_en, '/root-untranslated/child-translated/')
 
         grandchild1 = models.TestSlugPage1.objects.get(slug_de='grandchild1-untranslated')
-        self.assertEqual(grandchild1.url_path_de, '/child-untranslated/grandchild1-untranslated/')
-        self.assertEqual(grandchild1.url_path_en, '/child-translated/grandchild1-untranslated/')
+        self.assertEqual(grandchild1.url_path_de, '/root-untranslated/child-untranslated/grandchild1-untranslated/')
+        self.assertEqual(grandchild1.url_path_en, '/root-untranslated/child-translated/grandchild1-untranslated/')
 
         grandgrandchild = models.TestSlugPage1.objects.get(slug_de='grandgrandchild-untranslated')
         self.assertEqual(grandgrandchild.url_path_de,
-                         '/child-untranslated/grandchild1-untranslated/grandgrandchild-untranslated/')
+                         '/root-untranslated/child-untranslated/grandchild1-untranslated/grandgrandchild-untranslated/')
         self.assertEqual(grandgrandchild.url_path_en,
-                         '/child-translated/grandchild1-untranslated/grandgrandchild-untranslated/')
+                         '/root-untranslated/child-translated/grandchild1-untranslated/grandgrandchild-untranslated/')
 
     def test_fetch_translation_records(self):
         """
@@ -833,6 +838,59 @@ def test_url(self):
         self.assertEqual(page_01.url, '/en/url-en-01/')
         self.assertEqual(page_02.url, '/en/url-de-02/')
 
+    def test_root_page_slug(self):
+        site_pages = {
+            'model': models.TestRootPage,
+            'kwargs': {'title': 'root URL', 'slug_de': 'root-de', 'slug_en': 'root-en'},
+            'children': {
+                'child1': {
+                    'model': models.TestSlugPage1,
+                    'kwargs': {'title': 'child1 URL', 'slug_de': 'url-de-01', 'slug_en': 'url-en-01'},
+                },
+                'child2': {
+                    '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/')
+        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/')
+        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):
         """
         Assert set_translation_url_paths management command works correctly
@@ -904,20 +962,20 @@ def test_set_translation_url_paths_command(self):
         call_command('set_translation_url_paths', verbosity=0)
 
         grandchild1 = models.TestSlugPage1.objects.get(slug_de='grandchild1-untranslated')
-        self.assertEqual(grandchild1.url_path_de, '/child-untranslated/grandchild1-untranslated/')
-        self.assertEqual(grandchild1.url_path_en, '/child-untranslated/grandchild1-untranslated/')
+        self.assertEqual(grandchild1.url_path_de, '/root-untranslated/child-untranslated/grandchild1-untranslated/')
+        self.assertEqual(grandchild1.url_path_en, '/root-untranslated/child-untranslated/grandchild1-untranslated/')
         grandgrandchild = models.TestSlugPage1.objects.get(slug_de='grandgrandchild-untranslated')
         self.assertEqual(grandgrandchild.url_path_de,
-                         '/child-untranslated/grandchild1-untranslated/grandgrandchild-untranslated/')
+                         '/root-untranslated/child-untranslated/grandchild1-untranslated/grandgrandchild-untranslated/')
         self.assertEqual(grandgrandchild.url_path_en,
-                         '/child-untranslated/grandchild1-untranslated/grandgrandchild-untranslated/')
+                         '/root-untranslated/child-untranslated/grandchild1-untranslated/grandgrandchild-untranslated/')
         grandchild2 = models.TestSlugPage2.objects.get(slug_de='grandchild2-untranslated')
-        self.assertEqual(grandchild2.__dict__['url_path'], '/child-untranslated/grandchild2-untranslated/')
-        self.assertEqual(grandchild2.url_path_de, '/child-untranslated/grandchild2-untranslated/')
-        self.assertEqual(grandchild2.url_path_en, '/child-untranslated/grandchild2-untranslated/')
+        self.assertEqual(grandchild2.__dict__['url_path'], '/root-untranslated/child-untranslated/grandchild2-untranslated/')
+        self.assertEqual(grandchild2.url_path_de, '/root-untranslated/child-untranslated/grandchild2-untranslated/')
+        self.assertEqual(grandchild2.url_path_en, '/root-untranslated/child-untranslated/grandchild2-untranslated/')
 
         grandgrandchild_translated = models.TestSlugPage1.objects.get(slug_de='grandgrandchild1-translated')
         self.assertEqual(grandgrandchild_translated.url_path_de,
-                         '/child2-translated/grandchild1-translated/grandgrandchild1-translated/')
+                         '/root-untranslated/child2-translated/grandchild1-translated/grandgrandchild1-translated/')
         self.assertEqual(grandgrandchild_translated.url_path_en,
-                         '/child2-translated-en/grandchild1-translated-en/grandgrandchild1-translated-en/')
+                         '/root-untranslated/child2-translated-en/grandchild1-translated-en/grandgrandchild1-translated-en/')

wagtail_modeltranslation/tests/util.py

@@ -1,5 +1,3 @@
-
-
 class PageFactory(object):
 
     def __init__(self, initial_path=0):
@@ -33,12 +31,24 @@ def create_page_tree(self, nodes=None):
         if not nodes:
             return None
 
+        from .models import TestRootPage
         try:
             from wagtail.core.models import Site
         except ImportError:
             from wagtail.wagtailcore.models import Site
-        root_node = self.create_instance(nodes)
-        site = Site.objects.create(root_page=root_node)
+
+        # add a top root node to mimic Wagtail's real behaviour
+        all_nodes = {
+            'model': TestRootPage,
+            'kwargs': {'title': 'Root', 'slug': 'root', },
+            'children': {
+                'site_root': nodes,
+            },
+        }
+        self.create_instance(all_nodes)
+
+        site_root_node = nodes['instance']
+        site = Site.objects.create(root_page=site_root_node, hostname='localhost', port=80, is_default_site=True)
         return site
 
     def create_instance(self, node, parent=None, order=None):