Invalidate CDN cache on plain document edits

config/settings/base.py

@@ -627,6 +627,8 @@
 CLOUDFLARE_API_KEY = env("CLOUDFLARE_API_KEY", default="")
 CLOUDFLARE_API_ZONE = env("CLOUDFLARE_API_ZONE", default="")
 CLOUDFLARE_HOSTS = env.list("CLOUDFLARE_HOSTS", default=[])
+# max operations per purge request (Business plan caps this at 100)
+CLOUDFLARE_PURGE_LIMIT = env.int("CLOUDFLARE_PURGE_LIMIT", default=100)
 
 SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https")
 

documentcloud/documents/cache.py

@@ -0,0 +1,88 @@
+"""CDN cache invalidation for documents (CloudFront + Cloudflare)."""
+
+# Django
+from django.conf import settings
+
+# Standard Library
+import logging
+import uuid
+
+# Third Party
+import boto3
+import requests
+
+logger = logging.getLogger(__name__)
+
+
+def _chunk(items, size):
+    """Yield successive `size`-length chunks of `items`."""
+    for i in range(0, len(items), size):
+        yield items[i : i + size]
+
+
+def _invalidate_cloudfront(paths):
+    """Invalidate the given paths from CloudFront in one batch."""
+    distribution_id = settings.CLOUDFRONT_DISTRIBUTION_ID
+    if not distribution_id or not paths:
+        return
+    cloudfront = boto3.client("cloudfront")
+    cloudfront.create_invalidation(
+        DistributionId=distribution_id,
+        InvalidationBatch={
+            "Paths": {"Quantity": len(paths), "Items": paths},
+            "CallerReference": str(uuid.uuid4()),
+        },
+    )
+
+
+def _invalidate_cloudflare(files=None, tags=None):
+    """Purge the given files and tags from Cloudflare.
+
+    `files` and `tags` cannot be combined in a single purge request (the zone
+    purge API is a `oneOf`), so they are sent as separate requests, each
+    chunked to the plan's per-request operation cap.
+    """
+    zone = settings.CLOUDFLARE_API_ZONE
+    if not zone:
+        return
+    url = f"https://api.cloudflare.com/client/v4/zones/{zone}/purge_cache"
+    headers = {
+        "X-Auth-Email": settings.CLOUDFLARE_API_EMAIL,
+        "X-Auth-Key": settings.CLOUDFLARE_API_KEY,
+    }
+    for key, values in (("files", files), ("tags", tags)):
+        for chunk in _chunk(values or [], settings.CLOUDFLARE_PURGE_LIMIT):
+            requests.post(url, json={key: chunk}, headers=headers, timeout=10)
+
+
+def invalidate_cache_batch(documents):
+    """Invalidate the CloudFront and Cloudflare caches for many documents.
+
+    Cloudflare purges the API responses by Cache-Tag (`doc-{id}`) and the
+    frontend pages + public asset by URL; the two are mutually exclusive in a
+    single zone purge request, so they go in separate (chunked) requests.
+    CloudFront purges the underlying document file by path.
+    """
+    documents = list(documents)
+    if not documents:
+        return
+    logger.info("Invalidating cache for %s", [document.pk for document in documents])
+
+    cloudfront_paths = []
+    cloudflare_files = []
+    cloudflare_tags = []
+    for document in documents:
+        # the doc path without the s3 bucket name
+        doc_path = document.doc_path[document.doc_path.index("/") :]
+        cloudfront_paths.append(doc_path)
+        # always purge the frontend URLs: on a public -> private flip `access`
+        # is already private by now, but the public copy may still be cached at
+        # the edge - purging a URL that was never cached is harmless
+        cloudflare_files.extend(
+            host + document.get_absolute_url() for host in settings.CLOUDFLARE_HOSTS
+        )
+        cloudflare_files.append(settings.PUBLIC_ASSET_URL + doc_path[1:])
+        cloudflare_tags.append(document.cache_tag)
+
+    _invalidate_cloudfront(cloudfront_paths)
+    _invalidate_cloudflare(files=cloudflare_files, tags=cloudflare_tags)

