Merge branch 'kaedroho-clear-site-root-paths-on-delete'

CHANGELOG.txt

@@ -15,6 +15,7 @@ Changelog
  * Fix: created_at timestamps on page revisions were not being preserved on page copy, causing revisions to get out of sequence
  * Fix: When copying pages recursively, revisions of sub-pages were being copied regardless of the copy_revisions flag
  * Fix: Updated the migration dependencies within the project template to ensure that Wagtail's own migrations consistently apply first.
+ * Fix: The cache of site root paths is now cleared when a site is deleted.
 
 0.7 (09.10.2014)
 ~~~~~~~~~~~~~~~~

docs/releases/0.8.rst

@@ -31,6 +31,7 @@ Bug fixes
  * ``created_at`` timestamps on page revisions were not being preserved on page copy, causing revisions to get out of sequence
  * When copying pages recursively, revisions of sub-pages were being copied regardless of the ``copy_revisions`` flag
  * Updated the migration dependencies within the project template to ensure that Wagtail's own migrations consistently apply first
+ * The cache of site root paths is now cleared when a site is deleted
 
 
 Upgrade considerations

wagtail/wagtailcore/models.py

@@ -9,7 +9,7 @@ from modelcluster.models import ClusterableModel, get_all_child_relations
 
 from django.db import models, connection, transaction
 from django.db.models import Q
-from django.db.models.signals import pre_delete, post_delete
+from django.db.models.signals import post_save, pre_delete, post_delete
 from django.dispatch.dispatcher import receiver
 from django.http import Http404
 from django.core.cache import cache
@@ -121,12 +121,6 @@ class Site(models.Model):
                     ]}
                 )
 
-    # clear the wagtail_site_root_paths cache whenever Site records are updated
-    def save(self, *args, **kwargs):
-        result = super(Site, self).save(*args, **kwargs)
-        cache.delete('wagtail_site_root_paths')
-        return result
-
     @staticmethod
     def get_site_root_paths():
         """
@@ -145,6 +139,17 @@ class Site(models.Model):
         return result
 
 
+# Clear the wagtail_site_root_paths from the cache whenever Site records are updated
+@receiver(post_save, sender=Site)
+def clear_site_root_paths_on_save(sender, instance, **kwargs):
+    cache.delete('wagtail_site_root_paths')
+
+
+@receiver(post_delete, sender=Site)
+def clear_site_root_paths_on_delete(sender, instance, **kwargs):
+    cache.delete('wagtail_site_root_paths')
+
+
 PAGE_MODEL_CLASSES = []
 _PAGE_CONTENT_TYPES = []
 

wagtail/wagtailcore/tests/tests.py

@@ -1,4 +1,5 @@
 from django.test import TestCase
+from django.core.cache import cache
 
 from wagtail.wagtailcore.models import Page, Site
 from wagtail.tests.models import SimplePage
@@ -20,23 +21,73 @@ class TestPageUrlTags(TestCase):
                             '<a href="/events/">Back to events index</a>')
 
 
-class TestIssue7(TestCase):
-    """
-    This tests for an issue where if a site root page was moved, all
-    the page urls in that site would change to None.
-
-    The issue was caused by the 'wagtail_site_root_paths' cache
-    variable not being cleared when a site root page was moved. Which
-    left all the child pages thinking that they are no longer in the
-    site and return None as their url.
-
-    Fix: d6cce69a397d08d5ee81a8cbc1977ab2c9db2682
-    Discussion: https://github.com/torchbox/wagtail/issues/7
-    """
-
+class TestSiteRootPathsCache(TestCase):
     fixtures = ['test.json']
 
-    def test_issue7(self):
+    def test_cache(self):
+        """
+        This tests that the cache is populated when building URLs
+        """
+        # Get homepage
+        homepage = Page.objects.get(url_path='/home/')
+
+        # Warm up the cache by getting the url
+        _ = homepage.url
+
+        # Check that the cache has been set correctly
+        self.assertEqual(cache.get('wagtail_site_root_paths'), [(1, '/home/', 'http://localhost')])
+
+    def test_cache_clears_when_site_saved(self):
+        """
+        This tests that the cache is cleared whenever a site is saved
+        """
+        # Get homepage
+        homepage = Page.objects.get(url_path='/home/')
+
+        # Warm up the cache by getting the url
+        _ = homepage.url
+
+        # Check that the cache has been set
+        self.assertTrue(cache.get('wagtail_site_root_paths'))
+
+        # 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'))
+
+    def test_cache_clears_when_site_deleted(self):
+        """
+        This tests that the cache is cleared whenever a site is deleted
+        """
+        # Get homepage
+        homepage = Page.objects.get(url_path='/home/')
+
+        # Warm up the cache by getting the url
+        _ = homepage.url
+
+        # Check that the cache has been set
+        self.assertTrue(cache.get('wagtail_site_root_paths'))
+
+        # 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'))
+
+    def test_cache_clears_when_site_root_moves(self):
+        """
+        This tests for an issue where if a site root page was moved, all
+        the page urls in that site would change to None.
+
+        The issue was caused by the 'wagtail_site_root_paths' cache
+        variable not being cleared when a site root page was moved. Which
+        left all the child pages thinking that they are no longer in the
+        site and return None as their url.
+
+        Fix: d6cce69a397d08d5ee81a8cbc1977ab2c9db2682
+        Discussion: https://github.com/torchbox/wagtail/issues/7
+        """
         # Get homepage, root page and site
         root_page = Page.objects.get(id=1)
         homepage = Page.objects.get(url_path='/home/')
@@ -62,24 +113,19 @@ class TestIssue7(TestCase):
         # Check url
         self.assertEqual(new_homepage.url, '/')
 
+    def test_cache_clears_when_site_root_slug_changes(self):
+        """
+        This tests for an issue where if a site root pages slug was
+        changed, all the page urls in that site would change to None.
 
-class TestIssue157(TestCase):
-    """
-    This tests for an issue where if a site root pages slug was
-    changed, all the page urls in that site would change to None.
+        The issue was caused by the 'wagtail_site_root_paths' cache
+        variable not being cleared when a site root page was changed.
+        Which left all the child pages thinking that they are no longer in
+        the site and return None as their url.
 
-    The issue was caused by the 'wagtail_site_root_paths' cache
-    variable not being cleared when a site root page was changed.
-    Which left all the child pages thinking that they are no longer in
-    the site and return None as their url.
-
-    Fix: d6cce69a397d08d5ee81a8cbc1977ab2c9db2682
-    Discussion: https://github.com/torchbox/wagtail/issues/157
-    """
-
-    fixtures = ['test.json']
-
-    def test_issue157(self):
+        Fix: d6cce69a397d08d5ee81a8cbc1977ab2c9db2682
+        Discussion: https://github.com/torchbox/wagtail/issues/157
+        """
         # Get homepage
         homepage = Page.objects.get(url_path='/home/')