Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
39 changes: 39 additions & 0 deletions documentcloud/core/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,52 @@

# Standard Library
import logging
import re
import time

# Third Party
from ipware import get_client_ip

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):
Expand Down
2 changes: 0 additions & 2 deletions documentcloud/documents/decorators.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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",
),
),
]
6 changes: 6 additions & 0 deletions documentcloud/documents/models/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion documentcloud/documents/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
115 changes: 115 additions & 0 deletions documentcloud/documents/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"""
Expand All @@ -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"""
Expand Down Expand Up @@ -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"""
Expand Down
Loading
Loading