documentcloud/documents/models/document.py

@@ -10,13 +10,10 @@
 import logging
 import sys
 import time
-import uuid
 from io import BytesIO
 
 # Third Party
-import boto3
 import pymupdf
-import requests
 from listcrunch import crunch, uncrunch
 from pikepdf import Page as PikePage, Pdf, Rectangle
 
@@ -283,13 +280,19 @@ def save(self, *args, **kwargs):
     @transaction.atomic
     def destroy(self):
         # DocumentCloud
-        from documentcloud.documents.tasks import delete_document_files, solr_delete
+        from documentcloud.documents.tasks import (
+            delete_document_files,
+            invalidate_cache,
+            solr_delete,
+        )
 
         self.status = Status.deleted
         self.save()
         DeletedDocument.objects.create(pk=self.pk)
         transaction.on_commit(lambda: delete_document_files.delay(self.path))
         transaction.on_commit(lambda: solr_delete.delay(self.pk))
+        # the CDN may still be serving the (now deleted) public copy
+        transaction.on_commit(lambda: invalidate_cache.delay(self.pk))
 
     @property
     def path(self):
@@ -701,51 +704,15 @@ def page_filter(text):
 
         return solr_document
 
-    def invalidate_cache(self):
-        """
-        Invalidate public CDN cache for this document's underlying file,
-        plus frontend URLs in Cloudflare
-        """
-        logger.info("Invalidating cache for %s", self.pk)
-        doc_path = self.doc_path[self.doc_path.index("/") :]
-
-        # cloudfront
-        distribution_id = settings.CLOUDFRONT_DISTRIBUTION_ID
-        if distribution_id:
-            # we want the doc path without the s3 bucket name
-            cloudfront = boto3.client("cloudfront")
-            cloudfront.create_invalidation(
-                DistributionId=distribution_id,
-                InvalidationBatch={
-                    "Paths": {"Quantity": 1, "Items": [doc_path]},
-                    "CallerReference": str(uuid.uuid4()),
-                },
-            )
-
-        # cloudflare
-        cloudflare_email = settings.CLOUDFLARE_API_EMAIL
-        cloudflare_key = settings.CLOUDFLARE_API_KEY
-        cloudflare_zone = settings.CLOUDFLARE_API_ZONE
-        asset_url = settings.PUBLIC_ASSET_URL + doc_path[1:]
+    @property
+    def cache_tag(self):
+        """The Cloudflare Cache-Tag marking this document's cached API responses.
 
-        if self.access == Access.public:
-            public_urls = [
-                host + self.get_absolute_url() for host in settings.CLOUDFLARE_HOSTS
-            ] + [asset_url]
-        else:
-            public_urls = [asset_url]
-
-        if cloudflare_zone:
-            requests.post(
-                "https://api.cloudflare.com/client/v4/zones/"
-                f"{cloudflare_zone}/purge_cache",
-                json={"files": public_urls},
-                headers={
-                    "X-Auth-Email": cloudflare_email,
-                    "X-Auth-Key": cloudflare_key,
-                },
-                timeout=10,
-            )
+        Purging this one tag clears the bare `/api/documents/{pk}/` URL and
+        every `?expand=…` / per-`Origin` variant at once - the key spaces a
+        URL purge can't enumerate.
+        """
+        return f"doc-{self.pk}"
 
     def index_on_commit(self, **kwargs):
         """Index the document in Solr on tranasction commit"""

documentcloud/documents/tasks.py

@@ -20,6 +20,7 @@
 from documentcloud.common.environment import httpsub, storage
 from documentcloud.core.choices import Language
 from documentcloud.documents import entity_extraction, modifications, solr
+from documentcloud.documents.cache import invalidate_cache_batch
 from documentcloud.documents.choices import Access, Status
 from documentcloud.documents.models import Document, DocumentError
 from documentcloud.documents.search import SOLR, SOLR_NOTES
@@ -412,12 +413,20 @@ def publish_scheduled_documents():
 
 
 @shared_task
