Fix: cache per-request effective-document resolution for thumb/metadata/preview (#13166)

Django's condition() decorator invokes etag_func and last_modified_func
separately, and the view itself may resolve again -- each call to
resolve_effective_document_by_pk() was redoing the same root/version
DB lookups. Memoize the resolution on the request object so a single
thumb/metadata/preview request resolves the effective document once
instead of up to three times.

Related to paperless-ngx/paperless-ngx#13161.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

src/documents/conditionals.py

@@ -1,9 +1,9 @@
 from datetime import UTC
 from datetime import datetime
-from typing import Any
 
 from django.conf import settings
 from django.core.cache import cache
+from rest_framework.request import Request
 
 from documents.caching import CACHE_5_MINUTES
 from documents.caching import CACHE_50_MINUTES
@@ -117,7 +117,7 @@ def preview_last_modified(request, pk: int) -> datetime | None:
     return doc.modified
 
 
-def thumbnail_etag(request: Any, pk: int) -> str | None:
+def thumbnail_etag(request: Request, pk: int) -> str | None:
     """
     Thumbnails are version-dependent, so use the effective document checksum as
     the ETag to invalidate cache when the latest version changes.
@@ -128,7 +128,7 @@ def thumbnail_etag(request: Any, pk: int) -> str | None:
     return doc.checksum
 
 
-def thumbnail_last_modified(request: Any, pk: int) -> datetime | None:
+def thumbnail_last_modified(request: Request, pk: int) -> datetime | None:
     """
     Returns the filesystem last modified either from cache or from filesystem.
     Cache should be (slightly?) faster than filesystem

src/documents/tests/test_version_conditionals.py

@@ -1,8 +1,12 @@
 from datetime import timedelta
 from types import SimpleNamespace
+from typing import TYPE_CHECKING
+from typing import cast
 from unittest import mock
 
+from django.db import connection
 from django.test import TestCase
+from django.test.utils import CaptureQueriesContext
 from django.utils import timezone
 
 from documents.conditionals import metadata_etag
@@ -13,6 +17,9 @@ from documents.models import Document
 from documents.tests.utils import DirectoriesMixin
 from documents.versioning import resolve_effective_document_by_pk
 
+if TYPE_CHECKING:
+    from rest_framework.request import Request
+
 
 class TestConditionals(DirectoriesMixin, TestCase):
     def test_metadata_etag_uses_latest_version_for_root_request(self) -> None:
@@ -29,14 +36,18 @@ class TestConditionals(DirectoriesMixin, TestCase):
             mime_type="application/pdf",
             root_document=root,
         )
-        request = SimpleNamespace(query_params={})
+        request = cast("Request", SimpleNamespace(query_params={}))
 
         self.assertEqual(
             metadata_etag(request, root.id),
             f"{latest.checksum}:{latest.modified.isoformat()}",
         )
-        self.assertEqual(preview_etag(request, root.id), latest.archive_checksum)
-        self.assertEqual(thumbnail_etag(request, root.id), latest.checksum)
+        # preview_etag/thumbnail_etag resolve the same (pk, request) and should
+        # reuse metadata_etag's cached resolution instead of re-querying.
+        with CaptureQueriesContext(connection) as ctx:
+            self.assertEqual(preview_etag(request, root.id), latest.archive_checksum)
+            self.assertEqual(thumbnail_etag(request, root.id), latest.checksum)
+        self.assertEqual(len(ctx.captured_queries), 0)
 
     def test_metadata_etag_changes_when_document_modified_changes(self) -> None:
         doc = Document.objects.create(
@@ -44,15 +55,20 @@ class TestConditionals(DirectoriesMixin, TestCase):
             checksum="same-checksum",
             mime_type="application/pdf",
         )
-        request = SimpleNamespace(query_params={})
-
-        original_etag = metadata_etag(request, doc.id)
+        # Each call simulates a separate incoming HTTP request, so it gets its
+        # own request object -- the per-request resolution cache must not leak
+        # a stale result across genuinely different requests.
+        original_etag = metadata_etag(
+            cast("Request", SimpleNamespace(query_params={})),
+            doc.id,
+        )
         new_modified = timezone.now() + timedelta(seconds=5)
         Document.objects.filter(id=doc.id).update(modified=new_modified)
 
