From 023aeea7eac7c45702cc1998ac9887bb3f198f1a Mon Sep 17 00:00:00 2001 From: jonaswinkler Date: Sun, 29 Nov 2020 15:47:56 +0100 Subject: [PATCH] test cases for #67 --- docs/extending.rst | 3 ++- src/documents/consumer.py | 4 +++- src/documents/signals/handlers.py | 6 ++++- src/documents/tests/test_consumer.py | 27 ++++++++++++++++++++--- src/documents/tests/test_file_handling.py | 21 ++++++++---------- 5 files changed, 43 insertions(+), 18 deletions(-) diff --git a/docs/extending.rst b/docs/extending.rst index 29b08bf59..a0f14f2aa 100644 --- a/docs/extending.rst +++ b/docs/extending.rst @@ -53,7 +53,8 @@ them running. Testing and code style: * Run ``pytest`` in the src/ directory to execute all tests. This also generates a HTML coverage - report. + report. When runnings test, paperless.conf is loaded as well. However: the tests rely on the default + configuration. This is not ideal. But for now, make sure no settings except for DEBUG are overridden when testing. * Run ``pycodestyle`` to test your code for issues with the configured code style settings. .. note:: diff --git a/src/documents/consumer.py b/src/documents/consumer.py index 46c943d6b..1842fbb56 100755 --- a/src/documents/consumer.py +++ b/src/documents/consumer.py @@ -169,7 +169,9 @@ class Consumer(LoggingMixin): # Afte performing all database operations and moving files # into place, tell paperless where the file is. - document.filename = generate_filename(document) + document.filename = os.path.basename(document.source_path) + # Saving the document now will trigger the filename handling + # logic. document.save() # Delete the file only if it was successfully consumed diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 3afb0d1cf..7b53c073c 100755 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -218,7 +218,11 @@ def update_filename_and_move_files(sender, instance, **kwargs): try: os.rename(old_path, new_path) instance.filename = new_filename - instance.save() + # Don't save here to prevent infinite recursion. + Document.objects.filter(pk=instance.pk).update(filename=new_filename) + + logging.getLogger(__name__).debug( + f"Moved file {old_path} to {new_path}.") except OSError as e: instance.filename = old_filename diff --git a/src/documents/tests/test_consumer.py b/src/documents/tests/test_consumer.py index d1cd0adf1..b436f76a1 100644 --- a/src/documents/tests/test_consumer.py +++ b/src/documents/tests/test_consumer.py @@ -563,13 +563,34 @@ class TestConsumer(DirectoriesMixin, TestCase): document = self.consumer.try_consume_file(filename, override_filename="Bank - Test.pdf", override_title="new docs") - print(document.source_path) - print("===") - self.assertEqual(document.title, "new docs") self.assertEqual(document.correspondent.name, "Bank") self.assertEqual(document.filename, "bank/new-docs-0000001.pdf") + @override_settings(PAPERLESS_FILENAME_FORMAT="{correspondent}/{title}") + @mock.patch("documents.signals.handlers.generate_filename") + def testFilenameHandlingUnstableFormat(self, m): + + filenames = ["this", "that", "now this", "i cant decide"] + + def get_filename(): + f = filenames.pop() + filenames.insert(0, f) + return f + + m.side_effect = lambda f: get_filename() + + filename = self.get_test_file() + + Tag.objects.create(name="test", is_inbox_tag=True) + + document = self.consumer.try_consume_file(filename, override_filename="Bank - Test.pdf", override_title="new docs") + + self.assertEqual(document.title, "new docs") + self.assertEqual(document.correspondent.name, "Bank") + self.assertIsNotNone(os.path.isfile(document.title)) + self.assertTrue(os.path.isfile(document.source_path)) + @mock.patch("documents.consumer.DocumentClassifier") def testClassifyDocument(self, m): correspondent = Correspondent.objects.create(name="test") diff --git a/src/documents/tests/test_file_handling.py b/src/documents/tests/test_file_handling.py index 5ffd35f61..d799384e7 100644 --- a/src/documents/tests/test_file_handling.py +++ b/src/documents/tests/test_file_handling.py @@ -1,14 +1,15 @@ import os import shutil from pathlib import Path +from unittest import mock from uuid import uuid4 from django.conf import settings +from django.db import DatabaseError from django.test import TestCase, override_settings from ..file_handling import generate_filename, create_source_path_directory, delete_empty_directories from ..models import Document, Correspondent -from ..signals.handlers import update_filename_and_move_files class TestDate(TestCase): @@ -133,18 +134,14 @@ class TestDate(TestCase): document.correspondent = Correspondent.objects.get_or_create( name="test")[0] - # This will cause save() to fail. - document.checksum = document1.checksum + with mock.patch("documents.signals.handlers.Document.objects.filter") as m: + m.side_effect = DatabaseError() + document.save() - # Assume saving the document initially works, this gets called. - # After renaming, an error occurs, and filename is not saved: - # document should still be available at document.filename. - update_filename_and_move_files(None, document) - - # Check proper handling of files - self.assertTrue(os.path.isfile(document.source_path)) - self.assertEqual(os.path.isfile(settings.MEDIA_ROOT + "/documents/originals/none/none-{:07d}.pdf".format(document.pk)), True) - self.assertEqual(document.filename, "none/none-{:07d}.pdf".format(document.pk)) + # Check proper handling of files + self.assertTrue(os.path.isfile(document.source_path)) + self.assertEqual(os.path.isfile(settings.MEDIA_ROOT + "/documents/originals/none/none-{:07d}.pdf".format(document.pk)), True) + self.assertEqual(document.filename, "none/none-{:07d}.pdf".format(document.pk)) @override_settings(PAPERLESS_FILENAME_FORMAT="{correspondent}/{correspondent}") def test_document_delete(self):