-def invalidate_cache(document_pk):
-    """Invalidate the CloudFront and CloudFlare caches"""
-    document = Document.objects.get(pk=document_pk)
-    document.invalidate_cache()
-    document.cache_dirty = False
-    document.save()
+def invalidate_cache(*document_pks):
+    """Invalidate the CloudFront and CloudFlare caches for the given documents.
+
+    Variadic so the input is always iterable: `invalidate_cache.delay(pk)`
+    purges one document, `invalidate_cache.delay(*pks)` purges a batch in one
+    set of requests rather than one task per document.
+    """
+    # only pk/slug are needed to build the purge URLs and tags - skip the
+    # heavy columns (page_spec, data, description, ...)
+    documents = list(Document.objects.filter(pk__in=document_pks).only("pk", "slug"))
+    invalidate_cache_batch(documents)
+    # clear the flag with a queryset update so we don't bump `updated_at` (an
+    # AutoLastModifiedField) - a cache purge is not a content change
+    Document.objects.filter(pk__in=document_pks).update(cache_dirty=False)
 
 
 # page modifications

documentcloud/documents/tests/test_cache.py

@@ -0,0 +1,109 @@
+# Third Party
+import pytest
+
+# DocumentCloud
+from documentcloud.documents.cache import invalidate_cache_batch
+from documentcloud.documents.choices import Access
+from documentcloud.documents.tests.factories import DocumentFactory
+
+
+@pytest.mark.django_db()
+class TestDocumentCacheInvalidation:
+    """`invalidate_cache_batch` purges the API by Cache-Tag and URLs by URL."""
+
+    @pytest.fixture(autouse=True)
+    def cache_settings(self, settings):
+        settings.CLOUDFLARE_API_ZONE = "zone123"
+        settings.CLOUDFLARE_API_EMAIL = "cache@example.com"
+        settings.CLOUDFLARE_API_KEY = "secret"
+        settings.CLOUDFLARE_HOSTS = ["https://www.example.com"]
+        settings.CLOUDFRONT_DISTRIBUTION_ID = ""
+        settings.PUBLIC_ASSET_URL = "https://assets.example.com/documents/"
+
+    @pytest.fixture
+    def mock_post(self, mocker):
+        return mocker.patch("documentcloud.documents.cache.requests.post")
+
+    def test_cache_tag(self):
+        """The Cache-Tag is `doc-{pk}`."""
+        document = DocumentFactory()
+        assert document.cache_tag == f"doc-{document.pk}"
+
+    def test_batch_purges_tag_and_urls(self, mock_post):
+        """One Cloudflare request purges the `doc-{id}` tag, another the URLs.
+
+        `files` and `tags` are mutually exclusive in a single zone purge
+        request, so they must be sent separately.
+        """
+        document = DocumentFactory()
+
+        invalidate_cache_batch([document])
+
+        assert mock_post.call_count == 2
+        payloads = [call.kwargs["json"] for call in mock_post.call_args_list]
+        tags_payload = next(p for p in payloads if "tags" in p)
+        files_payload = next(p for p in payloads if "files" in p)
+        assert tags_payload["tags"] == [f"doc-{document.pk}"]
+        assert (
+            f"https://www.example.com{document.get_absolute_url()}"
+            in files_payload["files"]
+        )
+        # never both keys in one request
+        assert all(("tags" in p) != ("files" in p) for p in payloads)
+
+    def test_batch_always_purges_frontend_urls_even_when_private(self, mock_post):
+        """On a public -> private flip `access` is already private by purge
+        time, so the frontend URLs must be purged unconditionally (5b) - the
+        public copy may still be cached at the edge."""
+        document = DocumentFactory(access=Access.private)
+
+        invalidate_cache_batch([document])
+
+        files_payload = next(
+            call.kwargs["json"]
+            for call in mock_post.call_args_list
+            if "files" in call.kwargs["json"]
+        )
+        assert (
+            f"https://www.example.com{document.get_absolute_url()}"
+            in files_payload["files"]
+        )
+
+    def test_batch_chunks_to_the_purge_limit(self, mock_post, settings):
+        """Each purge request is chunked to the configured cap."""
+        settings.CLOUDFLARE_PURGE_LIMIT = 2
+        documents = DocumentFactory.create_batch(3)
+
+        invalidate_cache_batch(documents)
+
+        # 3 tags -> chunks of 2 -> 2 requests
+        # 3 docs x (1 host + 1 asset) = 6 files -> chunks of 2 -> 3 requests
+        assert mock_post.call_count == 5
+
+    def test_batch_no_op_without_zone(self, mock_post, settings):
+        """No Cloudflare zone configured means no purge request."""
+        settings.CLOUDFLARE_API_ZONE = ""
+        document = DocumentFactory()
+
+        invalidate_cache_batch([document])
+
+        mock_post.assert_not_called()
+
+    def test_batch_empty_is_noop(self, mock_post):
+        """An empty batch issues no requests."""
+        invalidate_cache_batch([])
+        mock_post.assert_not_called()
+
+    @pytest.mark.usefixtures("mock_post")
+    def test_batch_purges_cloudfront_paths(self, mocker, settings):
+        """CloudFront is invalidated by path for every document in the batch."""
+        settings.CLOUDFRONT_DISTRIBUTION_ID = "DIST123"
+        mock_boto = mocker.patch("documentcloud.documents.cache.boto3")
+        documents = DocumentFactory.create_batch(2)
+
+        invalidate_cache_batch(documents)
+
+        create_invalidation = mock_boto.client.return_value.create_invalidation
+        create_invalidation.assert_called_once()
+        paths = create_invalidation.call_args.kwargs["InvalidationBatch"]["Paths"]
+        assert paths["Quantity"] == 2

