Fix: extract document file moves out of the signal receiver

The file move receiver refreshed the caller's own instance from the database, which wiped unsaved in-memory changes. It ran on every m2m_changed action including pre_add and pre_remove, wrote modified even when nothing moved, and its no-move branch skipped trashed documents.

The move body now lives in documents.file_moves.move_files_for_document. It skips documents without a filename before taking media.lock, then takes the lock with a 300 second timeout, loads a fresh instance by pk, writes only when a filename column changes, and returns the instance. The receiver ignores reverse and pre_* m2m actions and copies just the final filename columns back onto the caller's instance.

src/documents/file_moves.py

@@ -15,7 +15,33 @@ its own transaction holds document rows.
 
 from __future__ import annotations
 
+import logging
+import shutil
+from pathlib import Path
+from typing import Final
+
+from django.conf import settings
+from django.db import DatabaseError
 from django.db import transaction
+from django.utils import timezone
+from filelock import Timeout
+
+from documents.caching import clear_document_caches
+from documents.file_handling import create_source_path_directory
+from documents.file_handling import delete_empty_directories
+from documents.file_handling import generate_filename
+from documents.file_handling import generate_unique_filename
+from documents.media_lock import media_lock
+from documents.models import Document
+from documents.utils import compute_checksum
+
+logger = logging.getLogger("paperless.handlers")
+
+MEDIA_LOCK_TIMEOUT_SECONDS: Final[int] = 300
+
+
+class CannotMoveFilesException(Exception):
+    pass
 
 
 def in_user_transaction() -> bool:
