fix #236

src/documents/apps.py

@@ -6,29 +6,21 @@ class DocumentsConfig(AppConfig):
     name = "documents"
 
     def ready(self):
-
-        from .signals import document_consumption_started
         from .signals import document_consumption_finished
         from .signals.handlers import (
             add_inbox_tags,
-            run_pre_consume_script,
-            run_post_consume_script,
             set_log_entry,
             set_correspondent,
             set_document_type,
             set_tags,
             add_to_index
-
         )
 
-        document_consumption_started.connect(run_pre_consume_script)
-
         document_consumption_finished.connect(add_inbox_tags)
         document_consumption_finished.connect(set_correspondent)
         document_consumption_finished.connect(set_document_type)
         document_consumption_finished.connect(set_tags)
         document_consumption_finished.connect(set_log_entry)
         document_consumption_finished.connect(add_to_index)
-        document_consumption_finished.connect(run_post_consume_script)
 
         AppConfig.ready(self)

src/documents/consumer.py

@@ -1,6 +1,7 @@
 import datetime
 import hashlib
 import os
+from subprocess import Popen
 
 import magic
 from django.conf import settings
@@ -8,6 +9,7 @@ from django.db import transaction
 from django.db.models import Q
 from django.utils import timezone
 from filelock import FileLock
+from rest_framework.reverse import reverse
 
 from .classifier import DocumentClassifier, IncompatibleClassifierVersionError
 from .file_handling import create_source_path_directory, \
@@ -65,6 +67,39 @@ class Consumer(LoggingMixin):
         os.makedirs(settings.ORIGINALS_DIR, exist_ok=True)
         os.makedirs(settings.ARCHIVE_DIR, exist_ok=True)
 
+    def run_pre_consume_script(self):
+        if not settings.PRE_CONSUME_SCRIPT:
+            return
+
+        try:
+            Popen((settings.PRE_CONSUME_SCRIPT, self.path)).wait()
+        except Exception as e:
+            raise ConsumerError(
+                f"Error while executing pre-consume script: {e}"
+            )
+
+    def run_post_consume_script(self, document):
+        if not settings.POST_CONSUME_SCRIPT:
+            return
+
+        try:
+            Popen((
+                settings.POST_CONSUME_SCRIPT,
+                str(document.pk),
+                document.get_public_filename(),
+                os.path.normpath(document.source_path),
+                os.path.normpath(document.thumbnail_path),
+                reverse("document-download", kwargs={"pk": document.pk}),
+                reverse("document-thumb", kwargs={"pk": document.pk}),
+                str(document.correspondent),
+                str(",".join(document.tags.all().values_list(
+                    "name", flat=True)))
+            )).wait()
+        except Exception as e:
+            raise ConsumerError(
+                f"Error while executing pre-consume script: {e}"
+            )
+
     def try_consume_file(self,
                          path,
                          override_filename=None,
@@ -118,6 +153,8 @@ class Consumer(LoggingMixin):
             logging_group=self.logging_group
         )
 
+        self.run_pre_consume_script()
+
         # This doesn't parse the document yet, but gives us a parser.
 
         document_parser = parser_class(self.logging_group)
@@ -214,6 +251,9 @@ class Consumer(LoggingMixin):
                 # Delete the file only if it was successfully consumed
                 self.log("debug", "Deleting file {}".format(self.path))
                 os.unlink(self.path)
