From 69724e4e3a3d372fa3cd47c66c11df74daa203bb Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Thu, 3 Nov 2022 17:35:17 +0000 Subject: [PATCH] Create preview-aware & page-enhanced cache template tags This can be used in other places, but ensures caches are invalidated whenever something about a page changes. - Add a util to get wagtail-specific fragment cache keys - Don't pollute context when injecting site variable - Add documentation on wagtail fragment caching - Define an intelligent cache key for pages - Allow the components of the cache key to be easily modified - Note that some manual changes may not create a new cache key Co-authored-by: Andy Babic Closes #5074 --- CHANGELOG.txt | 1 + docs/advanced_topics/performance.md | 31 +++ docs/reference/pages/model_reference.md | 4 + docs/releases/5.2.md | 2 +- docs/topics/writing_templates.md | 58 ++++++ wagtail/coreutils.py | 12 ++ wagtail/models/__init__.py | 31 ++- wagtail/templatetags/wagtail_cache.py | 98 +++++++++ wagtail/tests/test_page_model.py | 23 +++ wagtail/tests/tests.py | 263 +++++++++++++++++++++++- 10 files changed, 520 insertions(+), 3 deletions(-) create mode 100644 wagtail/templatetags/wagtail_cache.py diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 072d8d419a..64b577b639 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -4,6 +4,7 @@ Changelog 5.2 (xx.xx.xxxx) - IN DEVELOPMENT ~~~~~~~~~~~~~~~~ + * Add preview-aware and page-aware fragment caching template tags, `wagtailcache` & `wagtailpagecache` (Jake Howard) * Maintenance: Fix snippet search test to work on non-fallback database backends (Matt Westcott) diff --git a/docs/advanced_topics/performance.md b/docs/advanced_topics/performance.md index 6cbae1cd1f..9b046cec6c 100644 --- a/docs/advanced_topics/performance.md +++ b/docs/advanced_topics/performance.md @@ -100,6 +100,37 @@ For some images, it may be beneficial to lazy load images, so the rest of the pa This optimisation is already handled for you for images in the admin site. +## Template fragment caching + +Django supports [template fragment caching](https://docs.djangoproject.com/en/stable/topics/cache/#template-fragment-caching), which allows caching portions of a template. Using Django's `{% cache %}` tag natively with Wagtail can be [dangerous](https://github.com/wagtail/wagtail/issues/5074) as it can result in preview content being shown to end users. Instead, Wagtail provides 2 extra template tags: [`{% wagtailcache %}`](wagtailcache) and [`{% wagtailpagecache %}`](wagtailpagecache) which both avoid these issues. + +(page_cache_key)= + +## Page cache key + +It's often necessary to cache a value based on an entire page, rather than a specific value. For this, {attr}`~wagtail.models.Page.cache_key` can be used to get a unique value for the state of a page. Should something about the page change, so will its cache key. You can also use the value to create longer, more specific cache keys when using Django's caching framework directly. For example: + +```python +from django.core.cache import cache + +result = page.expensive_operation() +cache.set("expensive_result_" + page.cache_key, result, 3600) + +# Later... +cache.get("expensive_result_" + page.cache_key) +``` + +To modify the cache key, such as including a custom model field value, you can override {attr}`~wagtail.models.Page.get_cache_key_components`: + +```python +def get_cache_key_components(self): + components = super().get_cache_key_components() + components.append(self.external_slug) + return components +``` + +Manually updating a page might not result in a change to its cache key, unless the default component field values are modified directly. To be sure of a change in the cache key value, try saving the changes to a `Revision` instead, and then publishing it. + ## Django Wagtail is built on Django. Many of the [performance tips](django:topics/performance) set out by Django are also applicable to Wagtail. diff --git a/docs/reference/pages/model_reference.md b/docs/reference/pages/model_reference.md index 4bbe505dff..6b2f774834 100644 --- a/docs/reference/pages/model_reference.md +++ b/docs/reference/pages/model_reference.md @@ -315,6 +315,10 @@ See also [django-treebeard](https://django-treebeard.readthedocs.io/en/latest/in .. automethod:: create_alias .. automethod:: update_aliases + + .. automethod:: get_cache_key_components + + .. autoattribute:: cache_key ``` (site_model_ref)= diff --git a/docs/releases/5.2.md b/docs/releases/5.2.md index 002148d753..60d6928793 100644 --- a/docs/releases/5.2.md +++ b/docs/releases/5.2.md @@ -14,7 +14,7 @@ depth: 1 ### Other features - * ... + * Add [`wagtailcache`](wagtailcache) and [`wagtailpagecache`](wagtailpagecache) template tags to ensure previewing Pages or Snippets will not be cached (Jake Howard) ### Bug fixes diff --git a/docs/topics/writing_templates.md b/docs/topics/writing_templates.md index 6472d12532..7cec846ef0 100644 --- a/docs/topics/writing_templates.md +++ b/docs/topics/writing_templates.md @@ -290,3 +290,61 @@ Sometimes you may wish to vary the template output depending on whether the page If the page is being previewed, `request.preview_mode` can be used to determine the specific preview mode being used, if the page supports [multiple preview modes](wagtail.models.Page.preview_modes). + +(template_fragment_caching)= + +## Template fragment caching + +Django supports [template fragment caching](https://docs.djangoproject.com/en/stable/topics/cache/#template-fragment-caching), which allows caching portions of a template. Using Django's `{% cache %}` tag natively with Wagtail can be [dangerous](https://github.com/wagtail/wagtail/issues/5074) as it can result in preview content being shown to end users. Instead, Wagtail provides 2 extra template tags which can be loaded from `wagtail_cache`: + +(wagtailcache)= + +### Preview-aware caching + +The `{% wagtailcache %}` tag functions similarly to Django's `{% cache %}` tag, but will neither cache or serve cached content when previewing a page (or other model) in Wagtail. + +```html+django +{% load wagtail_cache %} + +{% wagtailcache 500 "sidebar" %} + +{% endwagtailcache %} +``` + +Much like `{% cache %}`, you can use [`make_template_fragment_key`](django.core.cache.utils.make_template_fragment_key) to obtain the cache key. + +(wagtailpagecache)= + +### Page-aware caching + +`{% wagtailpagecache %}` is an extension of `{% wagtailcache %}`, but is also aware of the current `page` and `site`, and includes those as part of the cache key. This makes it possible to easily add caching around parts of the page without worrying about the page it's on. `{% wagtailpagecache %}` intentionally makes assumptions - for more customisation it's recommended to use `{% wagtailcache %}`. + +```html+django +{% load wagtail_cache %} + +{% wagtailpagecache 500 "hero" %} + +{% endwagtailcache %} +``` + +This is identical to: + +```html+django +{% wagtail_site as current_site %} + +{% wagtailcache 500 "hero" page.cache_key current_site.id %} + +{% endwagtailcache %} +``` + +Note the use of the page's [cache key](page_cache_key), which ensures that when a page is updated, the cache is automatically invalidated. + +If you want to obtain the cache key, you can use `make_wagtail_template_fragment_key` (based on Django's [`make_template_fragment_key`](django.core.cache.utils.make_template_fragment_key)): + +```python +from django.core.cache import cache +from wagtail.coreutils import make_wagtail_template_fragment_key + +key = make_wagtail_template_fragment_key("hero", page, site) +cache.delete(key) # invalidates cached template fragment +``` diff --git a/wagtail/coreutils.py b/wagtail/coreutils.py index e2fb2364b1..6628666880 100644 --- a/wagtail/coreutils.py +++ b/wagtail/coreutils.py @@ -10,6 +10,7 @@ from anyascii import anyascii from django.apps import apps from django.conf import settings from django.conf.locale import LANG_INFO +from django.core.cache.utils import make_template_fragment_key from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation from django.core.signals import setting_changed from django.db.models import Model @@ -560,3 +561,14 @@ class BatchCreator(BatchProcessor): def get_summary(self): opts = self.model._meta return f"{self.created_count}/{self.added_count} {opts.verbose_name_plural} were created successfully." + + +def make_wagtail_template_fragment_key(fragment_name, page, site, vary_on=None): + """ + A modified version of `make_template_fragment_key` which varies on page and + site for use with `{% wagtailpagecache %}`. + """ + if vary_on is None: + vary_on = [] + vary_on.extend([page.cache_key, site.id]) + return make_template_fragment_key(fragment_name, vary_on) diff --git a/wagtail/models/__init__.py b/wagtail/models/__init__.py index 04018ce4c4..633731e6b6 100644 --- a/wagtail/models/__init__.py +++ b/wagtail/models/__init__.py @@ -42,7 +42,7 @@ from django.urls import NoReverseMatch, reverse from django.utils import timezone from django.utils import translation as translation from django.utils.cache import patch_cache_control -from django.utils.encoding import force_str +from django.utils.encoding import force_bytes, force_str from django.utils.functional import Promise, cached_property from django.utils.module_loading import import_string from django.utils.text import capfirst, slugify @@ -70,6 +70,7 @@ from wagtail.coreutils import ( get_content_type_label, get_supported_content_language_variant, resolve_model_string, + safe_md5, ) from wagtail.fields import StreamField from wagtail.forms import TaskStateCommentForm @@ -2429,6 +2430,34 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): """ return ["/"] + def get_cache_key_components(self): + """ + The components of a :class:`Page` which make up the :attr:`cache_key`. Any change to a + page should be reflected in a change to at least one of these components. + """ + + return [ + self.id, + self.url_path, + self.last_published_at.isoformat() if self.last_published_at else None, + ] + + @property + def cache_key(self): + """ + A generic cache key to identify a page in its current state. + Should the page change, so will the key. + + Customizations to the cache key should be made in :attr:`get_cache_key_components`. + """ + + hasher = safe_md5() + + for component in self.get_cache_key_components(): + hasher.update(force_bytes(component)) + + return hasher.hexdigest() + def get_sitemap_urls(self, request=None): return [ { diff --git a/wagtail/templatetags/wagtail_cache.py b/wagtail/templatetags/wagtail_cache.py new file mode 100644 index 0000000000..503b5f71ba --- /dev/null +++ b/wagtail/templatetags/wagtail_cache.py @@ -0,0 +1,98 @@ +from django import template +from django.template import Variable +from django.template.exceptions import TemplateSyntaxError +from django.templatetags.cache import CacheNode as DjangoCacheNode + +from wagtail.models import PAGE_TEMPLATE_VAR, Site + +register = template.Library() + + +class WagtailCacheNode(DjangoCacheNode): + """ + A modified version of Django's `CacheNode` which is aware of Wagtail's + page previews. + """ + + def render(self, context): + try: + request = context["request"] + except KeyError: + # When there's no request, it's not possible to tell whether this is a preview or not. + # Bypass the cache to be safe. + return self.nodelist.render(context) + + if getattr(request, "is_preview", False): + # Skip cache in preview + return self.nodelist.render(context) + + return super().render(context) + + +class WagtailPageCacheNode(WagtailCacheNode): + """ + A modified version of Django's `CacheNode` designed for caching fragments + of pages. + + This tag intentionally makes assumptions about what context is available. + If these assumptions aren't valid, it's recommended to just use `{% wagtailcache %}`. + """ + + CACHE_SITE_TEMPLATE_VAR = "wagtail_page_cache_site" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + # Pretend the user specified the page and site as part of context + self.vary_on.extend( + [ + Variable(f"{PAGE_TEMPLATE_VAR}.cache_key"), + Variable(f"{self.CACHE_SITE_TEMPLATE_VAR}.pk"), + ] + ) + + def render(self, context): + if "request" in context: + # Inject the site into context to be picked up when resolving `vary_on` + with context.update( + { + self.CACHE_SITE_TEMPLATE_VAR: Site.find_for_request( + context["request"] + ) + } + ): + return super().render(context) + return super().render(context) + + +def register_cache_tag(tag_name, node_class): + """ + A helper function to define cache tags without duplicating `do_cache`. + """ + + @register.tag(tag_name) + def do_cache(parser, token): + # Implementation copied from `django.templatetags.cache.do_cache` + nodelist = parser.parse((f"end{tag_name}",)) + parser.delete_first_token() + tokens = token.split_contents() + if len(tokens) < 3: + raise TemplateSyntaxError( + f"'{tokens[0]}' tag requires at least 2 arguments." + ) + if len(tokens) > 3 and tokens[-1].startswith("using="): + cache_name = parser.compile_filter(tokens[-1][len("using=") :]) + tokens = tokens[:-1] + else: + cache_name = None + return node_class( + nodelist, + parser.compile_filter(tokens[1]), + tokens[2], # fragment_name can't be a variable. + [parser.compile_filter(t) for t in tokens[3:]], + cache_name, + ) + + +register_cache_tag("wagtailcache", WagtailCacheNode) +register_cache_tag("wagtailpagecache", WagtailPageCacheNode) diff --git a/wagtail/tests/test_page_model.py b/wagtail/tests/test_page_model.py index 9b978369c5..231074ab3f 100644 --- a/wagtail/tests/test_page_model.py +++ b/wagtail/tests/test_page_model.py @@ -3795,3 +3795,26 @@ class TestGetLock(TestCase): # This is because it shouldn't be possible to create a separate draft from what is scheduled to be published superuser = get_user_model().objects.get(email="superuser@example.com") self.assertTrue(lock.for_user(superuser)) + + +class TestPageCacheKey(TestCase): + fixtures = ["test.json"] + + def setUp(self): + self.page = Page.objects.last() + self.other_page = Page.objects.first() + + def test_cache_key_consistent(self): + self.assertEqual(self.page.cache_key, self.page.cache_key) + self.assertEqual(self.other_page.cache_key, self.other_page.cache_key) + + def test_no_queries(self): + with self.assertNumQueries(0): + self.page.cache_key + self.other_page.cache_key + + def test_changes_when_slug_changes(self): + original_cache_key = self.page.cache_key + self.page.slug = "something-else" + self.page.save() + self.assertNotEqual(self.page.cache_key, original_cache_key) diff --git a/wagtail/tests/tests.py b/wagtail/tests/tests.py index 6aef2e85a3..445d726f75 100644 --- a/wagtail/tests/tests.py +++ b/wagtail/tests/tests.py @@ -2,18 +2,25 @@ import json from django import template from django.core.cache import cache +from django.core.cache.utils import make_template_fragment_key from django.http import HttpRequest +from django.template import TemplateSyntaxError, VariableDoesNotExist from django.test import TestCase from django.test.utils import override_settings from django.urls.exceptions import NoReverseMatch from django.utils.safestring import SafeString -from wagtail.coreutils import get_dummy_request, resolve_model_string +from wagtail.coreutils import ( + get_dummy_request, + make_wagtail_template_fragment_key, + resolve_model_string, +) from wagtail.models import Locale, Page, Site, SiteRootPath from wagtail.models.sites import ( SITE_ROOT_PATHS_CACHE_KEY, SITE_ROOT_PATHS_CACHE_VERSION, ) +from wagtail.templatetags.wagtail_cache import WagtailPageCacheNode from wagtail.templatetags.wagtailcore_tags import richtext, slugurl from wagtail.test.testapp.models import SimplePage @@ -543,3 +550,257 @@ class TestRichtextTag(TestCase): TypeError, "'richtext' template filter received an invalid value" ): richtext(b"Hello world!") + + +class TestWagtailCacheTag(TestCase): + def setUp(self): + cache.clear() + + def test_caches(self): + request = get_dummy_request() + tpl = template.Template( + """{% load wagtail_cache %}{% wagtailcache 100 test %}{{ foo.bar }}{% endwagtailcache %}""" + ) + + result = tpl.render( + template.Context({"request": request, "foo": {"bar": "foobar"}}) + ) + self.assertEqual(result, "foobar") + + result2 = tpl.render( + template.Context({"request": request, "foo": {"bar": "baz"}}) + ) + self.assertEqual(result2, "foobar") + + self.assertEqual(cache.get(make_template_fragment_key("test")), "foobar") + + def test_caches_on_additional_parameters(self): + request = get_dummy_request() + tpl = template.Template( + """{% load wagtail_cache %}{% wagtailcache 100 test foo %}{{ foo.bar }}{% endwagtailcache %}""" + ) + + result = tpl.render( + template.Context({"request": request, "foo": {"bar": "foobar"}}) + ) + self.assertEqual(result, "foobar") + + result2 = tpl.render( + template.Context({"request": request, "foo": {"bar": "baz"}}) + ) + self.assertEqual(result2, "baz") + + self.assertEqual( + cache.get(make_template_fragment_key("test", [{"bar": "foobar"}])), "foobar" + ) + self.assertEqual( + cache.get(make_template_fragment_key("test", [{"bar": "baz"}])), "baz" + ) + + def test_skips_cache_in_preview(self): + request = get_dummy_request() + request.is_preview = True + + tpl = template.Template( + """{% load wagtail_cache %}{% wagtailcache 100 test %}{{ foo.bar }}{% endwagtailcache %}""" + ) + + result = tpl.render( + template.Context({"request": request, "foo": {"bar": "foobar"}}) + ) + self.assertEqual(result, "foobar") + + result2 = tpl.render( + template.Context({"request": request, "foo": {"bar": "baz"}}) + ) + self.assertEqual(result2, "baz") + + self.assertIsNone(cache.get(make_template_fragment_key("test"))) + + def test_no_request(self): + tpl = template.Template( + """{% load wagtail_cache %}{% wagtailcache 100 test %}{{ foo.bar }}{% endwagtailcache %}""" + ) + + result = tpl.render(template.Context({"foo": {"bar": "foobar"}})) + self.assertEqual(result, "foobar") + + result2 = tpl.render(template.Context({"foo": {"bar": "baz"}})) + self.assertEqual(result2, "baz") + + self.assertIsNone(cache.get(make_template_fragment_key("test"))) # + + def test_invalid_usage(self): + with self.assertRaises(TemplateSyntaxError) as e: + template.Template( + """{% load wagtail_cache %}{% wagtailcache 100 %}{{ foo.bar }}{% endwagtailcache %}""" + ) + self.assertEqual( + e.exception.args[0], "'wagtailcache' tag requires at least 2 arguments." + ) + + +class TestWagtailPageCacheTag(TestCase): + fixtures = ["test.json"] + + @classmethod + def setUpTestData(cls): + cls.page_1 = Page.objects.first() + cls.page_2 = Page.objects.all()[2] + cls.site = Site.objects.get(hostname="localhost", port=80) + + def test_caches(self): + request = get_dummy_request(site=self.site) + tpl = template.Template( + """{% load wagtail_cache %}{% wagtailpagecache 100 test %}{{ foo.bar }}{% endwagtailpagecache %}""" + ) + + result = tpl.render( + template.Context( + {"request": request, "foo": {"bar": "foobar"}, "page": self.page_1} + ) + ) + self.assertEqual(result, "foobar") + + result2 = tpl.render( + template.Context( + {"request": request, "foo": {"bar": "baz"}, "page": self.page_1} + ) + ) + self.assertEqual(result2, "foobar") + + self.assertEqual( + cache.get( + make_wagtail_template_fragment_key("test", self.page_1, self.site) + ), + "foobar", + ) + + def test_caches_additional_parameters(self): + request = get_dummy_request(site=self.site) + tpl = template.Template( + """{% load wagtail_cache %}{% wagtailpagecache 100 test foo %}{{ foo.bar }}{% endwagtailpagecache %}""" + ) + + result = tpl.render( + template.Context( + {"request": request, "foo": {"bar": "foobar"}, "page": self.page_1} + ) + ) + self.assertEqual(result, "foobar") + + result2 = tpl.render( + template.Context( + {"request": request, "foo": {"bar": "baz"}, "page": self.page_1} + ) + ) + self.assertEqual(result2, "baz") + + self.assertEqual( + cache.get( + make_wagtail_template_fragment_key( + "test", self.page_1, self.site, [{"bar": "foobar"}] + ) + ), + "foobar", + ) + self.assertEqual( + cache.get( + make_wagtail_template_fragment_key( + "test", self.page_1, self.site, [{"bar": "baz"}] + ) + ), + "baz", + ) + + def test_doesnt_pollute_cache(self): + request = get_dummy_request(site=self.site) + tpl = template.Template( + """{% load wagtail_cache %}{% wagtailpagecache 100 test %}{{ foo.bar }}{% endwagtailpagecache %}""" + ) + + context = template.Context( + {"request": request, "foo": {"bar": "foobar"}, "page": self.page_1} + ) + result = tpl.render(context) + self.assertEqual(result, "foobar") + + self.assertNotIn(WagtailPageCacheNode.CACHE_SITE_TEMPLATE_VAR, context) + + def test_skips_cache_in_preview(self): + request = get_dummy_request(site=self.site) + request.is_preview = True + + tpl = template.Template( + """{% load wagtail_cache %}{% wagtailpagecache 100 test %}{{ foo.bar }}{% endwagtailpagecache %}""" + ) + + result = tpl.render( + template.Context( + {"request": request, "foo": {"bar": "foobar"}, "page": self.page_1} + ) + ) + self.assertEqual(result, "foobar") + + result2 = tpl.render( + template.Context( + {"request": request, "foo": {"bar": "baz"}, "page": self.page_1} + ) + ) + self.assertEqual(result2, "baz") + + self.assertIsNone( + cache.get( + make_wagtail_template_fragment_key("test", self.page_1, self.site) + ) + ) + + def test_no_request(self): + tpl = template.Template( + """{% load wagtail_cache %}{% wagtailpagecache 100 test %}{{ foo.bar }}{% endwagtailpagecache %}""" + ) + + result = tpl.render( + template.Context({"foo": {"bar": "foobar"}, "page": self.page_1}) + ) + self.assertEqual(result, "foobar") + + result2 = tpl.render( + template.Context({"foo": {"bar": "baz"}, "page": self.page_1}) + ) + self.assertEqual(result2, "baz") + + self.assertIsNone( + cache.get( + make_wagtail_template_fragment_key("test", self.page_1, self.site) + ) + ) + + def test_no_page(self): + request = get_dummy_request() + + tpl = template.Template( + """{% load wagtail_cache %}{% wagtailpagecache 100 test %}{{ foo.bar }}{% endwagtailpagecache %}""" + ) + + with self.assertRaises(VariableDoesNotExist) as e: + tpl.render(template.Context({"request": request, "foo": {"bar": "foobar"}})) + + self.assertEqual(e.exception.params[0], "page") + + def test_cache_key(self): + self.assertEqual( + make_wagtail_template_fragment_key("test", self.page_1, self.site), + make_template_fragment_key( + "test", vary_on=[self.page_1.cache_key, self.site.id] + ), + ) + + def test_invalid_usage(self): + with self.assertRaises(TemplateSyntaxError) as e: + template.Template( + """{% load wagtail_cache %}{% wagtailpagecache 100 %}{{ foo.bar }}{% endwagtailpagecache %}""" + ) + self.assertEqual( + e.exception.args[0], "'wagtailpagecache' tag requires at least 2 arguments." + )