diff --git a/rcvis/settings.py b/rcvis/settings.py index d1cb3166..dfb5b5c1 100644 --- a/rcvis/settings.py +++ b/rcvis/settings.py @@ -82,7 +82,7 @@ 'django.contrib.sessions.middleware.SessionMiddleware', # Order of the next 3 is important - 'visualizer.middleware.UpdateCacheWithoutMaxAgeMiddleware', + 'django.middleware.cache.UpdateCacheMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.http.ConditionalGetMiddleware', 'django.middleware.cache.FetchFromCacheMiddleware', @@ -289,6 +289,13 @@ } } +# Lifetime of pages in the server-side cache, and the max-age sent to browsers. +# In production Cloudflare ignores the origin's Cache-Control and applies its +# own Browser TTL of 5 minutes, so this mirrors that value to keep local +# development honest about how stale a browser can be after an update. +# Keep in sync with the Browser TTL in the Cloudflare cache rule. +CACHE_MIDDLEWARE_SECONDS = 300 + REST_FRAMEWORK = { # Use Django's standard `django.contrib.auth` permissions, # or allow read-only access for unauthenticated users. diff --git a/scripts/check-live-cache.sh b/scripts/check-live-cache.sh new file mode 100755 index 00000000..6a24cb65 --- /dev/null +++ b/scripts/check-live-cache.sh @@ -0,0 +1,109 @@ +#!/bin/bash +# Checks that a visualization is served from cache on the live site, so that +# a burst of viewers never reaches the database. Run it after a deploy, or +# any time caching is in doubt. +# +# Usage: +# scripts/check-live-cache.sh [origin-base-url] +# +# Slug of a public visualization, e.g. city-of-eastpointe-macomb-county-mi +# [origin-base-url] Optional. The Heroku app URL, e.g. https://.herokuapp.com, +# to check Django's own page cache behind Cloudflare. +# +# What is checked: +# Edge: a second request through Cloudflare must be cf-cache-status: HIT, +# so the origin is not contacted at all. +# Origin: the response must be storable by a shared cache (max-age, no +# no-cache/no-store/private, no Vary: Cookie, no Set-Cookie), and a +# second request must be served from Django's page cache. A page +# cache hit returns the stored copy, so its Expires header is the +# same as the first response; a miss would compute a new one. +# Django 5.1+ also adds an Age header on page cache hits. +# +# The same guarantees are enforced in CI by +# visualizer/tests/testSimple.py::test_second_viewer_never_touches_the_database. + +set -euo pipefail + +slug="${1:?usage: $0 [origin-base-url]}" +origin="${2:-}" +edge="https://www.rcvis.com" +paths=("/v/$slug" "/ve/$slug" "/vb/$slug") + +failures=0 + +header() { # header -> value, lowercase, or empty + grep -i "^$2:" "$1" | head -1 | cut -d: -f2- | tr -d '\r' | sed 's/^ *//' | tr '[:upper:]' '[:lower:]' +} + +fail() { echo " FAIL: $*"; failures=$((failures + 1)); } +pass() { echo " ok: $*"; } + +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT + +echo "== Edge (Cloudflare): $edge" +for path in "${paths[@]}"; do + echo "$path" + curl -sS -o /dev/null -D "$tmp/h1" "$edge$path" + curl -sS -o /dev/null -D "$tmp/h2" "$edge$path" + status="$(head -1 "$tmp/h2" | awk '{print $2}')" + cf="$(header "$tmp/h2" cf-cache-status)" + if [ "$status" != "200" ]; then + fail "status $status" + elif [ "$cf" = "hit" ]; then + pass "cf-cache-status HIT (browser max-age: $(header "$tmp/h2" cache-control))" + else + fail "second request was cf-cache-status '$cf', expected HIT" + fi +done + +if [ -n "$origin" ]; then + echo "== Origin (Django page cache): $origin" + for path in "${paths[@]}"; do + echo "$path" + curl -sS -o /dev/null -D "$tmp/h1" "$origin$path" + sleep 2 # a cache miss would produce a later Expires than the first response + curl -sS -o /dev/null -D "$tmp/h2" "$origin$path" + + status="$(head -1 "$tmp/h2" | awk '{print $2}')" + [ "$status" = "200" ] && pass "status 200" || fail "status $status" + + cc="$(header "$tmp/h1" cache-control)" + case "$cc" in + *no-cache*|*no-store*|*private*) fail "Cache-Control '$cc' is not cacheable" ;; + *max-age=*) pass "Cache-Control '$cc'" ;; + *) fail "Cache-Control '$cc' has no max-age" ;; + esac + + vary="$(header "$tmp/h1" vary)" + case "$vary" in + *cookie*) fail "Vary '$vary' splits the cache per viewer" ;; + *) pass "Vary '${vary:-}'" ;; + esac + + if [ -n "$(header "$tmp/h1" set-cookie)" ]; then + fail "response sets a cookie, Django will not cache it" + else + pass "no Set-Cookie" + fi + + e1="$(header "$tmp/h1" expires)"; e2="$(header "$tmp/h2" expires)" + age="$(header "$tmp/h2" age)" + if [ -n "$e1" ] && [ "$e1" = "$e2" ]; then + pass "second request served from page cache (Expires unchanged${age:+, Age $age})" + else + fail "second request recomputed the page (Expires '$e1' -> '$e2')" + fi + done +else + echo "== Origin check skipped (pass the Heroku app URL as the second argument to run it)" +fi + +echo +if [ "$failures" -eq 0 ]; then + echo "All cache checks passed." +else + echo "$failures cache check(s) failed." + exit 1 +fi diff --git a/visualizer/middleware.py b/visualizer/middleware.py deleted file mode 100644 index 4a650d72..00000000 --- a/visualizer/middleware.py +++ /dev/null @@ -1,34 +0,0 @@ -""" -Custom cache middleware. - -UpdateCacheWithoutMaxAgeMiddleware: prevents UpdateCacheMiddleware from -leaking max-age into browser responses for views that set Cache-Control: -no-cache. UpdateCacheMiddleware stores the rendered response in Django's -server-side file cache (good — avoids expensive graph recomputation) but -also appends max-age=600 to the outgoing Cache-Control header (bad — tells -the browser not to revalidate for 10 minutes). This subclass calls super() -to get the server-side caching, then strips max-age from any response that -already has no-cache, so the browser always revalidates via If-Modified-Since. -""" - -from django.middleware.cache import UpdateCacheMiddleware -from django.utils.cache import cc_delim_re - - -class UpdateCacheWithoutMaxAgeMiddleware(UpdateCacheMiddleware): - """UpdateCacheMiddleware that strips max-age when no-cache is set.""" - - def process_response(self, request, response): - response = super().process_response(request, response) - - # If the view set no-cache, strip max-age so the browser revalidates. - cc = response.get('Cache-Control', '') - if 'no-cache' in cc: - # Remove max-age directive from the Cache-Control header - directives = [ - d.strip() for d in cc_delim_re.split(cc) - if not d.strip().startswith('max-age') - ] - response['Cache-Control'] = ', '.join(directives) - - return response diff --git a/visualizer/models.py b/visualizer/models.py index 71a293eb..ef806ec8 100644 --- a/visualizer/models.py +++ b/visualizer/models.py @@ -10,21 +10,21 @@ from common.cloudflare import CloudflareAPI -class ColorTheme(models.IntegerChoices): +class ColorTheme(models.IntegerChoices): # pylint: disable=too-many-ancestors """ Describes the status of movie generation for this model """ RAINBOW = 0, _('Full color spectrum') PURPLE_TO_ORANGE = 1, _('Purple to orange') ALTERNATING = 2, _('Alternating colors') -class EliminationBarColor(models.IntegerChoices): +class EliminationBarColor(models.IntegerChoices): # pylint: disable=too-many-ancestors """ Describes the status of movie generation for this model """ GRAY = 0, _('Gray') HIDDEN = 1, _('Hidden') LAST_ROUND_COLOR = 2, _('Same color of transfer') -class TextForWinner(models.IntegerChoices): +class TextForWinner(models.IntegerChoices): # pylint: disable=too-many-ancestors """ Describes the status of movie generation for this model """ ELECTED = 0, _('Candidate was elected') WON = 1, _('Candidate won') @@ -32,7 +32,7 @@ class TextForWinner(models.IntegerChoices): LEAD = 3, _('Candidate is in the lead') -class MovieGenerationStatuses(models.IntegerChoices): +class MovieGenerationStatuses(models.IntegerChoices): # pylint: disable=too-many-ancestors """ Describes the status of movie generation for this model """ NOT_REQUESTED = 0, _('No movie generation has been requested') NOT_STARTED = 1, _('Movie generation has been requested but not started') diff --git a/visualizer/tests/testSimple.py b/visualizer/tests/testSimple.py index c42f88f6..910e35a7 100644 --- a/visualizer/tests/testSimple.py +++ b/visualizer/tests/testSimple.py @@ -7,9 +7,10 @@ import json from mock import patch +from django.core.cache import cache from django.core.files import File from django.core.management import call_command -from django.test import TestCase +from django.test import TestCase, Client from django.test.client import RequestFactory from django.urls import reverse from django.utils.http import http_date, parse_http_date @@ -646,20 +647,64 @@ def test_response_has_last_modified_header(self): expected = http_date(config.updatedAt.timestamp()) self.assertEqual(response['Last-Modified'], expected) - def test_response_has_no_cache_directive(self): + # Public visualization pages that must be served from the page cache. + # The rcvis.com traffic profile is a million viewers of a handful of + # visualizations within an hour, on one small server: only the first + # viewer of each page may touch the database. + SHARED_CACHE_VIEWS = ['visualize', 'visualizeEmbedded', 'visualizeBallotpedia'] + + def test_visualization_headers_allow_shared_caching(self): """ - Visualization responses should include Cache-Control: no-cache - so browsers always revalidate with the server. + Visualization responses must be storable by a shared cache (Django's + page cache, Cloudflare) and identical for every viewer. Any of these + failing means each viewer gets their own copy, or none is cached: + Vary: Cookie splits the cache per session, a Set-Cookie stops Django + from caching at all, and no-cache/no-store/private stop everyone. """ with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: self.client.post('/upload.html', {'jsonFile': f}) - config = TestHelpers.get_latest_upload() + slug = TestHelpers.get_latest_upload().slug - with self.settings(CACHES={'default': { - 'BACKEND': 'django.core.cache.backends.dummy.DummyCache'}}): - path = reverse('visualize', args=(config.slug,)) + for viewName in self.SHARED_CACHE_VIEWS: + path = reverse(viewName, args=(slug,)) response = self.client.get(path) - self.assertIn('no-cache', response.get('Cache-Control', '')) + self.assertEqual(response.status_code, 200, path) + + vary = [v.strip().lower() for v in response.get('Vary', '').split(',')] + self.assertNotIn('cookie', vary, f"{path} varies on Cookie: cache is split per viewer") + self.assertFalse(response.cookies, f"{path} sets a cookie: Django will not cache it") + + directives = [d.strip().lower() for d in response['Cache-Control'].split(',')] + for forbidden in ('no-cache', 'no-store', 'private'): + self.assertNotIn(forbidden, directives, f"{path} sends {forbidden}: not cacheable") + maxAge = [d for d in directives if d.startswith('max-age=')] + self.assertEqual(len(maxAge), 1, f"{path} has no max-age: {directives}") + self.assertGreater(int(maxAge[0].split('=')[1]), 0, path) + + def test_second_viewer_never_touches_the_database(self): + """ + Two different people loading the same visualization must hit the + database once in total. The first viewer (logged in) populates the + page cache; the second viewer (a separate anonymous client with no + cookies) must be served the identical page with zero queries. + """ + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + slug = TestHelpers.get_latest_upload().slug + + # Start from an empty page cache so the first request is a real miss + cache.clear() + + for viewName in self.SHARED_CACHE_VIEWS: + path = reverse(viewName, args=(slug,)) + firstViewer = self.client.get(path) + self.assertEqual(firstViewer.status_code, 200, path) + + secondViewer = Client() + with self.assertNumQueries(0): + response = secondViewer.get(path) + self.assertEqual(response.status_code, 200, path) + self.assertEqual(response.content, firstViewer.content, path) def test_save_purge_only_on_update(self): """ diff --git a/visualizer/views.py b/visualizer/views.py index b1a55dc1..d1857f78 100644 --- a/visualizer/views.py +++ b/visualizer/views.py @@ -165,19 +165,18 @@ class ConditionalGetMixin: # pylint: disable=too-few-public-methods """ Mixin for DetailView subclasses that serve JsonConfig visualizations. - Sets Last-Modified from the object's updatedAt and Cache-Control: no-cache - so browsers always revalidate. On cache misses (file cache empty), - short-circuits with 304 if the client already has a fresh copy, - avoiding expensive graph computation. On cache hits, Django's - ConditionalGetMiddleware handles the 304 conversion using the - Last-Modified header preserved in the cached response. - - Cache-Control: no-cache allows Django's server-side cache to store - the rendered response (via UpdateCacheWithoutMaxAgeMiddleware), - so subsequent requests from different clients or Cloudflare PoPs - can be served from the file cache without recomputing the graph. - The custom middleware strips the max-age that UpdateCacheMiddleware - would otherwise add, so browsers always revalidate. + Sets Last-Modified from the object's updatedAt so clients can + revalidate cheaply. On cache misses (file cache empty), short-circuits + with 304 if the client already has a fresh copy, avoiding expensive + graph computation. On cache hits, Django's ConditionalGetMiddleware + handles the 304 conversion using the Last-Modified header preserved + in the cached response, without touching the database. + + Browser and edge cache lifetimes are owned by Cloudflare, which + ignores the origin's Cache-Control for these pages and is purged + whenever the model is saved. The origin therefore sends Django's + default cacheable headers, which also lets UpdateCacheMiddleware + store the rendered response in the server-side file cache. """ def get(self, request, *args, **kwargs): @@ -197,13 +196,11 @@ def get(self, request, *args, **kwargs): if ifModifiedSince is not None and lastModified <= ifModifiedSince: response = HttpResponseNotModified() response['Last-Modified'] = http_date(lastModified) - patch_cache_control(response, no_cache=True) return response response = super().get(request, *args, **kwargs) if self.object.updatedAt: response['Last-Modified'] = http_date(self.object.updatedAt.timestamp()) - patch_cache_control(response, no_cache=True) return response