From 01a0880e6f07108702a54b6b9d888f22f4c4d215 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Wed, 19 Aug 2026 16:50:17 -0700 Subject: [PATCH] prevent overwriting index Nothing actually passed in something different for the effective_content args, so drop them! --- src/documents/search/_backend.py | 31 ++++++------------- src/documents/signals/handlers.py | 5 +-- src/documents/tasks.py | 11 ++++--- src/documents/tests/search/test_backend.py | 36 ++++++++++++++++++++++ src/documents/versioning.py | 21 +++++++++++++ 5 files changed, 73 insertions(+), 31 deletions(-) diff --git a/src/documents/search/_backend.py b/src/documents/search/_backend.py index fe3fc646e..8aaa6cccd 100644 --- a/src/documents/search/_backend.py +++ b/src/documents/search/_backend.py @@ -266,11 +266,7 @@ class WriteBatch: if self._lock is not None: self._lock.release() - def add_or_update( - self, - document: Document, - effective_content: str | None = None, - ) -> None: + def add_or_update(self, document: Document) -> None: """ Add or update a document in the batch. @@ -280,11 +276,9 @@ class WriteBatch: Args: document: Django Document instance to index - effective_content: Override document.content for indexing (used when - re-indexing with newer OCR text from document versions) """ self.remove(document.pk) - doc = self._backend._build_tantivy_doc(document, effective_content) + doc = self._backend._build_tantivy_doc(document) self._writer.add_document(doc) def remove(self, doc_id: int) -> None: @@ -425,18 +419,17 @@ class TantivyBackend: def _build_tantivy_doc( self, document: Document, - effective_content: str | None = None, viewer_ids: list[int] | None = None, viewer_group_ids: list[int] | None = None, ) -> tantivy.Document: """Build a tantivy Document from a Django Document instance. - ``effective_content`` overrides ``document.content`` for indexing — - used when re-indexing a root document with a newer version's OCR text. + A root document is indexed with its effective content, i.e. the newest + version's OCR text, so it is never indexed with its own outdated text. + Annotate the queryset with ``annotate_effective_content`` when indexing + more than a couple of documents, to resolve that without a query each. """ - content = ( - effective_content if effective_content is not None else document.content - ) + content = document.get_effective_content() or "" doc = tantivy.Document() @@ -584,11 +577,7 @@ class TantivyBackend: return doc - def add_or_update( - self, - document: Document, - effective_content: str | None = None, - ) -> None: + def add_or_update(self, document: Document) -> None: """ Add or update a single document with file locking. @@ -601,12 +590,11 @@ class TantivyBackend: Args: document: Django Document instance to index - effective_content: Override document.content for indexing """ self._ensure_open() try: with self.batch_update(lock_timeout=_LOCK_TIMEOUT_SECONDS) as batch: - batch.add_or_update(document, effective_content) + batch.add_or_update(document) except SearchIndexLockError: logger.error( "Search index lock exhausted for document %d after %d attempts; " @@ -1027,7 +1015,6 @@ class TantivyBackend: ): doc = self._build_tantivy_doc( document, - document.get_effective_content(), viewer_ids=viewer_ids, viewer_group_ids=viewer_group_ids, ) diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 2d6d5c441..66de47ac4 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -799,10 +799,7 @@ def add_to_index(sender, document, **kwargs) -> None: if document.root_document_id: document = document.root_document - get_backend().add_or_update( - document, - effective_content=document.get_effective_content(), - ) + get_backend().add_or_update(document) def run_workflows_added( diff --git a/src/documents/tasks.py b/src/documents/tasks.py index a5da8f1cf..fd61960a1 100644 --- a/src/documents/tasks.py +++ b/src/documents/tasks.py @@ -64,6 +64,7 @@ from documents.signals.handlers import send_websocket_document_updated from documents.utils import IterWrapper from documents.utils import compute_checksum from documents.utils import identity +from documents.versioning import annotate_effective_content from documents.workflows.utils import get_workflows_for_trigger from paperless.config import AIConfig from paperless.logging import consume_task_id @@ -114,10 +115,7 @@ def index_document(self, document_id: int) -> None: ) return with get_backend().batch_update() as batch: - batch.add_or_update( - document, - effective_content=document.get_effective_content(), - ) + batch.add_or_update(document) @shared_task( @@ -312,7 +310,10 @@ def bulk_update_documents(document_ids) -> None: from documents.search import get_backend document_ids = list(document_ids) - documents = Document.objects.filter(id__in=document_ids) + # Annotated so indexing below doesn't query the versions of each document + documents = annotate_effective_content( + Document.objects.filter(id__in=document_ids), + ) for doc in documents: clear_document_caches(doc.pk) diff --git a/src/documents/tests/search/test_backend.py b/src/documents/tests/search/test_backend.py index adab7c40f..21fed8f02 100644 --- a/src/documents/tests/search/test_backend.py +++ b/src/documents/tests/search/test_backend.py @@ -1070,6 +1070,42 @@ class TestVersionIndexing: assert backend.search_ids("unprotected", user=None) == [root.pk] +class TestEffectiveContentIndexing: + """ + GIVEN: + - A root document with a newer version + WHEN: + - The root document is indexed + THEN: + - The newest version's content is indexed, never the root's own + outdated text + """ + + def test_root_is_indexed_with_latest_version_content( + self, + backend: TantivyBackend, + ) -> None: + root = Document.objects.create( + title="Statement", + content="stale original text", + checksum="EFF1", + pk=95, + ) + Document.objects.create( + title="Statement", + content="latest version text", + checksum="EFF2", + pk=96, + root_document=root, + version_index=1, + ) + + backend.add_or_update(root) + + assert backend.search_ids("latest", user=None) == [root.pk] + assert backend.search_ids("stale", user=None) == [] + + class TestIndexDirectoryGarbageCollection: """Regression tests for Tantivy segment files leaking on disk when multiple long-lived worker processes (Granian/Celery) take turns writing diff --git a/src/documents/versioning.py b/src/documents/versioning.py index 2c32d7fe9..bba2495c8 100644 --- a/src/documents/versioning.py +++ b/src/documents/versioning.py @@ -6,7 +6,10 @@ from typing import TYPE_CHECKING from typing import Any from django.db.models import F +from django.db.models import OuterRef from django.db.models import QuerySet +from django.db.models import Subquery +from django.db.models.functions import Coalesce from documents.models import Document @@ -22,6 +25,24 @@ def versions_newest_first(documents: QuerySet[Document]) -> QuerySet[Document]: return documents.order_by(F("version_index").desc(nulls_last=True), "-id") +def annotate_effective_content(documents: QuerySet[Document]) -> QuerySet[Document]: + """ + Annotates documents with the content of their newest version, falling back + to their own, so get_effective_content() can answer from the row rather + than querying for the versions of each document + """ + return documents.annotate( + effective_content=Coalesce( + Subquery( + versions_newest_first( + Document.objects.filter(root_document=OuterRef("pk")), + ).values("content")[:1], + ), + F("content"), + ), + ) + + def sort_versions_newest_first(documents: list[Document]) -> list[Document]: """ Same sorting as versions_newest_first()