Refresh the document instance before doing workflow work, in case some other process has updated it (#6849)

src/documents/signals/handlers.py

@@ -363,24 +363,22 @@ class CannotMoveFilesException(Exception):
     pass
 
 
-def validate_move(instance, old_path, new_path):
-    if not os.path.isfile(old_path):
-        # Can't do anything if the old file does not exist anymore.
-        logger.fatal(f"Document {instance!s}: File {old_path} has gone.")
-        raise CannotMoveFilesException
-
-    if os.path.isfile(new_path):
-        # Can't do anything if the new file already exists. Skip updating file.
-        logger.warning(
-            f"Document {instance!s}: Cannot rename file "
-            f"since target path {new_path} already exists.",
-        )
-        raise CannotMoveFilesException
-
-
 @receiver(models.signals.m2m_changed, sender=Document.tags.through)
 @receiver(models.signals.post_save, sender=Document)
 def update_filename_and_move_files(sender, instance: Document, **kwargs):
+    def validate_move(instance, old_path, new_path):
+        if not os.path.isfile(old_path):
+            # 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 os.path.isfile(new_path):
+            # 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.
@@ -532,39 +530,7 @@ def run_workflow(
     document: Document,
     logging_group=None,
 ):
-    for workflow in (
-        Workflow.objects.filter(
-            enabled=True,
-            triggers__type=trigger_type,
-        )
-        .prefetch_related("actions")
-        .prefetch_related("actions__assign_view_users")
-        .prefetch_related("actions__assign_view_groups")
-        .prefetch_related("actions__assign_change_users")
-        .prefetch_related("actions__assign_change_groups")
-        .prefetch_related("actions__assign_custom_fields")
-        .prefetch_related("actions__remove_tags")
-        .prefetch_related("actions__remove_correspondents")
-        .prefetch_related("actions__remove_document_types")
-        .prefetch_related("actions__remove_storage_paths")
-        .prefetch_related("actions__remove_custom_fields")
-        .prefetch_related("actions__remove_owners")
-        .prefetch_related("triggers")
-        .order_by("order")
-    ):
-        if matching.document_matches_workflow(
-            document,
-            workflow,
-            trigger_type,
-        ):
-            action: WorkflowAction
-            for action in workflow.actions.all():
-                logger.info(
-                    f"Applying {action} from {workflow}",
-                    extra={"group": logging_group},
-                )
-
-                if action.type == WorkflowAction.WorkflowActionType.ASSIGNMENT:
+    def assignment_action():
         if action.assign_tags.all().count() > 0:
             document.tags.add(*action.assign_tags.all())
 
@@ -594,11 +560,7 @@ def run_workflow(
                         if document.document_type is not None
                         else ""
                     ),
-                                (
-                                    document.owner.username
-                                    if document.owner is not None
-                                    else ""
-                                ),
+                    (document.owner.username if document.owner is not None else ""),
                     timezone.localtime(document.added),
                     (
                         document.original_filename
@@ -674,7 +636,7 @@ def run_workflow(
                         document=document,
                     )
 
-                elif action.type == WorkflowAction.WorkflowActionType.REMOVAL:
+    def removal_action():
         if action.remove_all_tags:
             document.tags.clear()
         else:
@@ -758,6 +720,48 @@ def run_workflow(
                 document=document,
             ).delete()
 
+    for workflow in (
+        Workflow.objects.filter(
+            enabled=True,
+            triggers__type=trigger_type,
+        )
+        .prefetch_related("actions")
+        .prefetch_related("actions__assign_view_users")
+        .prefetch_related("actions__assign_view_groups")
+        .prefetch_related("actions__assign_change_users")
+        .prefetch_related("actions__assign_change_groups")
+        .prefetch_related("actions__assign_custom_fields")
+        .prefetch_related("actions__remove_tags")
+        .prefetch_related("actions__remove_correspondents")
+        .prefetch_related("actions__remove_document_types")
+        .prefetch_related("actions__remove_storage_paths")
+        .prefetch_related("actions__remove_custom_fields")
+        .prefetch_related("actions__remove_owners")
+        .prefetch_related("triggers")
+        .order_by("order")
+    ):
+        # This can be called from bulk_update_documents, which may be running multiple times
+        # Refresh this so the matching data is fresh and instance fields are re-freshed
+        # Otherwise, this instance might be behind and overwrite the work another process did
+        document.refresh_from_db()
+        if matching.document_matches_workflow(
+            document,
+            workflow,
+            trigger_type,
+        ):
+            action: WorkflowAction
+            for action in workflow.actions.all():
+                logger.info(
+                    f"Applying {action} from {workflow}",
+                    extra={"group": logging_group},
+                )
+
+                if action.type == WorkflowAction.WorkflowActionType.ASSIGNMENT:
+                    assignment_action()
+
+                elif action.type == WorkflowAction.WorkflowActionType.REMOVAL:
+                    removal_action()
+
             document.save()