diff --git a/src/documents/models.py b/src/documents/models.py index 2e46bfc57..5b1097d63 100644 --- a/src/documents/models.py +++ b/src/documents/models.py @@ -373,6 +373,7 @@ class Document(SoftDeleteModel, ModelWithOwner): # type: ignore[django-manager- If the queryset already annotated ``effective_content``, that value is used. """ # Here to avoid circular import + from documents.versioning import LATEST_VERSION_CONTENT_PREFETCH_ATTR from documents.versioning import sort_versions_newest_first from documents.versioning import versions_newest_first @@ -382,6 +383,19 @@ class Document(SoftDeleteModel, ModelWithOwner): # type: ignore[django-manager- if self.root_document_id is not None or self.pk is None: return self.content + latest_version_prefetch = getattr( + self, + LATEST_VERSION_CONTENT_PREFETCH_ATTR, + None, + ) + if latest_version_prefetch is not None: + # Empty list means prefetch ran and found no versions — use own content. + return ( + latest_version_prefetch[0].content + if latest_version_prefetch + else self.content + ) + prefetched_cache = getattr(self, "_prefetched_objects_cache", None) prefetched_versions = ( prefetched_cache.get("versions") diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 9affa2a1a..2dc19dac9 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -88,6 +88,7 @@ from documents.templating.utils import convert_format_str_to_template_format from documents.templating.workflows import validate_workflow_template from documents.validators import uri_validator from documents.validators import url_validator +from documents.versioning import has_prefetched_effective_content from documents.versioning import sort_versions_newest_first if TYPE_CHECKING: @@ -1146,12 +1147,13 @@ class DocumentSerializer( def to_representation(self, instance): doc = super().to_representation(instance) - if "content" in self.fields: - # get_effective_content() already falls back through the SQL - # annotation (when the queryset attached one), the prefetched - # "versions" cache, and finally a direct query -- so this stays - # correct whether or not DocumentViewSet.get_queryset() decided - # the annotation was needed for this request. + if "content" in self.fields and has_prefetched_effective_content(instance): + # Only resolve version-aware content when it's cheap: an SQL + # annotation or a versions prefetch is already on the instance. + # A caller that set up neither (e.g. TrashView, GlobalSearchView, + # which build their own querysets) gets the document's own, + # unresolved content instead of paying for an extra per-instance + # query -- same as before effective_content resolution existed. doc["content"] = instance.get_effective_content() or "" if self.truncate_content and "content" in self.fields: doc["content"] = doc.get("content")[0:550] diff --git a/src/documents/tests/test_document_list_effective_content_annotation.py b/src/documents/tests/test_document_list_effective_content_annotation.py index 015de0d6f..09f225541 100644 --- a/src/documents/tests/test_document_list_effective_content_annotation.py +++ b/src/documents/tests/test_document_list_effective_content_annotation.py @@ -8,7 +8,11 @@ from django.db import connection from django.test.utils import CaptureQueriesContext from rest_framework import status +from documents.models import Document from documents.tests.factories import DocumentFactory +from documents.versioning import LATEST_VERSION_CONTENT_PREFETCH_ATTR +from documents.versioning import has_prefetched_effective_content +from documents.versioning import latest_version_content_prefetch from documents.views import DocumentViewSet if TYPE_CHECKING: @@ -30,6 +34,9 @@ class TestNeedsEffectiveContentAnnotation: ({}, False), ({"ordering": "-added"}, False), ({"tags__id__in": "1,2"}, False), + ({"search": ""}, False), + ({"search": " "}, False), + ({"content__icontains": ""}, False), ({"search": "foo"}, True), ({"title_content": "foo"}, True), ({"content__istartswith": "foo"}, True), @@ -89,3 +96,144 @@ class TestDocumentListEffectiveContentAnnotation: assert not any( "effective_content" in query["sql"] for query in ctx.captured_queries ) + + def test_latest_version_content_prefetch_carries_only_the_newest_version( + self, + ) -> None: + # GIVEN a root document with two versions + root = DocumentFactory(content="root-content") + DocumentFactory( + root_document=root, + version_index=1, + content="older-version-content", + ) + DocumentFactory( + root_document=root, + version_index=2, + content="newest-version-content", + ) + + # WHEN fetching the root through latest_version_content_prefetch() + fetched_root = ( + Document.objects.filter(pk=root.pk) + .prefetch_related( + latest_version_content_prefetch(), + ) + .get() + ) + + # THEN the prefetch carries only the single newest version, not + # every historical version's content (the whole point of not + # reusing the metadata-only "versions" prefetch for this) + latest = getattr(fetched_root, LATEST_VERSION_CONTENT_PREFETCH_ATTR) + assert [v.content for v in latest] == ["newest-version-content"] + + +class TestHasPrefetchedEffectiveContent: + """ + DocumentSerializer.to_representation() only calls get_effective_content() + when has_prefetched_effective_content() says it's cheap -- otherwise a + caller that never set up an annotation or prefetch (TrashView, + GlobalSearchView, which build their own querysets and don't display + content at all) would pay for a per-instance query nobody asked for. + """ + + def test_false_with_no_annotation_or_prefetch(self) -> None: + document = Document() + assert has_prefetched_effective_content(document) is False + + def test_true_with_effective_content_annotation(self) -> None: + document = Document() + document.effective_content = "resolved" + assert has_prefetched_effective_content(document) is True + + def test_true_with_lean_prefetch_attr_even_when_empty(self) -> None: + document = Document() + setattr(document, LATEST_VERSION_CONTENT_PREFETCH_ATTR, []) + assert has_prefetched_effective_content(document) is True + + def test_true_with_metadata_versions_prefetch_cache(self) -> None: + document = Document() + document._prefetched_objects_cache = {"versions": []} + assert has_prefetched_effective_content(document) is True + + +def _get_effective_content_fallback_queries( + ctx: CaptureQueriesContext, +) -> list[dict[str, str]]: + """ + Document.get_effective_content()'s per-instance fallback (no annotation, + no prefetch) is a `.values_list("content", flat=True).first()` query -- + a SELECT of just the content column. Distinct from get_versions()'s own, + unrelated per-instance metadata query (id/checksum/added/etc, no + content) run to build the "versions" response field, which isn't part + of what this test file covers. + """ + return [ + q + for q in ctx.captured_queries + if q["sql"].startswith('SELECT "documents_document"."content" FROM') + ] + + +@pytest.mark.django_db +class TestTrashAndGlobalSearchDoNotResolveEffectiveContent: + """ + TrashView and GlobalSearchView serialize Document instances with + DocumentSerializer too, but build their querysets independently of + DocumentViewSet.get_queryset() -- and neither actually displays + document content. They should keep showing the document's own, + unresolved content with no extra query, exactly as before + effective_content resolution existed. + """ + + def test_trash_list_shows_unresolved_content_with_no_extra_query( + self, + admin_client: APIClient, + ) -> None: + # GIVEN a trashed root document whose own content differs from what + # a (also trashed, since deletion cascades) version would have had + root = DocumentFactory(content="own-content") + DocumentFactory( + root_document=root, + version_index=1, + content="version-content", + ) + root.delete() + + # WHEN listing trash + with CaptureQueriesContext(connection) as ctx: + response = admin_client.get("/api/trash/") + + # THEN the response shows the document's own content... + assert response.status_code == status.HTTP_200_OK + [result] = [r for r in response.data["results"] if r["id"] == root.id] + assert result["content"] == "own-content" + # ...without ever querying for versions to resolve it + assert _get_effective_content_fallback_queries(ctx) == [] + + def test_global_search_db_only_shows_unresolved_content_with_no_extra_query( + self, + admin_client: APIClient, + ) -> None: + # GIVEN a root document, findable by title, whose own content + # differs from its latest version's + root = DocumentFactory(title="findme", content="own-content") + DocumentFactory( + root_document=root, + version_index=1, + content="version-content", + ) + + # WHEN using the global search endpoint's db_only mode + with CaptureQueriesContext(connection) as ctx: + response = admin_client.get( + "/api/search/?query=findme&db_only=true", + ) + + # THEN the response shows the document's own content... + assert response.status_code == status.HTTP_200_OK + [result] = [d for d in response.data["documents"] if d["id"] == root.id] + assert result["content"] == "own-content" + # ...without ever querying for versions to resolve it + assert _get_effective_content_fallback_queries(ctx) == [] diff --git a/src/documents/versioning.py b/src/documents/versioning.py index bba2495c8..f6d8371c5 100644 --- a/src/documents/versioning.py +++ b/src/documents/versioning.py @@ -7,9 +7,12 @@ from typing import Any from django.db.models import F from django.db.models import OuterRef +from django.db.models import Prefetch from django.db.models import QuerySet from django.db.models import Subquery +from django.db.models import Window from django.db.models.functions import Coalesce +from django.db.models.functions import RowNumber from documents.models import Document @@ -43,6 +46,68 @@ def annotate_effective_content(documents: QuerySet[Document]) -> QuerySet[Docume ) +LATEST_VERSION_CONTENT_PREFETCH_ATTR = "_latest_version_content_prefetch" + + +def latest_version_content_prefetch() -> Prefetch: + """ + A Prefetch for Document.versions scoped to just the newest version's + content, for get_effective_content()'s fallback when no SQL annotation + is present. + + Deliberately not merged into a metadata-only "versions" prefetch (the one + used for the serialized versions list): that one fetches every historical + version of every document, and pulling full OCR content for versions + nobody will read wastes DB transfer/memory at scale. This one is windowed + down to a single row per root, then bounded by Prefetch's own IN-list to + whatever page/result set it's attached to -- one cheap bulk query total, + not one per document and not one per version. + """ + return Prefetch( + "versions", + queryset=( + Document.objects.filter( + root_document_id__isnull=False, + deleted_at__isnull=True, + ) + .annotate( + rn=Window( + RowNumber(), + partition_by=F("root_document_id"), + order_by=[ + F("version_index").desc(nulls_last=True), + F("id").desc(), + ], + ), + ) + .filter(rn=1) + .only("id", "root_document_id", "content") + ), + to_attr=LATEST_VERSION_CONTENT_PREFETCH_ATTR, + ) + + +def has_prefetched_effective_content(document: Document) -> bool: + """ + True if document.get_effective_content() can answer without an extra + per-instance query -- an SQL ``effective_content`` annotation, the lean + latest_version_content_prefetch(), or the metadata-only "versions" + prefetch is already present on the instance. + + Callers that haven't set any of those up (e.g. views that build their + own querysets independently of DocumentViewSet.get_queryset(), like + TrashView or GlobalSearchView) intentionally don't pay for version-aware + content resolution -- see DocumentSerializer.to_representation(), which + uses this to decide whether to call get_effective_content() at all. + """ + if hasattr(document, "effective_content"): + return True + if getattr(document, LATEST_VERSION_CONTENT_PREFETCH_ATTR, None) is not None: + return True + prefetched_cache = getattr(document, "_prefetched_objects_cache", None) + return isinstance(prefetched_cache, dict) and "versions" in prefetched_cache + + def sort_versions_newest_first(documents: list[Document]) -> list[Document]: """ Same sorting as versions_newest_first() diff --git a/src/documents/views.py b/src/documents/views.py index 3d40546cd..2b60f06fe 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -233,6 +233,7 @@ from documents.versioning import VersionResolutionError from documents.versioning import get_latest_version_for_root from documents.versioning import get_request_version_param from documents.versioning import get_root_document +from documents.versioning import latest_version_content_prefetch from documents.versioning import resolve_requested_version_for_root from documents.versioning import versions_newest_first from paperless import version @@ -1107,9 +1108,16 @@ class DocumentViewSet( # self-join once real candidate counts get large. Everything on this # list is deprecated in favor of the Tantivy-backed search endpoint # (see filters.py's TitleContentFilter/EffectiveContentFilter docs), - # so keep paying that cost only when one is actually used. + # so keep paying that cost only when one is actually used. Checked as + # a stripped, non-blank value (not just key presence) to match how + # DRF's SearchFilter and TitleContentFilter/EffectiveContentFilter + # themselves no-op on a blank value -- otherwise an empty `?search=` + # or a saved view with a cleared text filter would still pay for the + # annotation despite applying no actual predicate. params = self.request.query_params - return any(param in params for param in self._CONTENT_FILTER_PARAMS) + return any( + params.get(param, "").strip() for param in self._CONTENT_FILTER_PARAMS + ) def get_queryset(self): # A correlated subquery avoids the LEFT JOIN + Count() this used to @@ -1146,9 +1154,9 @@ class DocumentViewSet( "version_label", "root_document_id", "version_index", - "content", ), ), + latest_version_content_prefetch(), "tags", Prefetch( "custom_fields",