From 4b74cd567734f9605bc7ddf1dabcd7d06ef10d0f Mon Sep 17 00:00:00 2001 From: jonaswinkler Date: Fri, 1 Jan 2021 23:27:55 +0100 Subject: [PATCH] fix #236 --- src/documents/apps.py | 8 --- src/documents/consumer.py | 40 +++++++++++++ src/documents/signals/handlers.py | 27 --------- src/documents/tests/test_consumer.py | 50 +++++++++++++++++ .../tests/test_post_consume_handlers.py | 56 ------------------- 5 files changed, 90 insertions(+), 91 deletions(-) delete mode 100644 src/documents/tests/test_post_consume_handlers.py diff --git a/src/documents/apps.py b/src/documents/apps.py index 2cd7d6c0e..4278029cc 100644 --- a/src/documents/apps.py +++ b/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) diff --git a/src/documents/consumer.py b/src/documents/consumer.py index abd12d802..480537279 100755 --- a/src/documents/consumer.py +++ b/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", diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index f2743c212..be101b3af 100755 --- a/src/documents/signals/handlers.py +++ b/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): diff --git a/src/documents/tests/test_consumer.py b/src/documents/tests/test_consumer.py index 90c034f9e..743c43846 100644 --- a/src/documents/tests/test_consumer.py +++ b/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"]) diff --git a/src/documents/tests/test_post_consume_handlers.py b/src/documents/tests/test_post_consume_handlers.py deleted file mode 100644 index b4357448c..000000000 --- a/src/documents/tests/test_post_consume_handlers.py +++ /dev/null @@ -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"])