documentcloud/documents/tests/test_tasks.py

@@ -0,0 +1,60 @@
+# Third Party
+import pytest
+
+# DocumentCloud
+from documentcloud.documents.tasks import invalidate_cache
+from documentcloud.documents.tests.factories import DocumentFactory
+
+
+@pytest.mark.django_db()
+class TestInvalidateCacheTask:
+    """The `invalidate_cache` task batches purges and clears `cache_dirty`."""
+
+    def test_accepts_single_pk(self, mocker):
+        """A single pk arg purges one document."""
+        mock_batch = mocker.patch(
+            "documentcloud.documents.tasks.invalidate_cache_batch"
+        )
+        document = DocumentFactory(cache_dirty=True)
+
+        invalidate_cache(document.pk)
+
+        mock_batch.assert_called_once()
+        (documents,) = mock_batch.call_args[0]
+        assert [d.pk for d in documents] == [document.pk]
+        document.refresh_from_db()
+        assert document.cache_dirty is False
+
+    def test_accepts_many_pks(self, mocker):
+        """Several pk args are purged in a single batch."""
+        mock_batch = mocker.patch(
+            "documentcloud.documents.tasks.invalidate_cache_batch"
+        )
+        documents = DocumentFactory.create_batch(3, cache_dirty=True)
+
+        invalidate_cache(*[d.pk for d in documents])
+
+        assert mock_batch.call_count == 1
+        (called,) = mock_batch.call_args[0]
+        assert {d.pk for d in called} == {d.pk for d in documents}
+        for document in documents:
+            document.refresh_from_db()
+            assert document.cache_dirty is False
+
+    def test_clears_dirty_without_bumping_updated_at(self, mocker):
+        """Clearing the flag must not look like a content edit.
+
+        `updated_at` is an `AutoLastModifiedField`; bumping it on every purge
+        would silently reset the freshness signal (3) and demote the document
+        to the shortest TTL tier (4). The flag is cleared with a queryset
+        `.update()` precisely so `save()` (and the field) never fires.
+        """
+        mocker.patch("documentcloud.documents.tasks.invalidate_cache_batch")
+        document = DocumentFactory(cache_dirty=True)
+        original_updated_at = document.updated_at
+
+        invalidate_cache(document.pk)
+
+        document.refresh_from_db()
+        assert document.cache_dirty is False
+        assert document.updated_at == original_updated_at