Fix: stop full document saves from writing file path columns

A full save() of a Document instance loaded before another process moved its file wrote the old filename back. The DB then pointed at a path that no longer existed, and recovery only worked when the recomputed path happened to match the file's real location.

Document.save() now leaves filename and archive_filename out of full saves on existing rows. Writers that own those columns opt in with write_file_paths=True or an explicit update_fields. A full save of a row deleted elsewhere now raises instead of re-inserting it.

src/documents/consumer.py

@@ -762,7 +762,7 @@ class ConsumerPlugin(
                         # Don't save with the lock active. Saving will cause the file
                         # renaming logic to acquire the lock as well.
                         # This triggers things like file renaming
-                        document.save()
+                        document.save(write_file_paths=True)
 
                         if document.root_document_id:
                             document_updated.send(

src/documents/models.py

@@ -1,6 +1,7 @@
 import datetime
 import uuid
 from pathlib import Path
+from typing import Any
 from typing import Final
 
 import pathvalidate
@@ -157,6 +158,12 @@ class StoragePath(MatchingModel):
 
 class Document(SoftDeleteModel, ModelWithOwner):  # type: ignore[django-manager-missing]
     MAX_STORED_FILENAME_LENGTH: Final[int] = 1024
+    # Written only by the file mover, the consumer's file placement and the
+    # archiver. A full save() of a stale instance must never write them, or
+    # the DB can point at a path the file has already been moved away from.
+    FILE_PATH_FIELDS: Final[frozenset[str]] = frozenset(
+        {"filename", "archive_filename"},
+    )
     # Per-component limit of common filesystems (ext4, XFS, btrfs, ZFS, APFS)
     MAX_PUBLIC_FILENAME_BYTES: Final[int] = 255
 
@@ -368,6 +375,31 @@ class Document(SoftDeleteModel, ModelWithOwner):  # type: ignore[django-manager-
             res += f" {self.title}"
         return res
 
+    def save(self, *, write_file_paths: bool = False, **kwargs: Any) -> None:
+        """
+        Save the document without writing filename / archive_filename on an
+        existing row, unless write_file_paths=True or update_fields names them.
+
+        This mirrors how Django narrows a save on a deferred instance: only the
+        loaded, concrete, non-generated, non-primary-key fields are written.
+        """
+        if (
+            not write_file_paths
+            and not self._state.adding
+            and kwargs.get("update_fields") is None
+            and not kwargs.get("force_insert", False)
+        ):
+            deferred = self.get_deferred_fields()
+            kwargs["update_fields"] = [
+                field.attname
+                for field in self._meta.concrete_fields
+                if not field.primary_key
+                and not field.generated
+                and field.name not in self.FILE_PATH_FIELDS
+                and field.attname not in deferred
+            ]
+        super().save(**kwargs)
+
     def get_effective_barcodes(self) -> list["DocumentBarcode"]:
         """
         Returns the stored barcodes for the document, like

src/documents/tests/test_bulk_edit.py

@@ -739,7 +739,7 @@ class TestPDFActions(DirectoriesMixin, TestCase):
             mime_type="application/pdf",
         )
         self.doc1.archive_filename = sample1_archive
-        self.doc1.save()
+        self.doc1.save(write_file_paths=True)
         self.doc2 = Document.objects.create(
             checksum="B",
             title="B",
@@ -749,7 +749,7 @@ class TestPDFActions(DirectoriesMixin, TestCase):
             created=date(2023, 1, 2),
         )
         self.doc2.archive_filename = sample2_archive
-        self.doc2.save()
+        self.doc2.save(write_file_paths=True)
         self.doc3 = Document.objects.create(
             checksum="C",
             title="C",
@@ -774,7 +774,7 @@ class TestPDFActions(DirectoriesMixin, TestCase):
             created=date(2023, 1, 3),
         )
         self.img_doc.archive_filename = img_doc_archive
-        self.img_doc.save()
+        self.img_doc.save(write_file_paths=True)
 
     @staticmethod
     def fake_decrypt(

src/documents/tests/test_file_handling.py

@@ -92,7 +92,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
         # Ensure that filename is properly generated
         self.assertEqual(document.filename, Path("none/none.pdf"))
 
-        document.save()
+        document.save(write_file_paths=True)
 
         # test that creating dirs for the source_path creates the correct directory
         create_source_path_directory(document.source_path)
@@ -190,7 +190,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
 
         # Set a correspondent and save the document
         document.correspondent = Correspondent.objects.get_or_create(name="test")[0]
-        document.save()
+        document.save(write_file_paths=True)
 
         # Check proper handling of files
         self.assertIsFile(
@@ -232,7 +232,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
             disable_auditlog(),
         ):
             m.side_effect = DatabaseError()
-            document.save()
+            document.save(write_file_paths=True)
 
             # Check proper handling of files
             self.assertIsFile(document.source_path)
@@ -336,7 +336,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
 
         # Ensure that filename is properly generated
         document.filename = generate_filename(document)
-        document.save()
+        document.save(write_file_paths=True)
         self.assertEqual(document.filename, "none/none.pdf")
 
         create_source_path_directory(document.source_path)
@@ -362,7 +362,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
 
         # Ensure that filename is properly generated
         document.filename = generate_filename(document)
-        document.save()
+        document.save(write_file_paths=True)
         self.assertEqual(document.filename, "none/none.pdf")
 
         create_source_path_directory(document.source_path)
@@ -385,7 +385,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
 
         document.save()
         document.filename = generate_filename(document)
-        document.save()
+        document.save(write_file_paths=True)
         create_source_path_directory(document.source_path)
         Path(document.source_path).touch()
         document.delete()
@@ -421,7 +421,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
 
         # Set a correspondent and save the document
         document.correspondent = Correspondent.objects.get_or_create(name="test")[0]
-        document.save()
+        document.save(write_file_paths=True)
 
         # Check proper handling of files
         self.assertIsDir(settings.ORIGINALS_DIR / "test")
@@ -476,7 +476,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
     def test_filename_relative(self) -> None:
         doc = Document.objects.create(title="doc1", mime_type="application/pdf")
         doc.filename = generate_filename(doc)
-        doc.save()
+        doc.save(write_file_paths=True)
 
         self.assertEqual(
             doc.source_path,
@@ -532,7 +532,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
 
         # Ensure that filename is properly generated
         document.filename = generate_filename(document)
-        document.save()
+        document.save(write_file_paths=True)
         self.assertEqual(document.filename, "none/none/none.pdf")
         create_source_path_directory(document.source_path)
         Path(document.source_path).touch()
@@ -620,13 +620,13 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
         Path(document.source_path).touch()
         Path(document2.source_path).touch()
         document.filename = "0000001.pdf"
-        document.save()
+        document.save(write_file_paths=True)
 
         self.assertIsFile(document.source_path)
         self.assertEqual(document.filename, "qwe.pdf")
 
         document2.filename = "0000002.pdf"
-        document2.save()
+        document2.save(write_file_paths=True)
 
         self.assertIsFile(document.source_path)
         self.assertEqual(document2.filename, "qwe_01.pdf")

src/documents/tests/test_management.py

@@ -236,7 +236,7 @@ class TestRenamer(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
         doc = Document.objects.create(title="test", mime_type="image/jpeg")
         doc.filename = generate_filename(doc)
         doc.archive_filename = generate_filename(doc, archive_filename=True)
-        doc.save()
+        doc.save(write_file_paths=True)
 
         Path(doc.source_path).touch()
         Path(doc.archive_path).touch()

src/documents/tests/test_models.py

@@ -1,4 +1,7 @@
 import pytest
+from django.db import DatabaseError
+from django.db import models
+from django.db import transaction
 
 from documents.models import Correspondent
 from documents.models import Document
@@ -22,3 +25,101 @@ class TestDocument:
         assert Correspondent.objects.count() == 0
         assert Document.objects.count() == 1
         assert Document.objects.first().correspondent is None
+
+
+@pytest.mark.django_db
+@pytest.mark.usefixtures("paperless_dirs")
+class TestDocumentFilePathWrites:
+    def test_full_save_does_not_write_file_paths(self) -> None:
+        """
+        GIVEN:
+            - A document whose filename columns were changed in the DB after
+              an instance was loaded
+        WHEN:
+            - The stale instance is saved without update_fields
+        THEN:
+            - The DB filename columns keep the newer values
+            - Other fields from the stale instance are written
+        """
+        doc = DocumentFactory(filename="old.pdf", archive_filename="old.pdf")
+        stale = Document.objects.get(pk=doc.pk)
+        Document.objects.filter(pk=doc.pk).update(
+            filename="new.pdf",
+            archive_filename="new-archive.pdf",
+        )
+
+        stale.title = "changed"
+        stale.save()
+
+        doc.refresh_from_db()
+        assert doc.filename == "new.pdf"
+        assert doc.archive_filename == "new-archive.pdf"
+        assert doc.title == "changed"
+
+    @pytest.mark.parametrize(
+        "save_kwargs",
+        [
+            pytest.param({"write_file_paths": True}, id="write-file-paths"),
+            pytest.param(
+                {"update_fields": ["filename", "archive_filename"]},
+                id="explicit-update-fields",
+            ),
+        ],
+    )
+    def test_opt_in_writes_file_paths(self, save_kwargs: dict[str, object]) -> None:
+        """
+        GIVEN:
+            - An existing document
+        WHEN:
+            - Its filename columns are changed in memory and saved with an
+              explicit opt in
+        THEN:
+            - The DB filename columns are updated
+        """
+        doc = DocumentFactory(filename="old.pdf", archive_filename="old.pdf")
+
+        doc.filename = "new.pdf"
+        doc.archive_filename = "new.pdf"
+        doc.save(**save_kwargs)
+
+        doc.refresh_from_db()
+        assert doc.filename == "new.pdf"
+        assert doc.archive_filename == "new.pdf"
+
+    def test_creation_writes_file_paths(self) -> None:
+        """
+        GIVEN:
+            - A new, unsaved document with filename columns set
+        WHEN:
+            - It is saved
+        THEN:
+            - The filename columns are written
+        """
+        doc = DocumentFactory.build(filename="a.pdf", archive_filename="b.pdf")
+
+        doc.save()
+
+        doc.refresh_from_db()
+        assert doc.filename == "a.pdf"
+        assert doc.archive_filename == "b.pdf"
+
+    def test_full_save_of_deleted_row_raises(self) -> None:
+        """
+        GIVEN:
+            - An instance whose row was hard-deleted elsewhere
+        WHEN:
+            - The instance is saved without update_fields
+        THEN:
+            - A DatabaseError is raised instead of re-inserting the row
+        """
+        doc = DocumentFactory()
+        stale = Document.objects.get(pk=doc.pk)
+        # A plain QuerySet bypasses the soft-delete manager, so the row is
+        # really gone (as after emptying the trash).
+        models.QuerySet(model=Document).filter(pk=doc.pk).delete()
+
+        # The savepoint keeps the failed UPDATE from poisoning the test transaction.
+        with pytest.raises(DatabaseError), transaction.atomic():
+            stale.save()
+
+        assert not Document.global_objects.filter(pk=doc.pk).exists()

src/documents/tests/test_sanity_check.py

@@ -132,7 +132,7 @@ class TestCheckSanityOriginal:
 class TestCheckSanityArchive:
     def test_checksum_without_filename(self, sample_doc: Document) -> None:
         sample_doc.archive_filename = None
-        sample_doc.save()
+        sample_doc.save(write_file_paths=True)
         messages = check_sanity()
         assert messages.has_error
         assert any(
@@ -185,7 +185,7 @@ class TestCheckSanityArchive:
         Path(sample_doc.archive_path).unlink()
         sample_doc.archive_checksum = None
         sample_doc.archive_filename = None
-        sample_doc.save()
+        sample_doc.save(write_file_paths=True)
         messages = check_sanity()
         assert not messages.has_error
 

src/documents/tests/test_share_link_bundles.py

@@ -366,7 +366,7 @@ class ShareLinkBundleBuildTaskTests(DirectoriesMixin, APITestCase):
     def _write_document_file(self, *, archive: bool, content: bytes) -> Path:
         if archive:
             self.document.archive_filename = f"{self.document.pk:07}.pdf"
-            self.document.save()
+            self.document.save(write_file_paths=True)
             path = self.document.archive_path
             assert path is not None
         else: