diff --git a/src/documents/sanity_checker.py b/src/documents/sanity_checker.py index de3995eb7..578f1a936 100644 --- a/src/documents/sanity_checker.py +++ b/src/documents/sanity_checker.py @@ -1,6 +1,8 @@ import hashlib import logging -import os +from collections import defaultdict +from pathlib import Path +from typing import Final from django.conf import settings from documents.models import Document @@ -9,16 +11,20 @@ from tqdm import tqdm class SanityCheckMessages: def __init__(self): - self._messages = [] + self._messages = defaultdict(list) + self.has_error = False + self.has_warning = False - def error(self, message): - self._messages.append({"level": logging.ERROR, "message": message}) + def error(self, doc_pk, message): + self._messages[doc_pk].append({"level": logging.ERROR, "message": message}) + self.has_error = True - def warning(self, message): - self._messages.append({"level": logging.WARNING, "message": message}) + def warning(self, doc_pk, message): + self._messages[doc_pk].append({"level": logging.WARNING, "message": message}) + self.has_warning = True - def info(self, message): - self._messages.append({"level": logging.INFO, "message": message}) + def info(self, doc_pk, message): + self._messages[doc_pk].append({"level": logging.INFO, "message": message}) def log_messages(self): logger = logging.getLogger("paperless.sanity_checker") @@ -26,8 +32,19 @@ class SanityCheckMessages: if len(self._messages) == 0: logger.info("Sanity checker detected no issues.") else: - for msg in self._messages: - logger.log(msg["level"], msg["message"]) + + # Query once + all_docs = Document.objects.all() + + for doc_pk in self._messages: + if doc_pk is not None: + doc = all_docs.get(pk=doc_pk) + logger.info( + f"Detected following issue(s) with document #{doc.pk}," + f" titled {doc.title}", + ) + for msg in self._messages[doc_pk]: + logger.log(msg["level"], msg["message"]) def __len__(self): return len(self._messages) @@ -35,99 +52,94 @@ class SanityCheckMessages: def __getitem__(self, item): return self._messages[item] - def has_error(self): - return any([msg["level"] == logging.ERROR for msg in self._messages]) - - def has_warning(self): - return any([msg["level"] == logging.WARNING for msg in self._messages]) - class SanityCheckFailedException(Exception): pass -def check_sanity(progress=False): +def check_sanity(progress=False) -> SanityCheckMessages: messages = SanityCheckMessages() - present_files = [] - for root, subdirs, files in os.walk(settings.MEDIA_ROOT): - for f in files: - present_files.append(os.path.normpath(os.path.join(root, f))) + present_files = { + x.resolve() for x in Path(settings.MEDIA_ROOT).glob("**/*") if not x.is_dir() + } - lockfile = os.path.normpath(settings.MEDIA_LOCK) + lockfile = Path(settings.MEDIA_LOCK).resolve() if lockfile in present_files: present_files.remove(lockfile) for doc in tqdm(Document.objects.all(), disable=not progress): # Check sanity of the thumbnail - if not os.path.isfile(doc.thumbnail_path): - messages.error(f"Thumbnail of document {doc.pk} does not exist.") + thumbnail_path: Final[Path] = Path(doc.thumbnail_path).resolve() + if not thumbnail_path.exists() or not thumbnail_path.is_file(): + messages.error(doc.pk, "Thumbnail of document does not exist.") else: - if os.path.normpath(doc.thumbnail_path) in present_files: - present_files.remove(os.path.normpath(doc.thumbnail_path)) + if thumbnail_path in present_files: + present_files.remove(thumbnail_path) try: - with doc.thumbnail_file as f: - f.read() + _ = thumbnail_path.read_bytes() except OSError as e: - messages.error(f"Cannot read thumbnail file of document {doc.pk}: {e}") + messages.error(doc.pk, f"Cannot read thumbnail file of document: {e}") # Check sanity of the original file # TODO: extract method - if not os.path.isfile(doc.source_path): - messages.error(f"Original of document {doc.pk} does not exist.") + source_path: Final[Path] = Path(doc.source_path).resolve() + if not source_path.exists() or not source_path.is_file(): + messages.error(doc.pk, "Original of document does not exist.") else: - if os.path.normpath(doc.source_path) in present_files: - present_files.remove(os.path.normpath(doc.source_path)) + if source_path in present_files: + present_files.remove(source_path) try: - with doc.source_file as f: - checksum = hashlib.md5(f.read()).hexdigest() + checksum = hashlib.md5(source_path.read_bytes()).hexdigest() except OSError as e: - messages.error(f"Cannot read original file of document {doc.pk}: {e}") + messages.error(doc.pk, f"Cannot read original file of document: {e}") else: if not checksum == doc.checksum: messages.error( - f"Checksum mismatch of document {doc.pk}. " + doc.pk, + "Checksum mismatch. " f"Stored: {doc.checksum}, actual: {checksum}.", ) # Check sanity of the archive file. - if doc.archive_checksum and not doc.archive_filename: + if doc.archive_checksum is not None and doc.archive_filename is None: messages.error( - f"Document {doc.pk} has an archive file checksum, but no " - f"archive filename.", + doc.pk, + "Document has an archive file checksum, but no archive filename.", ) - elif not doc.archive_checksum and doc.archive_filename: + elif doc.archive_checksum is None and doc.archive_filename is not None: messages.error( - f"Document {doc.pk} has an archive file, but its checksum is " - f"missing.", + doc.pk, + "Document has an archive file, but its checksum is missing.", ) elif doc.has_archive_version: - if not os.path.isfile(doc.archive_path): - messages.error(f"Archived version of document {doc.pk} does not exist.") + archive_path: Final[Path] = Path(doc.archive_path).resolve() + if not archive_path.exists() or not archive_path.is_file(): + messages.error(doc.pk, "Archived version of document does not exist.") else: - if os.path.normpath(doc.archive_path) in present_files: - present_files.remove(os.path.normpath(doc.archive_path)) + if archive_path in present_files: + present_files.remove(archive_path) try: - with doc.archive_file as f: - checksum = hashlib.md5(f.read()).hexdigest() + checksum = hashlib.md5(archive_path.read_bytes()).hexdigest() except OSError as e: messages.error( - f"Cannot read archive file of document {doc.pk}: {e}", + doc.pk, + f"Cannot read archive file of document : {e}", ) else: if not checksum == doc.archive_checksum: messages.error( - f"Checksum mismatch of archived document " - f"{doc.pk}. " + doc.pk, + "Checksum mismatch of archived document. " f"Stored: {doc.archive_checksum}, " f"actual: {checksum}.", ) # other document checks if not doc.content: - messages.info(f"Document {doc.pk} has no content.") + messages.info(doc.pk, "Document contains no OCR data") for extra_file in present_files: - messages.warning(f"Orphaned file in media dir: {extra_file}") + messages.warning(None, f"Orphaned file in media dir: {extra_file}") return messages diff --git a/src/documents/tasks.py b/src/documents/tasks.py index 208f74f1d..4c57b2eee 100644 --- a/src/documents/tasks.py +++ b/src/documents/tasks.py @@ -338,9 +338,9 @@ def sanity_check(): messages.log_messages() - if messages.has_error(): + if messages.has_error: raise SanityCheckFailedException("Sanity check failed with errors. See log.") - elif messages.has_warning(): + elif messages.has_warning: return "Sanity check exited with warnings. See log." elif len(messages) > 0: return "Sanity check exited with infos. See log." diff --git a/src/documents/tests/test_management.py b/src/documents/tests/test_management.py index dbfc1267b..5e45086fe 100644 --- a/src/documents/tests/test_management.py +++ b/src/documents/tests/test_management.py @@ -238,5 +238,5 @@ class TestSanityChecker(DirectoriesMixin, TestCase): with self.assertLogs() as capture: call_command("document_sanity_checker") - self.assertEqual(len(capture.output), 1) - self.assertIn("Checksum mismatch of document", capture.output[0]) + self.assertEqual(len(capture.output), 2) + self.assertIn("Checksum mismatch. Stored: abc, actual:", capture.output[1]) diff --git a/src/documents/tests/test_management_exporter.py b/src/documents/tests/test_management_exporter.py index c509b9339..a9dcabc4d 100644 --- a/src/documents/tests/test_management_exporter.py +++ b/src/documents/tests/test_management_exporter.py @@ -190,7 +190,7 @@ class TestExportImport(DirectoriesMixin, TestCase): self.assertEqual(Document.objects.get(id=self.d4.id).title, "wow_dec") messages = check_sanity() # everything is alright after the test - self.assertEqual(len(messages), 0, str([str(m) for m in messages])) + self.assertEqual(len(messages), 0) def test_exporter_with_filename_format(self): shutil.rmtree(os.path.join(self.dirs.media_dir, "documents")) diff --git a/src/documents/tests/test_sanity_check.py b/src/documents/tests/test_sanity_check.py index 7a1b64ce4..5ebedd908 100644 --- a/src/documents/tests/test_sanity_check.py +++ b/src/documents/tests/test_sanity_check.py @@ -8,62 +8,9 @@ from django.conf import settings from django.test import TestCase from documents.models import Document from documents.sanity_checker import check_sanity -from documents.sanity_checker import SanityCheckMessages from documents.tests.utils import DirectoriesMixin -class TestSanityCheckMessages(TestCase): - def test_no_messages(self): - messages = SanityCheckMessages() - self.assertEqual(len(messages), 0) - self.assertFalse(messages.has_error()) - self.assertFalse(messages.has_warning()) - with self.assertLogs() as capture: - messages.log_messages() - self.assertEqual(len(capture.output), 1) - self.assertEqual(capture.records[0].levelno, logging.INFO) - self.assertEqual( - capture.records[0].message, - "Sanity checker detected no issues.", - ) - - def test_info(self): - messages = SanityCheckMessages() - messages.info("Something might be wrong") - self.assertEqual(len(messages), 1) - self.assertFalse(messages.has_error()) - self.assertFalse(messages.has_warning()) - with self.assertLogs() as capture: - messages.log_messages() - self.assertEqual(len(capture.output), 1) - self.assertEqual(capture.records[0].levelno, logging.INFO) - self.assertEqual(capture.records[0].message, "Something might be wrong") - - def test_warning(self): - messages = SanityCheckMessages() - messages.warning("Something is wrong") - self.assertEqual(len(messages), 1) - self.assertFalse(messages.has_error()) - self.assertTrue(messages.has_warning()) - with self.assertLogs() as capture: - messages.log_messages() - self.assertEqual(len(capture.output), 1) - self.assertEqual(capture.records[0].levelno, logging.WARNING) - self.assertEqual(capture.records[0].message, "Something is wrong") - - def test_error(self): - messages = SanityCheckMessages() - messages.error("Something is seriously wrong") - self.assertEqual(len(messages), 1) - self.assertTrue(messages.has_error()) - self.assertFalse(messages.has_warning()) - with self.assertLogs() as capture: - messages.log_messages() - self.assertEqual(len(capture.output), 1) - self.assertEqual(capture.records[0].levelno, logging.ERROR) - self.assertEqual(capture.records[0].message, "Something is seriously wrong") - - class TestSanityCheck(DirectoriesMixin, TestCase): def make_test_data(self): @@ -111,10 +58,30 @@ class TestSanityCheck(DirectoriesMixin, TestCase): archive_filename="0000001.pdf", ) - def assertSanityError(self, messageRegex): + def assertSanityError(self, doc: Document, messageRegex): messages = check_sanity() - self.assertTrue(messages.has_error()) - self.assertRegex(messages[0]["message"], messageRegex) + self.assertTrue(messages.has_error) + with self.assertLogs() as capture: + messages.log_messages() + self.assertEqual( + capture.records[0].message, + f"Detected following issue(s) with document #{doc.pk}, titled {doc.title}", + ) + self.assertRegex(capture.records[1].message, messageRegex) + + def test_no_issues(self): + self.make_test_data() + messages = check_sanity() + self.assertFalse(messages.has_error) + self.assertFalse(messages.has_warning) + with self.assertLogs() as capture: + messages.log_messages() + self.assertEqual(len(capture.output), 1) + self.assertEqual(capture.records[0].levelno, logging.INFO) + self.assertEqual( + capture.records[0].message, + "Sanity checker detected no issues.", + ) def test_no_docs(self): self.assertEqual(len(check_sanity()), 0) @@ -126,75 +93,82 @@ class TestSanityCheck(DirectoriesMixin, TestCase): def test_no_thumbnail(self): doc = self.make_test_data() os.remove(doc.thumbnail_path) - self.assertSanityError("Thumbnail of document .* does not exist") + self.assertSanityError(doc, "Thumbnail of document does not exist") def test_thumbnail_no_access(self): doc = self.make_test_data() os.chmod(doc.thumbnail_path, 0o000) - self.assertSanityError("Cannot read thumbnail file of document") + self.assertSanityError(doc, "Cannot read thumbnail file of document") os.chmod(doc.thumbnail_path, 0o777) def test_no_original(self): doc = self.make_test_data() os.remove(doc.source_path) - self.assertSanityError("Original of document .* does not exist.") + self.assertSanityError(doc, "Original of document does not exist.") def test_original_no_access(self): doc = self.make_test_data() os.chmod(doc.source_path, 0o000) - self.assertSanityError("Cannot read original file of document") + self.assertSanityError(doc, "Cannot read original file of document") os.chmod(doc.source_path, 0o777) def test_original_checksum_mismatch(self): doc = self.make_test_data() doc.checksum = "WOW" doc.save() - self.assertSanityError("Checksum mismatch of document") + self.assertSanityError(doc, "Checksum mismatch. Stored: WOW, actual: ") def test_no_archive(self): doc = self.make_test_data() os.remove(doc.archive_path) - self.assertSanityError("Archived version of document .* does not exist.") + self.assertSanityError(doc, "Archived version of document does not exist.") def test_archive_no_access(self): doc = self.make_test_data() os.chmod(doc.archive_path, 0o000) - self.assertSanityError("Cannot read archive file of document") + self.assertSanityError(doc, "Cannot read archive file of document") os.chmod(doc.archive_path, 0o777) def test_archive_checksum_mismatch(self): doc = self.make_test_data() doc.archive_checksum = "WOW" doc.save() - self.assertSanityError("Checksum mismatch of archived document") + self.assertSanityError(doc, "Checksum mismatch of archived document") def test_empty_content(self): doc = self.make_test_data() doc.content = "" doc.save() messages = check_sanity() - self.assertFalse(messages.has_error()) - self.assertFalse(messages.has_warning()) + self.assertFalse(messages.has_error) + self.assertFalse(messages.has_warning) self.assertEqual(len(messages), 1) - self.assertRegex(messages[0]["message"], "Document .* has no content.") + self.assertRegex( + messages[doc.pk][0]["message"], + "Document contains no OCR data", + ) def test_orphaned_file(self): doc = self.make_test_data() Path(self.dirs.originals_dir, "orphaned").touch() messages = check_sanity() - self.assertFalse(messages.has_error()) - self.assertTrue(messages.has_warning()) - self.assertEqual(len(messages), 1) - self.assertRegex(messages[0]["message"], "Orphaned file in media dir") + self.assertTrue(messages.has_warning) + self.assertRegex( + messages._messages[None][0]["message"], + "Orphaned file in media dir", + ) def test_archive_filename_no_checksum(self): doc = self.make_test_data() doc.archive_checksum = None doc.save() - self.assertSanityError("has an archive file, but its checksum is missing.") + self.assertSanityError(doc, "has an archive file, but its checksum is missing.") def test_archive_checksum_no_filename(self): doc = self.make_test_data() doc.archive_filename = None doc.save() - self.assertSanityError("has an archive file checksum, but no archive filename.") + self.assertSanityError( + doc, + "has an archive file checksum, but no archive filename.", + ) diff --git a/src/documents/tests/test_tasks.py b/src/documents/tests/test_tasks.py index 41b9380db..998b7d955 100644 --- a/src/documents/tests/test_tasks.py +++ b/src/documents/tests/test_tasks.py @@ -538,7 +538,7 @@ class TestTasks(DirectoriesMixin, TestCase): @mock.patch("documents.tasks.sanity_checker.check_sanity") def test_sanity_check_error(self, m): messages = SanityCheckMessages() - messages.error("Some error") + messages.error(None, "Some error") m.return_value = messages self.assertRaises(SanityCheckFailedException, tasks.sanity_check) m.assert_called_once() @@ -546,7 +546,7 @@ class TestTasks(DirectoriesMixin, TestCase): @mock.patch("documents.tasks.sanity_checker.check_sanity") def test_sanity_check_warning(self, m): messages = SanityCheckMessages() - messages.warning("Some warning") + messages.warning(None, "Some warning") m.return_value = messages self.assertEqual( tasks.sanity_check(), @@ -557,7 +557,7 @@ class TestTasks(DirectoriesMixin, TestCase): @mock.patch("documents.tasks.sanity_checker.check_sanity") def test_sanity_check_info(self, m): messages = SanityCheckMessages() - messages.info("Some info") + messages.info(None, "Some info") m.return_value = messages self.assertEqual( tasks.sanity_check(),