Fix: Defer password removal workflow action until the file is in place (#12814)

diff --git a/src/documents/tests/test_workflows.py b/src/documents/tests/test_workflows.py
index 2e31608bb733a85302ec29ee9228d888e3e8d058..1bfe294d00bffcfd782eb193d1a552c9b9870950 100644 (file)
--- a/src/documents/tests/test_workflows.py
+++ b/src/documents/tests/test_workflows.py
@@ -4301,10 +4301,11 @@ class TestWorkflows(
             title="Protected",
         )
 
-        document_consumption_finished.send(
-            sender=self.__class__,
-            document=doc,
-        )
+        with self.captureOnCommitCallbacks(execute=True):
+            document_consumption_finished.send(
+                sender=self.__class__,
+                document=doc,
+            )
 
         assert mock_remove_password.call_count == 2
         mock_remove_password.assert_has_calls(
@@ -4325,12 +4326,64 @@ class TestWorkflows(
         )
 
         # ensure handler disconnected after first run
-        document_consumption_finished.send(
-            sender=self.__class__,
-            document=doc,
-        )
+        with self.captureOnCommitCallbacks(execute=True):
+            document_consumption_finished.send(
+                sender=self.__class__,
+                document=doc,
+            )
         assert mock_remove_password.call_count == 2
 
+    @mock.patch("documents.bulk_edit.remove_password")
+    def test_password_removal_deferred_until_transaction_commit(
+        self,
+        mock_remove_password,
+    ) -> None:
+        """
+        GIVEN:
+            - Workflow password removal action triggered during consumption
+            - document_consumption_finished signal fires inside a transaction
+        WHEN:
+            - Signal fires but transaction has not yet committed
+        THEN:
+            - Password removal is not attempted until the transaction commits
+        """
+        from django.db import transaction
+
+        action = WorkflowAction.objects.create(
+            type=WorkflowAction.WorkflowActionType.PASSWORD_REMOVAL,
+            passwords=["secret"],
+        )
+
+        temp_dir = Path(tempfile.mkdtemp())
+        original_file = temp_dir / "file.pdf"
+        original_file.write_bytes(b"pdf content")
+        consumable = ConsumableDocument(
+            source=DocumentSource.ApiUpload,
+            original_file=original_file,
+        )
+
+        execute_password_removal_action(action, consumable, logging_group=None)
+
+        doc = Document.objects.create(
+            checksum="pw-checksum-on-commit",
+            title="Protected",
+        )
+
+        with self.captureOnCommitCallbacks(execute=True):
+            with transaction.atomic():
+                document_consumption_finished.send(
+                    sender=self.__class__,
+                    document=doc,
+                )
+                mock_remove_password.assert_not_called()
+
+        mock_remove_password.assert_called_once_with(
+            [doc.id],
+            password="secret",
+            update_document=True,
+            user=doc.owner,
+        )
+
     def test_workflow_trash_action_soft_delete(self) -> None:
         """
         GIVEN:

diff --git a/src/documents/workflows/actions.py b/src/documents/workflows/actions.py
index b6adc35b711edc3d815cb383d8779c6c4d0a25e3..948642dcb1dfef986e1f22e300df8655b582174a 100644 (file)
--- a/src/documents/workflows/actions.py
+++ b/src/documents/workflows/actions.py
@@ -4,6 +4,7 @@ from pathlib import Path
 
 from django.conf import settings
 from django.contrib.auth.models import User
+from django.db import transaction
 from django.utils import timezone
 
 from documents.data_models import ConsumableDocument
@@ -295,13 +296,15 @@ def execute_password_removal_action(
         # hook the consumption-finished signal to attempt password removal later
         def handler(sender, **kwargs):
             consumed_document: Document = kwargs.get("document")
+            document_consumption_finished.disconnect(handler)
             if consumed_document is not None:
-                execute_password_removal_action(
-                    action,
-                    consumed_document,
-                    logging_group,
+                transaction.on_commit(
+                    lambda: execute_password_removal_action(
+                        action,
+                        consumed_document,
+                        logging_group,
+                    ),
                 )
-            document_consumption_finished.disconnect(handler)
 
         document_consumption_finished.connect(handler, weak=False)
         return