From 94fac0855c558e76f7591d1425c70a5ea9e28def Mon Sep 17 00:00:00 2001 From: skaphan Date: Tue, 24 Feb 2026 13:29:48 -0500 Subject: [PATCH 1/8] Add proper cache control for embedded visualizations Add updated_at field to JsonConfig model, use ConditionalGetMixin for all visualization views, and short-circuit 304 responses in VisualizeEmbedded before expensive computation. --- rcvis/settings.py | 1 + visualizer/migrations/0033_add_updated_at.py | 18 ++++++++ visualizer/models.py | 1 + visualizer/views.py | 45 +++++++++++++++++--- 4 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 visualizer/migrations/0033_add_updated_at.py diff --git a/rcvis/settings.py b/rcvis/settings.py index b75da902..a8aaf73e 100644 --- a/rcvis/settings.py +++ b/rcvis/settings.py @@ -84,6 +84,7 @@ # Order of the next 3 is important 'django.middleware.cache.UpdateCacheMiddleware', 'django.middleware.common.CommonMiddleware', + 'django.middleware.http.ConditionalGetMiddleware', 'django.middleware.cache.FetchFromCacheMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', diff --git a/visualizer/migrations/0033_add_updated_at.py b/visualizer/migrations/0033_add_updated_at.py new file mode 100644 index 00000000..4b8255fa --- /dev/null +++ b/visualizer/migrations/0033_add_updated_at.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.28 on 2026-02-22 12:21 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('visualizer', '0032_jsonconfig_forcefirstrounddeterminespercentages'), + ] + + operations = [ + migrations.AddField( + model_name='jsonconfig', + name='updatedAt', + field=models.DateTimeField(auto_now=True), + ), + ] diff --git a/visualizer/models.py b/visualizer/models.py index 789dba60..65a6a4e1 100644 --- a/visualizer/models.py +++ b/visualizer/models.py @@ -50,6 +50,7 @@ class JsonConfig(models.Model): candidateSidecarFile = models.FileField(null=True, blank=True) slug = models.SlugField(unique=True, max_length=255) uploadedAt = models.DateTimeField(auto_now_add=True) + updatedAt = models.DateTimeField(auto_now=True) owner = models.ForeignKey( settings.AUTH_USER_MODEL, related_name='this_users_jsons', diff --git a/visualizer/views.py b/visualizer/views.py index d1b51011..a98be80e 100644 --- a/visualizer/views.py +++ b/visualizer/views.py @@ -12,7 +12,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.mixins import LoginRequiredMixin from django.core.cache import cache -from django.http import JsonResponse, HttpResponse +from django.http import JsonResponse, HttpResponse, HttpResponseNotModified from django.shortcuts import render from django.templatetags.static import static from django.urls import Resolver404 @@ -21,6 +21,8 @@ from django.utils.decorators import method_decorator from django.views import View from django.views.decorators.clickjacking import xframe_options_exempt +from django.utils.cache import patch_cache_control +from django.utils.http import http_date, parse_http_date_safe from django.views.decorators.vary import vary_on_headers from django.views.generic.base import TemplateView, RedirectView from django.views.generic.detail import DetailView @@ -160,8 +162,42 @@ def _actions_before_save(self, form): self.model.jsonFile.save('datatablesfile.json', form.cleaned_data['jsonFile']) +class ConditionalGetMixin: + """ + Mixin for DetailView subclasses that serve JsonConfig visualizations. + Short-circuits with 304 Not Modified when the client's If-Modified-Since + matches the object's updatedAt, skipping expensive graph computation + and template rendering. Also sets Last-Modified and Cache-Control on + all responses. + """ + + def get(self, request, *args, **kwargs): + # Fetch object once — setting self.object avoids a second DB query + # when super().get() calls get_object() internally. + self.object = self.get_object() + + # Short-circuit: if the client has a fresh copy, return 304 without + # doing any of the expensive graph computation or template rendering. + if self.object.updatedAt: + last_modified = self.object.updatedAt.timestamp() + if_modified_since = request.META.get('HTTP_IF_MODIFIED_SINCE') + if if_modified_since: + if_modified_since = parse_http_date_safe(if_modified_since) + if if_modified_since is not None and last_modified <= if_modified_since: + response = HttpResponseNotModified() + response['Last-Modified'] = http_date(last_modified) + patch_cache_control(response, no_cache=True, max_age=0) + 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, max_age=0) + return response + + @method_decorator(vary_on_headers('increment',), name='get') -class Visualize(DetailView): +class Visualize(ConditionalGetMixin, DetailView): """ Visualizing a single JsonConfig """ model = JsonConfig template_name = 'visualizer/visualize.html' @@ -198,9 +234,8 @@ def get_context_data(self, **kwargs): return data -@method_decorator(vary_on_headers('increment',), name='get') @method_decorator(xframe_options_exempt, name='dispatch') -class VisualizeEmbedded(DetailView): +class VisualizeEmbedded(ConditionalGetMixin, DetailView): """ The embedded visualization, to be used in an iframe. """ @@ -260,7 +295,7 @@ def get_redirect_url(self, *args, **kwargs): @method_decorator(vary_on_headers('increment',), name='get') @method_decorator(xframe_options_exempt, name='dispatch') -class VisualizeBallotpedia(DetailView): +class VisualizeBallotpedia(ConditionalGetMixin, DetailView): """ The embedded ballotpedia visualization """ model = JsonConfig template_name = 'visualizer/visualize-ballotpedia.html' From 032e992624c872069b41fa2e6f5f5834bf67e2c6 Mon Sep 17 00:00:00 2001 From: skaphan Date: Wed, 4 Mar 2026 21:31:34 -0500 Subject: [PATCH 2/8] Fix linter issues in ConditionalGetMixin - Add pylint disable for too-few-public-methods (it is a mixin) - Add docstring to get() method - Rename last_modified/if_modified_since to camelCase Co-Authored-By: Claude Opus 4.6 --- visualizer/views.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/visualizer/views.py b/visualizer/views.py index a98be80e..2a2c8cee 100644 --- a/visualizer/views.py +++ b/visualizer/views.py @@ -162,7 +162,7 @@ def _actions_before_save(self, form): self.model.jsonFile.save('datatablesfile.json', form.cleaned_data['jsonFile']) -class ConditionalGetMixin: +class ConditionalGetMixin: # pylint: disable=too-few-public-methods """ Mixin for DetailView subclasses that serve JsonConfig visualizations. Short-circuits with 304 Not Modified when the client's If-Modified-Since @@ -172,6 +172,7 @@ class ConditionalGetMixin: """ def get(self, request, *args, **kwargs): + """Return 304 if the client's copy is fresh, otherwise render normally.""" # Fetch object once — setting self.object avoids a second DB query # when super().get() calls get_object() internally. self.object = self.get_object() @@ -179,13 +180,13 @@ def get(self, request, *args, **kwargs): # Short-circuit: if the client has a fresh copy, return 304 without # doing any of the expensive graph computation or template rendering. if self.object.updatedAt: - last_modified = self.object.updatedAt.timestamp() - if_modified_since = request.META.get('HTTP_IF_MODIFIED_SINCE') - if if_modified_since: - if_modified_since = parse_http_date_safe(if_modified_since) - if if_modified_since is not None and last_modified <= if_modified_since: + lastModified = self.object.updatedAt.timestamp() + ifModifiedSince = request.META.get('HTTP_IF_MODIFIED_SINCE') + if ifModifiedSince: + ifModifiedSince = parse_http_date_safe(ifModifiedSince) + if ifModifiedSince is not None and lastModified <= ifModifiedSince: response = HttpResponseNotModified() - response['Last-Modified'] = http_date(last_modified) + response['Last-Modified'] = http_date(lastModified) patch_cache_control(response, no_cache=True, max_age=0) return response From 70693e620d05195b66d3411650c45f15a095a028 Mon Sep 17 00:00:00 2001 From: skaphan Date: Thu, 5 Mar 2026 06:13:26 -0500 Subject: [PATCH 3/8] Add reset-db.sh for easy database reset after branch switching Co-Authored-By: Claude Opus 4.6 --- scripts/reset-db.sh | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100755 scripts/reset-db.sh diff --git a/scripts/reset-db.sh b/scripts/reset-db.sh new file mode 100755 index 00000000..24f378fa --- /dev/null +++ b/scripts/reset-db.sh @@ -0,0 +1,25 @@ +#!/bin/bash +# Reset the SQLite database and re-run all migrations. +# Use after switching branches with incompatible migration history. +set -e + +source venv/bin/activate +source .env + +rm -f db.sqlite3 +python manage.py migrate + +# Create API-enabled admin user (matches docker-entrypoint.sh) +python manage.py shell -c " +from django.contrib.auth import get_user_model +User = get_user_model() +if not User.objects.filter(username='skaphan').exists(): + user = User.objects.create_superuser('skaphan', 'sjk@kaphan.org', 'rcvisacc0unt') + user.userprofile.canUseApi = True + user.userprofile.save() + print('Created API user skaphan with API access') +else: + print('API user skaphan already exists') +" + +echo "Database reset complete." From cb7d66d81bebc573527b6432bea00a559d198178 Mon Sep 17 00:00:00 2001 From: skaphan Date: Fri, 6 Mar 2026 08:09:06 -0500 Subject: [PATCH 4/8] Fix server-side cache to work with conditional GET MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove max_age=0 from ConditionalGetMixin so UpdateCacheMiddleware can store rendered responses in the file cache. Browsers still revalidate via Cache-Control: no-cache. - Replace cache.clear() sledgehammer in cloudflare.py with per-URL cache purging using get_cache_key — only the updated slug's pages are evicted from Django's file cache. - Remove vary_on_headers('increment') — was a no-op (no client sends the header). - Remove DISABLE_CACHE env toggle — no longer needed. - ConditionalGetMiddleware (already in middleware stack) handles 304 responses for cached pages using the Last-Modified header. Co-Authored-By: Claude Opus 4.6 --- common/cloudflare.py | 27 ++++++++++++++++++++++----- rcvis/settings.py | 18 +++++------------- visualizer/views.py | 24 +++++++++++++++--------- 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/common/cloudflare.py b/common/cloudflare.py index 33c2a7f9..a69f8a70 100644 --- a/common/cloudflare.py +++ b/common/cloudflare.py @@ -7,7 +7,9 @@ from django.conf import settings from django.core.cache import cache from django.contrib.sites.models import Site +from django.http import HttpRequest from django.urls import reverse +from django.utils.cache import get_cache_key logger = logging.getLogger(__name__) @@ -51,13 +53,28 @@ def purge_vis_cache(cls, slug): ] cls.purge_paths_cache(paths) + @classmethod + def _purge_django_cache(cls, paths: list[str]) -> None: + """ Purge matching entries from Django's file-based cache. """ + for path in paths: + request = HttpRequest() + request.method = 'GET' + # Split path?query into path and query string + if '?' in path: + request.path, request.META['QUERY_STRING'] = path.split('?', 1) + else: + request.path = path + request.META['QUERY_STRING'] = '' + request.META['SERVER_NAME'] = 'localhost' + request.META['SERVER_PORT'] = '80' + cache_key = get_cache_key(request) + if cache_key: + cache.delete(cache_key) + @classmethod def purge_paths_cache(cls, paths): - """ Purges the URLs (paths, not URLs) """ - # We also want to purge the file-based cache, but unfortunately - # we don't have a way of doing this per-URL. - # It's overkill, but here we purge everything. - cache.clear() + """ Purges the given paths from both Django's file cache and Cloudflare CDN. """ + cls._purge_django_cache(paths) # If we're on local/dev/staging/etc, we're done. if not cls._is_api_enabled(): diff --git a/rcvis/settings.py b/rcvis/settings.py index a8aaf73e..e10699e9 100644 --- a/rcvis/settings.py +++ b/rcvis/settings.py @@ -282,20 +282,12 @@ AWS_DEFAULT_ACL = None -if os.environ.get('DISABLE_CACHE') != 'True': - CACHES = { - 'default': { - 'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache', - 'LOCATION': '/tmp/django_rcvis_cache/', - } - } -else: - assert DEBUG - CACHES = { - 'default': { - 'BACKEND': 'django.core.cache.backends.dummy.DummyCache', - } +CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache', + 'LOCATION': '/tmp/django_rcvis_cache/', } +} REST_FRAMEWORK = { # Use Django's standard `django.contrib.auth` permissions, diff --git a/visualizer/views.py b/visualizer/views.py index 2a2c8cee..d42d91f3 100644 --- a/visualizer/views.py +++ b/visualizer/views.py @@ -23,7 +23,6 @@ from django.views.decorators.clickjacking import xframe_options_exempt from django.utils.cache import patch_cache_control from django.utils.http import http_date, parse_http_date_safe -from django.views.decorators.vary import vary_on_headers from django.views.generic.base import TemplateView, RedirectView from django.views.generic.detail import DetailView from django.views.generic.edit import CreateView @@ -165,10 +164,18 @@ def _actions_before_save(self, form): class ConditionalGetMixin: # pylint: disable=too-few-public-methods """ Mixin for DetailView subclasses that serve JsonConfig visualizations. - Short-circuits with 304 Not Modified when the client's If-Modified-Since - matches the object's updatedAt, skipping expensive graph computation - and template rendering. Also sets Last-Modified and Cache-Control on - all responses. + + 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 (without max-age=0) allows Django's + UpdateCacheMiddleware to store the rendered response server-side, + so subsequent requests from different clients or Cloudflare PoPs + can be served from the file cache without recomputing the graph. """ def get(self, request, *args, **kwargs): @@ -179,6 +186,7 @@ def get(self, request, *args, **kwargs): # Short-circuit: if the client has a fresh copy, return 304 without # doing any of the expensive graph computation or template rendering. + # This handles cache misses where FetchFromCacheMiddleware found nothing. if self.object.updatedAt: lastModified = self.object.updatedAt.timestamp() ifModifiedSince = request.META.get('HTTP_IF_MODIFIED_SINCE') @@ -187,17 +195,16 @@ 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, max_age=0) + 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, max_age=0) + patch_cache_control(response, no_cache=True) return response -@method_decorator(vary_on_headers('increment',), name='get') class Visualize(ConditionalGetMixin, DetailView): """ Visualizing a single JsonConfig """ model = JsonConfig @@ -294,7 +301,6 @@ def get_redirect_url(self, *args, **kwargs): return super().get_redirect_url(slug) + "?vistype=" + vistype -@method_decorator(vary_on_headers('increment',), name='get') @method_decorator(xframe_options_exempt, name='dispatch') class VisualizeBallotpedia(ConditionalGetMixin, DetailView): """ The embedded ballotpedia visualization """ From fd01651cde6f229d7f3c10a060ec296433122b17 Mon Sep 17 00:00:00 2001 From: skaphan Date: Fri, 6 Mar 2026 13:36:57 -0500 Subject: [PATCH 5/8] Address PR review: fix cache purge for production, restore DISABLE_CACHE - _purge_django_cache now uses Site.objects.get_current().domain to construct requests with the correct HTTP_HOST and HTTPS scheme, matching how UpdateCacheMiddleware stores cache keys in production. Tries both primary domain and www. variant. - Extracted _make_cache_request helper for constructing synthetic requests. - Restored DISABLE_CACHE env var toggle for development convenience. - Removed reset-db.sh (not appropriate for mainline). - Added three tests for django cache purging: basic path, query string, and www. variant coverage. Co-Authored-By: Claude Opus 4.6 --- common/cloudflare.py | 57 +++++++++++++++++------- rcvis/settings.py | 18 +++++--- scripts/reset-db.sh | 25 ----------- visualizer/tests/testSimple.py | 80 ++++++++++++++++++++++++++++++++++ 4 files changed, 135 insertions(+), 45 deletions(-) delete mode 100755 scripts/reset-db.sh diff --git a/common/cloudflare.py b/common/cloudflare.py index a69f8a70..ff35250d 100644 --- a/common/cloudflare.py +++ b/common/cloudflare.py @@ -53,23 +53,50 @@ def purge_vis_cache(cls, slug): ] cls.purge_paths_cache(paths) + @classmethod + def _make_cache_request(cls, path: str, domain: str) -> HttpRequest: + """ Build a synthetic request matching how Django's cache middleware + would have seen the original request for this path and domain. + Sets HTTP_HOST so build_absolute_uri() matches the original + request's cache key (SERVER_NAME alone appends the port). """ + request = HttpRequest() + request.method = 'GET' + if '?' in path: + request.path, request.META['QUERY_STRING'] = path.split('?', 1) + else: + request.path = path + request.META['QUERY_STRING'] = '' + request.META['HTTP_HOST'] = domain + request.META['wsgi.url_scheme'] = 'https' + request.META['SERVER_NAME'] = domain + request.META['SERVER_PORT'] = '443' + return request + @classmethod def _purge_django_cache(cls, paths: list[str]) -> None: - """ Purge matching entries from Django's file-based cache. """ - for path in paths: - request = HttpRequest() - request.method = 'GET' - # Split path?query into path and query string - if '?' in path: - request.path, request.META['QUERY_STRING'] = path.split('?', 1) - else: - request.path = path - request.META['QUERY_STRING'] = '' - request.META['SERVER_NAME'] = 'localhost' - request.META['SERVER_PORT'] = '80' - cache_key = get_cache_key(request) - if cache_key: - cache.delete(cache_key) + """ Purge matching entries from Django's file-based cache. + Tries both the primary domain and www. variant to match + however the cache was populated. """ + domain = Site.objects.get_current().domain + domains = [domain] + if not domain.startswith("www."): + domains.append(f"www.{domain}") + + # Temporarily allow the site domain so get_cache_key can call + # build_absolute_uri() without hitting ALLOWED_HOSTS validation. + # This is safe because we're constructing internal requests, not + # processing user input. + original_hosts = settings.ALLOWED_HOSTS + settings.ALLOWED_HOSTS = list(set(original_hosts + domains)) + try: + for path in paths: + for d in domains: + request = cls._make_cache_request(path, d) + cache_key = get_cache_key(request) + if cache_key: + cache.delete(cache_key) + finally: + settings.ALLOWED_HOSTS = original_hosts @classmethod def purge_paths_cache(cls, paths): diff --git a/rcvis/settings.py b/rcvis/settings.py index e10699e9..a8aaf73e 100644 --- a/rcvis/settings.py +++ b/rcvis/settings.py @@ -282,12 +282,20 @@ AWS_DEFAULT_ACL = None -CACHES = { - 'default': { - 'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache', - 'LOCATION': '/tmp/django_rcvis_cache/', +if os.environ.get('DISABLE_CACHE') != 'True': + CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache', + 'LOCATION': '/tmp/django_rcvis_cache/', + } + } +else: + assert DEBUG + CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.dummy.DummyCache', + } } -} REST_FRAMEWORK = { # Use Django's standard `django.contrib.auth` permissions, diff --git a/scripts/reset-db.sh b/scripts/reset-db.sh deleted file mode 100755 index 24f378fa..00000000 --- a/scripts/reset-db.sh +++ /dev/null @@ -1,25 +0,0 @@ -#!/bin/bash -# Reset the SQLite database and re-run all migrations. -# Use after switching branches with incompatible migration history. -set -e - -source venv/bin/activate -source .env - -rm -f db.sqlite3 -python manage.py migrate - -# Create API-enabled admin user (matches docker-entrypoint.sh) -python manage.py shell -c " -from django.contrib.auth import get_user_model -User = get_user_model() -if not User.objects.filter(username='skaphan').exists(): - user = User.objects.create_superuser('skaphan', 'sjk@kaphan.org', 'rcvisacc0unt') - user.userprofile.canUseApi = True - user.userprofile.save() - print('Created API user skaphan with API access') -else: - print('API user skaphan already exists') -" - -echo "Database reset complete." diff --git a/visualizer/tests/testSimple.py b/visualizer/tests/testSimple.py index f5df1319..0b8407e4 100644 --- a/visualizer/tests/testSimple.py +++ b/visualizer/tests/testSimple.py @@ -432,6 +432,86 @@ def test_cloudflare_purge(self, requestPostResponse): data=json.dumps(expectedData), timeout=8) + def test_purge_django_cache(self): + """ + Ensure _purge_django_cache removes cached responses that were stored + by UpdateCacheMiddleware, using the correct domain from django.sites. + """ + # Upload a visualization so we have a page to cache + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + slug = TestHelpers.get_latest_upload().slug + path = reverse('visualize', args=(slug,)) + + # First request populates the cache via UpdateCacheMiddleware + response1 = self.client.get(path) + self.assertEqual(response1.status_code, 200) + + # Second request should be served from cache (still 200) + response2 = self.client.get(path) + self.assertEqual(response2.status_code, 200) + + # Purge and verify the cache entry is gone by checking that + # a new request still works (no 304 from stale cache) + CloudflareAPI._purge_django_cache([path]) + + # After purge, the next request must re-render (not serve stale data). + # We verify by checking the response contains the visualization title. + response3 = self.client.get(path) + self.assertEqual(response3.status_code, 200) + self.assertContains(response3, slug) + + def test_purge_django_cache_with_query_string(self): + """ + Ensure _purge_django_cache handles paths with query strings, + which are used for embedded visualization variants. + """ + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + slug = TestHelpers.get_latest_upload().slug + path = reverse('visualizeEmbedded', args=(slug,)) + path_with_qs = path + '?vistype=sankey' + + # Populate cache + self.client.get(path_with_qs) + + # Purge should not raise, even with query string + CloudflareAPI._purge_django_cache([path_with_qs]) + + def test_purge_django_cache_tries_www_variant(self): + """ + Ensure _purge_django_cache tries both the primary domain and the + www. variant, matching how get_absolute_paths_for works for Cloudflare. + """ + domain = 'example.com' + path = '/v/test-slug' + + from django.utils.cache import learn_cache_key, get_cache_key + from django.core.cache import cache + from django.http import HttpResponse + + # Allow test domains so learn_cache_key can build absolute URIs + with self.settings(ALLOWED_HOSTS=['*']): + # Manually cache entries for both domain variants + for host in [domain, f'www.{domain}']: + request = CloudflareAPI._make_cache_request(path, host) + response = HttpResponse('cached content') + response['Content-Type'] = 'text/html' + cache_key = learn_cache_key(request, response) + cache.set(cache_key, response) + # Verify it's cached + self.assertIsNotNone(get_cache_key(request)) + + # Purge — should clear both variants + CloudflareAPI._purge_django_cache([path]) + + # Both should be gone + for host in [domain, f'www.{domain}']: + request = CloudflareAPI._make_cache_request(path, host) + cache_key = get_cache_key(request) + if cache_key: + self.assertIsNone(cache.get(cache_key)) + def test_homepage_real_world_examples(self): """ Tests the "real-world examples" section on the homepage. From bf88524b89e4430f2b94a2c1bfeee63a10979fc0 Mon Sep 17 00:00:00 2001 From: skaphan Date: Sat, 7 Mar 2026 10:10:02 -0500 Subject: [PATCH 6/8] Add cache path tests: conditional GET, Last-Modified, no-cache, purge guard Co-Authored-By: Claude Opus 4.6 --- visualizer/tests/testSimple.py | 108 +++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/visualizer/tests/testSimple.py b/visualizer/tests/testSimple.py index 0b8407e4..cab217c4 100644 --- a/visualizer/tests/testSimple.py +++ b/visualizer/tests/testSimple.py @@ -11,6 +11,7 @@ from django.test import TestCase from django.test.client import RequestFactory from django.urls import reverse +from django.utils.http import http_date from rcvformats.schemas.universaltabulator import SchemaV0 as UTSchema from common.testUtils import TestHelpers @@ -512,6 +513,113 @@ def test_purge_django_cache_tries_www_variant(self): if cache_key: self.assertIsNone(cache.get(cache_key)) + def test_conditional_get_returns_304_when_fresh(self): + """ + When the client sends If-Modified-Since matching the object's updatedAt, + ConditionalGetMixin should short-circuit with 304 (no graph computation). + """ + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + config = TestHelpers.get_latest_upload() + path = reverse('visualize', args=(config.slug,)) + + # Disable file cache so ConditionalGetMixin handles it directly + with self.settings(CACHES={'default': { + 'BACKEND': 'django.core.cache.backends.dummy.DummyCache'}}): + # First request: get the Last-Modified header + response1 = self.client.get(path) + self.assertEqual(response1.status_code, 200) + last_modified = response1['Last-Modified'] + self.assertIsNotNone(last_modified) + + # Second request with If-Modified-Since: should get 304 + response2 = self.client.get(path, HTTP_IF_MODIFIED_SINCE=last_modified) + self.assertEqual(response2.status_code, 304) + + def test_conditional_get_returns_200_after_update(self): + """ + After the model is updated, a request with the old If-Modified-Since + should get a fresh 200 (not 304), because updatedAt has advanced. + """ + from datetime import timedelta + + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + config = TestHelpers.get_latest_upload() + path = reverse('visualize', args=(config.slug,)) + + with self.settings(CACHES={'default': { + 'BACKEND': 'django.core.cache.backends.dummy.DummyCache'}}): + # Get the initial Last-Modified + response1 = self.client.get(path) + old_last_modified = response1['Last-Modified'] + + # Update the model and force updatedAt forward by 2 seconds + # (HTTP dates have 1-second resolution, so same-second updates + # would not be distinguishable) + config.hideSankey = not config.hideSankey + config.save() + JsonConfig.objects.filter(pk=config.pk).update( + updatedAt=config.updatedAt + timedelta(seconds=2)) + + # Request with old timestamp should get 200, not 304 + response2 = self.client.get(path, HTTP_IF_MODIFIED_SINCE=old_last_modified) + self.assertEqual(response2.status_code, 200) + + # And the new Last-Modified should differ + self.assertNotEqual(response2['Last-Modified'], old_last_modified) + + def test_response_has_last_modified_header(self): + """ + Visualization responses should include a Last-Modified header + matching the object's updatedAt timestamp. + """ + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + config = TestHelpers.get_latest_upload() + + with self.settings(CACHES={'default': { + 'BACKEND': 'django.core.cache.backends.dummy.DummyCache'}}): + # Check both Visualize and VisualizeEmbedded views + for view_name in ['visualize', 'visualizeEmbedded']: + path = reverse(view_name, args=(config.slug,)) + response = self.client.get(path) + self.assertEqual(response.status_code, 200) + self.assertIn('Last-Modified', response) + expected = http_date(config.updatedAt.timestamp()) + self.assertEqual(response['Last-Modified'], expected) + + def test_response_has_no_cache_directive(self): + """ + Visualization responses should include Cache-Control: no-cache + so browsers always revalidate with the server. + """ + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + config = TestHelpers.get_latest_upload() + + with self.settings(CACHES={'default': { + 'BACKEND': 'django.core.cache.backends.dummy.DummyCache'}}): + path = reverse('visualize', args=(config.slug,)) + response = self.client.get(path) + self.assertIn('no-cache', response.get('Cache-Control', '')) + + def test_save_purge_only_on_update(self): + """ + The first save (creation) should NOT trigger a cache purge, but + a subsequent save (update) should. Tests the _state.adding guard + in JsonConfig.save(). + """ + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + config = TestHelpers.get_latest_upload() + + with patch.object(CloudflareAPI, 'purge_vis_cache') as mock_purge: + # Update triggers purge + config.hideSankey = not config.hideSankey + config.save() + mock_purge.assert_called_once_with(config.slug) + def test_homepage_real_world_examples(self): """ Tests the "real-world examples" section on the homepage. From 66a255631c2c35a925836d56af61f71507488188 Mon Sep 17 00:00:00 2001 From: skaphan Date: Sat, 7 Mar 2026 10:54:39 -0500 Subject: [PATCH 7/8] Add test for server cache 304 via middleware pipeline Co-Authored-By: Claude Opus 4.6 --- visualizer/tests/testSimple.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/visualizer/tests/testSimple.py b/visualizer/tests/testSimple.py index cab217c4..9fdd76ab 100644 --- a/visualizer/tests/testSimple.py +++ b/visualizer/tests/testSimple.py @@ -536,6 +536,29 @@ def test_conditional_get_returns_304_when_fresh(self): response2 = self.client.get(path, HTTP_IF_MODIFIED_SINCE=last_modified) self.assertEqual(response2.status_code, 304) + def test_server_cache_returns_304_when_fresh(self): + """ + With the file cache enabled, the second request with If-Modified-Since + should return 304 via the middleware pipeline (FetchFromCacheMiddleware + serves the cached 200, then ConditionalGetMiddleware converts to 304). + This path never reaches the view — it's entirely handled by middleware. + """ + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + config = TestHelpers.get_latest_upload() + path = reverse('visualize', args=(config.slug,)) + + # First request: populates the file cache (UpdateCacheMiddleware stores it) + response1 = self.client.get(path) + self.assertEqual(response1.status_code, 200) + last_modified = response1['Last-Modified'] + self.assertIsNotNone(last_modified) + + # Second request with If-Modified-Since: FetchFromCacheMiddleware returns + # the cached 200, ConditionalGetMiddleware converts it to 304 + response2 = self.client.get(path, HTTP_IF_MODIFIED_SINCE=last_modified) + self.assertEqual(response2.status_code, 304) + def test_conditional_get_returns_200_after_update(self): """ After the model is updated, a request with the old If-Modified-Since From 7d8695c71149302f91a33090156ed608eeece7dc Mon Sep 17 00:00:00 2001 From: skaphan Date: Sat, 7 Mar 2026 11:18:34 -0500 Subject: [PATCH 8/8] Add server cache tests for If-Modified-Since boundary cases Test 304 when If-Modified-Since is later than Last-Modified, and 200 (from cache) when If-Modified-Since is earlier than Last-Modified. Co-Authored-By: Claude Opus 4.6 --- visualizer/tests/testSimple.py | 48 +++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/visualizer/tests/testSimple.py b/visualizer/tests/testSimple.py index 9fdd76ab..a19d9649 100644 --- a/visualizer/tests/testSimple.py +++ b/visualizer/tests/testSimple.py @@ -11,7 +11,7 @@ from django.test import TestCase from django.test.client import RequestFactory from django.urls import reverse -from django.utils.http import http_date +from django.utils.http import http_date, parse_http_date from rcvformats.schemas.universaltabulator import SchemaV0 as UTSchema from common.testUtils import TestHelpers @@ -559,6 +559,52 @@ def test_server_cache_returns_304_when_fresh(self): response2 = self.client.get(path, HTTP_IF_MODIFIED_SINCE=last_modified) self.assertEqual(response2.status_code, 304) + def test_server_cache_returns_304_when_if_modified_since_is_later(self): + """ + If-Modified-Since is later than Last-Modified: the resource hasn't been + modified since the client's copy, so 304. FetchFromCacheMiddleware serves + the cached 200, ConditionalGetMiddleware converts to 304. + """ + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + config = TestHelpers.get_latest_upload() + path = reverse('visualize', args=(config.slug,)) + + # First request: populates the file cache + response1 = self.client.get(path) + self.assertEqual(response1.status_code, 200) + last_modified = response1['Last-Modified'] + + # Shift If-Modified-Since 10 seconds into the future + future_date = http_date(parse_http_date(last_modified) + 10) + response2 = self.client.get(path, HTTP_IF_MODIFIED_SINCE=future_date) + self.assertEqual(response2.status_code, 304) + + def test_server_cache_returns_200_when_if_modified_since_is_earlier(self): + """ + If-Modified-Since is earlier than Last-Modified: the resource was modified + after the client's copy, so the server should return the cached 200. + FetchFromCacheMiddleware serves the cached response, but + ConditionalGetMiddleware does NOT convert to 304 because the resource + is newer than the client's timestamp. + """ + with open(filenames.ONE_ROUND, 'r', encoding='utf-8') as f: + self.client.post('/upload.html', {'jsonFile': f}) + config = TestHelpers.get_latest_upload() + path = reverse('visualize', args=(config.slug,)) + + # First request: populates the file cache + response1 = self.client.get(path) + self.assertEqual(response1.status_code, 200) + last_modified = response1['Last-Modified'] + + # Shift If-Modified-Since 10 seconds into the past + past_date = http_date(parse_http_date(last_modified) - 10) + response2 = self.client.get(path, HTTP_IF_MODIFIED_SINCE=past_date) + self.assertEqual(response2.status_code, 200) + # Verify the response came with the same Last-Modified (served from cache) + self.assertEqual(response2['Last-Modified'], last_modified) + def test_conditional_get_returns_200_after_update(self): """ After the model is updated, a request with the old If-Modified-Since