diff --git a/src/documents/models.py b/src/documents/models.py index 521f43b6a..76b0192aa 100644 --- a/src/documents/models.py +++ b/src/documents/models.py @@ -375,6 +375,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 @@ -384,6 +385,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 648052797..d87681e07 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -89,6 +89,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: @@ -1152,8 +1153,14 @@ class DocumentSerializer( def to_representation(self, instance): doc = super().to_representation(instance) - if "content" in self.fields and hasattr(instance, "effective_content"): - doc["content"] = getattr(instance, "effective_content") or "" + 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] return doc diff --git a/src/documents/tests/test_document_list_effective_content_annotation.py b/src/documents/tests/test_document_list_effective_content_annotation.py new file mode 100644 index 000000000..717ee950b --- /dev/null +++ b/src/documents/tests/test_document_list_effective_content_annotation.py @@ -0,0 +1,457 @@ +from __future__ import annotations + +from types import SimpleNamespace +from typing import TYPE_CHECKING + +import pytest +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: + from rest_framework.test import APIClient + + +class TestNeedsEffectiveContentAnnotation: + """ + DocumentViewSet._needs_effective_content_annotation() decides whether + the effective_content correlated subquery is worth attaching to the + queryset at all -- see TestDocumentListEffectiveContentAnnotation below + for why. This only checks that decision's own logic (a plain query-param + membership test), not that Django/DRF's filtering machinery works. + """ + + @pytest.mark.parametrize( + ("params", "expected"), + [ + ({}, 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), + ({"content__iendswith": "foo"}, True), + ({"content__icontains": "foo"}, True), + ({"content__iexact": "foo"}, True), + ], + ) + def test_detects_content_filter_params( + self, + params: dict[str, str], + expected: bool, # noqa: FBT001 + ) -> None: + """ + GIVEN: + - A view bound to a request carrying the given query params + WHEN: + - Checking whether the effective_content annotation is needed + THEN: + - It is needed only for requests that actually filter on it + """ + view = DocumentViewSet() + view.request = SimpleNamespace(query_params=params) + + assert view._needs_effective_content_annotation() is expected + + +class TestNeedsEffectiveContentPrefetch: + """ + DocumentViewSet._needs_effective_content_prefetch() decides whether the + single-version content prefetch is worth attaching. It has to read the + `fields` param exactly the way get_serializer() does, or a request whose + response includes content ends up without the prefetch and pays + get_effective_content()'s per-instance fallback instead. + """ + + @pytest.mark.parametrize( + ("params", "expected"), + [ + pytest.param({}, True, id="no-fields-param-keeps-every-field"), + pytest.param({"fields": ""}, True, id="blank-fields-keeps-every-field"), + pytest.param( + {"fields": "id,content"}, + True, + id="content-among-requested-fields", + ), + pytest.param({"fields": "content"}, True, id="content-only"), + pytest.param({"fields": "id"}, False, id="content-not-requested"), + pytest.param( + {"fields": "id,title"}, + False, + id="several-fields-without-content", + ), + ], + ) + def test_detects_whether_content_can_reach_the_response( + self, + params: dict[str, str], + expected: bool, # noqa: FBT001 + ) -> None: + """ + GIVEN: + - A view bound to a request carrying the given query params + WHEN: + - Checking whether the content prefetch is needed + THEN: + - It is needed exactly when get_serializer() would emit content, + which treats a blank `fields` the same as an absent one + """ + view = DocumentViewSet() + view.request = SimpleNamespace(query_params=params) + + assert view._needs_effective_content_prefetch() is expected + + +@pytest.mark.django_db +class TestDocumentListEffectiveContentAnnotation: + """ + DocumentViewSet.get_queryset() only attaches the effective_content + correlated subquery when a request actually filters on it. Attaching it + unconditionally re-executes it once per candidate row before the page's + LIMIT is applied -- fine on SQLite/Postgres, but pathological on + MariaDB's default cardinality estimation for the root_document_id + self-join once candidate counts get large (see the root_document_id / + effective_content perf investigation). + """ + + def test_list_without_content_filter_skips_annotation_but_returns_latest_content( + self, + admin_client: APIClient, + ) -> None: + """ + GIVEN: + - A root document whose latest version has different content + WHEN: + - Listing documents with no search/content-filter param + THEN: + - The response still reflects the latest version's content + - The database never evaluates effective_content per row + """ + root = DocumentFactory(content="old-root-content") + DocumentFactory( + root_document=root, + version_index=1, + content="new-version-content", + ) + + with CaptureQueriesContext(connection) as ctx: + response = admin_client.get("/api/documents/?fields=id,content") + + assert response.status_code == status.HTTP_200_OK + assert response.data["results"] == [ + {"id": root.id, "content": "new-version-content"}, + ] + assert not any( + "effective_content" in query["sql"] for query in ctx.captured_queries + ) + + @pytest.mark.parametrize( + "fields_param", + [ + pytest.param("", id="blank-fields"), + pytest.param("id,content", id="content-requested"), + ], + ) + def test_content_resolves_without_a_query_per_document( + self, + admin_client: APIClient, + fields_param: str, + ) -> None: + """ + GIVEN: + - One versioned root document, then two more + WHEN: + - Listing documents with a `fields` param that keeps content + THEN: + - Every root's content resolves to its latest version's + - The query count does not grow with the number of documents, + i.e. a blank `fields` does not skip the prefetch and fall back + to loading each root's deferred version content + """ + first = DocumentFactory(content="first-root-content") + DocumentFactory( + root_document=first, + version_index=1, + content="first-version-content", + ) + + with CaptureQueriesContext(connection) as one_document: + response = admin_client.get(f"/api/documents/?fields={fields_param}") + + assert response.status_code == status.HTTP_200_OK + assert [r["content"] for r in response.data["results"]] == [ + "first-version-content", + ] + + for index in range(2): + root = DocumentFactory(content=f"root-content-{index}") + DocumentFactory( + root_document=root, + version_index=1, + content=f"version-content-{index}", + ) + with CaptureQueriesContext(connection) as three_documents: + response = admin_client.get(f"/api/documents/?fields={fields_param}") + + assert response.status_code == status.HTTP_200_OK + assert sorted(r["content"] for r in response.data["results"]) == [ + "first-version-content", + "version-content-0", + "version-content-1", + ] + assert len(_get_document_queries(three_documents)) == len( + _get_document_queries(one_document), + ) + + def test_list_without_content_field_skips_prefetch_and_omits_content( + self, + admin_client: APIClient, + ) -> None: + """ + GIVEN: + - A versioned root document + WHEN: + - Listing documents without asking for content + THEN: + - Content is neither serialized nor resolved + - Nothing pays for the prefetch or the per-instance fallback + """ + root = DocumentFactory(content="root-content") + DocumentFactory( + root_document=root, + version_index=1, + content="version-content", + ) + + with CaptureQueriesContext(connection) as ctx: + response = admin_client.get("/api/documents/?fields=id") + + assert response.status_code == status.HTTP_200_OK + assert response.data["results"] == [{"id": root.id}] + assert _get_effective_content_fallback_queries(ctx) == [] + # Only the list query itself reads a content column: no extra query + # for the skipped prefetch, none for a per-instance fallback + content_queries = [ + query + for query in ctx.captured_queries + if '"documents_document"."content"' in query["sql"] + ] + assert len(content_queries) == 1 + + def test_latest_version_content_prefetch_carries_only_the_newest_version( + self, + ) -> None: + """ + GIVEN: + - A root document with two versions + WHEN: + - Fetching the root through latest_version_content_prefetch() + 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) + """ + 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", + ) + + fetched_root = ( + Document.objects.filter(pk=root.pk) + .prefetch_related( + latest_version_content_prefetch(), + ) + .get() + ) + + 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: + """ + GIVEN: + - A document the ORM never annotated or prefetched for + WHEN: + - Asking whether its effective content is already resolved + THEN: + - It is not, so the serializer must leave it alone + """ + document = DocumentFactory.build() + + assert has_prefetched_effective_content(document) is False + + def test_true_with_effective_content_annotation(self) -> None: + """ + GIVEN: + - A document carrying the queryset's effective_content annotation + WHEN: + - Asking whether its effective content is already resolved + THEN: + - It is, straight off the annotation + """ + document = DocumentFactory.build() + document.effective_content = "resolved" + + assert has_prefetched_effective_content(document) is True + + def test_true_with_lean_prefetch_attr_even_when_empty(self) -> None: + """ + GIVEN: + - A document the lean content prefetch ran for, finding no versions + WHEN: + - Asking whether its effective content is already resolved + THEN: + - It is: an empty prefetch is an answer, not a missing one + """ + document = DocumentFactory.build() + 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: + """ + GIVEN: + - A document carrying only the metadata "versions" prefetch + WHEN: + - Asking whether its effective content is already resolved + THEN: + - It is, via get_effective_content()'s prefetch-cache branch + """ + document = DocumentFactory.build() + document._prefetched_objects_cache = {"versions": []} + + assert has_prefetched_effective_content(document) is True + + +def _get_document_queries( + ctx: CaptureQueriesContext, +) -> list[dict[str, str]]: + """ + The queries a list request spends on the documents themselves, i.e. + everything but the one-time django_content_type lookup guardian's + permission filtering makes. That lookup is process-cached, and the + autouse fixture in conftest clears the cache before every test, so it + lands in whichever request happens to run first and never repeats -- + counting it makes a request look like it costs one query more than the + identical request after it. + """ + return [q for q in ctx.captured_queries if '"django_content_type"' not in q["sql"]] + + +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 TestTrashAndGlobalSearchEffectiveContentIsNeverPerInstance: + """ + TrashView and GlobalSearchView serialize Document instances with + DocumentSerializer too, but build their querysets independently of + DocumentViewSet.get_queryset(). TrashView doesn't display content at all, + so it keeps the document's own unresolved content; GlobalSearchView + annotates effective_content itself, so it shows the latest version's. + Neither should ever fall back to a per-instance query. + """ + + 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 + version would have had (also trashed, deletion cascades) + WHEN: + - Listing trash + THEN: + - The response shows the document's own content + - Nothing ever queries for versions to resolve it + """ + root = DocumentFactory(content="own-content") + DocumentFactory( + root_document=root, + version_index=1, + content="version-content", + ) + root.delete() + + with CaptureQueriesContext(connection) as ctx: + response = admin_client.get("/api/trash/") + + 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" + assert _get_effective_content_fallback_queries(ctx) == [] + + def test_global_search_db_only_shows_latest_version_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 + WHEN: + - Using the global search endpoint's db_only mode + THEN: + - The response shows the latest version's content, resolved by + GlobalSearchView's own effective_content annotation + - There is no per-instance fallback query + """ + root = DocumentFactory(title="findme", content="own-content") + DocumentFactory( + root_document=root, + version_index=1, + content="version-content", + ) + + with CaptureQueriesContext(connection) as ctx: + response = admin_client.get( + "/api/search/?query=findme&db_only=true", + ) + + assert response.status_code == status.HTTP_200_OK + [result] = [d for d in response.data["documents"] if d["id"] == root.id] + assert result["content"] == "version-content" + assert _get_effective_content_fallback_queries(ctx) == [] diff --git a/src/documents/versioning.py b/src/documents/versioning.py index 758391a0b..b1f0862f0 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 @@ -46,6 +49,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 00b166516..04bd68d2e 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -36,7 +36,6 @@ from django.db.migrations.recorder import MigrationRecorder from django.db.models import Avg from django.db.models import Case from django.db.models import Count -from django.db.models import F from django.db.models import IntegerField from django.db.models import Max from django.db.models import Model @@ -137,12 +136,14 @@ from documents.filters import CustomFieldFilterSet from documents.filters import DocumentFilterSet from documents.filters import DocumentsOrderingFilter from documents.filters import DocumentTypeFilterSet +from documents.filters import EffectiveContentFilter from documents.filters import PaperlessTaskFilterSet from documents.filters import PermittedObjectsFilter from documents.filters import ShareLinkBundleFilterSet from documents.filters import ShareLinkFilterSet from documents.filters import StoragePathFilterSet from documents.filters import TagFilterSet +from documents.filters import TitleContentFilter from documents.mail import EmailAttachment from documents.mail import send_email from documents.matching import match_correspondents @@ -236,6 +237,7 @@ from documents.versioning import annotate_effective_content 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 @@ -1084,12 +1086,59 @@ class DocumentViewSet( ], } - def get_queryset(self): - latest_version_content = Subquery( - versions_newest_first( - Document.objects.filter(root_document=OuterRef("pk")), - ).values("content")[:1], + @classmethod + def _content_filter_params(cls) -> tuple[str, ...]: + """ + Query params whose filtering needs effective_content evaluated in SQL + against every candidate row -- see + _needs_effective_content_annotation(). Derived rather than + hand-maintained so a new content-filtering param counts automatically. + """ + params = [ + name + for name, f in DocumentFilterSet.declared_filters.items() + if isinstance(f, (TitleContentFilter, EffectiveContentFilter)) + ] + if "effective_content" in cls.search_fields: + params.append(SearchFilter().search_param) + return tuple(params) + + def _needs_effective_content_annotation(self) -> bool: + # effective_content is a per-row correlated subquery resolving each + # document's latest version. Filtering *on* it forces the database to + # evaluate it for every candidate row before reaching the LIMIT, which + # the root_document_id self-join makes pathological on MariaDB + # specifically once real candidate counts get large; otherwise the + # "versions" prefetch + Document.get_effective_content() resolves only + # the page that survives pagination. Every param here is deprecated in + # favor of the Tantivy-backed search endpoint (see filters.py's + # TitleContentFilter/EffectiveContentFilter docs), so pay that cost + # only when one is actually used. Blank values don't count, matching + # how those filters themselves no-op on them -- an empty `?search=` + # applies no predicate. + params = self.request.query_params + return any( + params.get(param, "").strip() for param in self._content_filter_params() ) + + def _requested_fields(self) -> list[str] | None: + # The sparse-fieldset `fields` param, as DynamicFieldsModelSerializer + # wants it: None means "no restriction, serialize everything", which + # a blank value means too. get_queryset() and get_serializer() both + # branch on this, and they have to read it identically -- a queryset + # that skips the content prefetch for a response that still + # serializes content reintroduces get_effective_content()'s + # per-instance fallback. + fields_param = self.request.query_params.get("fields") + return fields_param.split(",") if fields_param else None + + def _needs_effective_content_prefetch(self) -> bool: + # The prefetch spares get_effective_content() a per-instance fallback + # query, but only earns itself when content can reach the response. + fields = self._requested_fields() + return fields is None or "content" in fields + + def get_queryset(self): # A correlated subquery avoids the LEFT JOIN + Count() this used to # be, which forced a GROUP BY aggregate over every matching document # before the query could even be sorted or limited. @@ -1109,40 +1158,43 @@ class DocumentViewSet( # ObjectFilter.filter(). A blanket .distinct() here forces the # database to fully sort and dedupe every visible document before # it can apply LIMIT, which is disastrous at scale. - return ( + prefetches = [ + Prefetch( + "versions", + queryset=Document.objects.only( + "id", + "added", + "checksum", + "version_label", + "root_document_id", + "version_index", + ), + ), + "tags", + Prefetch( + "custom_fields", + queryset=CustomFieldInstance.objects.select_related("field"), + ), + # NotesSerializer nests the author, this avoids query per note + Prefetch("notes", queryset=Note.objects.select_related("user")), + ] + if self._needs_effective_content_prefetch(): + prefetches.append(latest_version_content_prefetch()) + queryset = ( Document.objects.filter(root_document__isnull=True) .order_by("-created", "-id") - .annotate(effective_content=Coalesce(latest_version_content, F("content"))) .annotate(num_notes=Coalesce(note_count, 0)) .select_related("correspondent", "storage_path", "document_type", "owner") - .prefetch_related( - Prefetch( - "versions", - queryset=Document.objects.only( - "id", - "added", - "checksum", - "version_label", - "root_document_id", - "version_index", - ), - ), - "tags", - Prefetch( - "custom_fields", - queryset=CustomFieldInstance.objects.select_related("field"), - ), - # NotesSerializer nests the author, this avoids query per note - Prefetch("notes", queryset=Note.objects.select_related("user")), - ) + .prefetch_related(*prefetches) ) + if self._needs_effective_content_annotation(): + queryset = annotate_effective_content(queryset) + return queryset def get_serializer(self, *args, **kwargs): - fields_param = self.request.query_params.get("fields", None) - fields = fields_param.split(",") if fields_param else None truncate_content = self.request.query_params.get("truncate_content", "False") kwargs.setdefault("context", self.get_serializer_context()) - kwargs.setdefault("fields", fields) + kwargs.setdefault("fields", self._requested_fields()) kwargs.setdefault("truncate_content", truncate_content.lower() in ["true", "1"]) try: full_perms = get_boolean(