diff --git a/src/documents/consumer.py b/src/documents/consumer.py index e7f026e71..319e68ef9 100644 --- a/src/documents/consumer.py +++ b/src/documents/consumer.py @@ -57,9 +57,7 @@ from paperless.models import ArchiveFileGenerationChoices from paperless.parsers import ParserContext from paperless.parsers import ParserProtocol from paperless.parsers.registry import get_parser_registry -from paperless.parsers.utils import PDF_TEXT_MIN_LENGTH -from paperless.parsers.utils import extract_pdf_text -from paperless.parsers.utils import is_tagged_pdf +from paperless.parsers.utils import pdf_born_digital_text LOGGING_NAME: Final[str] = "paperless.consumer" @@ -160,29 +158,20 @@ def should_produce_archive( _log.debug("Archive: yes — image document, ARCHIVE_FILE_GENERATION=auto") return True if mime_type == "application/pdf": - text = extract_pdf_text(document_path) - has_text = text is not None and len(text) > 0 - if has_text and is_tagged_pdf(document_path): + text, born_digital = pdf_born_digital_text(document_path, log=_log) + if born_digital: _log.debug( - "Archive: no — born-digital PDF (structure tags detected)," - " ARCHIVE_FILE_GENERATION=auto", - ) - return False - if text is None or len(text) <= PDF_TEXT_MIN_LENGTH: - _log.debug( - "Archive: yes — scanned PDF (text_length=%d ≤ %d)," + "Archive: no - born-digital PDF (text_length=%d)," " ARCHIVE_FILE_GENERATION=auto", len(text) if text else 0, - PDF_TEXT_MIN_LENGTH, ) - return True + return False _log.debug( - "Archive: no — born-digital PDF (text_length=%d > %d)," + "Archive: yes - scanned/textless PDF (text_length=%d)," " ARCHIVE_FILE_GENERATION=auto", - len(text), - PDF_TEXT_MIN_LENGTH, + len(text) if text else 0, ) - return False + return True _log.debug( "Archive: no — MIME type %r not eligible for auto archive generation", mime_type, diff --git a/src/documents/tests/test_consumer_archive.py b/src/documents/tests/test_consumer_archive.py index c179162db..6644f2ba8 100644 --- a/src/documents/tests/test_consumer_archive.py +++ b/src/documents/tests/test_consumer_archive.py @@ -134,60 +134,29 @@ class TestShouldProduceArchive: assert should_produce_archive(parser, mime, Path("/tmp/doc")) is expected @pytest.mark.parametrize( - ("extracted_text", "expected"), + ("born_digital", "expected"), [ - pytest.param( - "This is a born-digital PDF with lots of text content. " * 10, - False, - id="born-digital-long-text-skips-archive", - ), - pytest.param(None, True, id="no-text-scanned-produces-archive"), - pytest.param("tiny", True, id="short-text-treated-as-scanned"), + pytest.param(True, False, id="born-digital-skips-archive"), + pytest.param(False, True, id="not-born-digital-produces-archive"), ], ) def test_auto_pdf_archive_decision( self, mocker: MockerFixture, settings, - extracted_text: str | None, + born_digital: bool, # noqa: FBT001 expected: bool, # noqa: FBT001 ) -> None: + """should_produce_archive() defers entirely to pdf_born_digital_text() + for the has-real-text decision, so both callers of that predicate + (this function and RasterisedDocumentParser.parse()) always agree.""" settings.ARCHIVE_FILE_GENERATION = "auto" - mocker.patch("documents.consumer.is_tagged_pdf", return_value=False) - mocker.patch("documents.consumer.extract_pdf_text", return_value=extracted_text) + mocker.patch( + "documents.consumer.pdf_born_digital_text", + return_value=("some text", born_digital), + ) parser = _parser_instance(can_produce=True, requires_rendition=False) assert ( should_produce_archive(parser, "application/pdf", Path("/tmp/doc.pdf")) is expected ) - - def test_tagged_pdf_skips_archive_in_auto_mode( - self, - mocker: MockerFixture, - settings, - ) -> None: - """Tagged PDFs (e.g. Word exports) with real text are treated as born-digital, even below PDF_TEXT_MIN_LENGTH.""" - settings.ARCHIVE_FILE_GENERATION = "auto" - mocker.patch("documents.consumer.is_tagged_pdf", return_value=True) - mocker.patch("documents.consumer.extract_pdf_text", return_value="tiny") - parser = _parser_instance(can_produce=True, requires_rendition=False) - assert ( - should_produce_archive(parser, "application/pdf", Path("/tmp/doc.pdf")) - is False - ) - - def test_tagged_pdf_without_text_produces_archive( - self, - mocker: MockerFixture, - settings, - ) -> None: - """A tagged PDF with no actual extractable text (e.g. some scanner firmware) is not - trusted as born-digital — the tag alone must not bypass OCR.""" - settings.ARCHIVE_FILE_GENERATION = "auto" - mocker.patch("documents.consumer.is_tagged_pdf", return_value=True) - mocker.patch("documents.consumer.extract_pdf_text", return_value=None) - parser = _parser_instance(can_produce=True, requires_rendition=False) - assert ( - should_produce_archive(parser, "application/pdf", Path("/tmp/doc.pdf")) - is True - ) diff --git a/src/paperless/parsers/tesseract.py b/src/paperless/parsers/tesseract.py index 78ce4c5c4..393beca39 100644 --- a/src/paperless/parsers/tesseract.py +++ b/src/paperless/parsers/tesseract.py @@ -3,7 +3,6 @@ from __future__ import annotations import importlib.resources import logging import os -import re import shutil import tempfile from pathlib import Path @@ -25,9 +24,9 @@ from paperless.config import OcrConfig from paperless.models import CleanChoices from paperless.models import ModeChoices from paperless.models import OutputTypeChoices -from paperless.parsers.utils import PDF_TEXT_MIN_LENGTH from paperless.parsers.utils import extract_pdf_text -from paperless.parsers.utils import is_tagged_pdf +from paperless.parsers.utils import pdf_born_digital_text +from paperless.parsers.utils import post_process_text from paperless.parsers.utils import read_file_handle_unicode_errors from paperless.version import __full_version_str__ @@ -509,11 +508,9 @@ class RasterisedDocumentParser: from ocrmypdf.exceptions import PriorOcrFoundError if mime_type == "application/pdf": - text_original = self.extract_text(None, document_path) - has_text = text_original is not None and len(text_original) > 0 - original_has_text = has_text and ( - is_tagged_pdf(document_path, log=self.log) - or len(text_original) > PDF_TEXT_MIN_LENGTH + text_original, original_has_text = pdf_born_digital_text( + document_path, + log=self.log, ) else: text_original = None @@ -658,17 +655,3 @@ class RasterisedDocumentParser: f"No text was found in {document_path}, the content will be empty.", ) self.text = "" - - -def post_process_text(text: str | None) -> str | None: - if not text: - return None - - collapsed_spaces = re.sub(r"([^\S\r\n]+)", " ", text) - no_leading_whitespace = re.sub(r"([\n\r]+)([^\S\n\r]+)", "\\1", collapsed_spaces) - no_trailing_whitespace = re.sub(r"([^\S\n\r]+)$", "", no_leading_whitespace) - - # TODO: this needs a rework - # replace \0 prevents issues with saving to postgres. - # text may contain \0 when this character is present in PDF files. - return no_trailing_whitespace.strip().replace("\0", " ") diff --git a/src/paperless/parsers/utils.py b/src/paperless/parsers/utils.py index 0257ab736..9b4590c21 100644 --- a/src/paperless/parsers/utils.py +++ b/src/paperless/parsers/utils.py @@ -111,6 +111,62 @@ def extract_pdf_text( return None +def post_process_text(text: str | None) -> str | None: + """Normalize extracted PDF/OCR text: collapse whitespace, strip padding. + + Returns ``None`` for ``None`` or whitespace-only input, so callers can + treat "no text" and "only layout padding" the same way. + """ + if not text: + return None + + collapsed_spaces = re.sub(r"([^\S\r\n]+)", " ", text) + no_leading_whitespace = re.sub(r"([\n\r]+)([^\S\n\r]+)", "\\1", collapsed_spaces) + no_trailing_whitespace = re.sub(r"([^\S\n\r]+)$", "", no_leading_whitespace) + + # replace \0 prevents issues with saving to postgres. + # text may contain \0 when this character is present in PDF files. + result = no_trailing_whitespace.strip().replace("\0", " ") + return result or None + + +def pdf_born_digital_text( + path: Path, + log: logging.Logger | None = None, +) -> tuple[str | None, bool]: + """Extract a PDF's text and decide whether it should be treated as born-digital. + + This is the single source of truth for "does this PDF already have real + text", used both to decide whether to produce an archive file and to + decide whether OCR can be skipped. Both decisions must agree, or a + tagged-but-textless PDF can end up with no archive AND a forced OCR pass + (see GH #13387): raw ``pdftotext -layout`` output can be non-empty + (whitespace/form-feed padding) even when there is no real content, so the + "has text" check must run against the *normalized* text, not the raw + extraction. + + Parameters + ---------- + path: + Absolute path to the PDF file. + log: + Logger for warnings. Falls back to the module-level logger when omitted. + + Returns + ------- + tuple[str | None, bool] + The normalized extracted text (or ``None``), and whether the PDF + counts as born-digital (has real text, and is either tagged or + exceeds ``PDF_TEXT_MIN_LENGTH``). + """ + text = post_process_text(extract_pdf_text(path, log=log)) + has_text = text is not None and len(text) > 0 + born_digital = has_text and ( + is_tagged_pdf(path, log=log) or len(text) > PDF_TEXT_MIN_LENGTH + ) + return text, born_digital + + def read_file_handle_unicode_errors( filepath: Path, log: logging.Logger | None = None, diff --git a/src/paperless/tests/parsers/conftest.py b/src/paperless/tests/parsers/conftest.py index 843ffdb88..61a3ac3ef 100644 --- a/src/paperless/tests/parsers/conftest.py +++ b/src/paperless/tests/parsers/conftest.py @@ -674,6 +674,21 @@ def single_page_mixed_pdf_file(tesseract_samples_dir: Path) -> Path: return tesseract_samples_dir / "single-page-mixed.pdf" +@pytest.fixture(scope="session") +def tagged_no_text_pdf_file(tesseract_samples_dir: Path) -> Path: + """Path to a tagged PDF whose only "text" is pdftotext layout padding. + + Reproduces GH #13387: ``/MarkInfo /Marked true`` is set, but the only + extractable content is a form-feed byte, not real text. + + Returns + ------- + Path + Absolute path to ``tesseract/tagged-but-no-text.pdf``. + """ + return tesseract_samples_dir / "tagged-but-no-text.pdf" + + @pytest.fixture(scope="session") def with_form_pdf_file(tesseract_samples_dir: Path) -> Path: """Path to a PDF with form sample file. diff --git a/src/paperless/tests/parsers/test_tesseract_parser.py b/src/paperless/tests/parsers/test_tesseract_parser.py index e56992d0b..f25efccc7 100644 --- a/src/paperless/tests/parsers/test_tesseract_parser.py +++ b/src/paperless/tests/parsers/test_tesseract_parser.py @@ -21,7 +21,7 @@ from documents.parsers import run_convert from paperless.models import ModeChoices from paperless.parsers import ParserProtocol from paperless.parsers.tesseract import RasterisedDocumentParser -from paperless.parsers.tesseract import post_process_text +from paperless.parsers.utils import is_tagged_pdf if TYPE_CHECKING: from pathlib import Path @@ -151,36 +151,6 @@ class TestRasterisedDocumentParserLifecycle: assert tempdir is not None and not tempdir.exists() -# --------------------------------------------------------------------------- -# post_process_text -# --------------------------------------------------------------------------- - - -class TestPostProcessText: - @pytest.mark.parametrize( - ("source", "expected"), - [ - pytest.param( - "simple string", - "simple string", - id="collapse-spaces", - ), - pytest.param( - "simple newline\n testing string", - "simple newline\ntesting string", - id="preserve-newline", - ), - pytest.param( - "utf-8 строка с пробелами в конце ", # noqa: RUF001 - "utf-8 строка с пробелами в конце", # noqa: RUF001 - id="utf8-trailing-spaces", - ), - ], - ) - def test_post_process_text(self, source: str, expected: str) -> None: - assert post_process_text(source) == expected - - # --------------------------------------------------------------------------- # Page count # --------------------------------------------------------------------------- @@ -910,25 +880,25 @@ class TestSkipArchive: self, mocker: MockerFixture, tesseract_parser: RasterisedDocumentParser, - tesseract_samples_dir: Path, + tagged_no_text_pdf_file: Path, ) -> None: """ GIVEN: - - A PDF that reports itself as tagged (/MarkInfo /Marked true) but - has no actual extractable text (some scanner firmware produces - this — see GitHub issue #13349) + - A real PDF that reports itself as tagged (/MarkInfo /Marked + true) but whose only pdftotext output is layout padding (a + lone form-feed byte), not real text (see GitHub issue #13387, + originally reported against #13349's tagged-PDF handling) - Mode: auto, produce_archive=False WHEN: - Document is parsed THEN: - The tag alone is not trusted as "has text"; OCRmyPDF still runs """ + assert is_tagged_pdf(tagged_no_text_pdf_file) is True tesseract_parser.settings.mode = ModeChoices.AUTO - mocker.patch("paperless.parsers.tesseract.is_tagged_pdf", return_value=True) - mocker.patch.object(tesseract_parser, "extract_text", return_value=None) mock_ocr = mocker.patch("ocrmypdf.ocr") tesseract_parser.parse( - tesseract_samples_dir / "multi-page-images.pdf", + tagged_no_text_pdf_file, "application/pdf", produce_archive=False, ) diff --git a/src/paperless/tests/samples/tesseract/tagged-but-no-text.pdf b/src/paperless/tests/samples/tesseract/tagged-but-no-text.pdf new file mode 100644 index 000000000..5a652345a Binary files /dev/null and b/src/paperless/tests/samples/tesseract/tagged-but-no-text.pdf differ diff --git a/src/paperless/tests/test_parser_utils.py b/src/paperless/tests/test_parser_utils.py index c6bb3e34a..ee0027ced 100644 --- a/src/paperless/tests/test_parser_utils.py +++ b/src/paperless/tests/test_parser_utils.py @@ -4,10 +4,18 @@ from __future__ import annotations import codecs from pathlib import Path +from typing import TYPE_CHECKING + +import pytest from paperless.parsers.utils import is_tagged_pdf +from paperless.parsers.utils import pdf_born_digital_text +from paperless.parsers.utils import post_process_text from paperless.parsers.utils import read_file_handle_unicode_errors +if TYPE_CHECKING: + from pytest_mock import MockerFixture + SAMPLES = Path(__file__).parent / "samples" / "tesseract" @@ -60,3 +68,95 @@ class TestIsTaggedPdf: bad = tmp_path / "bad.pdf" bad.write_bytes(b"not a pdf") assert is_tagged_pdf(bad) is False + + +class TestPostProcessText: + @pytest.mark.parametrize( + ("source", "expected"), + [ + pytest.param( + "simple string", + "simple string", + id="collapse-spaces", + ), + pytest.param( + "simple newline\n testing string", + "simple newline\ntesting string", + id="preserve-newline", + ), + pytest.param( + "utf-8 строка с пробелами в конце ", # noqa: RUF001 + "utf-8 строка с пробелами в конце", # noqa: RUF001 + id="utf8-trailing-spaces", + ), + pytest.param(None, None, id="none-input"), + pytest.param("", None, id="empty-string"), + pytest.param(" \n\x0c \n ", None, id="whitespace-and-formfeed-only"), + ], + ) + def test_post_process_text( + self, + source: str | None, + expected: str | None, + ) -> None: + assert post_process_text(source) == expected + + +class TestPdfBornDigitalText: + """Regression coverage for GH #13387. + + should_produce_archive() and RasterisedDocumentParser.parse() must agree + on whether a PDF has real text, so both go through this one function. + """ + + def test_tagged_pdf_with_real_text_is_born_digital( + self, + mocker: MockerFixture, + tmp_path: Path, + ) -> None: + mocker.patch( + "paperless.parsers.utils.extract_pdf_text", + return_value="tiny", + ) + mocker.patch("paperless.parsers.utils.is_tagged_pdf", return_value=True) + text, born_digital = pdf_born_digital_text(tmp_path / "doc.pdf") + assert text == "tiny" + assert born_digital is True + + def test_untagged_pdf_below_min_length_is_not_born_digital( + self, + mocker: MockerFixture, + tmp_path: Path, + ) -> None: + mocker.patch( + "paperless.parsers.utils.extract_pdf_text", + return_value="tiny", + ) + mocker.patch("paperless.parsers.utils.is_tagged_pdf", return_value=False) + text, born_digital = pdf_born_digital_text(tmp_path / "doc.pdf") + assert text == "tiny" + assert born_digital is False + + def test_untagged_pdf_above_min_length_is_born_digital( + self, + mocker: MockerFixture, + tmp_path: Path, + ) -> None: + mocker.patch( + "paperless.parsers.utils.extract_pdf_text", + return_value="x" * 51, + ) + mocker.patch("paperless.parsers.utils.is_tagged_pdf", return_value=False) + _text, born_digital = pdf_born_digital_text(tmp_path / "doc.pdf") + assert born_digital is True + + def test_no_text_is_not_born_digital( + self, + mocker: MockerFixture, + tmp_path: Path, + ) -> None: + mocker.patch("paperless.parsers.utils.extract_pdf_text", return_value=None) + mocker.patch("paperless.parsers.utils.is_tagged_pdf", return_value=True) + text, born_digital = pdf_born_digital_text(tmp_path / "doc.pdf") + assert text is None + assert born_digital is False