diff --git a/src/documents/file_moves.py b/src/documents/file_moves.py index 7ca349174f..a3f3cdb037 100644 --- a/src/documents/file_moves.py +++ b/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 diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 76dc65f9cd..ac90093f2e 100644 --- a/src/documents/signals/handlers.py +++ b/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 diff --git a/src/documents/tests/test_consumer.py b/src/documents/tests/test_consumer.py index 7c607e1e3f..d623070515 100644 --- a/src/documents/tests/test_consumer.py +++ b/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"] diff --git a/src/documents/tests/test_file_handling.py b/src/documents/tests/test_file_handling.py index 3746c3cdbb..ef130ece11 100644 --- a/src/documents/tests/test_file_handling.py +++ b/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"), diff --git a/src/documents/tests/test_file_moves.py b/src/documents/tests/test_file_moves.py new file mode 100644 index 0000000000..f198356704 --- /dev/null +++ b/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()