diff --git a/Dockerfile b/Dockerfile index d45922b11..9e128f5ef 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,10 +6,18 @@ COPY --from=ghcr.io/astral-sh/uv:0.11.31@sha256:ecd4de2f060c64bea0ff8ecb182ddf46 # 1. curl (required for container healthcheck probes) # 2. git (required for caldav dependency from git) # 3. sqlite for development with token db +# 4. libreoffice-writer/-calc: the only readers for legacy .doc/.xls, and the +# renderer behind the .doc/.docx PDF rendition that gives those formats page +# numbers and highlight geometry. Writer and Calc specifically, not the +# metapackage -- Impress/Draw/Base would roughly double the layer for +# formats we do not index. The code degrades to "no processor for type" when +# the binary is absent, so a slimmer image variant stays possible. RUN apt update && apt install --no-install-recommends --no-install-suggests -y \ curl \ git \ tesseract-ocr \ + libreoffice-writer \ + libreoffice-calc \ sqlite3 && apt clean # Build in /src, run in /app, keep the venv in /opt/venv. The three have diff --git a/docs/configuration.md b/docs/configuration.md index 95781abd3..827151e80 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -723,6 +723,7 @@ shorter OCR ceiling: ```dotenv DOCUMENT_PARSE_TIMEOUT_SECONDS=120 # Wall-clock cap per isolated parse (default: 120) +DOCUMENT_OFFICE_TIMEOUT_SECONDS=120 # Wall-clock cap per LibreOffice conversion: .doc/.docx -> pdf, .xls -> xlsx (default: 120) DOCUMENT_OCR_TIMEOUT_SECONDS=180 # OCR backend request timeout (default: 180) DOCUMENT_MAX_PDF_SIZE_MB=50 # Pre-parse size cap; 0 disables (default: 50) DOCUMENT_PARSE_PAGE_WINDOW=100 # Pages per extraction window; 0 disables (default: 100) @@ -751,6 +752,18 @@ A PDF larger than `DOCUMENT_MAX_PDF_SIZE_MB` fails fast with reason `oversize` of being handed to the tiers, where a 40+ MB scan would otherwise burn the full OCR timeout for zero recovered text. +The same cap is applied twice to a `.doc`/`.docx`: once to the source before it +is downloaded, and again to the PDF LibreOffice renders from it. Rendering is +not size-preserving — a modest document of dense vector figures can render much +larger — so a source under the cap can still produce a rendition over it, and +that rendition is what the parse tiers would have to hold. + +A LibreOffice conversion also pays `DOCUMENT_OFFICE_TIMEOUT_SECONDS` before the +parse cap applies, and it takes a slot from `DOCUMENT_PARSE_PROCESS_SLOTS` while +it runs: `soffice` holds the source and the rendered output at once, so it is +bounded by the same limiter as the parse workers rather than being allowed to +start one process per concurrent ingest task. + **Sizing the cap for a tenant.** Two metrics make the corpus visible instead of requiring a manual crawl: diff --git a/nextcloud_mcp_server/config.py b/nextcloud_mcp_server/config.py index 1a522c0ae..6e9e93a2a 100644 --- a/nextcloud_mcp_server/config.py +++ b/nextcloud_mcp_server/config.py @@ -248,6 +248,11 @@ # PDF parse isolation (OOM guard) "document_pdf_graphics_limit": 1000, "document_parse_timeout_seconds": 120.0, + # Wall-clock cap on one LibreOffice conversion (.doc/.docx -> pdf, + # .xls -> xlsx). Separate from document_parse_timeout_seconds: a rendition + # pays this first and then the parse cap, and the two are tuned against + # different costs (page layout vs. table detection). + "document_office_timeout_seconds": 120.0, # Optional wall-clock cap (seconds) on the SYNCHRONOUS parse inside the # nc_webdav_read_file MCP tool. None (default) = disabled: an interactive read # is bounded only by the underlying processor timeout (DOCLING_TIMEOUT / @@ -603,6 +608,11 @@ def _resolve_settings_files() -> list[str]: Validator("CHUNKING_CONFIG_VERSION", gte=1), Validator("DOCUMENT_PARSE_TIMEOUT_SECONDS", gte=1), Validator("DOCUMENT_OCR_TIMEOUT_SECONDS", gte=1), + # Same floor as its siblings: 0 or negative reaches anyio.fail_after in + # _libreoffice.convert and expires every conversion immediately, so the + # misconfiguration surfaces as "every .doc/.docx fails to parse" rather + # than as a startup error naming the setting. + Validator("DOCUMENT_OFFICE_TIMEOUT_SECONDS", gte=1), # DOCUMENT_OCR_MODE is normalised + membership-checked in # Settings.__post_init__ via _enum_fields (case-insensitive, like # DOCUMENT_OCR_PROVIDER) — no strict dynaconf Validator here, so @@ -1235,6 +1245,11 @@ class Settings: # float so a fractional DOCUMENT_PARSE_TIMEOUT_SECONDS is honoured, matching # anyio.move_on_after's float seconds. document_parse_timeout_seconds: float = 120.0 + # Wall-clock cap on one LibreOffice conversion (.doc/.docx -> pdf, + # .xls -> xlsx). Separate from document_parse_timeout_seconds because a + # rendition pays this first and the parse cap afterwards, and the two bound + # different costs -- page layout vs. table detection. + document_office_timeout_seconds: float = 120.0 # Optional cap (seconds) on the synchronous parse in the nc_webdav_read_file # tool. None = disabled (bounded only by the processor timeout). When set, # anyio.fail_after aborts a slow interactive convert and the tool returns diff --git a/nextcloud_mcp_server/document_processors/__init__.py b/nextcloud_mcp_server/document_processors/__init__.py index 80360f53c..e5c1be1dd 100644 --- a/nextcloud_mcp_server/document_processors/__init__.py +++ b/nextcloud_mcp_server/document_processors/__init__.py @@ -1,12 +1,20 @@ """Document processing plugins for extracting text from various file formats.""" +import logging + from nextcloud_mcp_server.config import get_settings +from . import _libreoffice from .base import DocumentProcessor, ProcessingResult, ProcessorError +from .msg import MsgProcessor from .ocr import OcrProcessor +from .office import OfficeDocumentProcessor from .pymupdf import PyMuPDFProcessor from .pypdfium2_fast import Pypdfium2FastProcessor from .registry import ProcessorRegistry, get_registry +from .spreadsheet import SpreadsheetProcessor + +logger = logging.getLogger(__name__) # Register processors at module initialization. The tiered PDF pipeline selects # by tier (not priority): Pypdfium2FastProcessor is the ``fast`` tier, @@ -37,13 +45,33 @@ priority=1, ) +# Office and email formats. Priority 15 puts them above the optional +# Unstructured processor (10), which also claims these types but flattens their +# structure, while leaving 20 for docling. Each reads its format the way that +# measured best rather than sharing one route -- see the module docstrings. +_office_timeout = _settings.document_office_timeout_seconds +_registry.register(SpreadsheetProcessor(timeout=_office_timeout), priority=15) +_registry.register(MsgProcessor(), priority=15) +if _libreoffice.LIBREOFFICE_AVAILABLE: + _registry.register(OfficeDocumentProcessor(timeout=_office_timeout), priority=15) +else: + # Not an error: the API image has no reason to carry LibreOffice. Leaving + # the processor unregistered means .doc/.docx report "no processor for + # type" once, rather than every document failing mid-parse. + logger.info( + "LibreOffice not found; .doc/.docx indexing is unavailable in this image" + ) + __all__ = [ "DocumentProcessor", "ProcessingResult", "ProcessorError", "ProcessorRegistry", "get_registry", + "MsgProcessor", + "OfficeDocumentProcessor", "PyMuPDFProcessor", "Pypdfium2FastProcessor", "OcrProcessor", + "SpreadsheetProcessor", ] diff --git a/nextcloud_mcp_server/document_processors/_libreoffice.py b/nextcloud_mcp_server/document_processors/_libreoffice.py new file mode 100644 index 000000000..d45aea223 --- /dev/null +++ b/nextcloud_mcp_server/document_processors/_libreoffice.py @@ -0,0 +1,145 @@ +"""LibreOffice headless conversion, used to reach formats nothing else reads. + +Legacy binary Office documents (``.doc``, ``.xls``) have no working pure-Python +reader: markitdown raises ``UnsupportedFormatException`` and docling-serve +returns ``status=failure``. LibreOffice is the only thing that opens them, and +it is already present in several of our images. + +Two conversion targets, chosen per source format rather than uniformly: + +* ``pdf`` -- for word-processor documents, whose page layout *is* their + structure. The rendition is a real PDF, so the existing fast/structured + ladder, ``page_boundaries`` and ``pdf_highlighter`` all work on it unchanged. +* ``xlsx`` -- for legacy spreadsheets, which are then read cell-by-cell. A + spreadsheet must NOT go to PDF: measured on a real workbook, PDF rendering + paginated it into 25 mixed-orientation pages and recalled only 63.8% of the + tokens a direct cell read recovers, because LibreOffice honours the print + layout and drops whatever falls outside it. +""" + +import logging +import pathlib +import shutil +import tempfile +from typing import Final + +import anyio + +from nextcloud_mcp_server.config import get_settings + +from ._isolation import parse_process_limiter + +logger = logging.getLogger(__name__) + +# Resolved once at import, like TesseractProcessor's availability probe, so a +# deployment without LibreOffice degrades to "no processor for this type" +# rather than failing per document. +SOFFICE_BIN: Final[str | None] = shutil.which("soffice") or shutil.which("libreoffice") +LIBREOFFICE_AVAILABLE: Final[bool] = SOFFICE_BIN is not None + + +class LibreOfficeError(Exception): + """Raised when a LibreOffice conversion fails or produces no output.""" + + +async def convert( + content: bytes, + filename: str, + target: str, + timeout_seconds: float = 120.0, +) -> bytes: + """Convert ``content`` to ``target`` ("pdf" / "xlsx") and return the bytes. + + The cap is taken as a value rather than left to the caller's own + ``fail_after`` because expiry is translated here into a + :class:`LibreOfficeError`: callers already handle that one exception type, + and a bare ``TimeoutError`` escaping into the processor would bypass the + "conversion failed" path each of them implements. + + Args: + content: Source document bytes. + filename: Source filename -- LibreOffice picks its import filter from + the extension, so a name without one converts as the wrong format. + target: LibreOffice output filter name. + timeout_seconds: Wall-clock cap; the process is killed past it. + + Raises: + LibreOfficeError: LibreOffice is absent, exits non-zero, times out, or + writes no output file. + """ + if SOFFICE_BIN is None: + raise LibreOfficeError("LibreOffice (soffice) is not installed") + + suffix = pathlib.Path(filename).suffix + if not suffix: + raise LibreOfficeError( + f"cannot convert {filename!r}: no extension to select an import filter" + ) + + with tempfile.TemporaryDirectory(prefix="lo-convert-") as tmp: + tmpdir = pathlib.Path(tmp) + src = tmpdir / f"source{suffix}" + src.write_bytes(content) + outdir = tmpdir / "out" + outdir.mkdir() + + # -env:UserInstallation gives this invocation a private profile + # directory. Without it every concurrent soffice shares one profile and + # the second one either blocks on the lock or exits 0 having written + # nothing -- which would surface as a random empty-output failure under + # parallel ingest rather than as anything diagnosable. + profile = tmpdir / "profile" + argv = [ + SOFFICE_BIN, + f"-env:UserInstallation=file://{profile}", + "--headless", + "--norestore", + "--convert-to", + target, + "--outdir", + str(outdir), + str(src), + ] + + # Bounded by the same limiter as the isolated PDF parse. A LibreOffice + # process is the heaviest thing the ingest path spawns -- it holds the + # source document and the rendered output at once -- so leaving it + # unbounded would let a folder of .doc files start one soffice per + # concurrent task and exhaust the pod's memory, which is exactly what + # that limiter exists to prevent for the (lighter) parse workers. + # Acquired and released here, before the delegated parse acquires it in + # turn, so the two never nest. + settings = get_settings() + limiter = parse_process_limiter(settings.document_parse_process_slots) + try: + async with limiter: + with anyio.fail_after(timeout_seconds): + result = await anyio.run_process(argv, check=False) + except TimeoutError as exc: + raise LibreOfficeError( + f"LibreOffice timed out after {timeout_seconds}s converting {filename!r}" + ) from exc + + if result.returncode != 0: + stderr = result.stderr.decode("utf-8", "replace").strip()[:500] + raise LibreOfficeError( + f"LibreOffice exited {result.returncode} for {filename!r}: {stderr}" + ) + + # soffice exits 0 on an unreadable input while writing nothing, so the + # output file's existence -- not the return code -- is the real check. + produced = sorted(outdir.iterdir()) + if not produced: + raise LibreOfficeError( + f"LibreOffice produced no {target} output for {filename!r}" + ) + + data = produced[0].read_bytes() + logger.debug( + "LibreOffice converted %s (%d bytes) -> %s (%d bytes)", + filename, + len(content), + target, + len(data), + ) + return data diff --git a/nextcloud_mcp_server/document_processors/_msg_reader.py b/nextcloud_mcp_server/document_processors/_msg_reader.py new file mode 100644 index 000000000..cc25d7ee1 --- /dev/null +++ b/nextcloud_mcp_server/document_processors/_msg_reader.py @@ -0,0 +1,194 @@ +"""A small Outlook ``.msg`` reader built directly on olefile. + +Why not ``extract-msg``, which does this properly: it depends on +``red-black-tree-mod``, which PyPI publishes as an **sdist only**. The image +build runs ``uv sync --no-build`` so that no dependency's ``setup.py`` executes +at build time (docker:S8541), and that guarantee is worth more than the library. +Reading the handful of fields we index is about eighty lines. + +A ``.msg`` is an OLE2 compound file. Each property lives in a stream named +``__substg1.0_``, where TYPE is ``001F`` for UTF-16LE text, ``001E`` +for text in the message's code page, and ``0102`` for binary. Fixed-size +properties (dates, the code page itself) live packed in +``__properties_version1.0`` instead. + +Reading **both** string variants is the point. markitdown's converter yields 27 +bytes -- the literal ``# Email Message\\n\\n## Content`` -- on a real 1.3 MB +thread, because it looks for a body variant that message does not carry. The +same message holds a 14,996-byte plain-text body in the ``001E`` variant. + +**Scope cut worth knowing:** only the plain-text body property (``1000``) is +read. Outlook writes it alongside the HTML (``1013``) and compressed-RTF +(``1009``) bodies for every message observed here, so nothing was lost -- but a +client that wrote *solely* an HTML or RTF body would be indexed with its headers +and an empty body. If that shows up, ``1013`` is plain HTML and can go through +``html_to_markdown``; ``1009`` needs the MS-OXRTFCP decompression that +``compressed-rtf`` implements, which is the point at which a dependency is +easier than more code here. +""" + +import logging +import struct +from datetime import datetime, timedelta, timezone +from typing import Any, BinaryIO + +import olefile + +logger = logging.getLogger(__name__) + +# Property tags (the high 16 bits of a MAPI property tag), as hex text because +# that is how they appear in the stream names. +TAG_SUBJECT = "0037" +TAG_SENDER_NAME = "0C1A" +TAG_SENDER_SMTP = "5D01" +TAG_TO = "0E04" +TAG_CC = "0E03" +TAG_BODY = "1000" +TAG_ATTACH_LONG_NAME = "3707" +TAG_ATTACH_SHORT_NAME = "3704" + +# Fixed-size properties, read from __properties_version1.0 as (tag, type). +PROP_CLIENT_SUBMIT_TIME = 0x0039 +PROP_MESSAGE_DELIVERY_TIME = 0x0E06 +PROP_INTERNET_CPID = 0x3FDE +PT_SYSTIME = 0x0040 +PT_LONG = 0x0003 + +_PROPERTIES_STREAM = "__properties_version1.0" +# FILETIME counts 100-nanosecond intervals from 1601-01-01 UTC. +_FILETIME_EPOCH = datetime(1601, 1, 1, tzinfo=timezone.utc) +# Fallback when the message declares no code page. cp1252 rather than utf-8: +# these streams predate utf-8's ubiquity and a mis-decode of Western punctuation +# is far more likely than of multibyte text. +_DEFAULT_ENCODING = "cp1252" + + +class MsgReadError(Exception): + """Raised when the container is not a readable Outlook message.""" + + +def _stream_bytes(ole: olefile.OleFileIO, path: list[str]) -> bytes | None: + """One stream's contents, or None when it is absent.""" + if not ole.exists("/".join(path)): + return None + with ole.openstream(path) as stream: + return stream.read() + + +def _decode(raw: bytes, unicode_variant: bool, encoding: str) -> str: + if unicode_variant: + return raw.decode("utf-16-le", errors="replace").rstrip("\x00") + return raw.decode(encoding, errors="replace").rstrip("\x00") + + +def _string_property( + ole: olefile.OleFileIO, tag: str, encoding: str, prefix: list[str] | None = None +) -> str | None: + """A string property, trying the UTF-16 variant then the code-page one. + + Both are tried because a message carries whichever its sender's client + wrote, and assuming one is exactly the bug that makes other readers return + an empty body. + """ + base = list(prefix or []) + for suffix, is_unicode in (("001F", True), ("001E", False)): + raw = _stream_bytes(ole, base + [f"__substg1.0_{tag}{suffix}"]) + if raw: + text = _decode(raw, is_unicode, encoding) + if text: + return text + return None + + +def _fixed_properties(ole: olefile.OleFileIO) -> dict[int, bytes]: + """Map ``(tag << 16 | type)`` to each fixed property's raw 8-byte value. + + The stream is a header followed by 16-byte entries: 4 bytes of property tag, + 4 of flags, 8 of value. The header is 32 bytes for a top-level message. + """ + raw = _stream_bytes(ole, [_PROPERTIES_STREAM]) + if not raw or len(raw) < 32: + return {} + entries: dict[int, bytes] = {} + for offset in range(32, len(raw) - 15, 16): + (prop_tag,) = struct.unpack_from(" datetime | None: + (ticks,) = struct.unpack(" list[str]: + """Filenames of the attached items, in storage order.""" + storages = sorted( + { + entry[0] + for entry in ole.listdir() + if entry and entry[0].startswith("__attach_version1.0") + } + ) + names = [] + for storage in storages: + name = _string_property( + ole, TAG_ATTACH_LONG_NAME, encoding, prefix=[storage] + ) or _string_property(ole, TAG_ATTACH_SHORT_NAME, encoding, prefix=[storage]) + names.append(name or "unnamed") + return names + + +def read_msg(stream: BinaryIO) -> dict[str, Any]: + """Read an Outlook message into ``{subject, sender, to, cc, date, body, + attachments}``. + + Every field is optional: ``.msg`` is also how Outlook saves contacts, tasks + and calendar items, and those carry no sender or body. A missing field comes + back as ``None`` rather than raising, so such an item is still indexed by + whatever it does have. + """ + if not olefile.isOleFile(stream): + raise MsgReadError("not an OLE2 compound file") + stream.seek(0) + + ole = olefile.OleFileIO(stream) + try: + props = _fixed_properties(ole) + + encoding = _DEFAULT_ENCODING + cpid_value = props.get(PROP_INTERNET_CPID << 16 | PT_LONG) + if cpid_value: + (codepage,) = struct.unpack_from(" str: + return "msg" + + @property + def tier(self) -> str: + return "fast" + + @property + def supported_mime_types(self) -> set[str]: + return MSG_MIME_TYPES + + async def process( + self, + content: bytes, + content_type: str, + filename: Optional[str] = None, + options: Optional[dict[str, Any]] = None, + progress_callback: Optional[ + Callable[[float, Optional[float], Optional[str]], Awaitable[None]] + ] = None, + ) -> ProcessingResult: + try: + text, metadata = await run_sync(_extract_msg, content) + except ProcessorError: + raise + except Exception as exc: + raise ProcessorError(f"Outlook message parse failed: {exc}") from exc + + metadata["text_length"] = len(text) + metadata["parse_mode"] = "markdown" + return ProcessingResult( + text=text, + metadata=metadata, + processor=self.name, + success=True, + ) + + async def health_check(self) -> bool: + try: + import olefile # noqa: F401, PLC0415 + except ImportError: + return False + return True + + +def _header_line(label: str, value: Any) -> str | None: + """``**Label:** value`` when there is a value, else nothing.""" + if value in (None, ""): + return None + return f"**{label}:** {value}" + + +def _extract_msg(content: bytes) -> tuple[str, dict[str, Any]]: + """Message body plus its envelope. Runs in a worker thread.""" + message = read_msg(io.BytesIO(content)) + + attachments = message["attachments"] + date = message["date"] + body = message["body"] or "" + + # The envelope is indexed as part of the text, not just held as metadata: + # "who sent this and when" is a large share of what anyone searches an + # inbox for, and metadata is not embedded. + header_lines = [ + line + for line in ( + _header_line("From", message["sender"]), + _header_line("To", message["to"]), + _header_line("Cc", message["cc"]), + _header_line("Date", date.isoformat() if date else None), + _header_line("Attachments", ", ".join(attachments)), + ) + if line + ] + heading = f"# {message['subject'] or '(no subject)'}\n\n" + text = heading + "\n".join(header_lines) + f"\n\n{body.strip()}\n" + + metadata: dict[str, Any] = { + "subject": message["subject"], + "sender": message["sender"], + "date": date.isoformat() if date else None, + ATTACHMENT_NAMES_KEY: attachments, + "attachment_count": len(attachments), + } + return text, metadata diff --git a/nextcloud_mcp_server/document_processors/office.py b/nextcloud_mcp_server/document_processors/office.py new file mode 100644 index 000000000..6db7415a3 --- /dev/null +++ b/nextcloud_mcp_server/document_processors/office.py @@ -0,0 +1,137 @@ +"""Word-processor documents, read through a LibreOffice PDF rendition.""" + +import logging +from collections.abc import Awaitable, Callable +from typing import Any, Optional + +from . import _libreoffice +from .base import DocumentProcessor, ProcessingResult, ProcessorError +from .pymupdf import PyMuPDFProcessor + +logger = logging.getLogger(__name__) + +DOC_MIME_TYPES = { + "application/msword", # legacy .doc (OLE2) + "application/vnd.openxmlformats-officedocument.wordprocessingml.document", +} + +# Metadata key naming the source format a rendition was produced from. Its +# presence is what tells downstream consumers that the geometry in this result +# (page_boundaries, and later the chunk bboxes derived from them) belongs to a +# derived PDF rather than to the file the user has in Nextcloud -- so a viewer +# must fetch the rendition, not the original path. +RENDERED_FROM_KEY = "rendered_from_mime" + + +class OfficeDocumentProcessor(DocumentProcessor): + """Extract ``.doc``/``.docx`` by rendering to PDF, then parsing that. + + Why a rendition rather than reading the document directly: + + * Legacy ``.doc`` has no working pure-Python reader at all. + * For ``.docx`` the rendition is *more* faithful, not less. Measured against + a direct ``mammoth`` parse on a 4-column questionnaire, the rendition + recalls 98.3% of the same tokens and reproduces 22 table rows against 17 + -- because ``mammoth``'s HTML drops a vertically-merged cell and shifts + every remaining cell in that row one column left, so answers land under + the wrong heading. Rendering resolves merged cells the way a reader sees + them. + * The rendition is a real PDF, so page numbers and highlight geometry come + from the existing PDF path instead of needing a second implementation. + + The trade-off is that hyperlink targets are lost -- LibreOffice renders the + display text and drops the href. + """ + + def __init__(self, timeout: float = 120.0): + self._timeout = timeout + # Forced structured tier, deliberately not the classifier's choice. The + # classifier scores a rendition on text quality and returns + # ``tier='fast'`` for these documents, and the fast tier extracts ZERO + # tables from a rendition: rendered table borders are vector line-art, + # so only the structured tier's find_tables recovers the grid. Letting + # the ladder choose would discard exactly the structure the rendition + # exists to preserve. + # + # extract_images=False regardless of ``settings.pymupdf_extract_images``: + # the images in a rendition are LibreOffice's raster of the source's own + # figures, one indirection removed from anything a user could be shown, + # and writing them would spend disk on a temporary artefact that is + # discarded with the rendition. + self._pdf = PyMuPDFProcessor(extract_images=False) + + @property + def name(self) -> str: + return "office" + + @property + def tier(self) -> str: + return "structured" + + @property + def supported_mime_types(self) -> set[str]: + return DOC_MIME_TYPES + + async def process( + self, + content: bytes, + content_type: str, + filename: Optional[str] = None, + options: Optional[dict[str, Any]] = None, + progress_callback: Optional[ + Callable[[float, Optional[float], Optional[str]], Awaitable[None]] + ] = None, + ) -> ProcessingResult: + """Render to PDF, then extract with the structured PDF processor.""" + name = filename or _default_name(content_type) + if progress_callback: + await progress_callback(0.0, None, "Rendering document to PDF...") + + try: + pdf_bytes = await _libreoffice.convert( + content, name, "pdf", timeout_seconds=self._timeout + ) + except _libreoffice.LibreOfficeError as exc: + raise ProcessorError(f"Office rendition failed: {exc}") from exc + + # A rendition reaches the PDF engine directly rather than back through + # ProcessorRegistry, so it would otherwise skip the size cap a PDF + # uploaded to Nextcloud has to pass. The *source* is already capped + # before download, but rendering is not size-preserving -- a modest .doc + # of dense vector figures can render far larger -- so the cap is applied + # again here, to the bytes actually about to be parsed. + # Imported here rather than at module scope: registry imports the + # processor package, so a top-level import would close a cycle. + from nextcloud_mcp_server.config import get_settings # noqa: PLC0415 + + from .registry import get_registry # noqa: PLC0415 + + oversize = get_registry().oversize_result_for_size( + len(pdf_bytes), name, get_settings() + ) + if oversize is not None: + oversize.metadata[RENDERED_FROM_KEY] = content_type + oversize.metadata["rendition_bytes"] = len(pdf_bytes) + return oversize + + result = await self._pdf.process( + pdf_bytes, "application/pdf", name, options, progress_callback + ) + result.metadata[RENDERED_FROM_KEY] = content_type + result.metadata["rendition_bytes"] = len(pdf_bytes) + result.processor = self.name + return result + + async def health_check(self) -> bool: + return _libreoffice.LIBREOFFICE_AVAILABLE + + +def _default_name(content_type: str) -> str: + """A filename whose extension selects the right LibreOffice import filter. + + Only used when the caller passed none: LibreOffice picks its filter from the + extension, so ``document`` with no suffix imports as the wrong format. + """ + if content_type.split(";")[0].strip().lower() == "application/msword": + return "document.doc" + return "document.docx" diff --git a/nextcloud_mcp_server/document_processors/spreadsheet.py b/nextcloud_mcp_server/document_processors/spreadsheet.py new file mode 100644 index 000000000..8fdef1240 --- /dev/null +++ b/nextcloud_mcp_server/document_processors/spreadsheet.py @@ -0,0 +1,203 @@ +"""Spreadsheets, read cell-by-cell rather than through a PDF rendition. + +Deliberately NOT the rendition route the word-processor formats take. Measured +on a real workbook, rendering to PDF paginated it into 25 mixed-orientation +pages and recalled only **63.8%** of the tokens a direct cell read recovers -- +LibreOffice honours the print layout, so columns past the print width are cut +and the question numbering (``1.2``, ``2.a``, ``3.a``..``3.d``) disappears +entirely. A direct read recovered 2013 cells against the rendition's 567. + +A spreadsheet also has no page geometry to highlight, so giving up the +rendition costs nothing a viewer could have used: chunks carry a sheet name and +cell range instead of a bounding box. +""" + +import io +import logging +from collections.abc import Awaitable, Callable +from typing import Any, Optional + +from anyio.to_thread import run_sync + +from . import _libreoffice +from .base import DocumentProcessor, ProcessingResult, ProcessorError + +logger = logging.getLogger(__name__) + +XLSX_MIME = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" +XLS_MIME = "application/vnd.ms-excel" +SPREADSHEET_MIME_TYPES = {XLSX_MIME, XLS_MIME} + +# Per-sheet spans into the joined text: ``{"sheet", "cell_range", +# "start_offset", "end_offset"}``. The spreadsheet counterpart to a PDF's +# ``page_boundaries`` -- it lets a chunk be attributed to the sheet and cell +# range it came from, which is the anchor a viewer can use where there is no +# bbox to draw. +SHEET_BOUNDARIES_KEY = "sheet_boundaries" + + +class SpreadsheetProcessor(DocumentProcessor): + """Extract ``.xlsx``/``.xls`` as one markdown table per sheet.""" + + def __init__(self, timeout: float = 120.0): + self._timeout = timeout + + @property + def name(self) -> str: + return "spreadsheet" + + @property + def tier(self) -> str: + return "fast" + + @property + def supported_mime_types(self) -> set[str]: + # Legacy .xls needs LibreOffice to reach a readable container. Where it + # is absent, claim only .xlsx: the registry then reports "no processor + # for application/vnd.ms-excel" once, instead of this processor + # accepting every .xls and failing on each one. + if _libreoffice.LIBREOFFICE_AVAILABLE: + return SPREADSHEET_MIME_TYPES + return {XLSX_MIME} + + async def process( + self, + content: bytes, + content_type: str, + filename: Optional[str] = None, + options: Optional[dict[str, Any]] = None, + progress_callback: Optional[ + Callable[[float, Optional[float], Optional[str]], Awaitable[None]] + ] = None, + ) -> ProcessingResult: + base_type = content_type.split(";")[0].strip().lower() + converted_from = None + + if base_type == XLS_MIME: + # openpyxl cannot read the legacy OLE2 format. Convert the container + # to xlsx rather than to PDF: this is a format change, not a + # re-layout, so it keeps every cell instead of losing a third of + # them to pagination. + if progress_callback: + await progress_callback(0.0, None, "Converting legacy spreadsheet...") + try: + content = await _libreoffice.convert( + content, + filename or "workbook.xls", + "xlsx", + timeout_seconds=self._timeout, + ) + except _libreoffice.LibreOfficeError as exc: + raise ProcessorError( + f"Legacy spreadsheet conversion failed: {exc}" + ) from exc + converted_from = XLS_MIME + + try: + text, boundaries, sheet_names = await run_sync(_extract_workbook, content) + except ProcessorError: + raise + except Exception as exc: + raise ProcessorError(f"Spreadsheet parse failed: {exc}") from exc + + metadata: dict[str, Any] = { + "sheet_count": len(sheet_names), + "sheet_names": sheet_names, + SHEET_BOUNDARIES_KEY: boundaries, + "text_length": len(text), + "parse_mode": "markdown", + } + if converted_from: + metadata["converted_from_mime"] = converted_from + + return ProcessingResult( + text=text, + metadata=metadata, + processor=self.name, + success=True, + ) + + async def health_check(self) -> bool: + try: + import openpyxl # noqa: F401, PLC0415 + except ImportError: + return False + return True + + +def _escape(value: Any) -> str: + """One cell as markdown-table-safe text. + + A literal ``|`` in a cell would end the column early and shift every value + after it into the wrong heading, so it is escaped; newlines inside a cell + would end the row entirely, so they become spaces. + """ + if value is None: + return "" + text = str(value).replace("|", "\\|") + return " ".join(text.split()) + + +def _extract_workbook(content: bytes) -> tuple[str, list[dict[str, Any]], list[str]]: + """Render every sheet as a markdown table. Runs in a worker thread.""" + import openpyxl # noqa: PLC0415 -- keep the import off the hot path + + # data_only: read the cached result of a formula rather than "=SUM(A1:A9)", + # which is what a reader searches for. read_only streams rows instead of + # building the whole object graph, so a large workbook stays bounded. + workbook = openpyxl.load_workbook( + io.BytesIO(content), data_only=True, read_only=True + ) + try: + parts: list[str] = [] + boundaries: list[dict[str, Any]] = [] + offset = 0 + for sheet in workbook.worksheets: + rows = _sheet_rows(sheet) + if not rows: + continue + body = f"## {sheet.title}\n\n" + "\n".join(rows) + "\n" + parts.append(body) + boundaries.append( + { + "sheet": sheet.title, + "cell_range": sheet.calculate_dimension(), + "start_offset": offset, + "end_offset": offset + len(body), + } + ) + offset += len(body) + return "".join(parts), boundaries, workbook.sheetnames + finally: + workbook.close() + + +def _sheet_rows(sheet: Any) -> list[str]: + """Markdown rows for one sheet, or [] when it holds nothing. + + The first non-empty row becomes the header. That is a guess -- a real + spreadsheet may open with a title banner -- but the alternative is an empty + header row, and the cells are all present either way, which is what a + retrieval index needs. + """ + rendered: list[list[str]] = [] + width = 0 + for row in sheet.iter_rows(values_only=True): + cells = [_escape(v) for v in row] + while cells and not cells[-1]: + cells.pop() + if not cells: + continue + width = max(width, len(cells)) + rendered.append(cells) + + if not rendered: + return [] + + lines = [] + for i, cells in enumerate(rendered): + padded = cells + [""] * (width - len(cells)) + lines.append("| " + " | ".join(padded) + " |") + if i == 0: + lines.append("| " + " | ".join(["---"] * width) + " |") + return lines diff --git a/pyproject.toml b/pyproject.toml index 9bda305af..633108a04 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,6 +54,8 @@ dependencies = [ "mistralai>=2.4.5", "sqlalchemy[asyncio]>=2.0", "pypdfium2>=5.9.0", + "openpyxl>=3.1.5", + "olefile>=0.47", ] classifiers = [ "Development Status :: 4 - Beta", diff --git a/tests/integration/test_office_documents.py b/tests/integration/test_office_documents.py new file mode 100644 index 000000000..fd1436e2f --- /dev/null +++ b/tests/integration/test_office_documents.py @@ -0,0 +1,156 @@ +"""Office formats end to end through real LibreOffice + the real PDF ladder. + +The unit tests stub the conversion, so nothing there would catch a wrong +LibreOffice argument, a missing import filter, or a rendition the PDF ladder +cannot read. These run the actual binary. + +Fixtures are generated by LibreOffice itself from HTML rather than committed as +binaries, so there is nothing opaque in the tree and the legacy ``.doc``/``.xls`` +cases exercise the same import filters a real document would. +""" + +import pytest + +from nextcloud_mcp_server.document_processors import _libreoffice +from nextcloud_mcp_server.document_processors.office import ( + RENDERED_FROM_KEY, + OfficeDocumentProcessor, +) +from nextcloud_mcp_server.document_processors.spreadsheet import ( + SHEET_BOUNDARIES_KEY, + XLS_MIME, + SpreadsheetProcessor, +) + +pytestmark = [ + pytest.mark.integration, + pytest.mark.skipif( + not _libreoffice.LIBREOFFICE_AVAILABLE, + reason="LibreOffice (soffice) is not installed", + ), +] + +DOCX_MIME = "application/vnd.openxmlformats-officedocument.wordprocessingml.document" +DOC_MIME = "application/msword" + +# A merged first cell spanning two rows: the exact shape that makes a +# mammoth/markdownify parse drop a cell and shift the rest of the row one +# column left, so answers land under the wrong heading. +TABLE_HTML = """ +

