From 0f7c02de5e8391d49e08031749ae95785956b670 Mon Sep 17 00:00:00 2001 From: Trenton H <797416+stumpylog@users.noreply.github.com> Date: Wed, 18 Mar 2026 13:31:09 -0700 Subject: [PATCH] Fix: test: add regression test for workflow save clobbering filename (#12390) Add test_workflow_document_updated_does_not_overwrite_filename to verify that run_workflows (DOCUMENT_UPDATED path) does not revert a DB filename that was updated by a concurrent bulk_update_documents task's update_filename_and_move_files call. The test replicates the race window by: - Updating the DB filename directly (simulating BUD-1 completing) - Mocking refresh_from_db so the stale in-memory filename persists - Asserting the DB filename is not clobbered after run_workflows Relates to: https://github.com/paperless-ngx/paperless-ngx/issues/12386 Co-authored-by: Claude Sonnet 4.6 --- src/documents/signals/handlers.py | 23 +++++++++-- src/documents/tests/test_workflows.py | 58 +++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 160e6021bf..f8d670e307 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -867,10 +867,25 @@ def run_workflows( if not use_overrides: # limit title to 128 characters document.title = document.title[:128] - # Make sure the filename and archive filename are accurate - document.refresh_from_db(fields=["filename", "archive_filename"]) - # save first before setting tags - document.save() + # Save only the fields that workflow actions can set directly. + # Deliberately excludes filename and archive_filename — those are + # managed exclusively by update_filename_and_move_files via the + # post_save signal. Writing stale in-memory values here would revert + # a concurrent update_filename_and_move_files DB write, leaving the + # DB pointing at the old path while the file is already at the new + # one (see: https://github.com/paperless-ngx/paperless-ngx/issues/12386). + # modified has auto_now=True but is not auto-added when update_fields + # is specified, so it must be listed explicitly. + document.save( + update_fields=[ + "title", + "correspondent", + "document_type", + "storage_path", + "owner", + "modified", + ], + ) document.tags.set(doc_tag_ids) WorkflowRun.objects.create( diff --git a/src/documents/tests/test_workflows.py b/src/documents/tests/test_workflows.py index ba1e72e1f1..9245336982 100644 --- a/src/documents/tests/test_workflows.py +++ b/src/documents/tests/test_workflows.py @@ -956,6 +956,64 @@ class TestWorkflows( self.assertEqual(Path(doc.filename), expected_filename) self.assertTrue(doc.source_path.is_file()) + def test_workflow_document_updated_does_not_overwrite_filename(self) -> None: + """ + GIVEN: + - A document whose filename has been updated in the DB by a concurrent + bulk_update_documents task (simulating update_filename_and_move_files + completing and writing the new filename to the DB) + - A stale in-memory document instance still holding the old filename + - An active DOCUMENT_UPDATED workflow + WHEN: + - run_workflows is called with the stale in-memory instance + (as would happen in the second concurrent bulk_update_documents task) + THEN: + - The DB filename is NOT overwritten with the stale in-memory value + (regression test for GH #12386 — the race window between + refresh_from_db and document.save in run_workflows) + """ + trigger = WorkflowTrigger.objects.create( + type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, + ) + action = WorkflowAction.objects.create( + type=WorkflowAction.WorkflowActionType.ASSIGNMENT, + assign_title="Updated by workflow", + ) + workflow = Workflow.objects.create(name="Race condition test workflow", order=0) + workflow.triggers.add(trigger) + workflow.actions.add(action) + workflow.save() + + doc = Document.objects.create( + title="race condition test", + mime_type="application/pdf", + checksum="racecondition123", + original_filename="old.pdf", + filename="old/path/old.pdf", + ) + + # Simulate BUD-1 completing update_filename_and_move_files: + # the DB now holds the new filename while BUD-2's in-memory instance is stale. + new_filename = "new/path/new.pdf" + Document.global_objects.filter(pk=doc.pk).update(filename=new_filename) + + # The stale instance still has filename="old/path/old.pdf" in memory. + # Mock refresh_from_db so the stale value persists through run_workflows, + # replicating the race window between refresh and save. + # Mock update_filename_and_move_files to prevent file-not-found errors + # since we are only testing DB state here. + with ( + mock.patch( + "documents.signals.handlers.update_filename_and_move_files", + ), + mock.patch.object(Document, "refresh_from_db"), + ): + run_workflows(WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, doc) + + # The DB filename must not have been reverted to the stale old value. + doc.refresh_from_db() + self.assertEqual(doc.filename, new_filename) + def test_document_added_workflow(self): trigger = WorkflowTrigger.objects.create( type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_ADDED,