From c76bc9c30e974d2333a042a223b53abb6b913fa1 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 17 Oct 2014 17:12:44 +0100 Subject: [PATCH 1/3] Clear wagtail_site_root_paths on Site delete Fixes #706 --- wagtail/wagtailcore/models.py | 19 +++-- wagtail/wagtailcore/tests/tests.py | 121 +++++++++++++++++++++-------- 2 files changed, 102 insertions(+), 38 deletions(-) diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index fdc30af672..8750881f95 100644 --- a/wagtail/wagtailcore/models.py +++ b/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 @@ -116,12 +116,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(): """ @@ -140,6 +134,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 = [] diff --git a/wagtail/wagtailcore/tests/tests.py b/wagtail/wagtailcore/tests/tests.py index 5fd5aa8025..febc3f4ae8 100644 --- a/wagtail/wagtailcore/tests/tests.py +++ b/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,86 @@ class TestPageUrlTags(TestCase): 'Back to events index') -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 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 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 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')) + + # 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_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 + print(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 +126,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/') From 5e06a4485af7eaa88075f16dc9ff9b62b81cccd2 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 27 Oct 2014 17:43:23 +0000 Subject: [PATCH 2/3] fix inaccurate comments and duplicate definition of test_cache_clears_when_site_deleted --- wagtail/wagtailcore/tests/tests.py | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/wagtail/wagtailcore/tests/tests.py b/wagtail/wagtailcore/tests/tests.py index febc3f4ae8..7cc1509cd2 100644 --- a/wagtail/wagtailcore/tests/tests.py +++ b/wagtail/wagtailcore/tests/tests.py @@ -26,7 +26,7 @@ class TestSiteRootPathsCache(TestCase): def test_cache(self): """ - This tests that the cache is cleared whenever a site is deleted + This tests that the cache is populated when building URLs """ # Get homepage homepage = Page.objects.get(url_path='/home/') @@ -39,7 +39,7 @@ class TestSiteRootPathsCache(TestCase): def test_cache_clears_when_site_saved(self): """ - This tests that the cache is cleared whenever a site is deleted + This tests that the cache is cleared whenever a site is saved """ # Get homepage homepage = Page.objects.get(url_path='/home/') @@ -75,19 +75,6 @@ class TestSiteRootPathsCache(TestCase): # 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 - print(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 From c440087c3edf252934818456259c9b8fe1026106 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 27 Oct 2014 17:44:28 +0000 Subject: [PATCH 3/3] release note for #719 --- CHANGELOG.txt | 1 + docs/releases/0.8.rst | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index fe6701c431..cb93761317 100644 --- a/CHANGELOG.txt +++ b/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) ~~~~~~~~~~~~~~~~ diff --git a/docs/releases/0.8.rst b/docs/releases/0.8.rst index c2d89d1975..9a6de865dc 100644 --- a/docs/releases/0.8.rst +++ b/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