Supplier Questionnaire

+

Please complete and return this questionnaire.

+ + + + +
DomainQuestionAnswer
CertificationsDo you hold ISO 27001?Yes
Do you hold Cyber Essentials?Not applicable
+""" + + +async def _word_fixture(target_filter: str) -> bytes: + """A real Word document, converted from HTML by LibreOffice. + + The export filter has to be named explicitly: a bare ``--convert-to docx`` + from HTML aborts with "no export filter found", because LibreOffice cannot + tell which module should claim the input. + """ + return await _libreoffice.convert( + TABLE_HTML.encode(), "q.html", target_filter, timeout_seconds=180.0 + ) + + +async def _xls_fixture() -> bytes: + """A real legacy .xls, via xlsx -- both formats belong to Calc, so the + conversion is unambiguous where HTML -> xls is not.""" + import io + + import openpyxl + + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.title = "Answers" + for row in ( + ["Domain", "Answer"], + ["Certifications", "ISO 27001"], + ["Testing", "Annual pen test"], + ): + sheet.append(row) + buf = io.BytesIO() + workbook.save(buf) + return await _libreoffice.convert( + buf.getvalue(), "s.xlsx", "xls", timeout_seconds=180.0 + ) + + +@pytest.mark.parametrize( + ("target_filter", "mime"), + [("docx:MS Word 2007 XML", DOCX_MIME), ("doc:MS Word 97", DOC_MIME)], +) +async def test_word_document_is_rendered_and_parsed(target_filter, mime): + """Both the modern and legacy Word formats reach text, pages and tables.""" + content = await _word_fixture(target_filter) + target = "docx" if "2007" in target_filter else "doc" + + result = await OfficeDocumentProcessor(timeout=180.0).process( + content, mime, f"q.{target}" + ) + + assert "Supplier Questionnaire" in result.text + assert "ISO 27001" in result.text + # The rendition is a real PDF, so page geometry comes from the existing path. + assert len(result.metadata["page_boundaries"]) >= 1 + assert result.metadata[RENDERED_FROM_KEY] == mime + assert result.processor == "office" + + +async def test_merged_cell_keeps_its_answer_in_the_answer_column(): + """The defect the rendition route exists to avoid. + + A direct mammoth parse emits three cells for the second body row where the + header has four, shifting "Not applicable" under the question column. + """ + content = await _word_fixture("docx:MS Word 2007 XML") + + result = await OfficeDocumentProcessor(timeout=180.0).process( + content, DOCX_MIME, "q.docx" + ) + + rows = [ln for ln in result.text.splitlines() if ln.count("|") >= 2] + assert rows, f"no markdown table recovered from the rendition: {result.text!r}" + answer_rows = [r for r in rows if "Not applicable" in r] + assert answer_rows, "the merged-cell row vanished from the table" + # "Not applicable" must be the last populated cell, i.e. still in the Answer + # column rather than shifted left into Question. + cells = [c.strip() for c in answer_rows[0].strip("|").split("|")] + assert cells[-1] == "Not applicable" + + +async def test_page_boundaries_index_the_returned_text_exactly(): + """The offsets the chunker and pdf_highlighter both rely on.""" + content = await _word_fixture("docx:MS Word 2007 XML") + + result = await OfficeDocumentProcessor(timeout=180.0).process( + content, DOCX_MIME, "q.docx" + ) + + boundaries = result.metadata["page_boundaries"] + assert boundaries[0]["start_offset"] == 0 + assert boundaries[-1]["end_offset"] == len(result.text) + for previous, following in zip(boundaries, boundaries[1:]): + assert previous["end_offset"] == following["start_offset"] + + +async def test_legacy_xls_is_read_cell_by_cell_not_paginated(): + """.xls converts container-to-container, so no cell is lost to a page break.""" + content = await _xls_fixture() + + result = await SpreadsheetProcessor(timeout=180.0).process( + content, XLS_MIME, "s.xls" + ) + + assert "| Certifications | ISO 27001 |" in result.text + assert "| Testing | Annual pen test |" in result.text + assert result.metadata["converted_from_mime"] == XLS_MIME + assert len(result.metadata[SHEET_BOUNDARIES_KEY]) >= 1 + # A spreadsheet has no page geometry, and must not pretend otherwise. + assert "page_boundaries" not in result.metadata diff --git a/tests/support/__init__.py b/tests/support/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/support/cfb_writer.py b/tests/support/cfb_writer.py new file mode 100644 index 000000000..c33feb2e6 --- /dev/null +++ b/tests/support/cfb_writer.py @@ -0,0 +1,215 @@ +"""Author a minimal OLE2 / Compound File Binary container for tests. + +``olefile`` reads CFB but cannot write it, and there is no Outlook to hand, so +without this the ``.msg`` reader could only be tested against a mock of the +format -- which is exactly the kind of test that passes while the real parser +returns nothing. This produces genuine containers that ``olefile`` opens. + +Deliberately restricted to what the tests need: + +* version 3 (512-byte sectors), no DIFAT sectors, so at most 109 FAT sectors -- + about 27 MB of payload, far past any fixture. +* streams smaller than 4096 bytes go in the mini-stream, as the format + requires. Writing a smaller cutoff in the header to avoid implementing it + does not work: olefile logs "Fixing the mini_stream_cutoff_size to 4096 + (mandatory value)" and reads small streams from the mini-stream regardless, + so a container without one yields empty strings for every field. +* the directory is a degenerate right-leaning tree rather than a balanced + red-black one. Readers walk the links rather than re-deriving the ordering. +""" + +import struct +from typing import Iterable + +SECTOR = 512 +MINI_SECTOR = 64 +# Mandatory per [MS-CFB]; olefile rewrites any other value in the header. +MINI_CUTOFF = 4096 +DIFAT_ENTRIES_IN_HEADER = 109 + +FREESECT = 0xFFFFFFFF +ENDOFCHAIN = 0xFFFFFFFE +FATSECT = 0xFFFFFFFD + +# Directory entry object types. +_EMPTY, _STORAGE, _STREAM, _ROOT = 0, 1, 2, 5 +_NOSTREAM = 0xFFFFFFFF + + +class _Entry: + def __init__(self, name: str, kind: int): + self.name = name + self.kind = kind + self.child = _NOSTREAM + self.right = _NOSTREAM + self.start = ENDOFCHAIN + self.size = 0 + + +def _pad(data: bytes, multiple: int = SECTOR) -> bytes: + remainder = len(data) % multiple + return data if not remainder else data + b"\x00" * (multiple - remainder) + + +def _directory_entry(entry: _Entry) -> bytes: + raw = bytearray(b"\x00" * 128) + encoded = entry.name.encode("utf-16-le")[:62] + raw[0 : len(encoded)] = encoded + struct.pack_into(" int: + """Chain ``indices`` as right siblings; return the first, or _NOSTREAM.""" + if not indices: + return _NOSTREAM + for current, following in zip(indices, indices[1:]): + entries[current].right = following + return indices[0] + + +def write_cfb(streams: dict[str, bytes]) -> bytes: + """Build a CFB container holding ``streams``. + + Keys are paths; a single ``/`` introduces one level of storage, which is all + a ``.msg``'s attachment folders need (``__attach_version1.0_#00000000/...``). + """ + root = _Entry("Root Entry", _ROOT) + entries: list[_Entry] = [root] + + # Regular payload and mini payload are accumulated separately: a stream's + # start sector is an index into whichever of the two holds it, so both must + # be complete before the directory naming them can be written. + regular = bytearray() + mini = bytearray() + + def allocate(data: bytes) -> tuple[int, int]: + if not data: + return ENDOFCHAIN, 0 + if len(data) < MINI_CUTOFF: + start = len(mini) // MINI_SECTOR + mini.extend(_pad(data, MINI_SECTOR)) + return start, len(data) + start = len(regular) // SECTOR + regular.extend(_pad(data)) + return start, len(data) + + top_level: list[int] = [] + storages: dict[str, tuple[_Entry, list[int]]] = {} + + for path, data in streams.items(): + storage_name, _, leaf = path.rpartition("/") + entry = _Entry(leaf, _STREAM) + entry.start, entry.size = allocate(data) + entries.append(entry) + index = len(entries) - 1 + if not storage_name: + top_level.append(index) + continue + if storage_name not in storages: + storage_entry = _Entry(storage_name, _STORAGE) + entries.append(storage_entry) + top_level.append(len(entries) - 1) + storages[storage_name] = (storage_entry, []) + storages[storage_name][1].append(index) + + for storage_entry, children in storages.values(): + storage_entry.child = _link_siblings(entries, children) + root.child = _link_siblings(entries, top_level) + + # The mini-stream container is itself an ordinary stream, owned by the root + # entry, laid down after the regular payload. + mini_container_start = len(regular) // SECTOR if mini else ENDOFCHAIN + root.start = mini_container_start + root.size = len(mini) + regular.extend(_pad(bytes(mini))) + + directory = _pad(b"".join(_directory_entry(e) for e in entries)) + directory_sectors = len(directory) // SECTOR + payload_sectors = len(regular) // SECTOR + + mini_fat = [FREESECT] * (len(mini) // MINI_SECTOR) + cursor = 0 + for data in streams.values(): + if not data or len(data) >= MINI_CUTOFF: + continue + count = -(-len(data) // MINI_SECTOR) + for offset in range(count): + slot = cursor + offset + mini_fat[slot] = ENDOFCHAIN if offset == count - 1 else slot + 1 + cursor += count + mini_fat_bytes = _pad(b"".join(struct.pack(" DIFAT_ENTRIES_IN_HEADER: + raise ValueError("payload too large for a header-only DIFAT") + + directory_start = payload_sectors + mini_fat_start = directory_start + directory_sectors + fat_start = mini_fat_start + mini_fat_sectors + + fat = [FREESECT] * (fat_sectors * (SECTOR // 4)) + + def chain(start: int, count: int) -> None: + for offset in range(count): + sector = start + offset + fat[sector] = ENDOFCHAIN if offset == count - 1 else sector + 1 + + # Every stream is contiguous, so each chain simply runs to its end. + cursor = 0 + for data in streams.values(): + if not data or len(data) < MINI_CUTOFF: + continue + length = len(_pad(data)) // SECTOR + chain(cursor, length) + cursor += length + if mini: + chain(mini_container_start, len(_pad(bytes(mini))) // SECTOR) + chain(directory_start, directory_sectors) + if mini_fat_sectors: + chain(mini_fat_start, mini_fat_sectors) + for offset in range(fat_sectors): + fat[fat_start + offset] = FATSECT + + header = bytearray(b"\x00" * SECTOR) + header[0:8] = b"\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1" + struct.pack_into(" bytes: + """``write_cfb`` from pairs, for callers building the list incrementally.""" + return write_cfb(dict(streams)) diff --git a/tests/unit/test_libreoffice_convert.py b/tests/unit/test_libreoffice_convert.py new file mode 100644 index 000000000..93b796d38 --- /dev/null +++ b/tests/unit/test_libreoffice_convert.py @@ -0,0 +1,128 @@ +"""LibreOffice invocation: private profile, real output check, clear failures.""" + +import pathlib + +import pytest + +from nextcloud_mcp_server.document_processors import _libreoffice + +pytestmark = pytest.mark.unit + + +class _Completed: + def __init__(self, returncode=0, stderr=b""): + self.returncode = returncode + self.stderr = stderr + self.stdout = b"" + + +def _patch_run(mocker, *, returncode=0, stderr=b"", writes: str | None = "out.pdf"): + """Fake soffice: optionally write an output file into --outdir.""" + + async def fake_run(argv, **kwargs): + if writes is not None: + outdir = pathlib.Path(argv[argv.index("--outdir") + 1]) + (outdir / writes).write_bytes(b"%PDF-1.7 rendered") + fake_run.argv = argv + return _Completed(returncode, stderr) + + mocker.patch.object(_libreoffice.anyio, "run_process", fake_run) + mocker.patch.object(_libreoffice, "SOFFICE_BIN", "/usr/bin/soffice") + return fake_run + + +async def test_each_invocation_gets_a_private_profile(mocker): + """Concurrent soffice runs sharing one profile silently produce nothing.""" + run = _patch_run(mocker) + + await _libreoffice.convert(b"x", "a.docx", "pdf") + + profile_args = [a for a in run.argv if a.startswith("-env:UserInstallation=")] + assert len(profile_args) == 1 + assert profile_args[0].startswith("-env:UserInstallation=file://") + + +async def test_output_bytes_are_returned(mocker): + _patch_run(mocker) + + assert await _libreoffice.convert(b"x", "a.docx", "pdf") == b"%PDF-1.7 rendered" + + +async def test_success_exit_with_no_output_file_is_still_a_failure(mocker): + """soffice exits 0 on an unreadable input while writing nothing at all.""" + _patch_run(mocker, writes=None) + + with pytest.raises(_libreoffice.LibreOfficeError, match="produced no pdf output"): + await _libreoffice.convert(b"x", "a.docx", "pdf") + + +async def test_nonzero_exit_reports_stderr(mocker): + _patch_run(mocker, returncode=1, stderr=b"source file could not be loaded") + + with pytest.raises(_libreoffice.LibreOfficeError, match="could not be loaded"): + await _libreoffice.convert(b"x", "a.docx", "pdf") + + +async def test_extensionless_name_is_rejected(mocker): + """Without a suffix LibreOffice picks the wrong import filter silently.""" + mocker.patch.object(_libreoffice, "SOFFICE_BIN", "/usr/bin/soffice") + + with pytest.raises(_libreoffice.LibreOfficeError, match="no extension"): + await _libreoffice.convert(b"x", "document", "pdf") + + +async def test_missing_libreoffice_is_reported_not_crashed(mocker): + mocker.patch.object(_libreoffice, "SOFFICE_BIN", None) + + with pytest.raises(_libreoffice.LibreOfficeError, match="not installed"): + await _libreoffice.convert(b"x", "a.docx", "pdf") + + +async def test_conversions_are_bounded_by_the_parse_limiter(mocker): + """Unbounded, a folder of .doc files would start one soffice per task. + + Counts processes actually in flight rather than inspecting the limiter + afterwards: the limiter always reads as empty once the work is done, so a + post-hoc assertion would pass even with no bound at all. + """ + import anyio as anyio_module + + in_flight = 0 + peak = 0 + + async def fake_run(argv, **kwargs): + nonlocal in_flight, peak + in_flight += 1 + peak = max(peak, in_flight) + # Yield so a sibling task can start if nothing is holding it back. + await anyio_module.sleep(0.01) + outdir = pathlib.Path(argv[argv.index("--outdir") + 1]) + (outdir / "out.pdf").write_bytes(b"%PDF-1.7") + in_flight -= 1 + return _Completed() + + mocker.patch.object(_libreoffice.anyio, "run_process", fake_run) + mocker.patch.object(_libreoffice, "SOFFICE_BIN", "/usr/bin/soffice") + mocker.patch.object( + _libreoffice, + "get_settings", + lambda: mocker.Mock(document_parse_process_slots=2), + ) + limiter = anyio_module.CapacityLimiter(2) + mocker.patch.object(_libreoffice, "parse_process_limiter", lambda slots: limiter) + + async with anyio_module.create_task_group() as tg: + for _ in range(6): + tg.start_soon(_libreoffice.convert, b"x", "a.docx", "pdf") + + assert peak == 2, f"expected at most 2 concurrent soffice processes, saw {peak}" + + +async def test_the_temp_directory_is_cleaned_up(mocker): + """A rendition holds the whole document twice; leaking it fills the disk.""" + run = _patch_run(mocker) + + await _libreoffice.convert(b"x", "a.docx", "pdf") + + src = pathlib.Path(run.argv[-1]) + assert not src.parent.exists() diff --git a/tests/unit/test_msg_processor.py b/tests/unit/test_msg_processor.py new file mode 100644 index 000000000..8ba812d03 --- /dev/null +++ b/tests/unit/test_msg_processor.py @@ -0,0 +1,166 @@ +"""Outlook messages keep their envelope, which is most of what gets searched.""" + +import io +import struct +from datetime import datetime, timezone + +import pytest + +from nextcloud_mcp_server.document_processors._msg_reader import ( + MsgReadError, + read_msg, +) +from nextcloud_mcp_server.document_processors.base import ProcessorError +from nextcloud_mcp_server.document_processors.msg import ( + ATTACHMENT_NAMES_KEY, + MsgProcessor, +) +from tests.support.cfb_writer import write_cfb + +pytestmark = pytest.mark.unit + +MSG_MIME = "application/vnd.ms-outlook" + +# 2026-02-07 10:02:02 UTC as a FILETIME (100ns ticks since 1601-01-01). +SUBMIT_FILETIME = int( + ( + datetime(2026, 2, 7, 10, 2, 2, tzinfo=timezone.utc) + - datetime(1601, 1, 1, tzinfo=timezone.utc) + ).total_seconds() + * 10_000_000 +) + + +def _build_msg( + *, + subject: str | None = "Re: Something for the weekend", + sender: str | None = "john@example.com", + to: str | None = "paul@example.org", + body: str | None = "Hi Paul,\n\nThe partners will catch up on Monday.", + unicode_body: bool = False, + attachments: tuple[str, ...] = (), + submit_time: int | None = SUBMIT_FILETIME, + codepage: int | None = None, +) -> bytes: + """A minimal but genuine OLE2 .msg carrying the streams we read.""" + streams: dict[str, bytes] = {} + + def put(tag: str, value: str | None, *, as_unicode: bool, prefix: str = ""): + if value is None: + return + suffix = "001F" if as_unicode else "001E" + raw = value.encode("utf-16-le" if as_unicode else "cp1252") + streams[f"{prefix}__substg1.0_{tag}{suffix}"] = raw + + put("0037", subject, as_unicode=False) + put("5D01", sender, as_unicode=False) + put("0E04", to, as_unicode=False) + put("1000", body, as_unicode=unicode_body) + for i, name in enumerate(attachments): + put("3707", name, as_unicode=False, prefix=f"__attach_version1.0_#{i:08X}/") + + # __properties_version1.0: 32-byte header, then 16-byte entries of + # (tag<<16|type, flags, 8-byte value). + props = bytearray(b"\x00" * 32) + if submit_time is not None: + props += struct.pack(" bytes: + """An .xlsx containing ``{sheet name: rows}``.""" + wb = openpyxl.Workbook() + wb.remove(wb.active) + for title, rows in sheets.items(): + ws = wb.create_sheet(title=title) + for row in rows: + ws.append(row) + buf = io.BytesIO() + wb.save(buf) + return buf.getvalue() + + +class TestLegacyXls: + """The .xls branch, mocked so it stays covered in the fast unit lane. + + The real conversion is exercised by + tests/integration/test_office_documents.py, which needs a soffice binary and + is skipped without one -- so without these the branch has no coverage at all + on a machine or CI lane lacking LibreOffice. + """ + + async def test_it_converts_to_xlsx_then_reads_cells(self, mocker): + converted = _workbook({"Answers": [["Domain", "Answer"], ["Certs", "ISO"]]}) + convert = mocker.patch.object( + _libreoffice, "convert", mocker.AsyncMock(return_value=converted) + ) + + result = await SpreadsheetProcessor().process( + b"legacy ole2 bytes", XLS_MIME, "s.xls" + ) + + # xlsx, never pdf: a PDF rendition loses a third of the cells. + assert convert.await_args.args[2] == "xlsx" + assert "| Certs | ISO |" in result.text + assert result.metadata["converted_from_mime"] == XLS_MIME + + async def test_conversion_failure_becomes_a_processor_error(self, mocker): + mocker.patch.object( + _libreoffice, + "convert", + mocker.AsyncMock(side_effect=_libreoffice.LibreOfficeError("no filter")), + ) + + with pytest.raises(ProcessorError, match="Legacy spreadsheet conversion"): + await SpreadsheetProcessor().process(b"x", XLS_MIME, "s.xls") + + async def test_xlsx_never_goes_near_libreoffice(self, mocker): + convert = mocker.patch.object(_libreoffice, "convert", mocker.AsyncMock()) + + await SpreadsheetProcessor().process( + _workbook({"S": [["a"]]}), XLSX_MIME, "s.xlsx" + ) + + convert.assert_not_awaited() + + +async def test_rows_become_a_markdown_table(): + content = _workbook( + {"Questions": [["Domain", "Answer"], ["Certifications", "ISO 27001"]]} + ) + + result = await SpreadsheetProcessor().process(content, XLSX_MIME, "q.xlsx") + + assert "## Questions" in result.text + assert "| Domain | Answer |" in result.text + assert "| Certifications | ISO 27001 |" in result.text + assert result.metadata["sheet_count"] == 1 + assert result.metadata["parse_mode"] == "markdown" + + +async def test_each_sheet_gets_its_own_boundary_span(): + content = _workbook({"First": [["a"]], "Second": [["b"]]}) + + result = await SpreadsheetProcessor().process(content, XLSX_MIME, "two.xlsx") + + spans = result.metadata[SHEET_BOUNDARIES_KEY] + assert [s["sheet"] for s in spans] == ["First", "Second"] + # Offsets must index the returned text exactly -- they are what attributes a + # chunk back to a sheet, the spreadsheet stand-in for a page number. + for span in spans: + segment = result.text[span["start_offset"] : span["end_offset"]] + assert segment.startswith(f"## {span['sheet']}") + assert spans[0]["end_offset"] == spans[1]["start_offset"] + + +async def test_pipe_in_a_cell_does_not_break_the_row(): + """An unescaped | would end the column early and shift later values.""" + content = _workbook({"S": [["head", "other"], ["a|b", "kept"]]}) + + result = await SpreadsheetProcessor().process(content, XLSX_MIME, "p.xlsx") + + assert r"| a\|b | kept |" in result.text + + +async def test_newline_in_a_cell_does_not_break_the_table(): + content = _workbook({"S": [["head"], ["line one\nline two"]]}) + + result = await SpreadsheetProcessor().process(content, XLSX_MIME, "n.xlsx") + + assert "| line one line two |" in result.text + + +async def test_ragged_rows_are_padded_to_the_widest(): + """A short row must not shift its cells under the wrong heading.""" + content = _workbook({"S": [["a", "b", "c"], ["only"]]}) + + result = await SpreadsheetProcessor().process(content, XLSX_MIME, "r.xlsx") + + assert "| only | | |" in result.text + + +async def test_empty_sheet_is_skipped_not_emitted_as_an_empty_table(): + content = _workbook({"Empty": [], "Real": [["x"]]}) + + result = await SpreadsheetProcessor().process(content, XLSX_MIME, "e.xlsx") + + assert "## Empty" not in result.text + assert "## Real" in result.text + assert [s["sheet"] for s in result.metadata[SHEET_BOUNDARIES_KEY]] == ["Real"] diff --git a/uv.lock b/uv.lock index 4c7859867..d9de75ede 100644 --- a/uv.lock +++ b/uv.lock @@ -618,6 +618,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/bb/7d/1e50069bb7d9774acbcb7d9509a4be95348e6476009f083d46cca67c5dbc/dynaconf-3.3.2-py3-none-any.whl", hash = "sha256:4bb8ac4222af0bb4315e3da40022c283ed140bafb80c36282a7973404a67cd8a", size = 270081, upload-time = "2026-06-29T20:55:40.066Z" }, ] +[[package]] +name = "et-xmlfile" +version = "2.0.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d3/38/af70d7ab1ae9d4da450eeec1fa3918940a5fafb9055e934af8d6eb0c2313/et_xmlfile-2.0.0.tar.gz", hash = "sha256:dab3f4764309081ce75662649be815c4c9081e88f0837825f90fd28317d4da54", size = 17234, upload-time = "2024-10-25T17:25:40.039Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/c1/8b/5fe2cc11fee489817272089c4203e679c63b570a5aaeb18d852ae3cbba6a/et_xmlfile-2.0.0-py3-none-any.whl", hash = "sha256:7a91720bc756843502c3b7504c77b8fe44217c85c537d85037f0f536151b2caa", size = 18059, upload-time = "2024-10-25T17:25:39.051Z" }, +] + [[package]] name = "eval-type-backport" version = "0.4.0" @@ -1953,7 +1962,9 @@ dependencies = [ { name = "markdownify" }, { name = "mcp", extra = ["cli"] }, { name = "mistralai" }, + { name = "olefile" }, { name = "openai" }, + { name = "openpyxl" }, { name = "opentelemetry-api" }, { name = "opentelemetry-exporter-otlp-proto-grpc" }, { name = "opentelemetry-instrumentation-asgi" }, @@ -2022,7 +2033,9 @@ requires-dist = [ { name = "markdownify", specifier = ">=0.14.1" }, { name = "mcp", extras = ["cli"], specifier = ">=1.29,<1.30" }, { name = "mistralai", specifier = ">=2.4.5" }, + { name = "olefile", specifier = ">=0.47" }, { name = "openai", specifier = ">=2.8.1" }, + { name = "openpyxl", specifier = ">=3.1.5" }, { name = "opentelemetry-api", specifier = ">=1.28.2" }, { name = "opentelemetry-exporter-otlp-proto-grpc", specifier = ">=1.28.2" }, { name = "opentelemetry-instrumentation-asgi", specifier = ">=0.49b2" }, @@ -2229,6 +2242,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/a1/5a/4d2b1601df3602dba7a14f3348ba9bfe94a18adb428e693df6154c293831/numpy-2.5.1-cp314-cp314t-win_arm64.whl", hash = "sha256:5a6db61f9aaa57e369905c67d852045d3c4f7126405b29d09b19dec118e9c9cb", size = 10697674, upload-time = "2026-07-04T17:07:58.506Z" }, ] +[[package]] +name = "olefile" +version = "0.47" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/69/1b/077b508e3e500e1629d366249c3ccb32f95e50258b231705c09e3c7a4366/olefile-0.47.zip", hash = "sha256:599383381a0bf3dfbd932ca0ca6515acd174ed48870cbf7fee123d698c192c1c", size = 112240, upload-time = "2023-12-01T16:22:53.025Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/17/d3/b64c356a907242d719fc668b71befd73324e47ab46c8ebbbede252c154b2/olefile-0.47-py2.py3-none-any.whl", hash = "sha256:543c7da2a7adadf21214938bb79c83ea12b473a4b6ee4ad4bf854e7715e13d1f", size = 114565, upload-time = "2023-12-01T16:22:51.518Z" }, +] + [[package]] name = "onnxruntime" version = "1.27.0" @@ -2286,6 +2308,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/ea/7b/206238ebcb50b235942b1c66dba4974776f2057402a8d91c399be587d66a/openai-2.46.0-py3-none-any.whl", hash = "sha256:672381db55efb3a1e2610f29304c130cccdd0b319bace4d492b2443cb64c1e7c", size = 1637556, upload-time = "2026-07-17T02:48:03.695Z" }, ] +[[package]] +name = "openpyxl" +version = "3.1.5" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "et-xmlfile" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/3d/f9/88d94a75de065ea32619465d2f77b29a0469500e99012523b91cc4141cd1/openpyxl-3.1.5.tar.gz", hash = "sha256:cf0e3cf56142039133628b5acffe8ef0c12bc902d2aadd3e0fe5878dc08d1050", size = 186464, upload-time = "2024-06-28T14:03:44.161Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/c0/da/977ded879c29cbd04de313843e76868e6e13408a94ed6b987245dc7c8506/openpyxl-3.1.5-py2.py3-none-any.whl", hash = "sha256:5282c12b107bffeef825f4617dc029afaf41d0ea60823bbb665ef3079dc79de2", size = 250910, upload-time = "2024-06-28T14:03:41.161Z" }, +] + [[package]] name = "opentelemetry-api" version = "1.39.1"