diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 75ab2b492f..77e80efedb 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -6,6 +6,7 @@ Changelog * Redirect to the last viewed listing page after deleting form submissions (Matthias Brück) * Provide `getTextLabel` method on date / time StreamField blocks (Vaughn Dickson) + * Purge frontend cache when modifying redirects (Jake Howard) * Fix: Prevent page type business rules from blocking reordering of pages (Andy Babic, Sage Abdullah) * Docs: Upgrade Sphinx to 7.3 (Matt Westcott) * Docs: Document how to customise date/time format settings (Vince Salvino) diff --git a/docs/releases/6.3.md b/docs/releases/6.3.md index 5ca43a5a48..981d2c1b7f 100644 --- a/docs/releases/6.3.md +++ b/docs/releases/6.3.md @@ -15,6 +15,7 @@ depth: 1 * Redirect to the last viewed listing page after deleting form submissions (Matthias Brück) * Provide `getTextLabel` method on date / time StreamField blocks (Vaughn Dickson) + * Purge frontend cache when modifying redirects (Jake Howard) ### Bug fixes diff --git a/wagtail/contrib/frontend_cache/backends/azure.py b/wagtail/contrib/frontend_cache/backends/azure.py index 4ac37e5808..4a142c79f8 100644 --- a/wagtail/contrib/frontend_cache/backends/azure.py +++ b/wagtail/contrib/frontend_cache/backends/azure.py @@ -100,7 +100,7 @@ class AzureBaseBackend(BaseBackend): return { "resource_group_name": self._resource_group_name, "custom_headers": self._custom_headers, - "content_paths": paths, + "content_paths": set(paths), } def _purge_content(self, paths): diff --git a/wagtail/contrib/frontend_cache/backends/cloudfront.py b/wagtail/contrib/frontend_cache/backends/cloudfront.py index 57c3ace53d..f92584222e 100644 --- a/wagtail/contrib/frontend_cache/backends/cloudfront.py +++ b/wagtail/contrib/frontend_cache/backends/cloudfront.py @@ -45,7 +45,7 @@ class CloudfrontBackend(BaseBackend): self.hostnames = list(self.cloudfront_distribution_id.keys()) def purge_batch(self, urls): - paths_by_distribution_id = defaultdict(list) + paths_by_distribution_id = defaultdict(set) for url in urls: url_parsed = urlparse(url) @@ -69,7 +69,7 @@ class CloudfrontBackend(BaseBackend): distribution_id = self.cloudfront_distribution_id if distribution_id: - paths_by_distribution_id[distribution_id].append(url_parsed.path) + paths_by_distribution_id[distribution_id].add(url_parsed.path) for distribution_id, paths in paths_by_distribution_id.items(): self._create_invalidation(distribution_id, paths) diff --git a/wagtail/contrib/frontend_cache/backends/dummy.py b/wagtail/contrib/frontend_cache/backends/dummy.py new file mode 100644 index 0000000000..e28889a292 --- /dev/null +++ b/wagtail/contrib/frontend_cache/backends/dummy.py @@ -0,0 +1,11 @@ +from .base import BaseBackend + + +class DummyBackend(BaseBackend): + def __init__(self): + super().__init__() + + self.urls = [] + + def purge(self, url) -> None: + self.urls.append(url) diff --git a/wagtail/contrib/frontend_cache/tests.py b/wagtail/contrib/frontend_cache/tests.py index 541d4796d7..35881c2a5d 100644 --- a/wagtail/contrib/frontend_cache/tests.py +++ b/wagtail/contrib/frontend_cache/tests.py @@ -360,7 +360,7 @@ class TestBackendConfiguration(SimpleTestCase): backends.get("cloudfront").purge("http://torchbox.com/blog/") _create_invalidation.assert_called_once_with( - "frontend", ["/home/events/christmas/"] + "frontend", {"/home/events/christmas/"} ) self.assertTrue( @@ -417,12 +417,12 @@ class TestBackendConfiguration(SimpleTestCase): self.assertEqual(backends["default"].cache_netloc, "localhost:8000") -PURGED_URLS = [] +PURGED_URLS = set() class MockBackend(BaseBackend): def purge(self, url): - PURGED_URLS.append(url) + PURGED_URLS.add(url) class MockCloudflareBackend(CloudflareBackend): @@ -430,7 +430,7 @@ class MockCloudflareBackend(CloudflareBackend): if len(urls) > self.CHUNK_SIZE: raise Exception("Cloudflare backend is not chunking requests as expected") - PURGED_URLS.extend(urls) + PURGED_URLS.update(urls) @override_settings( @@ -444,28 +444,27 @@ class TestCachePurgingFunctions(TestCase): fixtures = ["test.json"] def setUp(self): - # Reset PURGED_URLS to an empty list - PURGED_URLS[:] = [] + PURGED_URLS.clear() def test_purge_url_from_cache(self): purge_url_from_cache("http://localhost/foo") - self.assertEqual(PURGED_URLS, ["http://localhost/foo"]) + self.assertEqual(PURGED_URLS, {"http://localhost/foo"}) def test_purge_urls_from_cache(self): purge_urls_from_cache(["http://localhost/foo", "http://localhost/bar"]) - self.assertEqual(PURGED_URLS, ["http://localhost/foo", "http://localhost/bar"]) + self.assertEqual(PURGED_URLS, {"http://localhost/foo", "http://localhost/bar"}) def test_purge_page_from_cache(self): page = EventIndex.objects.get(url_path="/home/events/") purge_page_from_cache(page) self.assertEqual( - PURGED_URLS, ["http://localhost/events/", "http://localhost/events/past/"] + PURGED_URLS, {"http://localhost/events/", "http://localhost/events/past/"} ) def test_purge_pages_from_cache(self): purge_pages_from_cache(EventIndex.objects.all()) self.assertEqual( - PURGED_URLS, ["http://localhost/events/", "http://localhost/events/past/"] + PURGED_URLS, {"http://localhost/events/", "http://localhost/events/past/"} ) def test_purge_batch(self): @@ -477,11 +476,11 @@ class TestCachePurgingFunctions(TestCase): self.assertEqual( PURGED_URLS, - [ + { "http://localhost/events/", "http://localhost/events/past/", "http://localhost/foo", - ], + }, ) @override_settings( @@ -496,14 +495,14 @@ class TestCachePurgingFunctions(TestCase): with self.assertLogs(level="INFO") as log_output: purge_url_from_cache("http://localhost/foo") - self.assertEqual(PURGED_URLS, []) + self.assertEqual(PURGED_URLS, set()) self.assertIn( "Unable to find purge backend for localhost", log_output.output[0], ) purge_url_from_cache("http://example.com/foo") - self.assertEqual(PURGED_URLS, ["http://example.com/foo"]) + self.assertEqual(PURGED_URLS, {"http://example.com/foo"}) @override_settings( @@ -518,7 +517,7 @@ class TestCachePurgingFunctions(TestCase): class TestCloudflareCachePurgingFunctions(TestCase): def setUp(self): # Reset PURGED_URLS to an empty list - PURGED_URLS[:] = [] + PURGED_URLS.clear() def test_cloudflare_purge_batch_chunked(self): batch = PurgeBatch() @@ -526,7 +525,7 @@ class TestCloudflareCachePurgingFunctions(TestCase): batch.add_urls(urls) batch.purge() - self.assertCountEqual(PURGED_URLS, urls) + self.assertCountEqual(PURGED_URLS, set(urls)) @override_settings( @@ -541,20 +540,20 @@ class TestCachePurgingSignals(TestCase): def setUp(self): # Reset PURGED_URLS to an empty list - PURGED_URLS[:] = [] + PURGED_URLS.clear() def test_purge_on_publish(self): page = EventIndex.objects.get(url_path="/home/events/") page.save_revision().publish() self.assertEqual( - PURGED_URLS, ["http://localhost/events/", "http://localhost/events/past/"] + PURGED_URLS, {"http://localhost/events/", "http://localhost/events/past/"} ) def test_purge_on_unpublish(self): page = EventIndex.objects.get(url_path="/home/events/") page.unpublish() self.assertEqual( - PURGED_URLS, ["http://localhost/events/", "http://localhost/events/past/"] + PURGED_URLS, {"http://localhost/events/", "http://localhost/events/past/"} ) def test_purge_with_unroutable_page(self): @@ -562,7 +561,7 @@ class TestCachePurgingSignals(TestCase): page = EventIndex(title="new top-level page") root.add_child(instance=page) page.save_revision().publish() - self.assertEqual(PURGED_URLS, []) + self.assertEqual(PURGED_URLS, set()) @override_settings( ROOT_URLCONF="wagtail.test.urls_multilang", @@ -570,20 +569,19 @@ class TestCachePurgingSignals(TestCase): WAGTAILFRONTENDCACHE_LANGUAGES=["en", "fr", "pt-br"], ) def test_purge_on_publish_in_multilang_env(self): - PURGED_URLS[:] = [] # reset PURGED_URLS to the empty list page = EventIndex.objects.get(url_path="/home/events/") page.save_revision().publish() self.assertEqual( PURGED_URLS, - [ + { "http://localhost/en/events/", "http://localhost/en/events/past/", "http://localhost/fr/events/", "http://localhost/fr/events/past/", "http://localhost/pt-br/events/", "http://localhost/pt-br/events/past/", - ], + }, ) @override_settings( @@ -593,18 +591,17 @@ class TestCachePurgingSignals(TestCase): WAGTAIL_CONTENT_LANGUAGES=[("en", "English"), ("fr", "French")], ) def test_purge_on_publish_with_i18n_enabled(self): - PURGED_URLS[:] = [] # reset PURGED_URLS to the empty list page = EventIndex.objects.get(url_path="/home/events/") page.save_revision().publish() self.assertEqual( PURGED_URLS, - [ + { "http://localhost/en/events/", "http://localhost/en/events/past/", "http://localhost/fr/events/", "http://localhost/fr/events/past/", - ], + }, ) @override_settings( @@ -614,12 +611,11 @@ class TestCachePurgingSignals(TestCase): ) def test_purge_on_publish_without_i18n_enabled(self): # It should ignore WAGTAIL_CONTENT_LANGUAGES as WAGTAIL_I18N_ENABLED isn't set - PURGED_URLS[:] = [] # reset PURGED_URLS to the empty list page = EventIndex.objects.get(url_path="/home/events/") page.save_revision().publish() self.assertEqual( PURGED_URLS, - ["http://localhost/en/events/", "http://localhost/en/events/past/"], + {"http://localhost/en/events/", "http://localhost/en/events/past/"}, ) @@ -633,13 +629,13 @@ class TestPurgeBatchClass(TestCase): batch = PurgeBatch() batch.add_url("http://localhost/foo") - self.assertEqual(batch.urls, ["http://localhost/foo"]) + self.assertEqual(batch.urls, {"http://localhost/foo"}) def test_add_urls(self): batch = PurgeBatch() batch.add_urls(["http://localhost/foo", "http://localhost/bar"]) - self.assertEqual(batch.urls, ["http://localhost/foo", "http://localhost/bar"]) + self.assertEqual(batch.urls, {"http://localhost/foo", "http://localhost/bar"}) def test_add_page(self): page = EventIndex.objects.get(url_path="/home/events/") @@ -648,7 +644,7 @@ class TestPurgeBatchClass(TestCase): batch.add_page(page) self.assertEqual( - batch.urls, ["http://localhost/events/", "http://localhost/events/past/"] + batch.urls, {"http://localhost/events/", "http://localhost/events/past/"} ) def test_add_pages(self): @@ -656,7 +652,7 @@ class TestPurgeBatchClass(TestCase): batch.add_pages(EventIndex.objects.all()) self.assertEqual( - batch.urls, ["http://localhost/events/", "http://localhost/events/past/"] + batch.urls, {"http://localhost/events/", "http://localhost/events/past/"} ) def test_multiple_calls(self): @@ -669,11 +665,11 @@ class TestPurgeBatchClass(TestCase): self.assertEqual( batch.urls, - [ + { "http://localhost/events/", "http://localhost/events/past/", "http://localhost/foo", - ], + }, ) @mock.patch("wagtail.contrib.frontend_cache.backends.cloudflare.requests.delete") @@ -696,10 +692,8 @@ class TestPurgeBatchClass(TestCase): ) requests_delete_mock.side_effect = http_error - page = EventIndex.objects.get(url_path="/home/events/") - batch = PurgeBatch() - batch.add_page(page) + batch.add_url("http://localhost/events/") with self.assertLogs(level="ERROR") as log_output: batch.purge(backend_settings=backend_settings) diff --git a/wagtail/contrib/frontend_cache/utils.py b/wagtail/contrib/frontend_cache/utils.py index 25f6fbb063..a3f27a9f31 100644 --- a/wagtail/contrib/frontend_cache/utils.py +++ b/wagtail/contrib/frontend_cache/utils.py @@ -64,6 +64,15 @@ def purge_url_from_cache(url, backend_settings=None, backends=None): def purge_urls_from_cache(urls, backend_settings=None, backends=None): + if not urls: + return + + backends = get_backends(backend_settings, backends) + + # If no backends are configured, there's nothing to do + if not backends: + return + # Convert each url to urls one for each managed language (WAGTAILFRONTENDCACHE_LANGUAGES setting). # The managed languages are common to all the defined backends. # This depends on settings.USE_I18N @@ -105,8 +114,6 @@ def purge_urls_from_cache(urls, backend_settings=None, backends=None): for url in urls: urls_by_hostname[urlsplit(url).netloc].append(url) - backends = get_backends(backend_settings, backends) - for hostname, urls in urls_by_hostname.items(): backends_for_hostname = { backend_name: backend @@ -150,14 +157,14 @@ class PurgeBatch: """Represents a list of URLs to be purged in a single request""" def __init__(self, urls=None): - self.urls = [] + self.urls = set() if urls is not None: self.add_urls(urls) def add_url(self, url): """Adds a single URL""" - self.urls.append(url) + self.urls.add(url) def add_urls(self, urls): """ @@ -166,7 +173,7 @@ class PurgeBatch: This is equivalent to running ``.add_url(url)`` on each URL individually """ - self.urls.extend(urls) + self.urls.update(urls) def add_page(self, page): """ diff --git a/wagtail/contrib/redirects/models.py b/wagtail/contrib/redirects/models.py index 4053b263ce..adc6d8323f 100644 --- a/wagtail/contrib/redirects/models.py +++ b/wagtail/contrib/redirects/models.py @@ -5,7 +5,7 @@ from django.urls import Resolver404 from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ -from wagtail.models import Page +from wagtail.models import Page, Site class Redirect(models.Model): @@ -80,6 +80,21 @@ class Redirect(models.Model): return self.redirect_link return None + def old_links(self, site_root_paths=None): + """ + Determine the old URLs which this redirect might handle. + + :param site_root_paths: Pre-calculated root paths (obtained from `Site.get_site_root_paths`) + :return: List of old links + """ + if self.site_id is not None: + return {self.site.root_url + self.old_path} + + if site_root_paths is None: + site_root_paths = Site.get_site_root_paths() + + return {root_paths.root_url + self.old_path for root_paths in site_root_paths} + def get_is_permanent_display(self): if self.is_permanent: return _("permanent") diff --git a/wagtail/contrib/redirects/signal_handlers.py b/wagtail/contrib/redirects/signal_handlers.py index 13172596a7..1cdf93b60d 100644 --- a/wagtail/contrib/redirects/signal_handlers.py +++ b/wagtail/contrib/redirects/signal_handlers.py @@ -1,9 +1,11 @@ import logging from collections.abc import Iterable +from django.apps import apps from django.conf import settings from django.db.models import Q +from wagtail.contrib.frontend_cache.utils import PurgeBatch from wagtail.coreutils import BatchCreator, get_dummy_request from wagtail.models import Page, Site @@ -27,6 +29,19 @@ class BatchRedirectCreator(BatchCreator): clashes_q |= Q(old_path=item.old_path, site_id=item.site_id) Redirect.objects.filter(automatically_created=True).filter(clashes_q).delete() + def post_process(self): + if not apps.is_installed("wagtail.contrib.frontend_cache"): + return + + batch = PurgeBatch() + + site_root_paths = Site.get_site_root_paths() + + for redirect in self.items: + batch.add_urls(redirect.old_links(site_root_paths)) + + batch.purge() + def autocreate_redirects_on_slug_change( instance_before: Page, instance: Page, **kwargs diff --git a/wagtail/contrib/redirects/tests/test_redirects.py b/wagtail/contrib/redirects/tests/test_redirects.py index 4afb6a405d..708c9705d0 100644 --- a/wagtail/contrib/redirects/tests/test_redirects.py +++ b/wagtail/contrib/redirects/tests/test_redirects.py @@ -7,6 +7,7 @@ from django.urls import reverse from openpyxl.reader.excel import load_workbook from wagtail.admin.admin_url_finder import AdminURLFinder +from wagtail.contrib.frontend_cache.tests import PURGED_URLS from wagtail.contrib.redirects import models from wagtail.log_actions import registry as log_registry from wagtail.models import Page, Site @@ -791,11 +792,19 @@ class TestRedirectsIndexView(AdminTemplateTestUtils, WagtailTestUtils, TestCase) self.assertQuerySetEqual(qs, Page.objects.filter(pk=2)) +@override_settings( + WAGTAILFRONTENDCACHE={ + "dummy": { + "BACKEND": "wagtail.contrib.frontend_cache.tests.MockBackend", + }, + }, +) class TestRedirectsAddView(WagtailTestUtils, TestCase): fixtures = ["test.json"] def setUp(self): self.login() + PURGED_URLS.clear() def get(self, params={}): return self.client.get(reverse("wagtailredirects:add"), params) @@ -832,6 +841,8 @@ class TestRedirectsAddView(WagtailTestUtils, TestCase): log_entry = log_registry.get_logs_for_instance(redirect).first() self.assertEqual(log_entry.action, "wagtail.create") + self.assertEqual(PURGED_URLS, {"http://localhost/test"}) + def test_add_with_site(self): localhost = Site.objects.get(hostname="localhost") response = self.post( @@ -852,6 +863,8 @@ class TestRedirectsAddView(WagtailTestUtils, TestCase): self.assertEqual(redirects.first().redirect_link, "http://www.test.com/") self.assertEqual(redirects.first().site, localhost) + self.assertEqual(PURGED_URLS, {"http://localhost/test"}) + def test_add_validation_error(self): response = self.post( { @@ -864,6 +877,7 @@ class TestRedirectsAddView(WagtailTestUtils, TestCase): # Should not redirect to index self.assertEqual(response.status_code, 200) + self.assertEqual(PURGED_URLS, set()) def test_cannot_add_duplicate_with_no_site(self): models.Redirect.objects.create( @@ -880,6 +894,7 @@ class TestRedirectsAddView(WagtailTestUtils, TestCase): # Should not redirect to index self.assertEqual(response.status_code, 200) + self.assertEqual(PURGED_URLS, set()) def test_cannot_add_duplicate_on_same_site(self): localhost = Site.objects.get(hostname="localhost") @@ -897,6 +912,7 @@ class TestRedirectsAddView(WagtailTestUtils, TestCase): # Should not redirect to index self.assertEqual(response.status_code, 200) + self.assertEqual(PURGED_URLS, set()) def test_can_reuse_path_on_other_site(self): localhost = Site.objects.get(hostname="localhost") @@ -924,6 +940,8 @@ class TestRedirectsAddView(WagtailTestUtils, TestCase): redirects = models.Redirect.objects.filter(redirect_link="http://www.test.com/") self.assertEqual(redirects.count(), 1) + self.assertEqual(PURGED_URLS, redirects.get().old_links()) + def test_add_long_redirect(self): response = self.post( { @@ -946,7 +964,16 @@ class TestRedirectsAddView(WagtailTestUtils, TestCase): ) self.assertIsNone(redirects.first().site) + self.assertEqual(PURGED_URLS, redirects.get().old_links()) + +@override_settings( + WAGTAILFRONTENDCACHE={ + "dummy": { + "BACKEND": "wagtail.contrib.frontend_cache.tests.MockBackend", + }, + }, +) class TestRedirectsEditView(AdminTemplateTestUtils, WagtailTestUtils, TestCase): def setUp(self): # Create a redirect to edit @@ -958,6 +985,8 @@ class TestRedirectsEditView(AdminTemplateTestUtils, WagtailTestUtils, TestCase): # Login self.user = self.login() + PURGED_URLS.clear() + def get(self, params={}, redirect_id=None): return self.client.get( reverse("wagtailredirects:edit", args=(redirect_id or self.redirect.id,)), @@ -1010,6 +1039,8 @@ class TestRedirectsEditView(AdminTemplateTestUtils, WagtailTestUtils, TestCase): ) self.assertIsNone(redirects.first().site) + self.assertEqual(PURGED_URLS, {"http://localhost/test"}) + def test_edit_with_site(self): localhost = Site.objects.get(hostname="localhost") @@ -1032,6 +1063,7 @@ class TestRedirectsEditView(AdminTemplateTestUtils, WagtailTestUtils, TestCase): redirects.first().redirect_link, "http://www.test.com/ive-been-edited" ) self.assertEqual(redirects.first().site, localhost) + self.assertEqual(PURGED_URLS, {"http://localhost/test"}) def test_edit_validation_error(self): response = self.post( @@ -1045,6 +1077,7 @@ class TestRedirectsEditView(AdminTemplateTestUtils, WagtailTestUtils, TestCase): # Should not redirect to index self.assertEqual(response.status_code, 200) + self.assertEqual(PURGED_URLS, set()) def test_edit_duplicate(self): models.Redirect.objects.create( @@ -1061,6 +1094,7 @@ class TestRedirectsEditView(AdminTemplateTestUtils, WagtailTestUtils, TestCase): # Should not redirect to index self.assertEqual(response.status_code, 200) + self.assertEqual(PURGED_URLS, set()) def test_get_with_no_permission(self, redirect_id=None): self.user.is_superuser = False @@ -1096,6 +1130,13 @@ class TestRedirectsEditView(AdminTemplateTestUtils, WagtailTestUtils, TestCase): self.assertTemplateUsed(response, "wagtailredirects/edit.html") +@override_settings( + WAGTAILFRONTENDCACHE={ + "dummy": { + "BACKEND": "wagtail.contrib.frontend_cache.tests.MockBackend", + }, + }, +) class TestRedirectsDeleteView(WagtailTestUtils, TestCase): def setUp(self): # Create a redirect to edit @@ -1107,6 +1148,8 @@ class TestRedirectsDeleteView(WagtailTestUtils, TestCase): # Login self.login() + PURGED_URLS.clear() + def get(self, params={}, redirect_id=None): return self.client.get( reverse("wagtailredirects:delete", args=(redirect_id or self.redirect.id,)), @@ -1135,3 +1178,5 @@ class TestRedirectsDeleteView(WagtailTestUtils, TestCase): # Check that the redirect was deleted redirects = models.Redirect.objects.filter(old_path="/test") self.assertEqual(redirects.count(), 0) + + self.assertEqual(PURGED_URLS, {"http://localhost/test"}) diff --git a/wagtail/contrib/redirects/tests/test_signal_handlers.py b/wagtail/contrib/redirects/tests/test_signal_handlers.py index dc3b3e6626..99e87befde 100644 --- a/wagtail/contrib/redirects/tests/test_signal_handlers.py +++ b/wagtail/contrib/redirects/tests/test_signal_handlers.py @@ -1,6 +1,7 @@ from django.contrib.auth import get_user_model from django.test import TestCase, override_settings +from wagtail.contrib.frontend_cache.tests import PURGED_URLS from wagtail.contrib.redirects.models import Redirect from wagtail.coreutils import get_dummy_request from wagtail.models import Page, Site @@ -11,7 +12,14 @@ from wagtail.test.utils import WagtailTestUtils User = get_user_model() -@override_settings(WAGTAILREDIRECTS_AUTO_CREATE=True) +@override_settings( + WAGTAILREDIRECTS_AUTO_CREATE=True, + WAGTAILFRONTENDCACHE={ + "dummy": { + "BACKEND": "wagtail.contrib.frontend_cache.tests.MockBackend", + }, + }, +) class TestAutocreateRedirects(WagtailTestUtils, TestCase): fixtures = ["test.json"] @@ -25,6 +33,8 @@ class TestAutocreateRedirects(WagtailTestUtils, TestCase): self.event_index = EventIndex.objects.get() self.other_page = Page.objects.get(url_path="/home/about-us/") + PURGED_URLS.clear() + def trigger_page_slug_changed_signal(self, page): page.slug += "-extra" with self.captureOnCommitCallbacks(execute=True): @@ -76,9 +86,20 @@ class TestAutocreateRedirects(WagtailTestUtils, TestCase): # the automatically_created flag should have been set to True self.assertTrue(r.automatically_created) + self.assertEqual( + PURGED_URLS, + { + "http://localhost/events/saint-patrick/pointless-suffix", + "http://localhost/events/final-event", + "http://localhost/events/christmas", + "http://localhost/events", + }, + ) + def test_no_redirects_created_when_page_is_root_for_all_sites_it_belongs_to(self): self.trigger_page_slug_changed_signal(self.home_page) self.assertFalse(Redirect.objects.exists()) + self.assertEqual(len(PURGED_URLS), 0) def test_handling_of_existing_redirects(self): # the page we'll be triggering the change for here is... @@ -131,6 +152,16 @@ class TestAutocreateRedirects(WagtailTestUtils, TestCase): ).exists() ) + self.assertEqual( + PURGED_URLS, + { + "http://localhost/events/saint-patrick/pointless-suffix", + "http://localhost/events/final-event", + "http://localhost/events/christmas", + "http://localhost/events", + }, + ) + def test_redirect_creation_for_custom_route_paths(self): # Add a page that has overridden get_route_paths() homepage = Page.objects.get(id=2) @@ -165,6 +196,14 @@ class TestAutocreateRedirects(WagtailTestUtils, TestCase): ), ], ) + self.assertEqual( + PURGED_URLS, + { + "http://localhost/routable-page", + "http://localhost/routable-page/not-a-valid-route", + "http://localhost/routable-page/render-method-test", + }, + ) def test_no_redirects_created_when_pages_are_moved_to_a_different_site(self): # Add a new home page @@ -187,8 +226,10 @@ class TestAutocreateRedirects(WagtailTestUtils, TestCase): # No redirects should have been created self.assertFalse(Redirect.objects.exists()) + self.assertEqual(len(PURGED_URLS), 0) @override_settings(WAGTAILREDIRECTS_AUTO_CREATE=False) def test_no_redirects_created_if_disabled(self): self.trigger_page_slug_changed_signal(self.event_index) self.assertFalse(Redirect.objects.exists()) + self.assertEqual(len(PURGED_URLS), 0) diff --git a/wagtail/contrib/redirects/views.py b/wagtail/contrib/redirects/views.py index 35394a1a3e..950b3f3386 100644 --- a/wagtail/contrib/redirects/views.py +++ b/wagtail/contrib/redirects/views.py @@ -17,6 +17,7 @@ from wagtail.admin.forms.search import SearchForm from wagtail.admin.ui.tables import Column, StatusTagColumn, TitleColumn from wagtail.admin.views import generic from wagtail.admin.widgets.button import Button +from wagtail.contrib.frontend_cache.utils import PurgeBatch, purge_urls_from_cache from wagtail.contrib.redirects import models from wagtail.contrib.redirects.filters import RedirectsReportFilterSet from wagtail.contrib.redirects.forms import ( @@ -35,6 +36,7 @@ from wagtail.contrib.redirects.utils import ( write_to_file_storage, ) from wagtail.log_actions import log +from wagtail.models import Site permission_checker = PermissionPolicyChecker(permission_policy) @@ -153,6 +155,20 @@ class EditView(generic.EditView): "redirect_title": self.object.title } + def save_instance(self): + purger = PurgeBatch() + + root_paths = Site.get_site_root_paths() + purger.add_urls(self.object.old_links(root_paths)) + + instance = super().save_instance() + + purger.add_urls(self.object.old_links(root_paths)) + + purger.purge() + + return instance + @permission_checker.require("delete") def delete(request, redirect_id): @@ -167,6 +183,9 @@ def delete(request, redirect_id): with transaction.atomic(): log(instance=theredirect, action="wagtail.delete") theredirect.delete() + + purge_urls_from_cache(theredirect.old_links()) + messages.success( request, _("Redirect '%(redirect_title)s' deleted.") @@ -192,6 +211,8 @@ def add(request): theredirect = form.save() log(instance=theredirect, action="wagtail.create") + purge_urls_from_cache(theredirect.old_links()) + messages.success( request, _("Redirect '%(redirect_title)s' added.")