mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-11-04 22:09:24 +00:00
Fix: rename and retag documents one file move at a time
document_renamer sent a fake post_save per document to trigger the move, and document_retagger moved a document once for each field it assigned. The renamer calls move_files_for_document directly for each document, so progress reflects real work. The retagger opens a file move scope per document, so each document is moved once after all of its assignments.
This commit is contained in:
1 parent
7d5e7b1625
commit
0770b4fcc2
4 files changed
+172
-54
No files matched your search
@@ -1,5 +1,4 @@
|
||||
from django.db.models.signals import post_save
|
||||
|
||||
from documents.file_moves import move_files_for_document
|
||||
from documents.management.commands.base import PaperlessCommand
|
||||
from documents.models import Document
|
||||
|
||||
@@ -11,5 +10,8 @@ class Command(PaperlessCommand):
|
||||
supports_multiprocessing = False
|
||||
|
||||
def handle(self, *args, **options):
|
||||
for document in self.track(Document.objects.all(), description="Renaming..."):
|
||||
post_save.send(Document, instance=document, created=False)
|
||||
for document in self.track(
|
||||
Document.objects.only("pk"),
|
||||
description="Renaming...",
|
||||
):
|
||||
move_files_for_document(document.pk)
|
||||
@@ -9,6 +9,7 @@ from rich.table import Table
|
||||
from rich.text import Text
|
||||
|
||||
from documents.classifier import load_classifier
|
||||
from documents.file_moves import defer_file_moves
|
||||
from documents.management.commands.base import PaperlessCommand
|
||||
from documents.models import Document
|
||||
from documents.signals.handlers import set_correspondent
|
||||
@@ -273,61 +274,63 @@ class Command(PaperlessCommand):
|
||||
description="Retagging...",
|
||||
stats_renderer=render_stats,
|
||||
):
|
||||
suggestion = DocumentSuggestion(document=document)
|
||||
# One file move per document, after all of its assignments.
|
||||
with defer_file_moves():
|
||||
suggestion = DocumentSuggestion(document=document)
|
||||
|
||||
if do_correspondent:
|
||||
correspondent = set_correspondent(
|
||||
None,
|
||||
document,
|
||||
classifier=classifier,
|
||||
replace=overwrite,
|
||||
use_first=use_first,
|
||||
dry_run=suggest,
|
||||
)
|
||||
if correspondent is not None:
|
||||
stats.correspondents += 1
|
||||
suggestion.correspondent = correspondent
|
||||
if do_correspondent:
|
||||
correspondent = set_correspondent(
|
||||
None,
|
||||
document,
|
||||
classifier=classifier,
|
||||
replace=overwrite,
|
||||
use_first=use_first,
|
||||
dry_run=suggest,
|
||||
)
|
||||
if correspondent is not None:
|
||||
stats.correspondents += 1
|
||||
suggestion.correspondent = correspondent
|
||||
|
||||
if do_document_type:
|
||||
document_type = set_document_type(
|
||||
None,
|
||||
document,
|
||||
classifier=classifier,
|
||||
replace=overwrite,
|
||||
use_first=use_first,
|
||||
dry_run=suggest,
|
||||
)
|
||||
if document_type is not None:
|
||||
stats.document_types += 1
|
||||
suggestion.document_type = document_type
|
||||
if do_document_type:
|
||||
document_type = set_document_type(
|
||||
None,
|
||||
document,
|
||||
classifier=classifier,
|
||||
replace=overwrite,
|
||||
use_first=use_first,
|
||||
dry_run=suggest,
|
||||
)
|
||||
if document_type is not None:
|
||||
stats.document_types += 1
|
||||
suggestion.document_type = document_type
|
||||
|
||||
if do_tags:
|
||||
tags_to_add, tags_to_remove = set_tags(
|
||||
None,
|
||||
document,
|
||||
classifier=classifier,
|
||||
replace=overwrite,
|
||||
dry_run=suggest,
|
||||
)
|
||||
stats.tags_added += len(tags_to_add)
|
||||
stats.tags_removed += len(tags_to_remove)
|
||||
suggestion.tags_to_add = frozenset(tags_to_add)
|
||||
suggestion.tags_to_remove = frozenset(tags_to_remove)
|
||||
if do_tags:
|
||||
tags_to_add, tags_to_remove = set_tags(
|
||||
None,
|
||||
document,
|
||||
classifier=classifier,
|
||||
replace=overwrite,
|
||||
dry_run=suggest,
|
||||
)
|
||||
stats.tags_added += len(tags_to_add)
|
||||
stats.tags_removed += len(tags_to_remove)
|
||||
suggestion.tags_to_add = frozenset(tags_to_add)
|
||||
suggestion.tags_to_remove = frozenset(tags_to_remove)
|
||||
|
||||
if do_storage_path:
|
||||
storage_path = set_storage_path(
|
||||
None,
|
||||
document,
|
||||
classifier=classifier,
|
||||
replace=overwrite,
|
||||
use_first=use_first,
|
||||
dry_run=suggest,
|
||||
)
|
||||
if storage_path is not None:
|
||||
stats.storage_paths += 1
|
||||
suggestion.storage_path = storage_path
|
||||
if do_storage_path:
|
||||
storage_path = set_storage_path(
|
||||
None,
|
||||
document,
|
||||
classifier=classifier,
|
||||
replace=overwrite,
|
||||
use_first=use_first,
|
||||
dry_run=suggest,
|
||||
)
|
||||
if storage_path is not None:
|
||||
stats.storage_paths += 1
|
||||
suggestion.storage_path = storage_path
|
||||
|
||||
stats.documents_processed += 1
|
||||
stats.documents_processed += 1
|
||||
|
||||
if suggest:
|
||||
suggestions.append(suggestion)
|
||||
|
||||
@@ -18,6 +18,7 @@ if TYPE_CHECKING:
|
||||
from pytest_mock import MockerFixture
|
||||
|
||||
from documents.file_handling import generate_filename
|
||||
from documents.file_moves import move_files_for_document
|
||||
from documents.models import Document
|
||||
from documents.tasks import update_document_content_maybe_archive_file
|
||||
from paperless_testing.assertions import FileSystemAssertsMixin
|
||||
@@ -276,6 +277,43 @@ class TestRenamer(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
|
||||
self.assertIsFile(doc2.source_path)
|
||||
self.assertIsFile(doc2.archive_path)
|
||||
|
||||
@override_settings(FILENAME_FORMAT="")
|
||||
def test_rename_moves_each_document_by_direct_call(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- Two documents whose filename format changed
|
||||
WHEN:
|
||||
- document_renamer runs
|
||||
THEN:
|
||||
- Each document is moved by a direct call, one per document,
|
||||
not by a fake post_save signal
|
||||
- Both files are at their new paths
|
||||
"""
|
||||
docs = []
|
||||
for title in ("first", "second"):
|
||||
doc = Document.objects.create(title=title, mime_type="image/jpeg")
|
||||
doc.filename = generate_filename(doc)
|
||||
doc.save(write_file_paths=True)
|
||||
Path(doc.source_path).touch()
|
||||
docs.append(doc)
|
||||
|
||||
with (
|
||||
override_settings(FILENAME_FORMAT="{title}"),
|
||||
mock.patch(
|
||||
"documents.management.commands.document_renamer.move_files_for_document",
|
||||
wraps=move_files_for_document,
|
||||
) as mover,
|
||||
):
|
||||
call_command("document_renamer", skip_checks=True)
|
||||
|
||||
self.assertCountEqual(
|
||||
[c.args[0] for c in mover.call_args_list],
|
||||
[d.pk for d in docs],
|
||||
)
|
||||
for doc in docs:
|
||||
doc.refresh_from_db()
|
||||
self.assertIsFile(doc.source_path)
|
||||
|
||||
|
||||
@pytest.mark.management
|
||||
class TestCreateClassifier:
|
||||
|
||||
@@ -4,10 +4,13 @@ Tests for the document_retagger management command.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import pytest
|
||||
from django.core.management import call_command
|
||||
from django.core.management.base import CommandError
|
||||
|
||||
from documents.file_moves import _active_collector
|
||||
from documents.models import Correspondent
|
||||
from documents.models import Document
|
||||
from documents.models import DocumentType
|
||||
@@ -21,6 +24,12 @@ from paperless_testing.factories import DocumentTypeFactory
|
||||
from paperless_testing.factories import StoragePathFactory
|
||||
from paperless_testing.factories import TagFactory
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from pytest_django.fixtures import Settings
|
||||
from pytest_mock import MockerFixture
|
||||
|
||||
from paperless_testing.dirs import PaperlessDirs
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Module-level type aliases
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -484,3 +493,69 @@ class TestRetaggerEdgeCases(DirectoriesMixin):
|
||||
|
||||
assert d_first.tags.count() == 0
|
||||
assert d_unrelated.tags.count() == 2
|
||||
|
||||
|
||||
@pytest.mark.management
|
||||
@pytest.mark.django_db
|
||||
class TestRetaggerFileMoves:
|
||||
def test_each_document_is_processed_in_its_own_scope(
|
||||
self,
|
||||
paperless_dirs: PaperlessDirs,
|
||||
settings: Settings,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- Two documents and a filename format using the correspondent
|
||||
- A classifier suggestion assigning a correspondent to each
|
||||
WHEN:
|
||||
- document_retagger runs with --correspondent
|
||||
THEN:
|
||||
- Each document is processed inside an open file move scope
|
||||
- When the second document is processed, the first one's file has
|
||||
already moved
|
||||
- Both files end at their final paths
|
||||
"""
|
||||
settings.FILENAME_FORMAT = "{correspondent}/{title}"
|
||||
(paperless_dirs.originals_dir / "none").mkdir(exist_ok=True)
|
||||
for title in ("first", "second"):
|
||||
(paperless_dirs.originals_dir / "none" / f"{title}.pdf").write_bytes(b"x")
|
||||
DocumentFactory(
|
||||
title=title,
|
||||
filename=f"none/{title}.pdf",
|
||||
mime_type="application/pdf",
|
||||
)
|
||||
correspondent = CorrespondentFactory(name="acme")
|
||||
processed: list[Document] = []
|
||||
scope_open: list[bool] = []
|
||||
seen_first_moved: list[bool] = []
|
||||
|
||||
def fake_set_correspondent(sender, document, **kwargs):
|
||||
scope_open.append(_active_collector.get() is not None)
|
||||
# Order-agnostic: on the second call, check the first one's file.
|
||||
if processed:
|
||||
first = processed[0]
|
||||
seen_first_moved.append(
|
||||
(
|
||||
paperless_dirs.originals_dir / "acme" / f"{first.title}.pdf"
|
||||
).is_file(),
|
||||
)
|
||||
processed.append(document)
|
||||
document.correspondent = correspondent
|
||||
document.save(update_fields=("correspondent",))
|
||||
return correspondent
|
||||
|
||||
mocker.patch(
|
||||
"documents.management.commands.document_retagger.load_classifier",
|
||||
)
|
||||
mocker.patch(
|
||||
"documents.management.commands.document_retagger.set_correspondent",
|
||||
side_effect=fake_set_correspondent,
|
||||
)
|
||||
|
||||
call_command("document_retagger", "--correspondent", skip_checks=True)
|
||||
|
||||
assert scope_open == [True, True]
|
||||
assert seen_first_moved == [True]
|
||||
for title in ("first", "second"):
|
||||
assert (paperless_dirs.originals_dir / "acme" / f"{title}.pdf").is_file()
|
||||
Reference in new issue
Block a user