diff --git a/config/settings/base.py b/config/settings/base.py index 513b3f61..970acd14 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -166,6 +166,7 @@ MIDDLEWARE = [ "django.middleware.security.SecurityMiddleware", "dogslow.WatchdogMiddleware", + "documentcloud.core.middleware.StripCookieVaryMiddleware", "django.contrib.sessions.middleware.SessionMiddleware", "django.middleware.locale.LocaleMiddleware", "corsheaders.middleware.CorsMiddleware", diff --git a/documentcloud/core/middleware.py b/documentcloud/core/middleware.py index bd25417d..925616eb 100644 --- a/documentcloud/core/middleware.py +++ b/documentcloud/core/middleware.py @@ -5,6 +5,7 @@ # Standard Library import logging +import re import time # Third Party @@ -12,6 +13,44 @@ logger = logging.getLogger(__name__) +# matches Django's own comma-splitting for Cache-Control/Vary header values +# (django.utils.cache.cc_delim_re), defined locally to avoid depending on an +# undocumented Django internal +_VARY_DELIM_RE = re.compile(r"\s*,\s*") + + +class StripCookieVaryMiddleware: + """Drop `Cookie` from `Vary` on responses explicitly marked `public`. + + DRF's `SessionAuthentication` touches `request.session` on every + request (even anonymous ones) to check for a session-based user, which + makes Django's `SessionMiddleware` unconditionally add `Vary: Cookie` + to the response - that can't be prevented from the view layer, since + the session is already marked accessed before any view code runs. A + response we've explicitly marked `public` was meant to be shared across + users by the CDN, so `Vary: Cookie` on it is always unwanted; must run + after `SessionMiddleware` in the response phase, so it needs to be + listed before it in `MIDDLEWARE`. + """ + + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + response = self.get_response(request) + vary = response.get("Vary") + if vary and "public" in response.get("Cache-Control", ""): + values = [ + value + for value in _VARY_DELIM_RE.split(vary) + if value.lower() != "cookie" + ] + if values: + response["Vary"] = ", ".join(values) + else: + del response["Vary"] + return response + class LogHTTPMiddleware: def __init__(self, get_response): diff --git a/documentcloud/documents/decorators.py b/documentcloud/documents/decorators.py index 098760c1..adbcdfc1 100644 --- a/documentcloud/documents/decorators.py +++ b/documentcloud/documents/decorators.py @@ -1,7 +1,6 @@ # Django from django.conf import settings from django.utils.cache import patch_cache_control -from django.views.decorators.vary import vary_on_cookie # Standard Library from functools import wraps @@ -30,7 +29,6 @@ def anonymous_cache_control(viewfunc): """Cache this view only if the user is anonymous""" @wraps(viewfunc) - @vary_on_cookie def inner(request, *args, **kwargs): response = viewfunc(request, *args, **kwargs) has_auth_token = hasattr(request, "auth") and request.auth is not None diff --git a/documentcloud/documents/migrations/0055_section_created_at_section_updated_at.py b/documentcloud/documents/migrations/0055_section_created_at_section_updated_at.py new file mode 100644 index 00000000..bd7083be --- /dev/null +++ b/documentcloud/documents/migrations/0055_section_created_at_section_updated_at.py @@ -0,0 +1,35 @@ +# Generated by Django 5.2.13 on 2026-07-22 19:32 + +import django.utils.timezone +import documentcloud.core.fields +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("documents", "0054_savedsearch"), + ] + + operations = [ + migrations.AddField( + model_name="section", + name="created_at", + field=documentcloud.core.fields.AutoCreatedField( + default=django.utils.timezone.now, + editable=False, + help_text="Timestamp of when the section was created", + verbose_name="created at", + ), + ), + migrations.AddField( + model_name="section", + name="updated_at", + field=documentcloud.core.fields.AutoLastModifiedField( + default=django.utils.timezone.now, + editable=False, + help_text="Timestamp of when the section was last updated", + verbose_name="updated at", + ), + ), + ] diff --git a/documentcloud/documents/models/document.py b/documentcloud/documents/models/document.py index 50563a45..5e4b7a41 100644 --- a/documentcloud/documents/models/document.py +++ b/documentcloud/documents/models/document.py @@ -852,6 +852,12 @@ class Section(models.Model): _("page number"), help_text=_("Which page this section appears on") ) title = models.TextField(_("title"), help_text=_("A title for the section")) + created_at = AutoCreatedField( + _("created at"), help_text=_("Timestamp of when the section was created") + ) + updated_at = AutoLastModifiedField( + _("updated at"), help_text=_("Timestamp of when the section was last updated") + ) class Meta: ordering = ("document", "page_number") diff --git a/documentcloud/documents/serializers.py b/documentcloud/documents/serializers.py index 1b977647..f16725d8 100644 --- a/documentcloud/documents/serializers.py +++ b/documentcloud/documents/serializers.py @@ -583,7 +583,7 @@ def get_edit_access(self, obj): class SectionSerializer(PageNumberValidationMixin, serializers.ModelSerializer): class Meta: model = Section - fields = ["id", "page_number", "title"] + fields = ["id", "page_number", "title", "created_at", "updated_at"] extra_kwargs = {"title": {"max_length": 200}} def validate_page_number(self, value): diff --git a/documentcloud/documents/tests/test_views.py b/documentcloud/documents/tests/test_views.py index 92fa0009..93a4cdbf 100644 --- a/documentcloud/documents/tests/test_views.py +++ b/documentcloud/documents/tests/test_views.py @@ -2,10 +2,12 @@ from django.conf import settings from django.db import connection, reset_queries from django.test.utils import override_settings +from django.utils.http import http_date from rest_framework import status # Standard Library import json +from datetime import timedelta # Third Party import pytest @@ -341,6 +343,9 @@ def test_retrieve(self, client, document): assert f"max-age={settings.CACHE_CONTROL_MAX_AGE}" in response["Cache-Control"] assert "private" not in response["Cache-Control"] assert "no-cache" not in response["Cache-Control"] + # anonymous reads don't vary on cookie - the cookie value doesn't + # change the response, so it shouldn't fragment the CDN cache + assert "Cookie" not in response["Vary"] def test_retrieve_auth(self, client, document): """Test retrieving a document""" @@ -352,6 +357,113 @@ def test_retrieve_auth(self, client, document): assert "no-cache" in response["Cache-Control"] assert "public" not in response["Cache-Control"] assert "max-age" not in response["Cache-Control"] + # the authenticated branch still varies per user + assert "Cookie" in response["Vary"] + + def test_retrieve_last_modified(self, client, document): + """Retrieving a document should send a real Last-Modified header + derived from the document's updated_at, not the response time""" + response = client.get(f"/api/documents/{document.pk}/") + assert response.status_code == status.HTTP_200_OK + assert response["Last-Modified"] == http_date(document.updated_at.timestamp()) + + def test_retrieve_not_modified(self, client, document): + """A conditional GET with a current If-Modified-Since should return + 304 with no body""" + response = client.get( + f"/api/documents/{document.pk}/", + HTTP_IF_MODIFIED_SINCE=http_date(document.updated_at.timestamp()), + ) + assert response.status_code == status.HTTP_304_NOT_MODIFIED + assert not response.content + + def test_retrieve_modified_since_stale(self, client, document): + """A conditional GET with an If-Modified-Since older than the + document's last edit should return 200 with the full body""" + stale = document.updated_at - timedelta(days=1) + response = client.get( + f"/api/documents/{document.pk}/", + HTTP_IF_MODIFIED_SINCE=http_date(stale.timestamp()), + ) + assert response.status_code == status.HTTP_200_OK + assert response.content + + def test_retrieve_expand_notes_last_modified(self, client, document): + """When notes are expanded, Last-Modified reflects the newest note - + editing a note does not bump the document's own updated_at""" + note = NoteFactory.create(document=document, access=Access.public) + assert note.updated_at > document.updated_at + response = client.get(f"/api/documents/{document.pk}/", {"expand": "notes"}) + assert response.status_code == status.HTTP_200_OK + assert response["Last-Modified"] == http_date(note.updated_at.timestamp()) + + def test_retrieve_expand_notes_not_modified(self, client, document): + """A conditional GET with expanded notes returns 304 when nothing has + changed since the newest of the document and its notes""" + note = NoteFactory.create(document=document, access=Access.public) + latest = max(document.updated_at, note.updated_at) + response = client.get( + f"/api/documents/{document.pk}/", + {"expand": "notes"}, + HTTP_IF_MODIFIED_SINCE=http_date(latest.timestamp()), + ) + assert response.status_code == status.HTTP_304_NOT_MODIFIED + + def test_retrieve_expand_sections_last_modified(self, client, document): + """When sections are expanded, Last-Modified reflects the newest + section - editing a section does not bump the document's updated_at""" + section = SectionFactory.create(document=document) + assert section.updated_at > document.updated_at + response = client.get(f"/api/documents/{document.pk}/", {"expand": "sections"}) + assert response.status_code == status.HTTP_200_OK + assert response["Last-Modified"] == http_date(section.updated_at.timestamp()) + + def test_retrieve_expand_organization_last_modified(self, client, document): + """Expanding the organization (now timestamped) enables conditional + caching rather than disabling it""" + response = client.get( + f"/api/documents/{document.pk}/", {"expand": "organization"} + ) + assert response.status_code == status.HTTP_200_OK + latest = max(document.updated_at, document.organization.updated_at) + assert response["Last-Modified"] == http_date(latest.timestamp()) + + def test_retrieve_expand_nested_no_conditional(self, client, document): + """A nested expansion (notes.user) serializes related fields we can't + cheaply track, so conditional caching is disabled to avoid a stale 304""" + NoteFactory.create(document=document, access=Access.public) + response = client.get( + f"/api/documents/{document.pk}/", {"expand": "notes.user"} + ) + assert response.status_code == status.HTTP_200_OK + assert "Last-Modified" not in response + # even a matching If-Modified-Since must still return the full body + response = client.get( + f"/api/documents/{document.pk}/", + {"expand": "notes.user"}, + HTTP_IF_MODIFIED_SINCE=http_date(document.updated_at.timestamp()), + ) + assert response.status_code == status.HTTP_200_OK + assert response.content + + def test_retrieve_expand_all_no_conditional(self, client, document): + """Expanding ~all serializes deeply nested content, so conditional + caching is disabled""" + response = client.get(f"/api/documents/{document.pk}/", {"expand": "~all"}) + assert response.status_code == status.HTTP_200_OK + assert "Last-Modified" not in response + + def test_conditional_expands_cover_expandable_fields(self): + """CONDITIONAL_EXPANDS must stay in sync with the serializer's + expandable_fields - a newly-added expandable relation should force a + deliberate choice here rather than silently emitting a stale 304""" + # DocumentCloud + from documentcloud.documents.views import DocumentViewSet + + assert ( + set(DocumentSerializer.Meta.expandable_fields) + == DocumentViewSet.CONDITIONAL_EXPANDS + ) def test_retrieve_no_file(self, client): """Test retrieving a document with a presigned url""" @@ -1518,6 +1630,9 @@ def test_retrieve(self, client, section): response_json = response.json() serializer = SectionSerializer(section) assert response_json == serializer.data + # timestamps are exposed in the API output + assert "created_at" in response_json + assert "updated_at" in response_json def test_retrieve_bad_document(self, client): """Test retrieving a section on a document you do not have access to""" diff --git a/documentcloud/documents/views.py b/documentcloud/documents/views.py index dd8c9254..091cb698 100644 --- a/documentcloud/documents/views.py +++ b/documentcloud/documents/views.py @@ -1,9 +1,11 @@ # Django from django.conf import settings from django.db import transaction -from django.db.models import Q, prefetch_related_objects +from django.db.models import Max, Q, prefetch_related_objects from django.db.models.query import Prefetch +from django.utils.cache import get_conditional_response from django.utils.decorators import method_decorator +from django.utils.http import http_date from django.utils.translation import gettext_lazy as _ from rest_framework import exceptions, mixins, parsers, serializers, status, viewsets from rest_framework.decorators import action @@ -102,6 +104,15 @@ # pylint: disable=too-many-lines, line-too-long, too-many-public-methods +def _max_updated_at(instances): + """Latest `updated_at` across an iterable of objects, or None if empty. + + Reads from an already-prefetched relation cache, so it avoids a redundant + aggregate query for relations `preload()`/`get_object()` have loaded. + """ + return max((obj.updated_at for obj in instances), default=None) + + @method_decorator(conditional_cache_control(no_cache=True), name="dispatch") @method_decorator(anonymous_cache_control, name="retrieve") class DocumentViewSet(BulkModelMixin, FlexFieldsModelViewSet): @@ -118,6 +129,13 @@ class DocumentViewSet(BulkModelMixin, FlexFieldsModelViewSet): permission_classes = ( DjangoObjectPermissionsOrAnonReadOnly | DocumentTokenPermissions, ) + # expandable relations `_retrieve_last_modified` can derive a freshness + # timestamp for; must stay in sync with the serializer's `expandable_fields` + # (enforced by test_conditional_expands_cover_expandable_fields). A request + # expanding anything outside this set falls back to no conditional handling + CONDITIONAL_EXPANDS = frozenset( + {"user", "organization", "sections", "notes", "projects", "revisions"} + ) @extend_schema( request={ @@ -630,7 +648,76 @@ def list(self, request, *args, **kwargs): ], ) def retrieve(self, request, *args, **kwargs): - return super().retrieve(request, *args, **kwargs) + instance = self.get_object() + + # last_modified is None when we can't safely determine freshness (a + # nested or ~all expansion); in that case skip conditional handling + last_modified = self._retrieve_last_modified(request, instance) + if last_modified is not None: + # short-circuit a 304 before serializing - conditional GET exists + # to avoid exactly that (often expensive) work on unchanged content + not_modified = get_conditional_response( + request, last_modified=last_modified + ) + if not_modified is not None: + not_modified["Last-Modified"] = http_date(last_modified) + return not_modified + + response = Response(self.get_serializer(instance).data) + if last_modified is not None: + response["Last-Modified"] = http_date(last_modified) + return response + + def _retrieve_last_modified(self, request, instance): + """Latest modification time across the document and any expanded + relations, as a whole-number unix timestamp. + + Every top-level expandable relation carries a modification timestamp, + so an explicit expand set can be tracked precisely. Returns None - skip + conditional handling rather than risk a stale 304 - for a nested (e.g. + `notes.user`) or `~all` expansion, whose related objects' own fields we + can't cheaply follow, or for any relation outside CONDITIONAL_EXPANDS + (so adding a new expandable relation fails safe until it's handled + here). Truncated to whole seconds because `If-Modified-Since` has only + 1s resolution; comparing against a raw microsecond timestamp would + never match. + """ + top_expands, nested_expands = split_levels( + request.query_params.get("expand", "") + ) + if ( + "~all" in top_expands + or nested_expands + or not set(top_expands) <= self.CONDITIONAL_EXPANDS + ): + return None + + # sections/notes/projects are already prefetched by get_queryset()'s + # preload() and get_object(), so read timestamps from the prefetch + # cache rather than issuing a redundant (and re-filtered) aggregate + timestamps = [instance.updated_at] + if "user" in top_expands: + timestamps.append(instance.user.updated_at) + if "organization" in top_expands: + timestamps.append(instance.organization.updated_at) + if "sections" in top_expands: + timestamps.append(_max_updated_at(instance.sections.all())) + if "notes" in top_expands: + timestamps.append(_max_updated_at(instance.notes.all())) + if "projects" in top_expands: + timestamps.append(_max_updated_at(instance.projects.all())) + # revisions aren't prefetched (so an indexed MAX beats loading every + # row) and are only serialized for users with edit access (matching the + # serializer's `_expandable_fields`) - otherwise Last-Modified would + # reflect content the requester can't see + if "revisions" in top_expands and request.user.has_perm( + "documents.change_document", instance + ): + timestamps.append( + instance.revisions.aggregate(latest=Max("created_at"))["latest"] + ) + + return int(max(ts for ts in timestamps if ts is not None).timestamp()) @extend_schema( request=DocumentSerializer, diff --git a/documentcloud/organizations/migrations/0022_organization_created_at_organization_updated_at.py b/documentcloud/organizations/migrations/0022_organization_created_at_organization_updated_at.py new file mode 100644 index 00000000..2b9ec9d5 --- /dev/null +++ b/documentcloud/organizations/migrations/0022_organization_created_at_organization_updated_at.py @@ -0,0 +1,35 @@ +# Generated by Django 5.2.13 on 2026-07-22 19:32 + +import django.utils.timezone +import documentcloud.core.fields +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("organizations", "0021_organization_members_organization_parent_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="organization", + name="created_at", + field=documentcloud.core.fields.AutoCreatedField( + default=django.utils.timezone.now, + editable=False, + help_text="Timestamp of when the organization was created", + verbose_name="created at", + ), + ), + migrations.AddField( + model_name="organization", + name="updated_at", + field=documentcloud.core.fields.AutoLastModifiedField( + default=django.utils.timezone.now, + editable=False, + help_text="Timestamp of when the organization was last updated", + verbose_name="updated at", + ), + ), + ] diff --git a/documentcloud/organizations/models.py b/documentcloud/organizations/models.py index a4261b42..422a7663 100644 --- a/documentcloud/organizations/models.py +++ b/documentcloud/organizations/models.py @@ -16,7 +16,7 @@ # DocumentCloud from documentcloud.core.choices import Language -from documentcloud.core.fields import AutoCreatedField +from documentcloud.core.fields import AutoCreatedField, AutoLastModifiedField from documentcloud.organizations.exceptions import InsufficientAICreditsError from documentcloud.organizations.querysets import OrganizationQuerySet @@ -67,6 +67,14 @@ class Organization(AbstractOrganization): blank=True, help_text=_("The default document language for user's in this organization"), ) + created_at = AutoCreatedField( + _("created at"), + help_text=_("Timestamp of when the organization was created"), + ) + updated_at = AutoLastModifiedField( + _("updated at"), + help_text=_("Timestamp of when the organization was last updated"), + ) def __str__(self): if self.individual: diff --git a/documentcloud/organizations/serializers.py b/documentcloud/organizations/serializers.py index 31828887..476c19f1 100644 --- a/documentcloud/organizations/serializers.py +++ b/documentcloud/organizations/serializers.py @@ -69,6 +69,8 @@ class Meta: "credit_reset_date", "monthly_credit_allowance", "plan", + "created_at", + "updated_at", ] extra_kwargs = { "avatar_url": {"read_only": True}, diff --git a/documentcloud/organizations/tests/test_views.py b/documentcloud/organizations/tests/test_views.py index a1477de2..f11e51bc 100644 --- a/documentcloud/organizations/tests/test_views.py +++ b/documentcloud/organizations/tests/test_views.py @@ -63,6 +63,9 @@ def test_retrieve(self, client, user, organization): response_json = json.loads(response.content) serializer = OrganizationSerializer(organization) assert response_json == serializer.data + # timestamps are exposed in the API output + assert "created_at" in response_json + assert "updated_at" in response_json def test_retrieve_bad(self, client, user): """Cannot view a private organization"""