@@ -29,3 +55,272 @@ def in_user_transaction() -> bool:
     """
     connection = transaction.get_connection()
     return any(not block._from_testcase for block in connection.atomic_blocks)
+
+
+def _path_matches_checksum(path: Path, checksum: str | None) -> bool:
+    if checksum is None or not path.is_file():
+        return False
+
+    return compute_checksum(path) == checksum
+
+
+def move_files_for_document(pk: int) -> Document | None:
+    """
+    Move the document's original and archive to the paths its current
+    committed state generates, then do the same for its versions.
+
+    Returns the fresh instance with its final filename columns, or None when
+    the document does not exist, has no filename yet, or the lock timed out.
+    """
+    # Cheap check without the lock: most saves during consumption happen
+    # before the file is placed. filename, once set, is never cleared, so a
+    # non-empty value here stays non-empty under the lock. This goes through
+    # values_list() (not the manager's filter()) on purpose.
+    filename = (
+        Document.global_objects.values_list("filename", flat=True).filter(pk=pk).first()
+    )
+    if not filename:
+        return None
+
+    try:
+        with media_lock(timeout=MEDIA_LOCK_TIMEOUT_SECONDS):
+            try:
+                document = Document.global_objects.get(pk=pk)
+            except Document.DoesNotExist:
+                logger.debug(f"Document {pk} no longer exists, skipping file move")
+                return None
+            _move_document_files(document)
+    except Timeout:
+        logger.error(
+            f"Timed out after {MEDIA_LOCK_TIMEOUT_SECONDS}s waiting for "
+            f"{settings.MEDIA_LOCK} to move files of document {pk}; the file "
+            f"keeps its current name and will be renamed on its next save",
+        )
+        return None
+
+    # Keep version files in sync with root
+    if document.root_document_id is None:
+        for version_pk in Document.objects.filter(root_document_id=pk).values_list(
+            "pk",
+            flat=True,
+        ):
+            move_files_for_document(version_pk)
+
+    return document
+
+
+def _validate_move(
+    document: Document,
+    old_path: Path,
+    new_path: Path,
+    root: Path,
+) -> None:
+    if not new_path.is_relative_to(root):
+        msg = (
+            f"Document {document!s}: Refusing to move file outside root {root}: "
+            f"{new_path}."
+        )
+        logger.warning(msg)
+        raise CannotMoveFilesException(msg)
+
+    if not old_path.is_file():
+        # Can't do anything if the old file does not exist anymore.
+        msg = f"Document {document!s}: File {old_path} doesn't exist."
+        logger.fatal(msg)
+        raise CannotMoveFilesException(msg)
+
+    if new_path.is_file():
+        # Can't do anything if the new file already exists. Skip updating file.
+        msg = f"Document {document!s}: Cannot rename file since target path {new_path} already exists."
+        logger.warning(msg)
+        raise CannotMoveFilesException(msg)
+
+
+def _move_document_files(document: Document) -> None:
+    try:
+        old_filename = document.filename
+        old_source_path = document.source_path
+        move_original = False
+        original_already_moved = False
+
+        old_archive_filename = document.archive_filename
+        old_archive_path = document.archive_path
+        move_archive = False
+        archive_already_moved = False
+
+        candidate_filename = generate_filename(document)
+        if len(str(candidate_filename)) > Document.MAX_STORED_FILENAME_LENGTH:
+            msg = (
+                f"Document {document!s}: Generated filename exceeds db path "
+                f"limit ({len(str(candidate_filename))} > "
+                f"{Document.MAX_STORED_FILENAME_LENGTH}): {candidate_filename!s}"
+            )
+            logger.warning(msg)
+            raise CannotMoveFilesException(msg)
+
+        candidate_source_path = (settings.ORIGINALS_DIR / candidate_filename).resolve()
+        if candidate_filename == Path(old_filename):
+            new_filename = Path(old_filename)
+        elif (
+            candidate_source_path.exists() and candidate_source_path != old_source_path
+        ):
+            if not old_source_path.is_file() and _path_matches_checksum(
+                candidate_source_path,
+                document.checksum,
+            ):
+                new_filename = candidate_filename
+                original_already_moved = True
+            else:
+                # Only fall back to unique search when there is an actual conflict
+                new_filename = generate_unique_filename(document)
+        else:
+            new_filename = candidate_filename
+
+        # Need to convert to string to be able to save it to the db
+        document.filename = str(new_filename)
+        move_original = old_filename != document.filename and not original_already_moved
+
+        if document.has_archive_version:
+            archive_candidate = generate_filename(document, archive_filename=True)
+            if len(str(archive_candidate)) > Document.MAX_STORED_FILENAME_LENGTH:
+                msg = (
+                    f"Document {document!s}: Generated archive filename exceeds "
+                    f"db path limit ({len(str(archive_candidate))} > "
+                    f"{Document.MAX_STORED_FILENAME_LENGTH}): {archive_candidate!s}"
+                )
+                logger.warning(msg)
+                raise CannotMoveFilesException(msg)
+            archive_candidate_path = (
+                settings.ARCHIVE_DIR / archive_candidate
+            ).resolve()
+            if archive_candidate == Path(old_archive_filename):
+                new_archive_filename = Path(old_archive_filename)
+            elif (
+                archive_candidate_path.exists()
+                and archive_candidate_path != old_archive_path
+            ):
+                if not old_archive_path.is_file() and _path_matches_checksum(
+                    archive_candidate_path,
+                    document.archive_checksum,
+                ):
+                    new_archive_filename = archive_candidate
+                    archive_already_moved = True
+                else:
+                    new_archive_filename = generate_unique_filename(
+                        document,
+                        archive_filename=True,
+                    )
+            else:
+                new_archive_filename = archive_candidate
+
+            document.archive_filename = str(new_archive_filename)
+
+            move_archive = (
+                old_archive_filename != document.archive_filename
+                and not archive_already_moved
+            )
+        else:
+            move_archive = False
+
+        if not move_original and not move_archive:
+            updates: dict[str, object] = {}
+            if old_filename != document.filename:
+                updates["filename"] = document.filename
+            if old_archive_filename != document.archive_filename:
+                updates["archive_filename"] = document.archive_filename
+            if updates:
+                # global_objects so a trashed document's path is fixed too.
+                # modified only changes when a filename column changes.
+                updates["modified"] = timezone.now()
+                # Don't save() here to prevent infinite recursion.
+                Document.global_objects.filter(pk=document.pk).update(**updates)
+            return
+
+        if move_original:
+            _validate_move(
+                document,
+                old_source_path,
+                document.source_path,
+                settings.ORIGINALS_DIR,
+            )
+            create_source_path_directory(document.source_path)
+            shutil.move(old_source_path, document.source_path)
+
+        if move_archive:
+            _validate_move(
+                document,
+                old_archive_path,
+                document.archive_path,
+                settings.ARCHIVE_DIR,
+            )
+            create_source_path_directory(document.archive_path)
+            shutil.move(old_archive_path, document.archive_path)
+
+        # Don't save() here to prevent infinite recursion.
+        Document.global_objects.filter(pk=document.pk).update(
+            filename=document.filename,
+            archive_filename=document.archive_filename,
+            modified=timezone.now(),
+        )
+        # Clear any caching for this document.  Slightly overkill, but not terrible
+        clear_document_caches(document.pk)
+
+    except (OSError, DatabaseError, CannotMoveFilesException) as e:
+        logger.warning(f"Exception during file handling: {e}")
+        # This happens when either:
+        #  - moving the files failed due to file system errors
+        #  - saving to the database failed due to database errors
+        # In both cases, we need to revert to the original state.
+
+        # Try to move files to their original location.
+        try:
+            if move_original and document.source_path.is_file():
+                logger.info("Restoring previous original path")
+                shutil.move(document.source_path, old_source_path)
+
+            if move_archive and document.archive_path.is_file():
+                logger.info("Restoring previous archive path")
+                shutil.move(document.archive_path, old_archive_path)
+
+        except Exception:
+            # This is fine, since:
+            # A: if we managed to move source from A to B, we will also
+            #  manage to move it from B to A. If not, we have a serious
+            #  issue that's going to get caught by the santiy checker.
+            #  All files remain in place and will never be overwritten,
+            #  so this is not the end of the world.
+            # B: if moving the original file failed, nothing has changed
+            #  anyway.
+            pass
+
+        # restore old values on the document
+        document.filename = old_filename
+        document.archive_filename = old_archive_filename
+
+    # finally, remove any empty sub folders. This will do nothing if
+    # something has failed above.
+    if not old_source_path.is_file():
+        delete_empty_directories(
+            Path(old_source_path).parent,
+            root=settings.ORIGINALS_DIR,
+        )
+
+    if document.has_archive_version and not old_archive_path.is_file():
+        delete_empty_directories(
+            Path(old_archive_path).parent,
+            root=settings.ARCHIVE_DIR,
+        )
+
+
+def request_file_move(document: Document) -> None:
+    """
+    Ask for the document's files to follow its current committed state.
+
+    Moves immediately and copies only the final filename columns onto the
+    caller's instance, so code that saves and then reads source_path keeps
+    working without any other in-memory field being touched.
+    """
+    moved = move_files_for_document(document.pk)
+    if moved is not None:
+        document.filename = moved.filename
+        document.archive_filename = moved.archive_filename

src/documents/signals/handlers.py

@@ -19,7 +19,6 @@ from celery.signals import worker_process_shutdown
 from django.conf import settings
 from django.contrib.auth.models import Group
 from django.contrib.auth.models import User
-from django.db import DatabaseError
 from django.db import close_old_connections
 from django.db import connections
 from django.db import models
@@ -29,14 +28,12 @@ from django.utils import timezone
 from rest_framework import serializers
 
 from documents import matching
-from documents.caching import clear_document_caches
 from documents.caching import invalidate_llm_suggestions_cache
 from documents.caching import invalidate_suggestions_cache
 from documents.data_models import ConsumableDocument
-from documents.file_handling import create_source_path_directory
 from documents.file_handling import delete_empty_directories
-from documents.file_handling import generate_filename
-from documents.file_handling import generate_unique_filename
+from documents.file_moves import move_files_for_document
+from documents.file_moves import request_file_move
 from documents.media_lock import media_lock
 from documents.models import Correspondent
 from documents.models import CustomField
@@ -55,7 +52,6 @@ from documents.models import WorkflowTrigger
 from documents.permissions import restrict_queryset_to_visible
 from documents.plugins.helpers import DocumentsStatusManager
 from documents.templating.utils import convert_format_str_to_template_format
-from documents.utils import compute_checksum
 from documents.utils import copy_file_with_basic_stats
 from documents.workflows.actions import build_workflow_action_context
 from documents.workflows.actions import execute_email_action
@@ -405,17 +401,6 @@ def cleanup_document_deletion(sender, instance, **kwargs) -> None:
             )
 
 
-class CannotMoveFilesException(Exception):
-    pass
-
-
-def _path_matches_checksum(path: Path, checksum: str | None) -> bool:
-    if checksum is None or not path.is_file():
-        return False
-
-    return compute_checksum(path) == checksum
-
-
 def _filename_template_uses_custom_fields(doc: Document) -> bool:
     template = None
     if doc.storage_path is not None:
@@ -438,235 +423,21 @@ def update_filename_and_move_files(
     instance: Document | CustomFieldInstance,
     **kwargs,
 ) -> None:
+    if kwargs.get("reverse"):
+        # tag.documents.add(doc): instance is the Tag, not a Document.
+        return
+    action = kwargs.get("action")
+    if action is not None and action not in {"post_add", "post_remove", "post_clear"}:
+        # m2m_changed fires pre_* and post_* actions; only post_* describe the
+        # relation state the filename should follow.
+        return
+
     if isinstance(instance, CustomFieldInstance):
         if not _filename_template_uses_custom_fields(instance.document):
             return
         instance = instance.document
 
-    def validate_move(instance, old_path: Path, new_path: Path, root: Path) -> None:
-        if not new_path.is_relative_to(root):
-            msg = (
-                f"Document {instance!s}: Refusing to move file outside root {root}: "
-                f"{new_path}."
-            )
-            logger.warning(msg)
-            raise CannotMoveFilesException(msg)
-
-        if not old_path.is_file():
-            # Can't do anything if the old file does not exist anymore.
-            msg = f"Document {instance!s}: File {old_path} doesn't exist."
-            logger.fatal(msg)
-            raise CannotMoveFilesException(msg)
-
-        if new_path.is_file():
-            # Can't do anything if the new file already exists. Skip updating file.
-            msg = f"Document {instance!s}: Cannot rename file since target path {new_path} already exists."
-            logger.warning(msg)
-            raise CannotMoveFilesException(msg)
-
-    if not instance.filename:
-        # Can't update the filename if there is no filename to begin with
-        # This happens when the consumer creates a new document.
-        # The document is modified and saved multiple times, and only after
-        # everything is done (i.e., the generated filename is final),
-        # filename will be set to the location where the consumer has put
-        # the file.
-        #
-        # This will in turn cause this logic to move the file where it belongs.
-        return
-
-    with media_lock():
-        # If this was waiting for the lock, the filename or archive_filename
-        # of this document may have been updated.  This happens if multiple updates
-        # get queued from the UI for the same document
-        # So freshen up the data before doing anything
-        instance.refresh_from_db()
-
-        try:
-            old_filename = instance.filename
-            old_source_path = instance.source_path
-            move_original = False
-            original_already_moved = False
-
-            old_archive_filename = instance.archive_filename
-            old_archive_path = instance.archive_path
-            move_archive = False
-            archive_already_moved = False
-
-            candidate_filename = generate_filename(instance)
-            if len(str(candidate_filename)) > Document.MAX_STORED_FILENAME_LENGTH:
-                msg = (
-                    f"Document {instance!s}: Generated filename exceeds db path "
-                    f"limit ({len(str(candidate_filename))} > "
-                    f"{Document.MAX_STORED_FILENAME_LENGTH}): {candidate_filename!s}"
-                )
-                logger.warning(msg)
-                raise CannotMoveFilesException(msg)
-
-            candidate_source_path = (
-                settings.ORIGINALS_DIR / candidate_filename
-            ).resolve()
-            if candidate_filename == Path(old_filename):
-                new_filename = Path(old_filename)
-            elif (
-                candidate_source_path.exists()
-                and candidate_source_path != old_source_path
-            ):
-                if not old_source_path.is_file() and _path_matches_checksum(
-                    candidate_source_path,
-                    instance.checksum,
-                ):
-                    new_filename = candidate_filename
-                    original_already_moved = True
-                else:
-                    # Only fall back to unique search when there is an actual conflict
-                    new_filename = generate_unique_filename(instance)
-            else:
-                new_filename = candidate_filename
-
-            # Need to convert to string to be able to save it to the db
-            instance.filename = str(new_filename)
-            move_original = (
-                old_filename != instance.filename and not original_already_moved
-            )
-
-            if instance.has_archive_version:
-                archive_candidate = generate_filename(instance, archive_filename=True)
-                if len(str(archive_candidate)) > Document.MAX_STORED_FILENAME_LENGTH:
-                    msg = (
-                        f"Document {instance!s}: Generated archive filename exceeds "
-                        f"db path limit ({len(str(archive_candidate))} > "
-                        f"{Document.MAX_STORED_FILENAME_LENGTH}): {archive_candidate!s}"
-                    )
-                    logger.warning(msg)
-                    raise CannotMoveFilesException(msg)
-                archive_candidate_path = (
-                    settings.ARCHIVE_DIR / archive_candidate
-                ).resolve()
-                if archive_candidate == Path(old_archive_filename):
-                    new_archive_filename = Path(old_archive_filename)
-                elif (
-                    archive_candidate_path.exists()
-                    and archive_candidate_path != old_archive_path
-                ):
-                    if not old_archive_path.is_file() and _path_matches_checksum(
-                        archive_candidate_path,
-                        instance.archive_checksum,
-                    ):
-                        new_archive_filename = archive_candidate
-                        archive_already_moved = True
-                    else:
-                        new_archive_filename = generate_unique_filename(
-                            instance,
-                            archive_filename=True,
-                        )
-                else:
-                    new_archive_filename = archive_candidate
-
-                instance.archive_filename = str(new_archive_filename)
-
-                move_archive = (
-                    old_archive_filename != instance.archive_filename
-                    and not archive_already_moved
-                )
-            else:
-                move_archive = False
-
-            if not move_original and not move_archive:
-                updates = {"modified": timezone.now()}
-                if old_filename != instance.filename:
-                    updates["filename"] = instance.filename
-                if old_archive_filename != instance.archive_filename:
-                    updates["archive_filename"] = instance.archive_filename
-
-                # Don't save() here to prevent infinite recursion.
-                Document.objects.filter(pk=instance.pk).update(**updates)
-                return
-
-            if move_original:
-                validate_move(
-                    instance,
-                    old_source_path,
-                    instance.source_path,
-                    settings.ORIGINALS_DIR,
-                )
-                create_source_path_directory(instance.source_path)
-                shutil.move(old_source_path, instance.source_path)
-
-            if move_archive:
-                validate_move(
-                    instance,
-                    old_archive_path,
-                    instance.archive_path,
-                    settings.ARCHIVE_DIR,
-                )
-                create_source_path_directory(instance.archive_path)
-                shutil.move(old_archive_path, instance.archive_path)
-
-            # Don't save() here to prevent infinite recursion.
-            Document.global_objects.filter(pk=instance.pk).update(
-                filename=instance.filename,
-                archive_filename=instance.archive_filename,
-                modified=timezone.now(),
-            )
-            # Clear any caching for this document.  Slightly overkill, but not terrible
-            clear_document_caches(instance.pk)
-
-        except (OSError, DatabaseError, CannotMoveFilesException) as e:
-            logger.warning(f"Exception during file handling: {e}")
-            # This happens when either:
-            #  - moving the files failed due to file system errors
-            #  - saving to the database failed due to database errors
-            # In both cases, we need to revert to the original state.
-
-            # Try to move files to their original location.
-            try:
-                if move_original and instance.source_path.is_file():
-                    logger.info("Restoring previous original path")
-                    shutil.move(instance.source_path, old_source_path)
-
-                if move_archive and instance.archive_path.is_file():
-                    logger.info("Restoring previous archive path")
-                    shutil.move(instance.archive_path, old_archive_path)
-
-            except Exception:
-                # This is fine, since:
-                # A: if we managed to move source from A to B, we will also
-                #  manage to move it from B to A. If not, we have a serious
-                #  issue that's going to get caught by the santiy checker.
-                #  All files remain in place and will never be overwritten,
-                #  so this is not the end of the world.
-                # B: if moving the original file failed, nothing has changed
-                #  anyway.
-                pass
-
-            # restore old values on the instance
-            instance.filename = old_filename
-            instance.archive_filename = old_archive_filename
-
-        # finally, remove any empty sub folders. This will do nothing if
-        # something has failed above.
-        if not old_source_path.is_file():
-            delete_empty_directories(
-                Path(old_source_path).parent,
-                root=settings.ORIGINALS_DIR,
-            )
-
-        if instance.has_archive_version and not old_archive_path.is_file():
-            delete_empty_directories(
-                Path(old_archive_path).parent,
-                root=settings.ARCHIVE_DIR,
-            )
-
-    # Keep version files in sync with root
-    if instance.root_document_id is None:
-        for version_doc in Document.objects.filter(root_document_id=instance.pk).only(
-            "pk",
-        ):
-            update_filename_and_move_files(
-                Document,
-                version_doc,
-            )
+    request_file_move(instance)
 
 
 @shared_task
@@ -691,7 +462,8 @@ def process_cf_select_update(custom_field: CustomField) -> None:
 
     for cf_instance in custom_field.fields.select_related("document").iterator():
         # Update the filename and move files if necessary
-        update_filename_and_move_files(CustomFieldInstance, cf_instance)
+        if _filename_template_uses_custom_fields(cf_instance.document):
+            move_files_for_document(cf_instance.document_id)
 
 
 # should be disabled in /src/documents/management/commands/document_importer.py handle

src/documents/tests/test_consumer.py

@@ -750,7 +750,7 @@ class TestConsumer(
         FILENAME_FORMAT="{correspondent}/{title}",
         ARCHIVE_FILE_GENERATION="always",
     )
-    @mock.patch("documents.signals.handlers.generate_unique_filename")
+    @mock.patch("documents.file_moves.generate_unique_filename")
     def testFilenameHandlingUnstableFormat(self, m) -> None:
         filenames = ["this", "that", "now this", "i cannot decide"]
 

src/documents/tests/test_file_handling.py

@@ -656,19 +656,17 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
         self.assertEqual(document2.filename, "qwe.pdf")
 
     @override_settings(FILENAME_FORMAT="{title}")
-    @mock.patch("documents.signals.handlers.Document.objects.filter")
-    @mock.patch("documents.signals.handlers.shutil.move")
+    @mock.patch("documents.file_moves.Document.global_objects.filter")
+    @mock.patch("documents.file_moves.shutil.move")
     def test_no_move_only_save(self, mock_move, mock_filter) -> None:
         """
         GIVEN:
             - A document with a filename