+
+                self.run_post_consume_script(document)
+
         except Exception as e:
             self.log(
                 "error",

src/documents/signals/handlers.py

@@ -11,7 +11,6 @@ from django.db.models import Q
 from django.dispatch import receiver
 from django.utils import timezone
 from filelock import FileLock
-from rest_framework.reverse import reverse
 
 from .. import index, matching
 from ..file_handling import delete_empty_directories, \
@@ -147,32 +146,6 @@ def set_tags(sender,
     document.tags.add(*relevant_tags)
 
 
-def run_pre_consume_script(sender, filename, **kwargs):
-
-    if not settings.PRE_CONSUME_SCRIPT:
-        return
-
-    Popen((settings.PRE_CONSUME_SCRIPT, filename)).wait()
-
-
-def run_post_consume_script(sender, document, **kwargs):
-
-    if not settings.POST_CONSUME_SCRIPT:
-        return
-
-    Popen((
-        settings.POST_CONSUME_SCRIPT,
-        str(document.pk),
-        document.get_public_filename(),
-        os.path.normpath(document.source_path),
-        os.path.normpath(document.thumbnail_path),
-        reverse("document-download", kwargs={"pk": document.pk}),
-        reverse("document-thumb", kwargs={"pk": document.pk}),
-        str(document.correspondent),
-        str(",".join(document.tags.all().values_list("name", flat=True)))
-    )).wait()
-
-
 @receiver(models.signals.post_delete, sender=Document)
 def cleanup_document_deletion(sender, instance, using, **kwargs):
     with FileLock(settings.MEDIA_LOCK):

src/documents/tests/test_consumer.py

@@ -466,3 +466,53 @@ class TestConsumer(DirectoriesMixin, TestCase):
         self.assertTrue(os.path.isfile(dst))
         self.assertRaises(ConsumerError, self.consumer.try_consume_file, dst)
         self.assertTrue(os.path.isfile(dst))
+
+
+class PostConsumeTestCase(TestCase):
+
+    @mock.patch("documents.signals.handlers.Popen")
+    @override_settings(POST_CONSUME_SCRIPT=None)
+    def test_no_post_consume_script(self, m):
+        doc = Document.objects.create(title="Test", mime_type="application/pdf")
+        tag1 = Tag.objects.create(name="a")
+        tag2 = Tag.objects.create(name="b")
+        doc.tags.add(tag1)
+        doc.tags.add(tag2)
+
+        Consumer().run_post_consume_script(doc)
+
+        m.assert_not_called()
+
+    @mock.patch("documents.signals.handlers.Popen")
+    @override_settings(POST_CONSUME_SCRIPT="script")
+    def test_post_consume_script_simple(self, m):
+        doc = Document.objects.create(title="Test", mime_type="application/pdf")
+
+        Consumer().run_post_consume_script(doc)
+
+        m.assert_called_once()
+
+    @mock.patch("documents.signals.handlers.Popen")
+    @override_settings(POST_CONSUME_SCRIPT="script")
+    def test_post_consume_script_with_correspondent(self, m):
+        c = Correspondent.objects.create(name="my_bank")
+        doc = Document.objects.create(title="Test", mime_type="application/pdf", correspondent=c)
+        tag1 = Tag.objects.create(name="a")
+        tag2 = Tag.objects.create(name="b")
+        doc.tags.add(tag1)
+        doc.tags.add(tag2)
+
+        Consumer().run_post_consume_script(doc)
+
+        m.assert_called_once()
+
+        args, kwargs = m.call_args
+
+        command = args[0]
+
+        self.assertEqual(command[0], "script")
+        self.assertEqual(command[1], str(doc.pk))
+        self.assertEqual(command[5], f"/api/documents/{doc.pk}/download/")
+        self.assertEqual(command[6], f"/api/documents/{doc.pk}/thumb/")
+        self.assertEqual(command[7], "my_bank")
+        self.assertCountEqual(command[8].split(","), ["a", "b"])

src/documents/tests/test_post_consume_handlers.py

@@ -1,56 +0,0 @@
-from unittest import mock
-
-from django.test import TestCase, override_settings
-
-from documents.models import Document, Tag, Correspondent
-from documents.signals.handlers import run_post_consume_script
-
-
-class PostConsumeTestCase(TestCase):
-
-    @mock.patch("documents.signals.handlers.Popen")
-    @override_settings(POST_CONSUME_SCRIPT=None)
-    def test_no_post_consume_script(self, m):
-        doc = Document.objects.create(title="Test", mime_type="application/pdf")
-        tag1 = Tag.objects.create(name="a")
-        tag2 = Tag.objects.create(name="b")
-        doc.tags.add(tag1)
-        doc.tags.add(tag2)
-
-        run_post_consume_script(None, doc)
-
-        m.assert_not_called()
-
-    @mock.patch("documents.signals.handlers.Popen")
-    @override_settings(POST_CONSUME_SCRIPT="script")
-    def test_post_consume_script_simple(self, m):
-        doc = Document.objects.create(title="Test", mime_type="application/pdf")
-
-        run_post_consume_script(None, doc)
-
-        m.assert_called_once()
-
-    @mock.patch("documents.signals.handlers.Popen")
-    @override_settings(POST_CONSUME_SCRIPT="script")
-    def test_post_consume_script_with_correspondent(self, m):
-        c = Correspondent.objects.create(name="my_bank")
-        doc = Document.objects.create(title="Test", mime_type="application/pdf", correspondent=c)
-        tag1 = Tag.objects.create(name="a")
-        tag2 = Tag.objects.create(name="b")
-        doc.tags.add(tag1)
-        doc.tags.add(tag2)
-
-        run_post_consume_script(None, doc)
-
-        m.assert_called_once()
-
-        args, kwargs = m.call_args
-
-        command = args[0]
-
-        self.assertEqual(command[0], "script")
-        self.assertEqual(command[1], str(doc.pk))
-        self.assertEqual(command[5], f"/api/documents/{doc.pk}/download/")
-        self.assertEqual(command[6], f"/api/documents/{doc.pk}/thumb/")
-        self.assertEqual(command[7], "my_bank")
-        self.assertCountEqual(command[8].split(","), ["a", "b"])