From f86ddcf2214199a973ecd9b78a9db4485e6e4ccb Mon Sep 17 00:00:00 2001 From: Trenton H <797416+stumpylog@users.noreply.github.com> Date: Thu, 19 Mar 2026 17:19:06 -0700 Subject: [PATCH] refactor: drop get_parser_class_for_mime_type; callers use registry directly All callers now call get_parser_registry().get_parser_for_file() with the actual filename and path, enabling score() to use file extension hints. The MIME-only helper is removed. - consumer.py: passes self.filename + self.working_copy - tasks.py: passes document.original_filename + document.source_path - document_thumbnails.py: same pattern - views.py: passes Path(file).name + Path(file) - parsers.py: internal helpers inline the registry call with filename="" - test_parsers.py: drop TestParserDiscovery (was testing mock behavior); TestParserAvailability uses registry directly - test_consumer.py: mocks switch to documents.consumer.get_parser_registry Co-Authored-By: Claude Sonnet 4.6 --- src/documents/consumer.py | 10 ++- .../commands/document_thumbnails.py | 8 +- src/documents/parsers.py | 42 +-------- src/documents/tasks.py | 8 +- src/documents/tests/test_consumer.py | 35 ++++---- src/documents/tests/test_parsers.py | 89 ++----------------- src/documents/views.py | 8 +- src/paperless/parsers/registry.py | 2 +- 8 files changed, 52 insertions(+), 150 deletions(-) diff --git a/src/documents/consumer.py b/src/documents/consumer.py index 8cbbb9d44..809d6c647 100644 --- a/src/documents/consumer.py +++ b/src/documents/consumer.py @@ -33,7 +33,6 @@ from documents.models import StoragePath from documents.models import Tag from documents.models import WorkflowTrigger from documents.parsers import ParseError -from documents.parsers import get_parser_class_for_mime_type from documents.permissions import set_permissions_for_object from documents.plugins.base import AlwaysRunPluginMixin from documents.plugins.base import ConsumeTaskPlugin @@ -52,6 +51,7 @@ from documents.utils import copy_file_with_basic_stats from documents.utils import run_subprocess from paperless.parsers import ParserContext from paperless.parsers import ParserProtocol +from paperless.parsers.registry import get_parser_registry LOGGING_NAME: Final[str] = "paperless.consumer" @@ -392,8 +392,12 @@ class ConsumerPlugin( self.log.error(f"Error attempting to clean PDF: {e}") # Based on the mime type, get the parser for that type - parser_class: type[ParserProtocol] | None = get_parser_class_for_mime_type( - mime_type, + parser_class: type[ParserProtocol] | None = ( + get_parser_registry().get_parser_for_file( + mime_type, + self.filename, + self.working_copy, + ) ) if not parser_class: tempdir.cleanup() diff --git a/src/documents/management/commands/document_thumbnails.py b/src/documents/management/commands/document_thumbnails.py index 2812e0dc0..3d779ae18 100644 --- a/src/documents/management/commands/document_thumbnails.py +++ b/src/documents/management/commands/document_thumbnails.py @@ -3,14 +3,18 @@ import shutil from documents.management.commands.base import PaperlessCommand from documents.models import Document -from documents.parsers import get_parser_class_for_mime_type +from paperless.parsers.registry import get_parser_registry logger = logging.getLogger("paperless.management.thumbnails") def _process_document(doc_id: int) -> None: document: Document = Document.objects.get(id=doc_id) - parser_class = get_parser_class_for_mime_type(document.mime_type) + parser_class = get_parser_registry().get_parser_for_file( + document.mime_type, + document.original_filename or "", + document.source_path, + ) if parser_class is None: logger.warning( diff --git a/src/documents/parsers.py b/src/documents/parsers.py index 94233c648..69ee4e285 100644 --- a/src/documents/parsers.py +++ b/src/documents/parsers.py @@ -3,7 +3,6 @@ from __future__ import annotations import logging import mimetypes import os -import re import shutil import subprocess import tempfile @@ -20,36 +19,6 @@ from paperless.parsers.registry import get_parser_registry if TYPE_CHECKING: import datetime - from paperless.parsers import ParserProtocol - -# This regular expression will try to find dates in the document at -# hand and will match the following formats: -# - XX.YY.ZZZZ with XX + YY being 1 or 2 and ZZZZ being 2 or 4 digits -# - XX/YY/ZZZZ with XX + YY being 1 or 2 and ZZZZ being 2 or 4 digits -# - XX-YY-ZZZZ with XX + YY being 1 or 2 and ZZZZ being 2 or 4 digits -# - ZZZZ.XX.YY with XX + YY being 1 or 2 and ZZZZ being 2 or 4 digits -# - ZZZZ/XX/YY with XX + YY being 1 or 2 and ZZZZ being 2 or 4 digits -# - ZZZZ-XX-YY with XX + YY being 1 or 2 and ZZZZ being 2 or 4 digits -# - XX. MONTH ZZZZ with XX being 1 or 2 and ZZZZ being 2 or 4 digits -# - MONTH ZZZZ, with ZZZZ being 4 digits -# - MONTH XX, ZZZZ with XX being 1 or 2 and ZZZZ being 4 digits -# - XX MON ZZZZ with XX being 1 or 2 and ZZZZ being 4 digits. MONTH is 3 letters -# - XXPP MONTH ZZZZ with XX being 1 or 2 and PP being 2 letters and ZZZZ being 4 digits - -# TODO: isn't there a date parsing library for this? - -DATE_REGEX = re.compile( - r"(\b|(?!=([_-])))(\d{1,2})[\.\/-](\d{1,2})[\.\/-](\d{4}|\d{2})(\b|(?=([_-])))|" - r"(\b|(?!=([_-])))(\d{4}|\d{2})[\.\/-](\d{1,2})[\.\/-](\d{1,2})(\b|(?=([_-])))|" - r"(\b|(?!=([_-])))(\d{1,2}[\. ]+[a-zéûäëčžúřěáíóńźçŞğü]{3,9} \d{4}|[a-zéûäëčžúřěáíóńźçŞğü]{3,9} \d{1,2}, \d{4})(\b|(?=([_-])))|" - r"(\b|(?!=([_-])))([^\W\d_]{3,9} \d{1,2}, (\d{4}))(\b|(?=([_-])))|" - r"(\b|(?!=([_-])))([^\W\d_]{3,9} \d{4})(\b|(?=([_-])))|" - r"(\b|(?!=([_-])))(\d{1,2}[^ 0-9]{2}[\. ]+[^ ]{3,9}[ \.\/-]\d{4})(\b|(?=([_-])))|" - r"(\b|(?!=([_-])))(\b\d{1,2}[ \.\/-][a-zéûäëčžúřěáíóńźçŞğü]{3}[ \.\/-]\d{4})(\b|(?=([_-])))", - re.IGNORECASE, -) - - logger = logging.getLogger("paperless.parsing") @@ -57,7 +26,7 @@ def is_mime_type_supported(mime_type: str) -> bool: """ Returns True if the mime type is supported, False otherwise """ - return get_parser_class_for_mime_type(mime_type) is not None + return get_parser_registry().get_parser_for_file(mime_type, "") is not None def get_default_file_extension(mime_type: str) -> str: @@ -65,7 +34,7 @@ def get_default_file_extension(mime_type: str) -> str: Returns the default file extension for a mimetype, or an empty string if it could not be determined """ - parser_class = get_parser_class_for_mime_type(mime_type) + parser_class = get_parser_registry().get_parser_for_file(mime_type, "") if parser_class is not None: supported = parser_class.supported_mime_types() if mime_type in supported: @@ -99,13 +68,6 @@ def get_supported_file_extensions() -> set[str]: return extensions -def get_parser_class_for_mime_type(mime_type: str) -> type[ParserProtocol] | None: - """ - Returns the best parser for the given mimetype or None if no parser exists. - """ - return get_parser_registry().get_parser_for_file(mime_type, "") - - def run_convert( input_file, output_file, diff --git a/src/documents/tasks.py b/src/documents/tasks.py index e8186a4f1..751990c62 100644 --- a/src/documents/tasks.py +++ b/src/documents/tasks.py @@ -52,7 +52,6 @@ from documents.models import StoragePath from documents.models import Tag from documents.models import WorkflowRun from documents.models import WorkflowTrigger -from documents.parsers import get_parser_class_for_mime_type from documents.plugins.base import ConsumeTaskPlugin from documents.plugins.base import ProgressManager from documents.plugins.base import StopConsumeTaskError @@ -65,6 +64,7 @@ from documents.signals.handlers import send_websocket_document_updated from documents.workflows.utils import get_workflows_for_trigger from paperless.config import AIConfig from paperless.parsers import ParserContext +from paperless.parsers.registry import get_parser_registry from paperless_ai.indexing import llm_index_add_or_update_document from paperless_ai.indexing import llm_index_remove_document from paperless_ai.indexing import update_llm_index @@ -304,7 +304,11 @@ def update_document_content_maybe_archive_file(document_id) -> None: mime_type = document.mime_type - parser_class = get_parser_class_for_mime_type(mime_type) + parser_class = get_parser_registry().get_parser_for_file( + mime_type, + document.original_filename or "", + document.source_path, + ) if not parser_class: logger.error( diff --git a/src/documents/tests/test_consumer.py b/src/documents/tests/test_consumer.py index 7ea438187..3fb25f978 100644 --- a/src/documents/tests/test_consumer.py +++ b/src/documents/tests/test_consumer.py @@ -191,9 +191,9 @@ class TestConsumer( def setUp(self) -> None: super().setUp() - patcher = mock.patch("documents.consumer.get_parser_class_for_mime_type") - m = patcher.start() - m.return_value = DummyParser + patcher = mock.patch("documents.consumer.get_parser_registry") + mock_registry = patcher.start() + mock_registry.return_value.get_parser_for_file.return_value = DummyParser self.addCleanup(patcher.stop) def get_test_file(self): @@ -562,9 +562,9 @@ class TestConsumer( ) as consumer: consumer.run() - @mock.patch("documents.consumer.get_parser_class_for_mime_type") + @mock.patch("documents.consumer.get_parser_registry") def testNoParsers(self, m) -> None: - m.return_value = None + m.return_value.get_parser_for_file.return_value = None with self.assertRaisesMessage( ConsumerError, @@ -575,9 +575,9 @@ class TestConsumer( self._assert_first_last_send_progress(last_status="FAILED") - @mock.patch("documents.consumer.get_parser_class_for_mime_type") + @mock.patch("documents.consumer.get_parser_registry") def testFaultyParser(self, m) -> None: - m.return_value = FaultyParser + m.return_value.get_parser_for_file.return_value = FaultyParser with self.get_consumer(self.get_test_file()) as consumer: with self.assertRaisesMessage( @@ -588,9 +588,9 @@ class TestConsumer( self._assert_first_last_send_progress(last_status="FAILED") - @mock.patch("documents.consumer.get_parser_class_for_mime_type") + @mock.patch("documents.consumer.get_parser_registry") def testGenericParserException(self, m) -> None: - m.return_value = FaultyGenericExceptionParser + m.return_value.get_parser_for_file.return_value = FaultyGenericExceptionParser with self.get_consumer(self.get_test_file()) as consumer: with self.assertRaisesMessage( @@ -1014,7 +1014,7 @@ class TestConsumer( self._assert_first_last_send_progress() @override_settings(FILENAME_FORMAT="{title}") - @mock.patch("documents.consumer.get_parser_class_for_mime_type") + @mock.patch("documents.consumer.get_parser_registry") def test_similar_filenames(self, m) -> None: shutil.copy( Path(__file__).parent / "samples" / "simple.pdf", @@ -1028,7 +1028,7 @@ class TestConsumer( Path(__file__).parent / "samples" / "simple-noalpha.png", settings.CONSUMPTION_DIR / "simple.png.pdf", ) - m.return_value = CopyParser + m.return_value.get_parser_for_file.return_value = CopyParser with self.get_consumer(settings.CONSUMPTION_DIR / "simple.png") as consumer: consumer.run() @@ -1056,9 +1056,10 @@ class TestConsumer( sanity_check() - @mock.patch("documents.consumer.get_parser_class_for_mime_type", return_value=None) + @mock.patch("documents.consumer.get_parser_registry") @mock.patch("documents.consumer.run_subprocess") - def test_try_to_clean_invalid_pdf(self, m, _) -> None: + def test_try_to_clean_invalid_pdf(self, m, mock_registry) -> None: + mock_registry.return_value.get_parser_for_file.return_value = None shutil.copy( Path(__file__).parent / "samples" / "invalid_pdf.pdf", settings.CONSUMPTION_DIR / "invalid_pdf.pdf", @@ -1080,10 +1081,10 @@ class TestConsumer( @mock.patch("paperless_mail.models.MailRule.objects.get") @mock.patch("paperless.parsers.mail.MailDocumentParser.parse") - @mock.patch("documents.consumer.get_parser_class_for_mime_type") + @mock.patch("documents.consumer.get_parser_registry") def test_mail_parser_receives_mailrule( self, - mock_get_parser_class: mock.Mock, + mock_get_parser_registry: mock.Mock, mock_mail_parser_parse: mock.Mock, mock_mailrule_get: mock.Mock, ) -> None: @@ -1097,7 +1098,9 @@ class TestConsumer( """ from paperless.parsers.mail import MailDocumentParser - mock_get_parser_class.return_value = MailDocumentParser + mock_get_parser_registry.return_value.get_parser_for_file.return_value = ( + MailDocumentParser + ) mock_mailrule_get.return_value = mock.Mock( pdf_layout=MailRule.PdfLayout.HTML_ONLY, ) diff --git a/src/documents/tests/test_parsers.py b/src/documents/tests/test_parsers.py index fbd88adf0..30963df70 100644 --- a/src/documents/tests/test_parsers.py +++ b/src/documents/tests/test_parsers.py @@ -1,95 +1,16 @@ -from unittest import mock - from django.test import TestCase from django.test import override_settings from documents.parsers import get_default_file_extension -from documents.parsers import get_parser_class_for_mime_type from documents.parsers import get_supported_file_extensions from documents.parsers import is_file_ext_supported +from paperless.parsers.registry import get_parser_registry from paperless.parsers.registry import reset_parser_registry from paperless.parsers.tesseract import RasterisedDocumentParser from paperless.parsers.text import TextDocumentParser from paperless.parsers.tika import TikaDocumentParser -class TestParserDiscovery(TestCase): - def _mock_registry(self, return_value): - """Return a context manager that patches get_parser_registry.""" - mock_registry = mock.MagicMock() - mock_registry.get_parser_for_file.return_value = return_value - mock_registry.all_parsers.return_value = ( - [return_value] if return_value is not None else [] - ) - return mock.patch( - "documents.parsers.get_parser_registry", - return_value=mock_registry, - ) - - def test_get_parser_class_1_parser(self) -> None: - """ - GIVEN: - - Parser declared for a given mimetype - WHEN: - - Attempt to get parser for the mimetype - THEN: - - Declared parser class is returned - """ - - class DummyParser: - pass - - with self._mock_registry(DummyParser): - self.assertEqual( - get_parser_class_for_mime_type("application/pdf"), - DummyParser, - ) - - def test_get_parser_class_n_parsers(self) -> None: - """ - GIVEN: - - Two parsers declared for a given mimetype, second has higher score - WHEN: - - Attempt to get parser for the mimetype - THEN: - - Second (higher-score) parser class is returned - """ - - class DummyParser2: - pass - - # The registry already handles scoring; get_parser_for_file returns the winner. - with self._mock_registry(DummyParser2): - self.assertEqual( - get_parser_class_for_mime_type("application/pdf"), - DummyParser2, - ) - - def test_get_parser_class_0_parsers(self) -> None: - """ - GIVEN: - - No parsers are declared - WHEN: - - Attempt to get parser for the mimetype - THEN: - - No parser class is returned - """ - with self._mock_registry(None): - self.assertIsNone(get_parser_class_for_mime_type("application/pdf")) - - def test_get_parser_class_no_valid_parser(self) -> None: - """ - GIVEN: - - No parser declared for a given mimetype - WHEN: - - Attempt to get parser for the given mimetype - THEN: - - No parser class is returned - """ - with self._mock_registry(None): - self.assertIsNone(get_parser_class_for_mime_type("image/tiff")) - - class TestParserAvailability(TestCase): def test_tesseract_parser(self) -> None: """ @@ -114,7 +35,7 @@ class TestParserAvailability(TestCase): self.assertIn(ext, supported_exts) self.assertEqual(get_default_file_extension(mime_type), ext) self.assertIsInstance( - get_parser_class_for_mime_type(mime_type)(), + get_parser_registry().get_parser_for_file(mime_type, "")(), RasterisedDocumentParser, ) @@ -138,7 +59,7 @@ class TestParserAvailability(TestCase): self.assertIn(ext, supported_exts) self.assertEqual(get_default_file_extension(mime_type), ext) self.assertIsInstance( - get_parser_class_for_mime_type(mime_type)(), + get_parser_registry().get_parser_for_file(mime_type, "")(), TextDocumentParser, ) @@ -172,12 +93,12 @@ class TestParserAvailability(TestCase): self.assertIn(ext, supported_exts) self.assertEqual(get_default_file_extension(mime_type), ext) self.assertIsInstance( - get_parser_class_for_mime_type(mime_type)(), + get_parser_registry().get_parser_for_file(mime_type, "")(), TikaDocumentParser, ) def test_no_parser_for_mime(self) -> None: - self.assertIsNone(get_parser_class_for_mime_type("text/sdgsdf")) + self.assertIsNone(get_parser_registry().get_parser_for_file("text/sdgsdf", "")) def test_default_extension(self) -> None: # Test no parser declared still returns a an extension diff --git a/src/documents/views.py b/src/documents/views.py index 91568673b..298e4d03c 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -157,7 +157,6 @@ from documents.models import UiSettings from documents.models import Workflow from documents.models import WorkflowAction from documents.models import WorkflowTrigger -from documents.parsers import get_parser_class_for_mime_type from documents.permissions import AcknowledgeTasksPermissions from documents.permissions import PaperlessAdminPermissions from documents.permissions import PaperlessNotePermissions @@ -225,6 +224,7 @@ from paperless.celery import app as celery_app from paperless.config import AIConfig from paperless.config import GeneralConfig from paperless.models import ApplicationConfiguration +from paperless.parsers.registry import get_parser_registry from paperless.serialisers import GroupSerializer from paperless.serialisers import UserSerializer from paperless.views import StandardPagination @@ -1081,7 +1081,11 @@ class DocumentViewSet( if not Path(file).is_file(): return None - parser_class = get_parser_class_for_mime_type(mime_type) + parser_class = get_parser_registry().get_parser_for_file( + mime_type, + Path(file).name, + Path(file), + ) if parser_class: try: with parser_class() as parser: diff --git a/src/paperless/parsers/registry.py b/src/paperless/parsers/registry.py index 3c4740216..895c00fcf 100644 --- a/src/paperless/parsers/registry.py +++ b/src/paperless/parsers/registry.py @@ -351,7 +351,7 @@ class ParserRegistry: mime_type: The detected MIME type of the file. filename: - The original filename, including extension. + The original filename, including extension. May be empty in some cases path: Optional filesystem path to the file. Forwarded to each parser's score method.