-            - The document is saved
-            - The filename is not changed
         WHEN:
-            - The document is saved
+            - The document is saved without a change that affects its path
         THEN:
-            - The document modified date is updated
-            - The document is not moved
+            - The document modified date is updated by the save itself
+            - The file is not moved and the mover writes nothing
         """
         with disable_auditlog():
             doc = Document.objects.create(
@@ -686,7 +684,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
             doc.save()
             doc.refresh_from_db()
 
-            mock_filter.assert_called()
+            mock_filter.assert_not_called()
             self.assertNotEqual(original_modified, doc.modified)
             mock_move.assert_not_called()
 
@@ -694,7 +692,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
         FILENAME_FORMAT="{{title}}_{{custom_fields|get_cf_value('test')}}",
         CELERY_TASK_ALWAYS_EAGER=True,
     )
-    @mock.patch("documents.signals.handlers.update_filename_and_move_files")
+    @mock.patch("documents.signals.handlers.move_files_for_document")
     def test_select_cf_updated(self, m) -> None:
         """
         GIVEN:
@@ -2045,7 +2043,7 @@ class TestCustomFieldFilenameUpdates(
         Path(self.doc.source_path).parent.mkdir(parents=True, exist_ok=True)
         Path(self.doc.source_path).touch()
 
-        with mock.patch("documents.signals.handlers.generate_unique_filename") as m:
+        with mock.patch("documents.file_moves.generate_unique_filename") as m:
             m.side_effect = generate_unique_filename
             self.cfi.value_text = "updated"
             self.cfi.save()
@@ -2067,7 +2065,7 @@ class TestCustomFieldFilenameUpdates(
         Path(self.doc.source_path).parent.mkdir(parents=True, exist_ok=True)
         Path(self.doc.source_path).touch()
 
-        with mock.patch("documents.signals.handlers.generate_unique_filename") as m:
+        with mock.patch("documents.file_moves.generate_unique_filename") as m:
             m.side_effect = generate_unique_filename
             self.cfi.value_text = "updated"
             self.cfi.save()
@@ -2145,7 +2143,7 @@ def test_refresh_failure_surfaces_database_error(
     GIVEN:
         - A document with a stored filename
     WHEN:
-        - Saving it triggers a filename update and refreshing it from the database fails
+        - Saving it triggers a filename update and loading it from the database fails
     THEN:
         - The database error is raised, not an unbound variable error from the rollback
     """
@@ -2153,8 +2151,8 @@ def test_refresh_failure_surfaces_database_error(
 
     with (
         mock.patch.object(
-            Document,
-            "refresh_from_db",
+            Document.global_objects,
+            "get",
             side_effect=DatabaseError("database is locked"),
         ),
         pytest.raises(DatabaseError, match="database is locked"),

src/documents/tests/test_file_moves.py

@@ -0,0 +1,237 @@
+from __future__ import annotations
+
+import datetime
+import hashlib
+import logging
+from typing import TYPE_CHECKING
+
+import pytest
+from django.utils import timezone
+from filelock import Timeout
+
+from documents.file_moves import move_files_for_document
+from documents.file_moves import request_file_move
+from documents.models import Document
+from paperless_testing.factories import CorrespondentFactory
+from paperless_testing.factories import DocumentFactory
+from paperless_testing.factories import TagFactory
+from paperless_testing.locks import record_media_lock_acquisitions
+
+if TYPE_CHECKING:
+    from pytest_django.fixtures import Settings
+    from pytest_mock import MockerFixture
+
+    from paperless_testing.dirs import PaperlessDirs
+
+CONTENT = b"original content"
+
+
+@pytest.fixture
+def title_format(settings: Settings) -> None:
+    settings.FILENAME_FORMAT = "{title}"
+
+
+@pytest.fixture
+def document(paperless_dirs: PaperlessDirs, title_format: None) -> Document:
+    """A document named after its title, with its original on disk."""
+    (paperless_dirs.originals_dir / "original.pdf").write_bytes(CONTENT)
+    return DocumentFactory(
+        title="original",
+        filename="original.pdf",
+        mime_type="application/pdf",
+        checksum=hashlib.sha256(CONTENT).hexdigest(),
+    )
+
+
+@pytest.mark.django_db
+class TestMoveFilesForDocument:
+    def test_moves_using_fresh_db_state(
+        self,
+        document: Document,
+        paperless_dirs: PaperlessDirs,
+    ) -> None:
+        """
+        GIVEN:
+            - A document whose title changed in the DB without any signal
+        WHEN:
+            - move_files_for_document is called with its pk
+        THEN:
+            - The file and the DB filename follow the DB title
+            - The returned instance carries the final filename
+        """
+        Document.objects.filter(pk=document.pk).update(title="renamed")
+
+        moved = move_files_for_document(document.pk)
+
+        assert moved is not None
+        assert moved.filename == "renamed.pdf"
+        assert (paperless_dirs.originals_dir / "renamed.pdf").is_file()
+        assert not (paperless_dirs.originals_dir / "original.pdf").exists()
+        document.refresh_from_db()
+        assert document.filename == "renamed.pdf"
+
+    def test_missing_document_returns_none(self, paperless_dirs: PaperlessDirs) -> None:
+        """
+        GIVEN:
+            - A pk with no document row
+        WHEN:
+            - move_files_for_document is called
+        THEN:
+            - It returns None without raising
+        """
+        assert move_files_for_document(999_999) is None
+
+    def test_document_without_filename_skips_lock(
+        self,
+        paperless_dirs: PaperlessDirs,
+        title_format: None,
+    ) -> None:
+        """
+        GIVEN:
+            - A document still being consumed (no filename yet)
+        WHEN:
+            - move_files_for_document is called
+        THEN:
+            - It returns None, writes nothing, and never takes media.lock
+        """
+        doc = DocumentFactory(filename=None, mime_type="application/pdf")
+
+        with record_media_lock_acquisitions() as acquisitions:
+            assert move_files_for_document(doc.pk) is None
+
+        assert acquisitions == []
+        doc.refresh_from_db()
+        assert doc.filename is None
+
+    def test_no_change_does_not_write_modified(
+        self,
+        document: Document,
+    ) -> None:
+        """
+        GIVEN:
+            - A document already at its generated path, modified in the past
+        WHEN:
+            - move_files_for_document is called
+        THEN:
+            - modified is not written
+        """
+        past = timezone.now() - datetime.timedelta(days=1)
+        Document.objects.filter(pk=document.pk).update(modified=past)
+
+        move_files_for_document(document.pk)
+
+        document.refresh_from_db()
+        assert document.modified == past
+
+    def test_trashed_already_moved_document_gets_db_path_fixed(
+        self,
+        paperless_dirs: PaperlessDirs,
+        title_format: None,
+    ) -> None:
+        """
+        GIVEN:
+            - A trashed document whose DB filename is stale
+            - Its file sits at the generated path (checksum matches), placed
+              after the creation-time move already failed
+        WHEN:
+            - move_files_for_document is called
+        THEN:
+            - The DB filename is updated to where the file is
+        """
+        doc = DocumentFactory(
+            title="current",
+            filename="stale.pdf",
+            mime_type="application/pdf",
+            checksum=hashlib.sha256(CONTENT).hexdigest(),
+        )
+        assert Document.objects.get(pk=doc.pk).filename == "stale.pdf"
+        doc.delete()
+        (paperless_dirs.originals_dir / "current.pdf").write_bytes(CONTENT)
+
+        move_files_for_document(doc.pk)
+
+        assert Document.global_objects.get(pk=doc.pk).filename == "current.pdf"
+
+    def test_lock_timeout_leaves_consistent_state(
+        self,
+        document: Document,
+        paperless_dirs: PaperlessDirs,
+        mocker: MockerFixture,
+        caplog: pytest.LogCaptureFixture,
+    ) -> None:
+        """
+        GIVEN:
+            - A document whose title changed in the DB
+            - media.lock cannot be acquired
+        WHEN:
+            - move_files_for_document is called
+        THEN:
+            - An error is logged and None is returned
+            - The lock was requested with the 300 second timeout
+            - The DB filename and the file on disk are unchanged
+        """
+        Document.objects.filter(pk=document.pk).update(title="renamed")
+        lock = mocker.patch("documents.file_moves.media_lock")
+        lock.return_value.__enter__.side_effect = Timeout("media.lock")
+
+        with caplog.at_level(logging.ERROR, logger="paperless.handlers"):
+            assert move_files_for_document(document.pk) is None
+
+        assert "Timed out" in caplog.text
+        lock.assert_called_once_with(timeout=300)
+        document.refresh_from_db()
+        assert document.filename == "original.pdf"
+        assert (paperless_dirs.originals_dir / "original.pdf").is_file()
+
+
+@pytest.mark.django_db
+class TestRequestFileMoveWithoutScope:
+    def test_moves_immediately_and_syncs_only_file_columns(
+        self,
+        document: Document,
+        paperless_dirs: PaperlessDirs,
+    ) -> None:
+        """
+        GIVEN:
+            - No file move scope is open
+            - A caller instance with an unsaved in-memory change
+        WHEN:
+            - request_file_move is called after the title changed in the DB
+        THEN:
+            - The file moves immediately
+            - The caller's filename is updated
+            - The caller's unsaved change is kept
+        """
+        Document.objects.filter(pk=document.pk).update(title="renamed")
+        unsaved = CorrespondentFactory()
+        document.correspondent = unsaved
+
+        request_file_move(document)
+
+        assert (paperless_dirs.originals_dir / "renamed.pdf").is_file()
+        assert document.filename == "renamed.pdf"
+        assert document.correspondent == unsaved
+        assert document.title == "original"
+
+
+@pytest.mark.django_db
+class TestReceiver:
+    def test_reverse_m2m_does_not_request_a_move(
+        self,
+        document: Document,
+        mocker: MockerFixture,
+    ) -> None:
+        """
+        GIVEN:
+            - A tag
+        WHEN:
+            - A document is added through the tag's reverse relation
+        THEN:
+            - No move is requested with the tag as the instance
+        """
+        tag = TagFactory()
+        request = mocker.patch("documents.signals.handlers.request_file_move")
+
+        tag.documents.add(document)
+
+        request.assert_not_called()