-        self.assertNotEqual(metadata_etag(request, doc.id), original_etag)
+        second_request = cast("Request", SimpleNamespace(query_params={}))
+        self.assertNotEqual(metadata_etag(second_request, doc.id), original_etag)
         self.assertEqual(
-            metadata_etag(request, doc.id),
+            metadata_etag(second_request, doc.id),
             f"{doc.checksum}:{new_modified.isoformat()}",
         )
 
@@ -76,9 +92,13 @@ class TestConditionals(DirectoriesMixin, TestCase):
             root_document=other_root,
         )
 
-        invalid_request = SimpleNamespace(query_params={"version": "not-a-number"})
-        unrelated_request = SimpleNamespace(
-            query_params={"version": str(other_version.id)},
+        invalid_request = cast(
+            "Request",
+            SimpleNamespace(query_params={"version": "not-a-number"}),
+        )
+        unrelated_request = cast(
+            "Request",
+            SimpleNamespace(query_params={"version": str(other_version.id)}),
         )
 
         self.assertIsNone(
@@ -105,7 +125,7 @@ class TestConditionals(DirectoriesMixin, TestCase):
         latest.thumbnail_path.parent.mkdir(parents=True, exist_ok=True)
         latest.thumbnail_path.write_bytes(b"thumb")
 
-        request = SimpleNamespace(query_params={})
+        request = cast("Request", SimpleNamespace(query_params={}))
         with mock.patch(
             "documents.conditionals.get_thumbnail_modified_key",
             return_value="thumb-modified-key",

src/documents/versioning.py

@@ -8,7 +8,7 @@ from typing import Any
 from documents.models import Document
 
 if TYPE_CHECKING:
-    from django.http import HttpRequest
+    from rest_framework.request import Request
 
 
 class VersionResolutionError(StrEnum):
@@ -26,7 +26,7 @@ def _document_manager(*, include_deleted: bool) -> Any:
     return Document.global_objects if include_deleted else Document.objects
 
 
-def get_request_version_param(request: HttpRequest) -> str | None:
+def get_request_version_param(request: Request) -> str | None:
     if hasattr(request, "query_params"):
         return request.query_params.get("version")
     return None
@@ -57,7 +57,7 @@ def get_latest_version_for_root(
 
 def resolve_requested_version_for_root(
     root_doc: Document,
-    request: Any,
+    request: Request,
     *,
     include_deleted: bool = False,
 ) -> VersionResolution:
@@ -86,7 +86,7 @@ def resolve_requested_version_for_root(
 
 def resolve_effective_document(
     request_doc: Document,
-    request: Any,
+    request: Request,
     *,
     include_deleted: bool = False,
 ) -> VersionResolution:
@@ -107,18 +107,41 @@ def resolve_effective_document(
     return VersionResolution(document=request_doc)
 
 
+_EFFECTIVE_DOCUMENT_CACHE_ATTR = "_effective_document_resolution_cache"
+
+
 def resolve_effective_document_by_pk(
     pk: int,
-    request: Any,
+    request: Request,
     *,
     include_deleted: bool = False,
 ) -> VersionResolution:
+    # Django's `condition()` decorator (used for ETag/Last-Modified) invokes the
+    # etag_func and last_modified_func separately, and the view itself may resolve
+    # again -- all against the same request. Cache per-request so a single thumb/
+    # metadata/preview request doesn't redo this resolution multiple times.
+    cache = getattr(request, _EFFECTIVE_DOCUMENT_CACHE_ATTR, None)
+    if cache is None:
+        cache = {}
+        setattr(request, _EFFECTIVE_DOCUMENT_CACHE_ATTR, cache)
+
+    key = (pk, include_deleted)
+    if key in cache:
+        return cache[key]
+
     manager = _document_manager(include_deleted=include_deleted)
     request_doc = manager.only("id", "root_document_id").filter(pk=pk).first()
     if request_doc is None:
-        return VersionResolution(document=None, error=VersionResolutionError.NOT_FOUND)
-    return resolve_effective_document(
-        request_doc,
-        request,
-        include_deleted=include_deleted,
-    )
+        resolution = VersionResolution(
+            document=None,
+            error=VersionResolutionError.NOT_FOUND,
+        )
+    else:
+        resolution = resolve_effective_document(
+            request_doc,
+            request,
+            include_deleted=include_deleted,
+        )
+
+    cache[key] = resolution
+    return resolution