Add no-cache headers to draft blog posts

[alarthast]

Fixes #2158.

Add headers to never cache draft blog posts (since we want changes to be reflected immediately).

Note that Django's cache middleware will not cache responses when the Cache-Control header is set to no-cache, no-store, or private, which is our case here for draft blog posts.

[alarthast]

(Note that when looking at this, @thibaudcolas and I found some code at the model layer related to caching:

djangoproject.com/blog/models.py

Lines 189 to 204 in ff9ea01

	self.invalidate_cached_entry()
	def invalidate_cached_entry(self):
	url = urlparse(self.get_absolute_url())
	rf = RequestFactory(
	SERVER_NAME=url.netloc,
	HTTP_X_FORWARDED_PROTOCOL=url.scheme,
	)
	is_secure = url.scheme == "https"
	request = rf.get(url.path, secure=is_secure)
	request.LANGUAGE_CODE = "en"
	cache = caches[settings.CACHE_MIDDLEWARE_ALIAS]
	cache_key = _generate_cache_header_key(
	settings.CACHE_MIDDLEWARE_KEY_PREFIX, request
	)
	cache.delete(cache_key)

We ignored these lines, since we suspect that this block of code might be doing nothing or related to a different cache than the one we are interested in.)

[ulgens]

Do we have the same need for any other model/view? I'd say that this looks more of a middleware problem, but I guess it would be an overkill if this is the only place we need it.

[tobiasmcnulty]

Nice find! I thought Django's cache middleware might still cache the page, but TIL setting the Cache-Control header manually prevents that. Thanks. :)

In testing this locally I came up with a few nitpicks, feel free to go ahead without them though.

diff --git a/blog/tests.py b/blog/tests.py
index 85c884c7..1f999ca4 100644
--- a/blog/tests.py
+++ b/blog/tests.py
@@ -3,10 +3,12 @@ from datetime import date, timedelta
 from io import StringIO
 
 import time_machine
+from django.conf import settings
 from django.contrib.auth.models import Permission, User
 from django.contrib.contenttypes.models import ContentType
 from django.core.files.base import ContentFile
 from django.test import TestCase
+from django.test.utils import override_settings
 from django.urls import reverse
 from django.utils import timezone, translation
 
@@ -406,9 +408,20 @@ class ViewsTestCase(DateTimeMixin, TestCase):
         response = self.client.get(published_url)
         self.assertEqual(response.status_code, 200)
 
+
+@override_settings(
+    # Caching middleware is added in the production settings file;
+    # simulate that here for the tests.
+    MIDDLEWARE=(
+        ["django.middleware.cache.UpdateCacheMiddleware"]
+        + settings.MIDDLEWARE
+        + ["django.middleware.cache.FetchFromCacheMiddleware"]
+    ),
+)
+class ViewsCachingTestCase(DateTimeMixin, TestCase):
     def test_drafts_have_no_cache_headers(self):
         """
-        Test that draft (unpublished) entries have no-cache headers.
+        Draft (unpublished) entries have no-cache headers.
         """
         user = User.objects.create(username="staff", is_staff=True)
         content_type = ContentType.objects.get_for_model(Entry)
@@ -438,11 +451,14 @@ class ViewsTestCase(DateTimeMixin, TestCase):
 
         self.assertEqual(response.status_code, 200)
         self.assertIn("Cache-Control", response.headers)
-        self.assertEqual(response.headers["Cache-Control"], "private, no-cache")
+        self.assertEqual(
+            response.headers["Cache-Control"],
+            "max-age=0, no-cache, no-store, must-revalidate, private",
+        )
 
-    def test_published_blogs_do_not_have_cache_control_headers(self):
+    def test_published_blogs_have_cache_control_headers(self):
         """
-        Test that published blog posts don't have Cache-Control headers.
+        Published blog posts has Cache-Control header.
         """
         entry = Entry.objects.create(
             pub_date=self.yesterday,
@@ -461,7 +477,7 @@ class ViewsTestCase(DateTimeMixin, TestCase):
         )
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-        self.assertNotIn("Cache-Control", response.headers)
+        self.assertEqual(response.headers["Cache-Control"], "max-age=300")
 
 
 class SitemapTests(DateTimeMixin, TestCase):
diff --git a/blog/views.py b/blog/views.py
index ca104380..ac51f7c1 100644
--- a/blog/views.py
+++ b/blog/views.py
@@ -1,3 +1,4 @@
+from django.utils.cache import add_never_cache_headers
 from django.views.generic.dates import (
     ArchiveIndexView,
     DateDetailView,
@@ -62,5 +63,5 @@ class BlogDateDetailView(BlogViewMixin, DateDetailView):
     def get(self, request, *args, **kwargs):
         response = super().get(request, *args, **kwargs)
         if not self.object.is_published():
-            response["Cache-Control"] = "private, no-cache"
+            add_never_cache_headers(response)
         return response

[alarthast]

Thank you for the review! I have included all the changes and edited the commit message. I hope that adding you as a co-author is all right.

And thanks for for letting me have a go at this! :)

[tobiasmcnulty]

Thank you! Looks good to me. 🚢

I'll leave this open for another day or two in case anyone else would like to review first.