mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-07-29 07:14:56 +00:00
Fix: unify born-digital PDF detection between archive decision and OCR
should_produce_archive() and RasterisedDocumentParser.parse() each reimplemented the "does this PDF have real text" check independently, using different normalization of pdftotext output. Raw pdftotext output can be non-empty (whitespace/form-feed layout padding) even when there is no real content, so the two checks could disagree: consumer.py treated a tagged-but-textless PDF as born-digital and skipped the archive, while the parser's own (stricter, normalized) check found no text and ran OCR anyway, leaving the document with no archive despite real OCR text (GH #13387). Both call sites now share one predicate, pdf_born_digital_text() in paperless/parsers/utils.py, so they can no longer drift apart.
This commit is contained in:
stumpylog
@@ -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,
|
||||
|
||||
@@ -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
|
||||
)
|
||||
|
||||
@@ -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", " ")
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
Binary file not shown.
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user