From 807e206826ae419aa93bea99be38f0ff4895dd15 Mon Sep 17 00:00:00 2001 From: harumiWeb Date: Sat, 3 Oct 2026 16:35:42 +0900 Subject: [PATCH 1/2] feat(extraction): add reusable OOXML core session for issue 149 --- CHANGELOG.md | 4 + dev-docs/specs/excel-extraction.md | 8 + dev-docs/specs/ooxml-core-extraction.md | 126 +++++ src/exstruct/core/backends/ooxml_backend.py | 17 +- src/exstruct/core/ooxml_drawing.py | 143 +++--- src/exstruct/core/ooxml_package.py | 87 ++++ src/exstruct/core/ooxml_session.py | 525 ++++++++++++++++++++ tests/core/test_ooxml_extraction_session.py | 348 +++++++++++++ tests/core/test_ooxml_package.py | 147 ++++++ 9 files changed, 1327 insertions(+), 78 deletions(-) create mode 100644 dev-docs/specs/ooxml-core-extraction.md create mode 100644 src/exstruct/core/ooxml_package.py create mode 100644 src/exstruct/core/ooxml_session.py create mode 100644 tests/core/test_ooxml_extraction_session.py create mode 100644 tests/core/test_ooxml_package.py diff --git a/CHANGELOG.md b/CHANGELOG.md index c316011e..247b0d1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ All notable changes to this project are documented in this file. This changelog ### Added +- Added an internal streaming OOXML core extraction session for cells, shared + strings, formulas, hyperlinks, merged cells, defined names, print areas and + explicit tables, with shared ZIP access for optional shapes and charts. + - Added regression coverage for previous-reader parity, workbook resource lifetime and loader counts, subprocess import boundaries, and clustering equivalence with SciPy-free execution. diff --git a/dev-docs/specs/excel-extraction.md b/dev-docs/specs/excel-extraction.md index 18c677f5..4fde3657 100644 --- a/dev-docs/specs/excel-extraction.md +++ b/dev-docs/specs/excel-extraction.md @@ -30,6 +30,14 @@ This document summarizes the current specification for Excel extraction processi Existing helper overrides remain observable and may intentionally replace the optimized session path. +## Internal OOXML core alternative + +Issue #149 adds a streaming `OoxmlExtractionSession` for core worksheet data +and optional rich extraction through one ZIP. It is available for parity and +future migration work; the pipeline above continues to use openpyxl. +See [Core OOXML extraction session](ooxml-core-extraction.md) for supported +values, formulas, relationships, tables, lifecycle and limitations. + ## Coordinate System - Rows are 1-based diff --git a/dev-docs/specs/ooxml-core-extraction.md b/dev-docs/specs/ooxml-core-extraction.md new file mode 100644 index 00000000..a3a52ab5 --- /dev/null +++ b/dev-docs/specs/ooxml-core-extraction.md @@ -0,0 +1,126 @@ +# Core OOXML extraction session + +Issue: [#149](https://github.com/harumiWeb/exstruct/issues/149) + +## Scope and selection + +`exstruct.core.ooxml_session.OoxmlExtractionSession` is an internal, read-only +alternative for `.xlsx` / `.xlsm` core extraction. It does not replace the +openpyxl pipeline or change public models. Default-path migration is separate. + +```python +from pathlib import Path +from exstruct.core.ooxml_session import OoxmlExtractionSession +from exstruct.core.backends.ooxml_backend import OoxmlRichBackend + +path = Path("book.xlsx") +with OoxmlExtractionSession(path) as session: + sheets = session.sheets() + cells = session.extract_cells(include_links=True) + formulas = session.extract_formulas_map() + merged = session.extract_merged_cells() + areas = session.extract_print_areas() + tables = session.extract_explicit_tables() + names = session.defined_names() + charts = OoxmlRichBackend(path, session=session).extract_charts(mode="light") +``` + +## Resource lifetime and parsing + +- One lazy ZIP is owned by the context and shared by core and optional rich + extraction. Independent sessions have independent caches. +- Worksheet XML and shared strings use `defusedxml.ElementTree.iterparse`. + Completed cell/row and shared-string elements are removed from their parents. +- Sparse values, formulas, merges, hyperlinks and table references are cached + only after complete worksheet parsing. Subsequent core methods do not reparse + worksheets. Memory scales with extracted data and the shared-string dictionary; + this is not a constant-memory iterator API. +- Small workbook, relationship, style and table parts use safe XML trees. + Existing drawing parsing retains its separate streaming geometry scan. +- Closing releases the ZIP and caches, including after context-body errors. + Accessing a closed session raises RuntimeError. +- No openpyxl workbook/worksheet model is created. Existing value/formula + normalization and openpyxl scalar format, date and formula-translation helpers + are reused; openpyxl remains a dependency. +- Core malformed XML, missing required parts and invalid scalar data raise + errors. Missing optional relationship parts return an empty mapping. No + partially parsed worksheet is cached. Rich extraction retains its existing + per-sheet best-effort fallback contract. + +## Data contract + +- `sheets()` returns workbook order, sheet ID, visibility, resolved part path + and relationship kind, including chart sheets. Core extraction excludes + non-worksheet sheets. Empty worksheets remain present in mappings. +- `defined_names()` preserves raw text and optional `localSheetId`; named + formulas are not evaluated. +- `extract_cells()` returns `CellRow`: rows 1-based, columns 0-based numeric-string + keys, existing missing-token filtering and numeric-string normalization. Sparse + gaps do not cause rectangular blank cell materialization. +- Shared strings (`s`), plain/rich inline strings (`inlineStr`), formula string + caches (`str`), numeric (`n`), boolean (`b`) and ISO date (`d`) values are decoded. + Formatting/phonetic annotations are omitted. Excel errors (`e`) are omitted, + matching current extraction. +- Numeric date/time/duration styles and 1900/1904 workbook epochs are honored. + Styles serve scalar conversion, not border heuristics or colors. +- Formula values use stored caches without calculation. `extract_formulas_map()` + groups normalized `=`-prefixed formulas at `(row, zero-based column)` coordinates. + Shared followers translate relative references from the shared anchor. + Array/dynamic formulas retain explicit anchor text without inventing follower + formulas. Unresolved shared followers are omitted; unsupported translation + is logged and skipped. +- External hyperlink URLs are preserved exactly. Internal `location` links are + omitted, matching current output. Links on filtered values survive only when + their row has another emitted value. Range links use zero-based column keys. +- Merged ranges contain cached raw anchor text or a space for an empty anchor; + covered follower values are excluded from rows. +- Print areas come from `_xlnm.Print_Area`, respecting local scope and quoted + names including commas/apostrophes. Unsupported non-finite ranges are logged + and skipped. +- `extract_explicit_tables()` returns referenced table names, display names, + ranges and column names, without the border heuristics of `table_candidates`. + +## Relationships + +`ooxml_package.read_relationships(archive, source_path)` resolves IDs to type, +target and external status. Internal relative paths resolve against the source +directory; absolute targets resolve from the ZIP root. External targets are +preserved and never fetched or read as ZIP parts. These primitives serve +hyperlinks, tables, worksheets, drawings and charts. Legacy drawing helpers +remain available as wrappers. + +## ADR assessment + +- Verdict: `not-needed`; next action: `no-adr`. +- Rationale: internal parsing capabilities are added without changing backend + selection, priority, mode meanings, public output or fallback policy. A future + pipeline migration requires a fresh ADR assessment. +- Domains: extraction implementation and resource reuse. +- Existing decisions: ADR-0010 (light OOXML rich baseline), ADR-0002 (rich fallback). +- Evidence triad: this spec and `dev-docs/specs/excel-extraction.md`; + `src/exstruct/core/ooxml_session.py`, `ooxml_package.py`, `ooxml_drawing.py`, + `backends/ooxml_backend.py`; `tests/core/test_ooxml_extraction_session.py`, + `test_ooxml_package.py`, `test_ooxml_drawing.py` and existing pipeline tests. + +## Validation + +Parity fixtures compare with `OpenpyxlBackend` for both suffixes, link flags, +cells, formulas, merges and print areas. Additional tests cover shared/rich +strings, cached and shared formulas, relationships, tables, archive/read reuse, +sparse sheets, exception cleanup and XML entity rejection. Existing drawing +and pipeline tests retain rich-artifact guarantees. + +```powershell +rtk uv run --extra all pytest tests/core/test_ooxml_extraction_session.py tests/core/test_ooxml_package.py tests/core/test_ooxml_drawing.py -q +rtk uv run --extra all pytest -m "not com and not render and not libreoffice" -q --cov=exstruct --cov-report=term --cov-fail-under=80 +rtk uv run ruff check . --no-fix +rtk uv run --extra all mypy src/exstruct --strict +``` + +Local Windows/Python 3.11 verification: 23 focused tests passed; 1,036 tests +passed with 12 external-runtime tests deselected, total coverage 83.51% (80% +gate passed). Ruff and strict mypy passed. Excel COM/render and real LibreOffice +runtime verification are not part of this internal-parser validation. +An additional comparison of all seven tracked `sample/**/*.xlsx` workbooks and +`tests/assets/multiple_print_ranges_4sheets.xlsx` matched cells with links, +formula maps, print areas and merged-range sets against `OpenpyxlBackend`. diff --git a/src/exstruct/core/backends/ooxml_backend.py b/src/exstruct/core/backends/ooxml_backend.py index 15de21c0..1e65d053 100644 --- a/src/exstruct/core/backends/ooxml_backend.py +++ b/src/exstruct/core/backends/ooxml_backend.py @@ -4,6 +4,7 @@ import logging from pathlib import Path +from typing import TYPE_CHECKING from zipfile import BadZipFile from defusedxml import ElementTree @@ -13,6 +14,9 @@ from .base import ChartData, RichBackend, ShapeData from .ooxml_shapes import _build_shapes_from_ooxml +if TYPE_CHECKING: + from ..ooxml_session import OoxmlExtractionSession + logger = logging.getLogger(__name__) _OOXML_SUFFIXES = {".xlsx", ".xlsm"} @@ -21,10 +25,15 @@ class OoxmlRichBackend(RichBackend): """Best-effort rich extraction backed only by OOXML workbook parts.""" - def __init__(self, file_path: Path) -> None: + def __init__( + self, file_path: Path, *, session: OoxmlExtractionSession | None = None + ) -> None: """Store the workbook path for lazy OOXML parsing.""" self.file_path = file_path + if session is not None and session.file_path.resolve() != file_path.resolve(): + raise ValueError("OOXML session belongs to a different workbook") + self.session = session self._drawings: dict[str, SheetDrawingData] | None = None def extract_shapes(self, *, mode: str) -> ShapeData: @@ -77,7 +86,11 @@ def _read_drawings(self) -> dict[str, SheetDrawingData]: self._drawings = {} return self._drawings try: - self._drawings = read_sheet_drawings(self.file_path) + self._drawings = ( + self.session.read_drawings() + if self.session is not None + else read_sheet_drawings(self.file_path) + ) except ( BadZipFile, ElementTree.ParseError, diff --git a/src/exstruct/core/ooxml_drawing.py b/src/exstruct/core/ooxml_drawing.py index 554026a9..685bc9f3 100644 --- a/src/exstruct/core/ooxml_drawing.py +++ b/src/exstruct/core/ooxml_drawing.py @@ -4,13 +4,15 @@ from dataclasses import dataclass, field import logging -from pathlib import Path, PurePosixPath +from pathlib import Path from typing import Literal from zipfile import BadZipFile, ZipFile from defusedxml import ElementTree from ..models import ChartSeries +from . import ooxml_package +from .ooxml_package import OoxmlRelationship, relationship_part_path _NS = { "a": "http://schemas.openxmlformats.org/drawingml/2006/main", @@ -82,14 +84,6 @@ class DrawingConnectorRef: end_drawing_id: int | None -@dataclass(frozen=True) -class OoxmlRelationship: - """Relationship metadata extracted from an OOXML ``.rels`` part.""" - - target: str - relationship_type: str - - @dataclass(frozen=True) class OoxmlChartInfo: """Chart metadata extracted from OOXML chart and drawing parts.""" @@ -185,32 +179,48 @@ def row_offset_points(self, row_index: int) -> float: def read_sheet_drawings(file_path: Path) -> dict[str, SheetDrawingData]: """Read worksheet drawing metadata directly from OOXML parts.""" - result: dict[str, SheetDrawingData] = {} with ZipFile(file_path) as archive: - for sheet_name, sheet_xml_path in _iter_sheet_xml_paths(archive): - try: - drawing_path = _resolve_sheet_drawing_path(archive, sheet_xml_path) - if drawing_path is None: - continue - result[sheet_name] = _parse_sheet_drawing( - archive, - drawing_path, - _read_sheet_metrics(archive, sheet_xml_path), - ) - except ( - BadZipFile, - ElementTree.ParseError, - FileNotFoundError, - KeyError, - OSError, - ValueError, - ) as exc: - logger.warning( - "Skipping OOXML drawing metadata for sheet %s in %s. (%r)", - sheet_name, - file_path, - exc, - ) + return _read_sheet_drawings_from_archive(archive, file_path) + + +def read_sheet_drawings_from_archive(archive: ZipFile) -> dict[str, SheetDrawingData]: + """Read worksheet drawing metadata from an already-open OOXML archive.""" + + source = getattr(archive, "filename", None) or "" + return _read_sheet_drawings_from_archive(archive, source) + + +def _read_sheet_drawings_from_archive( + archive: ZipFile, + source: object, +) -> dict[str, SheetDrawingData]: + """Read drawing metadata while preserving per-sheet parse fallbacks.""" + + result: dict[str, SheetDrawingData] = {} + for sheet_name, sheet_xml_path in _iter_sheet_xml_paths(archive): + try: + drawing_path = _resolve_sheet_drawing_path(archive, sheet_xml_path) + if drawing_path is None: + continue + result[sheet_name] = _parse_sheet_drawing( + archive, + drawing_path, + _read_sheet_metrics(archive, sheet_xml_path), + ) + except ( + BadZipFile, + ElementTree.ParseError, + FileNotFoundError, + KeyError, + OSError, + ValueError, + ) as exc: + logger.warning( + "Skipping OOXML drawing metadata for sheet %s in %s. (%r)", + sheet_name, + source, + exc, + ) return result @@ -219,7 +229,7 @@ def _iter_sheet_xml_paths(archive: ZipFile) -> list[tuple[str, str]]: workbook_xml = archive.read("xl/workbook.xml") workbook_root = ElementTree.fromstring(workbook_xml) - rel_map = _read_relationships(archive, "xl/_rels/workbook.xml.rels") + rel_map = _read_relationships(archive, relationship_part_path("xl/workbook.xml")) paths: list[tuple[str, str]] = [] for sheet in workbook_root.findall("spreadsheetml:sheets/spreadsheetml:sheet", _NS): name = sheet.attrib.get("name") @@ -227,7 +237,10 @@ def _iter_sheet_xml_paths(archive: ZipFile) -> list[tuple[str, str]]: if not name or not rel_id or rel_id not in rel_map: continue relationship = rel_map[rel_id] - if relationship.relationship_type != _WORKSHEET_REL_TYPE: + if ( + relationship.external + or relationship.relationship_type != _WORKSHEET_REL_TYPE + ): continue paths.append((name, relationship.target)) return paths @@ -236,12 +249,12 @@ def _iter_sheet_xml_paths(archive: ZipFile) -> list[tuple[str, str]]: def _resolve_sheet_drawing_path(archive: ZipFile, sheet_xml_path: str) -> str | None: """Resolve the drawing part referenced by a worksheet, if any.""" - rels_path = _rels_path(sheet_xml_path) + rels_path = relationship_part_path(sheet_xml_path) if rels_path not in archive.namelist(): return None rel_map = _read_relationships(archive, rels_path) for relationship in rel_map.values(): - if relationship.relationship_type != _DRAWING_REL_TYPE: + if relationship.external or relationship.relationship_type != _DRAWING_REL_TYPE: continue return relationship.target return None @@ -256,7 +269,7 @@ def _parse_sheet_drawing( root = ElementTree.fromstring(archive.read(drawing_path)) rel_map = {} - drawing_rels_path = _rels_path(drawing_path) + drawing_rels_path = relationship_part_path(drawing_path) if drawing_rels_path in archive.namelist(): rel_map = _read_relationships(archive, drawing_rels_path) @@ -420,7 +433,11 @@ def _parse_chart_node( if rel_id is None: return None relationship = rel_map.get(rel_id.attrib.get(f"{{{_NS['r']}}}id", "")) - if relationship is None or relationship.relationship_type != _CHART_REL_TYPE: + if ( + relationship is None + or relationship.external + or relationship.relationship_type != _CHART_REL_TYPE + ): return None target = relationship.target if target not in archive.namelist(): @@ -863,62 +880,36 @@ def _column_width_to_points(width: float) -> float: def _read_relationships( archive: ZipFile, rels_path: str ) -> dict[str, OoxmlRelationship]: - """Read a relationships part into a relationship-id keyed metadata map.""" - - root = ElementTree.fromstring(archive.read(rels_path)) - base_path = _base_dir(_source_path_from_rels(rels_path)) - rel_map: dict[str, OoxmlRelationship] = {} - for rel in root.findall("rel:Relationship", _NS): - rel_id = rel.attrib.get("Id") - target = rel.attrib.get("Target") - relationship_type = rel.attrib.get("Type") - if not rel_id or not target or not relationship_type: - continue - rel_map[rel_id] = OoxmlRelationship( - target=_normalize_zip_path(base_path, target), - relationship_type=relationship_type, - ) - return rel_map + """Compatibility wrapper accepting the relationships part path.""" + + return ooxml_package.read_relationships( + archive, + ooxml_package._source_path_from_rels(rels_path), + ) def _source_path_from_rels(rels_path: str) -> str: """Recover the source part path that owns a relationships part.""" - rels = PurePosixPath(rels_path) - if rels.parent.name != "_rels": - return rels_path - stem = rels.name.removesuffix(".rels") - return str(rels.parent.parent / stem) + return ooxml_package._source_path_from_rels(rels_path) def _rels_path(source_path: str) -> str: """Return the relationships part path for a source part.""" - path = PurePosixPath(source_path) - return str(path.parent / "_rels" / f"{path.name}.rels") + return relationship_part_path(source_path) def _base_dir(path: str) -> str: """Return the POSIX parent directory for a zip path.""" - return str(PurePosixPath(path).parent) + return ooxml_package._base_dir(path) def _normalize_zip_path(base_dir: str, target: str) -> str: """Normalize a relative OOXML zip target against a base directory.""" - base = PurePosixPath(base_dir) - normalized = base.joinpath(PurePosixPath(target)).as_posix() - parts: list[str] = [] - for part in normalized.split("/"): - if part in {"", "."}: - continue - if part == "..": - if parts: - parts.pop() - continue - parts.append(part) - return "/".join(parts) + return ooxml_package.normalize_part_path(base_dir, target) def _extract_text(node: ElementTree.Element | None) -> str: diff --git a/src/exstruct/core/ooxml_package.py b/src/exstruct/core/ooxml_package.py new file mode 100644 index 00000000..d3e55a3e --- /dev/null +++ b/src/exstruct/core/ooxml_package.py @@ -0,0 +1,87 @@ +"""Reusable OOXML package relationship helpers.""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import PurePosixPath +from zipfile import ZipFile + +from defusedxml import ElementTree + +_RELATIONSHIPS_NS = "http://schemas.openxmlformats.org/package/2006/relationships" +_NS = {"rel": _RELATIONSHIPS_NS} + + +@dataclass(frozen=True) +class OoxmlRelationship: + """Relationship metadata extracted from an OOXML ``.rels`` part.""" + + target: str + relationship_type: str + external: bool = False + + +def read_relationships( + archive: ZipFile, source_path: str +) -> dict[str, OoxmlRelationship]: + """Read relationships owned by ``source_path`` from an open OOXML archive.""" + + rels_path = relationship_part_path(source_path) + if rels_path not in archive.NameToInfo: + return {} + root = ElementTree.fromstring(archive.read(rels_path)) + base_dir = _base_dir(source_path) + rel_map: dict[str, OoxmlRelationship] = {} + for rel in root.findall("rel:Relationship", _NS): + rel_id = rel.attrib.get("Id") + target = rel.attrib.get("Target") + relationship_type = rel.attrib.get("Type") + if not rel_id or not target or not relationship_type: + continue + external = rel.attrib.get("TargetMode") == "External" + rel_map[rel_id] = OoxmlRelationship( + target=target if external else normalize_part_path(base_dir, target), + relationship_type=relationship_type, + external=external, + ) + return rel_map + + +def relationship_part_path(source_path: str) -> str: + """Return the package path of the relationships part for a source part.""" + + path = PurePosixPath(source_path) + return str(path.parent / "_rels" / f"{path.name}.rels") + + +def normalize_part_path(base_dir: str, target: str) -> str: + """Resolve an internal POSIX target against its source part directory.""" + + base = PurePosixPath(base_dir) + normalized = base.joinpath(PurePosixPath(target)).as_posix() + parts: list[str] = [] + for part in normalized.split("/"): + if part in {"", "."}: + continue + if part == "..": + if parts: + parts.pop() + continue + parts.append(part) + return "/".join(parts) + + +def _source_path_from_rels(rels_path: str) -> str: + """Recover the source part path that owns a relationships part.""" + + rels = PurePosixPath(rels_path) + if rels.parent.name != "_rels": + return rels_path + stem = rels.name.removesuffix(".rels") + return str(rels.parent.parent / stem) + + +def _base_dir(path: str) -> str: + """Return the POSIX parent directory for a package path.""" + + return str(PurePosixPath(path).parent) diff --git a/src/exstruct/core/ooxml_session.py b/src/exstruct/core/ooxml_session.py new file mode 100644 index 00000000..3dab81c8 --- /dev/null +++ b/src/exstruct/core/ooxml_session.py @@ -0,0 +1,525 @@ +"""Extraction-scoped, streaming access to core OOXML workbook parts (#149).""" + +from __future__ import annotations + +from dataclasses import dataclass, field +import logging +from pathlib import Path +from typing import TYPE_CHECKING +from zipfile import ZipFile + +from defusedxml import ElementTree +from openpyxl.formula.translate import Translator, TranslatorError +from openpyxl.styles.numbers import BUILTIN_FORMATS, is_date_format, is_timedelta_format +from openpyxl.utils.cell import coordinate_to_tuple, get_column_letter, range_boundaries +from openpyxl.utils.datetime import MAC_EPOCH, WINDOWS_EPOCH, from_excel, from_ISO8601 + +from ..models import CellRow, PrintArea +from .cell_types import MergedCellRange, SheetFormulasMap, WorkbookFormulasMap +from .cells import _normalize_cell_value, _normalize_formula_value +from .ooxml_package import OoxmlRelationship, read_relationships + +if TYPE_CHECKING: + from .ooxml_drawing import SheetDrawingData + +logger = logging.getLogger(__name__) +_MAIN = "http://schemas.openxmlformats.org/spreadsheetml/2006/main" +_REL = "http://schemas.openxmlformats.org/officeDocument/2006/relationships" +_NS = {"s": _MAIN} + + +@dataclass(frozen=True) +class OoxmlSheet: + """Sheet identity, visibility and resolved part path in workbook order.""" + + name: str + sheet_id: str + state: str + part_path: str + kind: str + + +@dataclass(frozen=True) +class OoxmlDefinedName: + """Raw defined-name text and optional workbook-local sheet index.""" + + name: str + text: str + local_sheet_id: int | None = None + + +@dataclass(frozen=True) +class OoxmlTable: + """Explicit table identity, extent and column names (no heuristics).""" + + name: str + display_name: str + ref: str + columns: tuple[str, ...] + + +@dataclass(frozen=True) +class _Formula: + coordinate: str + text: str | None + kind: str + shared_id: str | None + + +@dataclass +class _SheetData: + values: dict[tuple[int, int], object] = field(default_factory=dict) + formulas: list[_Formula] = field(default_factory=list) + merged: list[str] = field(default_factory=list) + links: list[tuple[str, str]] = field(default_factory=list) + table_ids: list[str] = field(default_factory=list) + + +def _string_text(node: ElementTree.Element) -> str: + """Join plain/rich text runs without phonetic annotations.""" + return "".join(t.text or "" for t in node.findall("s:t", _NS)) + "".join( + t.text or "" for t in node.findall("s:r/s:t", _NS) + ) + + +def _bounds(ref: str) -> tuple[int, int, int, int]: + """Validate finite cell bounds rather than expanding whole-row references.""" + c1, r1, c2, r2 = range_boundaries(ref) + if None in (c1, r1, c2, r2): + raise ValueError(f"Unsupported cell range: {ref}") + if not (1 <= c1 <= c2 <= 16384 and 1 <= r1 <= r2 <= 1048576): + raise ValueError(f"Invalid cell range: {ref}") + return c1, r1, c2, r2 + + +class OoxmlExtractionSession: + """Reuse one ZIP and parsed metadata; never calculate or edit a workbook. + + This internal alternative is not selected by the extraction pipeline yet. + Worksheet XML and shared strings are streamed; returned sparse data is cached + until close. Scalar format and formula utilities reuse openpyxl, but no + openpyxl workbook or worksheet object is constructed. + """ + + def __init__(self, file_path: Path) -> None: + self.file_path = file_path + self._archive: ZipFile | None = None + self._closed = False + self._workbook_root: ElementTree.Element | None = None + self._workbook_path = "xl/workbook.xml" + self._relationships: dict[str, dict[str, OoxmlRelationship]] = {} + self._sheet_cache: dict[str, _SheetData] = {} + self._strings: list[str] | None = None + self._formats: list[str] | None = None + self._drawings: dict[str, SheetDrawingData] | None = None + + def __enter__(self) -> OoxmlExtractionSession: + self._ensure_open() + return self + + def _ensure_open(self) -> None: + if self._closed: + raise RuntimeError("OOXML extraction session is closed") + + def archive(self) -> ZipFile: + """Open the archive lazily, once per session.""" + self._ensure_open() + if self._archive is None: + if self.file_path.suffix.lower() not in {".xlsx", ".xlsm"}: + raise ValueError("OOXML extraction requires .xlsx or .xlsm") + self._archive = ZipFile(self.file_path) + return self._archive + + def relationships(self, source_path: str) -> dict[str, OoxmlRelationship]: + """Resolve relationships once per source part.""" + self._ensure_open() + if source_path not in self._relationships: + self._relationships[source_path] = read_relationships( + self.archive(), source_path + ) + return self._relationships[source_path] + + def _workbook(self) -> ElementTree.Element: + self._ensure_open() + if self._workbook_root is None: + for rel in self.relationships("").values(): + if ( + not rel.external + and rel.relationship_type == f"{_REL}/officeDocument" + ): + self._workbook_path = rel.target + break + self._workbook_root = ElementTree.fromstring( + self.archive().read(self._workbook_path) + ) + return self._workbook_root + + def sheets(self) -> list[OoxmlSheet]: + """Return all sheet metadata, including non-worksheet sheet kinds.""" + root = self._workbook() + rels = self.relationships(self._workbook_path) + result: list[OoxmlSheet] = [] + for node in root.findall("s:sheets/s:sheet", _NS): + rel = rels.get(node.get(f"{{{_REL}}}id", "")) + if rel is None or rel.external: + continue + result.append( + OoxmlSheet( + name=node.attrib["name"], + sheet_id=node.attrib["sheetId"], + state=node.get("state", "visible"), + part_path=rel.target, + kind=rel.relationship_type.rsplit("/", 1)[-1], + ) + ) + return result + + def defined_names(self) -> list[OoxmlDefinedName]: + """Preserve names and scope without evaluating named formulas.""" + return [ + OoxmlDefinedName( + name=node.attrib["name"], + text=node.text or "", + local_sheet_id=int(node.attrib["localSheetId"]) + if "localSheetId" in node.attrib + else None, + ) + for node in self._workbook().findall("s:definedNames/s:definedName", _NS) + ] + + def _related_part(self, kind: str) -> str | None: + self._workbook() + return next( + ( + rel.target + for rel in self.relationships(self._workbook_path).values() + if not rel.external and rel.relationship_type == f"{_REL}/{kind}" + ), + None, + ) + + def _shared_strings(self) -> list[str]: + if self._strings is None: + strings: list[str] = [] + part = self._related_part("sharedStrings") + if part is not None: + with self.archive().open(part) as stream: + root: ElementTree.Element | None = None + for event, node in ElementTree.iterparse( + stream, events=("start", "end") + ): + if root is None: + root = node + if event == "end" and node.tag == f"{{{_MAIN}}}si": + strings.append(_string_text(node)) + root.remove(node) + self._strings = strings + return self._strings + + def _number_formats(self) -> list[str]: + if self._formats is None: + formats: list[str] = [] + part = self._related_part("styles") + if part is not None: + root = ElementTree.fromstring(self.archive().read(part)) + custom = { + int(node.attrib["numFmtId"]): node.attrib["formatCode"] + for node in root.findall("s:numFmts/s:numFmt", _NS) + } + formats = [ + custom.get(fmt_id, BUILTIN_FORMATS.get(fmt_id, "General")) + for node in root.findall("s:cellXfs/s:xf", _NS) + for fmt_id in [int(node.get("numFmtId", "0"))] + ] + self._formats = formats + return self._formats + + def _cell_value(self, node: ElementTree.Element) -> object: + kind = node.get("t", "n") + text = node.findtext("s:v", namespaces=_NS) + if kind == "inlineStr": + inline = node.find("s:is", _NS) + return _string_text(inline) if inline is not None else None + if text in (None, "") or kind == "e": + return None + if kind == "s": + index = int(text) + if index < 0: + raise ValueError("Negative OOXML shared-string index") + return self._shared_strings()[index] + if kind == "b": + return bool(int(text)) + if kind == "d": + return from_ISO8601(text) + if kind != "n": + return text + value = float(text) if any(c in text for c in ".eE") else int(text) + style = int(node.get("s", "0")) + formats = self._number_formats() + fmt = formats[style] if 0 <= style < len(formats) else "General" + if is_date_format(fmt): + props = self._workbook().find("s:workbookPr", _NS) + epoch = ( + MAC_EPOCH + if props is not None and props.get("date1904") in {"1", "true"} + else WINDOWS_EPOCH + ) + try: + return from_excel( + value, epoch=epoch, timedelta=is_timedelta_format(fmt) + ) + except (OverflowError, ValueError): + logger.warning("Skipping out-of-range OOXML date at %s", node.get("r")) + return None + return int(value) if int(value) == value else value + + def _read_sheet(self, sheet: OoxmlSheet) -> _SheetData: + self._ensure_open() + if sheet.name in self._sheet_cache: + return self._sheet_cache[sheet.name] + data = _SheetData() + rels = self.relationships(sheet.part_path) + stack: list[ElementTree.Element] = [] + row_number = 0 + column_number = 0 + with self.archive().open(sheet.part_path) as stream: + for event, node in ElementTree.iterparse(stream, events=("start", "end")): + if event == "start": + stack.append(node) + if node.tag == f"{{{_MAIN}}}row": + number = float(node.get("r", str(row_number + 1))) + if not number.is_integer(): + raise ValueError("Invalid OOXML row number") + row_number = int(number) + column_number = 0 + continue + tag = node.tag.removeprefix(f"{{{_MAIN}}}") + if tag == "c": + coordinate = node.get("r") + if coordinate: + _, column_number = coordinate_to_tuple(coordinate) + else: + column_number += 1 + node.set("r", f"{get_column_letter(column_number)}{row_number}") + self._sheet_element(tag, node, data, rels) + if tag in {"c", "row", "mergeCell", "hyperlink", "tablePart"}: + stack[-2].remove(node) + stack.pop() + self._sheet_cache[sheet.name] = data + return data + + def _sheet_element( + self, + tag: str, + node: ElementTree.Element, + data: _SheetData, + rels: dict[str, OoxmlRelationship], + ) -> None: + if tag == "c": + coordinate = node.attrib["r"] + data.values[coordinate_to_tuple(coordinate)] = self._cell_value(node) + formula = node.find("s:f", _NS) + if formula is not None: + data.formulas.append( + _Formula( + coordinate, + formula.text, + formula.get("t", "normal"), + formula.get("si"), + ) + ) + elif tag == "mergeCell": + data.merged.append(node.attrib["ref"]) + elif tag == "hyperlink": + rel = rels.get(node.get(f"{{{_REL}}}id", "")) + if ( + rel is not None + and rel.external + and rel.relationship_type == f"{_REL}/hyperlink" + ): + data.links.append((node.attrib["ref"], rel.target)) + elif tag == "tablePart": + data.table_ids.append(node.attrib[f"{{{_REL}}}id"]) + + def _worksheets(self) -> list[OoxmlSheet]: + return [sheet for sheet in self.sheets() if sheet.kind == "worksheet"] + + def extract_cells(self, *, include_links: bool = False) -> dict[str, list[CellRow]]: + """Read cached values with the current CellRow normalization contract.""" + return { + sheet.name: self._cell_rows( + self._read_sheet(sheet), include_links=include_links + ) + for sheet in self._worksheets() + } + + def _cell_rows(self, data: _SheetData, *, include_links: bool) -> list[CellRow]: + rows: dict[int, dict[str, int | float | str]] = {} + merged = [_bounds(ref) for ref in data.merged] + for (row, col), raw in sorted(data.values.items()): + if any( + c1 <= col <= c2 and r1 <= row <= r2 and (row, col) != (r1, c1) + for c1, r1, c2, r2 in merged + ): + continue + value = _normalize_cell_value(raw) + if value is not None: + rows.setdefault(row, {})[str(col - 1)] = value + links: dict[int, dict[str, str]] = {} + if include_links: + for ref, target in data.links: + c1, r1, c2, r2 = _bounds(ref) + for row in rows: + if r1 <= row <= r2: + links.setdefault(row, {}).update( + {str(col - 1): target for col in range(c1, c2 + 1)} + ) + return [ + CellRow(r=row, c=values, links=links.get(row)) + for row, values in rows.items() + ] + + def extract_formulas_map(self) -> WorkbookFormulasMap: + """Normalize ordinary/array formulas and translate shared followers.""" + return WorkbookFormulasMap( + sheets={ + sheet.name: self._formulas_map(sheet.name, self._read_sheet(sheet)) + for sheet in self._worksheets() + } + ) + + def _formulas_map(self, name: str, data: _SheetData) -> SheetFormulasMap: + shared = { + f.shared_id: f for f in data.formulas if f.kind == "shared" and f.text + } + result: dict[str, list[tuple[int, int]]] = {} + for formula in sorted( + data.formulas, key=lambda f: coordinate_to_tuple(f.coordinate) + ): + text = _normalize_formula_value(formula.text) + if text is None and formula.kind == "shared": + anchor = shared.get(formula.shared_id) + if anchor is not None: + try: + text = Translator( + _normalize_formula_value(anchor.text), + origin=anchor.coordinate, + ).translate_formula(formula.coordinate) + except (TranslatorError, ValueError): + logger.warning( + "Skipping unsupported shared formula at %s!%s", + name, + formula.coordinate, + ) + if text is not None: + row, col = coordinate_to_tuple(formula.coordinate) + result.setdefault(text, []).append((row, col - 1)) + return SheetFormulasMap(sheet_name=name, formulas_map=result) + + def extract_merged_cells(self) -> dict[str, list[MergedCellRange]]: + """Read merged extents and unnormalized cached anchor display values.""" + result: dict[str, list[MergedCellRange]] = {} + for sheet in self._worksheets(): + data = self._read_sheet(sheet) + ranges: list[MergedCellRange] = [] + for ref in data.merged: + c1, r1, c2, r2 = _bounds(ref) + raw = data.values.get((r1, c1)) + ranges.append( + MergedCellRange( + r1, + c1 - 1, + r2, + c2 - 1, + (str(raw) if raw is not None else "") or " ", + ) + ) + result[sheet.name] = ranges + return result + + def extract_print_areas(self) -> dict[str, list[PrintArea]]: + """Read scoped print areas without splitting commas in quoted names.""" + from openpyxl.workbook.defined_name import DefinedName + + sheets = self.sheets() + worksheet_names = {sheet.name for sheet in sheets if sheet.kind == "worksheet"} + result: dict[str, list[PrintArea]] = {} + for name in self.defined_names(): + if name.name != "_xlnm.Print_Area": + continue + defined = DefinedName(name=name.name, attr_text=name.text) + for sheet_name, ref in defined.destinations: + sheet_name = sheet_name.replace("''", "'") + if sheet_name not in worksheet_names: + continue + if name.local_sheet_id is not None and ( + not 0 <= name.local_sheet_id < len(sheets) + or sheets[name.local_sheet_id].name != sheet_name + ): + continue + try: + c1, r1, c2, r2 = _bounds(ref) + except ValueError: + logger.warning("Skipping unsupported OOXML print area %s", ref) + continue + result.setdefault(sheet_name, []).append( + PrintArea(r1=r1, c1=c1 - 1, r2=r2, c2=c2 - 1) + ) + return result + + def extract_explicit_tables(self) -> dict[str, list[OoxmlTable]]: + """Read table parts referenced by worksheets; no border inference.""" + result: dict[str, list[OoxmlTable]] = {} + for sheet in self._worksheets(): + tables: list[OoxmlTable] = [] + rels = self.relationships(sheet.part_path) + for rel_id in self._read_sheet(sheet).table_ids: + rel = rels.get(rel_id) + if ( + rel is None + or rel.external + or rel.relationship_type != f"{_REL}/table" + ): + continue + root = ElementTree.fromstring(self.archive().read(rel.target)) + tables.append( + OoxmlTable( + root.get("name", ""), + root.get("displayName", ""), + root.attrib["ref"], + tuple( + node.get("name", "") + for node in root.findall( + "s:tableColumns/s:tableColumn", _NS + ) + ), + ) + ) + result[sheet.name] = tables + return result + + def read_drawings(self) -> dict[str, SheetDrawingData]: + """Reuse the same ZIP for the existing best-effort drawing parser.""" + from .ooxml_drawing import read_sheet_drawings_from_archive + + self._ensure_open() + if self._drawings is None: + self._drawings = read_sheet_drawings_from_archive(self.archive()) + return self._drawings + + def close(self) -> None: + """Release the archive and caches, including after extraction errors.""" + if self._closed: + return + self._closed = True + if self._archive is not None: + self._archive.close() + self._relationships.clear() + self._sheet_cache.clear() + self._strings = None + self._formats = None + self._workbook_root = None + self._drawings = None + + def __exit__(self, *exc_info: object) -> None: + self.close() diff --git a/tests/core/test_ooxml_extraction_session.py b/tests/core/test_ooxml_extraction_session.py new file mode 100644 index 00000000..1eeb6074 --- /dev/null +++ b/tests/core/test_ooxml_extraction_session.py @@ -0,0 +1,348 @@ +"""Core OOXML output parity, streaming and resource lifetime (#149).""" + +from datetime import datetime, time, timedelta +from pathlib import Path +from unittest.mock import Mock +from xml.etree import ElementTree as ET +from zipfile import ZipFile + +from defusedxml.common import EntitiesForbidden +from openpyxl import Workbook +from openpyxl.chart import BarChart, Reference +from openpyxl.utils.datetime import MAC_EPOCH +from openpyxl.workbook.defined_name import DefinedName +from openpyxl.worksheet.formula import ArrayFormula +from openpyxl.worksheet.hyperlink import Hyperlink +from openpyxl.worksheet.table import Table +import pytest + +from exstruct.core import ooxml_session +from exstruct.core.backends.ooxml_backend import OoxmlRichBackend +from exstruct.core.backends.openpyxl_backend import OpenpyxlBackend +from exstruct.core.ooxml_session import OoxmlExtractionSession + +MAIN = "http://schemas.openxmlformats.org/spreadsheetml/2006/main" +REL = "http://schemas.openxmlformats.org/officeDocument/2006/relationships" +PKG = "http://schemas.openxmlformats.org/package/2006/relationships" + + +def make_book(path: Path) -> None: + wb = Workbook() + ws = wb.active + assert ws is not None + ws.title = "O'Brien, data" + ws.append(["Name", "Count"]) + ws.append(["item", 7]) + ws["A2"].hyperlink = "https://example.com/a?q=1#top" + for coord, value in { + "D1": "merged", + "D3": True, + "E3": False, + "F3": "0012", + "G3": "1.250", + "H3": " NA ", + "I3": "NA", + "J3": "#DIV/0!", + "A5": datetime(2026, 1, 2, 3, 4, 5), + "B5": time(12, 30), + "C5": timedelta(days=2, hours=3), + "D5": -1.5, + "A8": "=SUM(B2)", + "B8": "=A2", + "A10000": "sparse", + }.items(): + ws[coord] = value + ws["I3"].hyperlink = "https://example.com/filtered" + ws["A7"] = "NA" + ws["A7"].hyperlink = "https://example.com/only-filtered" + ws["B2"].hyperlink = Hyperlink(ref="B2", location="'Other'!A1") + ws["C8"] = ArrayFormula(ref="C8:C9", text="=SUM(B2)") + ws.merge_cells("D1:E2") + ws.merge_cells("D10:E10") + ws.print_area = ["A1:B5", "D1:E2"] + ws.add_table(Table(displayName="Items", ref="A1:B2")) + chart = BarChart() + chart.add_data( + Reference(ws, min_col=2, min_row=1, max_row=2), titles_from_data=True + ) + ws.add_chart(chart, "L1") + wb.defined_names.add(DefinedName("Global", attr_text="'O''Brien, data'!$A$1")) + other = wb.create_sheet("Other") + other.sheet_state = "hidden" + wb.save(path) + wb.close() + + +def replace_parts(path: Path, parts: dict[str, bytes]) -> None: + with ZipFile(path) as archive: + payload = {name: archive.read(name) for name in archive.namelist()} + payload.update(parts) + with ZipFile(path, "w") as archive: + for name, data in payload.items(): + archive.writestr(name, data) + + +@pytest.mark.parametrize("suffix", [".xlsx", ".xlsm"]) +@pytest.mark.parametrize("include_links", [False, True]) +def test_core_parity(tmp_path: Path, suffix: str, include_links: bool) -> None: + path = tmp_path / f"book{suffix}" + make_book(path) + expected = OpenpyxlBackend(path) + with OoxmlExtractionSession(path) as session: + assert session.extract_cells( + include_links=include_links + ) == expected.extract_cells(include_links=include_links) + assert session.extract_formulas_map() == expected.extract_formulas_map() + assert session.extract_print_areas() == expected.extract_print_areas() + actual_merged = session.extract_merged_cells() + expected_merged = expected.extract_merged_cells() + for name in actual_merged: + assert set(actual_merged[name]) == set(expected_merged[name]) + tables = session.extract_explicit_tables() + assert [t.ref for t in tables["O'Brien, data"]] == ["A1:B2"] + assert tables["O'Brien, data"][0].columns == ("Name", "Count") + assert tables["O'Brien, data"][0].display_name == "Items" + assert tables["Other"] == [] + assert [(s.name, s.state) for s in session.sheets()] == [ + ("O'Brien, data", "visible"), + ("Other", "hidden"), + ] + assert any( + n.name == "Global" and n.local_sheet_id is None + for n in session.defined_names() + ) + + +def test_shared_strings_cached_formulas_and_shared_formula_parity( + tmp_path: Path, +) -> None: + path = tmp_path / "book.xlsx" + make_book(path) + with ZipFile(path) as archive: + rels = ET.fromstring(archive.read("xl/_rels/workbook.xml.rels")) + content_types = ET.fromstring(archive.read("[Content_Types].xml")) + ET.SubElement( + content_types, + "{http://schemas.openxmlformats.org/package/2006/content-types}Override", + PartName="/xl/sharedStrings.xml", + ContentType="application/vnd.openxmlformats-officedocument.spreadsheetml.sharedStrings+xml", + ) + ET.SubElement( + rels, + f"{{{PKG}}}Relationship", + Id="strings", + Type=f"{REL}/sharedStrings", + Target="sharedStrings.xml", + ) + sheet = f''' + 0inlinephonetic + B2+$C$14TEXT(1,"0")cached text + 51 + #N/A2026-01-02T03:04:05 + SUM(B2)2 + ''' + # Keep the rich inline string outside the merge extent. + sheet = sheet.replace('ref="A1:C1"', 'ref="D1:E1"') + strings = f'''shared stringignored''' + replace_parts( + path, + { + "xl/_rels/workbook.xml.rels": ET.tostring(rels), + "xl/worksheets/sheet1.xml": sheet.encode(), + "xl/sharedStrings.xml": strings.encode(), + "[Content_Types].xml": ET.tostring(content_types), + }, + ) + expected = OpenpyxlBackend(path) + with OoxmlExtractionSession(path) as session: + assert session.extract_cells() == expected.extract_cells(include_links=False) + assert session.extract_formulas_map() == expected.extract_formulas_map() + rows = session.extract_cells()["O'Brien, data"] + assert rows[0].c == {"0": "shared string", "1": "inline"} + assert session.extract_formulas_map().sheets["O'Brien, data"].formulas_map[ + "=B3+$C$1" + ] == [(3, 0)] + + +def test_archive_and_worksheet_reads_reused_and_rich_parity( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + path = tmp_path / "book.xlsx" + make_book(path) + standalone = OoxmlRichBackend(path) + expected_charts = standalone.extract_charts(mode="light") + opened: list[ZipFile] = [] + real_zip = ZipFile + + def track(path: Path) -> ZipFile: + archive = real_zip(path) + opened.append(archive) + return archive + + monkeypatch.setattr(ooxml_session, "ZipFile", track) + with OoxmlExtractionSession(path) as session: + assert opened == [] + session.extract_cells() + archive = session.archive() + original_open = archive.open + reads: list[str] = [] + + def read(name: str, *args: object, **kwargs: object) -> object: + reads.append(name) + return original_open(name, *args, **kwargs) + + monkeypatch.setattr(archive, "open", read) + session.extract_cells(include_links=True) + session.extract_formulas_map() + session.extract_merged_cells() + session.extract_print_areas() + session.extract_explicit_tables() + assert "xl/worksheets/sheet1.xml" not in reads + rich = OoxmlRichBackend(path, session=session) + assert rich.extract_charts(mode="light") == expected_charts + assert rich.extract_shapes(mode="light") == standalone.extract_shapes( + mode="light" + ) + assert session.read_drawings() is session.read_drawings() + assert len(opened) == 1 + assert opened[0].fp is None + + +def test_exception_closes_and_reuse_rejected(tmp_path: Path) -> None: + path = tmp_path / "book.xlsx" + make_book(path) + replace_parts(path, {"xl/worksheets/sheet1.xml": b""}) + session = OoxmlExtractionSession(path) + with pytest.raises(ET.ParseError), session: + archive = session.archive() + session.extract_cells() + assert archive.fp is None + session.close() + for call in ( + session.sheets, + session.extract_cells, + session.archive, + session.__enter__, + ): + with pytest.raises(RuntimeError, match="closed"): + call() + + +def test_defused_stream_rejects_entities(tmp_path: Path) -> None: + path = tmp_path / "book.xlsx" + make_book(path) + replace_parts( + path, + { + "xl/worksheets/sheet1.xml": b']>&e;' + }, + ) + with pytest.raises(EntitiesForbidden), OoxmlExtractionSession(path) as session: + session.extract_cells() + + +def test_worksheet_stream_never_uses_archive_read( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + path = tmp_path / "book.xlsx" + make_book(path) + with OoxmlExtractionSession(path) as session: + archive = session.archive() + original = archive.read + + def read(name: str, *args: object, **kwargs: object) -> bytes: + assert not ("/worksheets/" in name and name.endswith(".xml")) + return original(name, *args, **kwargs) + + monkeypatch.setattr(archive, "read", read) + monkeypatch.setattr( + "openpyxl.load_workbook", + Mock(side_effect=AssertionError("no workbook model")), + ) + assert session.extract_cells()["O'Brien, data"][-1].r == 10000 + + +def test_sessions_observe_updates_and_reject_biff(tmp_path: Path) -> None: + path = tmp_path / "book.xlsx" + make_book(path) + with OoxmlExtractionSession(path) as first: + previous = first.extract_cells() + make_book(path) + replace_parts( + path, + { + "xl/worksheets/sheet2.xml": f'new'.encode() + }, + ) + with OoxmlExtractionSession(path) as second: + assert previous != second.extract_cells() + with OoxmlExtractionSession(tmp_path / "book.xls") as session: + with pytest.raises(ValueError, match="xlsx"): + session.extract_cells() + + +def test_implicit_coordinates_and_out_of_range_date_parity(tmp_path: Path) -> None: + path = tmp_path / "implicit.xlsx" + make_book(path) + with ZipFile(path) as archive: + root = ET.fromstring(archive.read("xl/worksheets/sheet1.xml")) + style = root.find(f"{{{MAIN}}}sheetData/{{{MAIN}}}row[@r='5']/{{{MAIN}}}c[@r='A5']") + assert style is not None + date_style = style.attrib["s"] + replace_parts( + path, + { + "xl/worksheets/sheet1.xml": f''' + first2D1+13 + 4100000000 + 5 + '''.encode() + }, + ) + with OoxmlExtractionSession(path) as session: + backend = OpenpyxlBackend(path) + with pytest.warns(UserWarning, match="outside the limits"): + assert session.extract_cells() == backend.extract_cells(include_links=False) + with pytest.warns(UserWarning, match="outside the limits"): + assert session.extract_formulas_map() == backend.extract_formulas_map() + assert session.extract_cells()["O'Brien, data"][0].c == { + "0": "first", + "3": 2, + "4": 3, + } + + +def test_mac_epoch_and_iso_date_parity(tmp_path: Path) -> None: + path = tmp_path / "epoch.xlsx" + wb = Workbook() + wb.epoch = MAC_EPOCH + ws = wb.active + assert ws is not None + ws["A1"] = datetime(2026, 1, 2) + ws["B1"] = time(6, 0) + wb.save(path) + wb.close() + with OoxmlExtractionSession(path) as session: + assert session.extract_cells() == OpenpyxlBackend(path).extract_cells( + include_links=False + ) + + +def test_foreign_rich_session_rejected_and_unresolved_formula_skipped( + tmp_path: Path, +) -> None: + path = tmp_path / "formulas.xlsx" + make_book(path) + replace_parts( + path, + { + "xl/worksheets/sheet1.xml": f''' + 1 + '''.encode() + }, + ) + with OoxmlExtractionSession(path) as session: + assert session.extract_formulas_map().sheets["O'Brien, data"].formulas_map == {} + assert session.extract_cells()["O'Brien, data"][0].c == {"0": 1} + with pytest.raises(ValueError, match="different workbook"): + OoxmlRichBackend(tmp_path / "foreign.xlsx", session=session) diff --git a/tests/core/test_ooxml_package.py b/tests/core/test_ooxml_package.py new file mode 100644 index 00000000..f3ca893a --- /dev/null +++ b/tests/core/test_ooxml_package.py @@ -0,0 +1,147 @@ +from pathlib import Path +from zipfile import ZipFile + +from exstruct.core.ooxml_drawing import read_sheet_drawings_from_archive +from exstruct.core.ooxml_package import ( + OoxmlRelationship, + normalize_part_path, + read_relationships, + relationship_part_path, +) + + +def test_relationship_paths_use_posix_package_paths() -> None: + """Resolve relative and package-absolute internal relationship targets.""" + + assert relationship_part_path("xl/workbook.xml") == "xl/_rels/workbook.xml.rels" + assert relationship_part_path("") == "_rels/.rels" + assert normalize_part_path("xl/worksheets", "../drawings/drawing1.xml") == ( + "xl/drawings/drawing1.xml" + ) + assert normalize_part_path("xl/worksheets", "/xl/styles.xml") == "xl/styles.xml" + + +def test_read_relationships_resolves_internal_and_preserves_external_targets( + tmp_path: Path, +) -> None: + """Keep external URLs intact while resolving internal targets from the source.""" + + book = tmp_path / "relationships.xlsx" + with ZipFile(book, "w") as archive: + archive.writestr( + "xl/_rels/workbook.xml.rels", + """ + + + + + + """, + ) + + with ZipFile(book) as archive: + relationships = read_relationships(archive, "xl/workbook.xml") + + assert relationships == { + "relative": OoxmlRelationship("xl/worksheets/sheet1.xml", "worksheet"), + "absolute": OoxmlRelationship("xl/styles.xml", "styles"), + "external": OoxmlRelationship( + "https://example.test/a?x=1&y=two#part", "hyperlink", external=True + ), + } + + +def test_read_sheet_drawings_from_open_archive_ignores_external_relationships( + tmp_path: Path, +) -> None: + """Skip external worksheet/drawing/chart targets without closing the archive.""" + + book = tmp_path / "external-relationships.xlsx" + with ZipFile(book, "w") as archive: + archive.writestr( + "xl/workbook.xml", + """ + + + + + + + + """, + ) + archive.writestr( + "xl/_rels/workbook.xml.rels", + """ + + + + + + """, + ) + archive.writestr( + "xl/worksheets/sheet2.xml", + '', + ) + archive.writestr( + "xl/worksheets/sheet3.xml", + '', + ) + archive.writestr( + "xl/worksheets/_rels/sheet2.xml.rels", + """ + + + + """, + ) + archive.writestr( + "xl/worksheets/_rels/sheet3.xml.rels", + """ + + + + """, + ) + archive.writestr( + "xl/drawings/drawing1.xml", + """ + + + + + + + + + + + + + + + + + """, + ) + archive.writestr( + "xl/drawings/_rels/drawing1.xml.rels", + """ + + + + """, + ) + + with ZipFile(book) as archive: + drawings = read_sheet_drawings_from_archive(archive) + + assert archive.fp is not None + + assert set(drawings) == {"ExternalChart"} + assert drawings["ExternalChart"].charts == [] From 40296a522169196860a7e5aabf0f6d84d2b0d5a9 Mon Sep 17 00:00:00 2001 From: harumiWeb Date: Sat, 3 Oct 2026 17:25:00 +0900 Subject: [PATCH 2/2] fix(ooxml): address PR 154 text, drawing and sparse-row review findings --- CHANGELOG.md | 4 + dev-docs/README.md | 3 + dev-docs/agents/contributing.md | 2 +- dev-docs/roadmap.md | 13 ++ dev-docs/specs/ooxml-core-extraction.md | 31 ++++ src/exstruct/core/ooxml_drawing.py | 25 ++- src/exstruct/core/ooxml_session.py | 81 +++++++-- tests/core/test_ooxml_extraction_session.py | 85 +++++++++- tests/core/test_ooxml_relocated_workbook.py | 175 ++++++++++++++++++++ 9 files changed, 397 insertions(+), 22 deletions(-) create mode 100644 dev-docs/roadmap.md create mode 100644 tests/core/test_ooxml_relocated_workbook.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 247b0d1d..e7ad0406 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,10 @@ All notable changes to this project are documented in this file. This changelog ### Fixed +- Preserved accepted plain/rich string segment order and retained drawings when + an OOXML session resolves a non-default workbook part; avoided repeated full + merge/link scans when constructing sparse cell rows. + - Preserved live pipeline backend overrides through lazy backend resolution. - Avoided materializing blank cells when reading a shared sparse worksheet. diff --git a/dev-docs/README.md b/dev-docs/README.md index 04db3645..ff721280 100644 --- a/dev-docs/README.md +++ b/dev-docs/README.md @@ -9,6 +9,9 @@ ## Reading order +Implementation milestones and explicitly deferred extraction work are recorded +in the [extraction roadmap](roadmap.md). + When you change the codebase, read these materials in the following order: 1. Check the public contract in `docs/`. diff --git a/dev-docs/agents/contributing.md b/dev-docs/agents/contributing.md index 691f1367..e11e9aac 100644 --- a/dev-docs/agents/contributing.md +++ b/dev-docs/agents/contributing.md @@ -10,7 +10,7 @@ This file contains **special guidelines** for AI coding agents such as ChatGPT, 4. The `models` layer must remain completely side-effect-free. 5. Do not mix I/O processing with core logic. 6. Keep exception handling fail-safe. -7. Update the roadmap whenever you add a new feature. +7. Update the [developer roadmap](../roadmap.md) whenever you add a new feature. ## Reference priority diff --git a/dev-docs/roadmap.md b/dev-docs/roadmap.md new file mode 100644 index 00000000..aaf180f5 --- /dev/null +++ b/dev-docs/roadmap.md @@ -0,0 +1,13 @@ +# Extraction roadmap + +This records the scope already established by the extraction issues. It does +not introduce a new public mode or approve a default-backend migration. + +| Milestone | Status | Contract and evidence | +| --- | --- | --- | +| Core OOXML session foundation (Issue #149, PR #154) | Implemented; pending merge | Direct cells, strings, formulas, links, merges, names, print areas and explicit tables; shared ZIP/relationships for optional drawings. See [core OOXML specification](specs/ooxml-core-extraction.md) and `tests/core/test_ooxml_extraction_session.py`. | +| Default-path OOXML migration | Deferred to a separate issue | Current openpyxl pipeline remains selected. Evaluate parity and capability gaps, then assess the migration's ADR and public-contract impact before changing selection. | +| OOXML table heuristics | Outside Issue #149 | Explicit tables are available now; border/style-based heuristics remain on the existing backend. Any extension needs its own scope and parity evidence. | + +Current behavior is defined by [Excel extraction](specs/excel-extraction.md), +not by the roadmap status labels. diff --git a/dev-docs/specs/ooxml-core-extraction.md b/dev-docs/specs/ooxml-core-extraction.md index a3a52ab5..1a47d316 100644 --- a/dev-docs/specs/ooxml-core-extraction.md +++ b/dev-docs/specs/ooxml-core-extraction.md @@ -31,6 +31,8 @@ with OoxmlExtractionSession(path) as session: extraction. Independent sessions have independent caches. - Worksheet XML and shared strings use `defusedxml.ElementTree.iterparse`. Completed cell/row and shared-string elements are removed from their parents. +- Accepted plain/rich text segments retain XML document order; phonetic + annotations do not contribute cell text. - Sparse values, formulas, merges, hyperlinks and table references are cached only after complete worksheet parsing. Subsequent core methods do not reparse worksheets. Memory scales with extracted data and the shared-string dictionary; @@ -57,6 +59,10 @@ with OoxmlExtractionSession(path) as session: - `extract_cells()` returns `CellRow`: rows 1-based, columns 0-based numeric-string keys, existing missing-token filtering and numeric-string normalization. Sparse gaps do not cause rectangular blank cell materialization. +- Merged follower membership is indexed once per parsed sheet over stored + coordinates. Row/column bisect limits checks to each merge extent without + expanding empty rectangles. Hyperlinks use bisect over emitted row keys, + preserving last-link-wins behavior without scanning unrelated rows. - Shared strings (`s`), plain/rich inline strings (`inlineStr`), formula string caches (`str`), numeric (`n`), boolean (`b`) and ISO date (`d`) values are decoded. Formatting/phonetic annotations are omitted. Excel errors (`e`) are omitted, @@ -88,6 +94,31 @@ directory; absolute targets resolve from the ZIP root. External targets are preserved and never fetched or read as ZIP parts. These primitives serve hyperlinks, tables, worksheets, drawings and charts. Legacy drawing helpers remain available as wrappers. +When drawing extraction shares a core session, workbook enumeration uses the +same resolved officeDocument part as core extraction, including a relocated +workbook. Standalone path-based drawing reads retain their conventional default. + +## Review follow-up validation + +PR #154 adds regressions for accepted interleaved text segments in shared and +inline strings, a relocated `custom/book.xml` read before any cell extraction, +and sheet-wide sparse merged ranges with overlapping hyperlink overrides. +The relocated fixture includes both a chart and a shape and verifies one ZIP +open per session. Merge coverage indexes only stored coordinates and is reused +across linked/unlinked row extraction. + +A local Windows synthetic comparison of the original `807e206` row builder +and the revised builder used 40,000 stored cells (20,000 rows, two columns), +200 single-row merges and 200 links. Exact output equality was asserted: +original 0.684129 s, revised first call 0.124096 s, revised cached call 0.099105 s. +These are row-construction timings from one run, not workbook extraction or +memory measurements. + +Follow-up validation: 27 focused tests passed; 1,040 full tests passed with +12 external-runtime tests deselected and 83.57% coverage. Existing eight-workbook +parity, Ruff, strict mypy and diff checks passed. These fixes restore extraction +semantics and improve internal indexing; they introduce no backend-selection, +public mode or fallback policy change and require no new ADR. ## ADR assessment diff --git a/src/exstruct/core/ooxml_drawing.py b/src/exstruct/core/ooxml_drawing.py index 685bc9f3..103f9a30 100644 --- a/src/exstruct/core/ooxml_drawing.py +++ b/src/exstruct/core/ooxml_drawing.py @@ -183,21 +183,31 @@ def read_sheet_drawings(file_path: Path) -> dict[str, SheetDrawingData]: return _read_sheet_drawings_from_archive(archive, file_path) -def read_sheet_drawings_from_archive(archive: ZipFile) -> dict[str, SheetDrawingData]: +def read_sheet_drawings_from_archive( + archive: ZipFile, + *, + workbook_path: str = "xl/workbook.xml", +) -> dict[str, SheetDrawingData]: """Read worksheet drawing metadata from an already-open OOXML archive.""" source = getattr(archive, "filename", None) or "" - return _read_sheet_drawings_from_archive(archive, source) + return _read_sheet_drawings_from_archive( + archive, + source, + workbook_path=workbook_path, + ) def _read_sheet_drawings_from_archive( archive: ZipFile, source: object, + *, + workbook_path: str = "xl/workbook.xml", ) -> dict[str, SheetDrawingData]: """Read drawing metadata while preserving per-sheet parse fallbacks.""" result: dict[str, SheetDrawingData] = {} - for sheet_name, sheet_xml_path in _iter_sheet_xml_paths(archive): + for sheet_name, sheet_xml_path in _iter_sheet_xml_paths(archive, workbook_path): try: drawing_path = _resolve_sheet_drawing_path(archive, sheet_xml_path) if drawing_path is None: @@ -224,12 +234,15 @@ def _read_sheet_drawings_from_archive( return result -def _iter_sheet_xml_paths(archive: ZipFile) -> list[tuple[str, str]]: +def _iter_sheet_xml_paths( + archive: ZipFile, + workbook_path: str = "xl/workbook.xml", +) -> list[tuple[str, str]]: """Return workbook sheet names paired with their OOXML worksheet paths.""" - workbook_xml = archive.read("xl/workbook.xml") + workbook_xml = archive.read(workbook_path) workbook_root = ElementTree.fromstring(workbook_xml) - rel_map = _read_relationships(archive, relationship_part_path("xl/workbook.xml")) + rel_map = _read_relationships(archive, relationship_part_path(workbook_path)) paths: list[tuple[str, str]] = [] for sheet in workbook_root.findall("spreadsheetml:sheets/spreadsheetml:sheet", _NS): name = sheet.attrib.get("name") diff --git a/src/exstruct/core/ooxml_session.py b/src/exstruct/core/ooxml_session.py index 3dab81c8..1db426f7 100644 --- a/src/exstruct/core/ooxml_session.py +++ b/src/exstruct/core/ooxml_session.py @@ -2,6 +2,7 @@ from __future__ import annotations +from bisect import bisect_left, bisect_right from dataclasses import dataclass, field import logging from pathlib import Path @@ -60,6 +61,8 @@ class OoxmlTable: @dataclass(frozen=True) class _Formula: + """Stored formula text and optional shared-formula anchor identity.""" + coordinate: str text: str | None kind: str @@ -68,18 +71,52 @@ class _Formula: @dataclass class _SheetData: + """Sparse worksheet artifacts cached only after successful XML parsing.""" + values: dict[tuple[int, int], object] = field(default_factory=dict) formulas: list[_Formula] = field(default_factory=list) merged: list[str] = field(default_factory=list) links: list[tuple[str, str]] = field(default_factory=list) table_ids: list[str] = field(default_factory=list) + covered_followers: set[tuple[int, int]] | None = None def _string_text(node: ElementTree.Element) -> str: """Join plain/rich text runs without phonetic annotations.""" - return "".join(t.text or "" for t in node.findall("s:t", _NS)) + "".join( - t.text or "" for t in node.findall("s:r/s:t", _NS) - ) + parts: list[str] = [] + for child in node: + if child.tag == f"{{{_MAIN}}}t": + parts.append(child.text or "") + elif child.tag == f"{{{_MAIN}}}r": + parts.extend(text.text or "" for text in child.findall("s:t", _NS)) + return "".join(parts) + + +def _covered_followers(data: _SheetData) -> set[tuple[int, int]]: + """Index stored merge followers once without expanding empty coordinates.""" + if data.covered_followers is not None: + return data.covered_followers + if not data.merged: + data.covered_followers = set() + return data.covered_followers + columns_by_row: dict[int, list[int]] = {} + for row, col in sorted(data.values): + columns_by_row.setdefault(row, []).append(col) + row_keys = list(columns_by_row) + covered: set[tuple[int, int]] = set() + for ref in data.merged: + c1, r1, c2, r2 = _bounds(ref) + start = bisect_left(row_keys, r1) + stop = bisect_right(row_keys, r2) + for row in row_keys[start:stop]: + columns = columns_by_row[row] + left = bisect_left(columns, c1) + right = bisect_right(columns, c2) + covered.update( + (row, col) for col in columns[left:right] if (row, col) != (r1, c1) + ) + data.covered_followers = covered + return covered def _bounds(ref: str) -> tuple[int, int, int, int]: @@ -102,6 +139,7 @@ class OoxmlExtractionSession: """ def __init__(self, file_path: Path) -> None: + """Initialize lazy archive ownership and extraction-local caches.""" self.file_path = file_path self._archive: ZipFile | None = None self._closed = False @@ -114,10 +152,12 @@ def __init__(self, file_path: Path) -> None: self._drawings: dict[str, SheetDrawingData] | None = None def __enter__(self) -> OoxmlExtractionSession: + """Enter without opening a ZIP until a feature needs its parts.""" self._ensure_open() return self def _ensure_open(self) -> None: + """Reject access after the session has released its resources.""" if self._closed: raise RuntimeError("OOXML extraction session is closed") @@ -140,6 +180,7 @@ def relationships(self, source_path: str) -> dict[str, OoxmlRelationship]: return self._relationships[source_path] def _workbook(self) -> ElementTree.Element: + """Resolve the package officeDocument relationship and cache its XML.""" self._ensure_open() if self._workbook_root is None: for rel in self.relationships("").values(): @@ -188,6 +229,7 @@ def defined_names(self) -> list[OoxmlDefinedName]: ] def _related_part(self, kind: str) -> str | None: + """Find a workbook-owned internal relationship of the requested kind.""" self._workbook() return next( ( @@ -199,6 +241,7 @@ def _related_part(self, kind: str) -> str | None: ) def _shared_strings(self) -> list[str]: + """Stream the shared-string dictionary once without retaining XML.""" if self._strings is None: strings: list[str] = [] part = self._related_part("sharedStrings") @@ -217,6 +260,7 @@ def _shared_strings(self) -> list[str]: return self._strings def _number_formats(self) -> list[str]: + """Cache scalar number formats from built-in and custom style records.""" if self._formats is None: formats: list[str] = [] part = self._related_part("styles") @@ -235,6 +279,7 @@ def _number_formats(self) -> list[str]: return self._formats def _cell_value(self, node: ElementTree.Element) -> object: + """Decode the stored scalar cache and honor date/time number formats.""" kind = node.get("t", "n") text = node.findtext("s:v", namespaces=_NS) if kind == "inlineStr": @@ -274,6 +319,7 @@ def _cell_value(self, node: ElementTree.Element) -> object: return int(value) if int(value) == value else value def _read_sheet(self, sheet: OoxmlSheet) -> _SheetData: + """Stream worksheet artifacts, publishing the cache only after success.""" self._ensure_open() if sheet.name in self._sheet_cache: return self._sheet_cache[sheet.name] @@ -315,6 +361,7 @@ def _sheet_element( data: _SheetData, rels: dict[str, OoxmlRelationship], ) -> None: + """Collect a completed worksheet element before its XML is released.""" if tag == "c": coordinate = node.attrib["r"] data.values[coordinate_to_tuple(coordinate)] = self._cell_value(node) @@ -342,6 +389,7 @@ def _sheet_element( data.table_ids.append(node.attrib[f"{{{_REL}}}id"]) def _worksheets(self) -> list[OoxmlSheet]: + """Filter workbook-order metadata to worksheet relationships.""" return [sheet for sheet in self.sheets() if sheet.kind == "worksheet"] def extract_cells(self, *, include_links: bool = False) -> dict[str, list[CellRow]]: @@ -354,26 +402,26 @@ def extract_cells(self, *, include_links: bool = False) -> dict[str, list[CellRo } def _cell_rows(self, data: _SheetData, *, include_links: bool) -> list[CellRow]: + """Normalize sparse values and restrict links to emitted rows in range.""" rows: dict[int, dict[str, int | float | str]] = {} - merged = [_bounds(ref) for ref in data.merged] + covered = _covered_followers(data) for (row, col), raw in sorted(data.values.items()): - if any( - c1 <= col <= c2 and r1 <= row <= r2 and (row, col) != (r1, c1) - for c1, r1, c2, r2 in merged - ): + if (row, col) in covered: continue value = _normalize_cell_value(raw) if value is not None: rows.setdefault(row, {})[str(col - 1)] = value links: dict[int, dict[str, str]] = {} if include_links: + row_keys = list(rows) for ref, target in data.links: c1, r1, c2, r2 = _bounds(ref) - for row in rows: - if r1 <= row <= r2: - links.setdefault(row, {}).update( - {str(col - 1): target for col in range(c1, c2 + 1)} - ) + start = bisect_left(row_keys, r1) + stop = bisect_right(row_keys, r2) + for row in row_keys[start:stop]: + links.setdefault(row, {}).update( + {str(col - 1): target for col in range(c1, c2 + 1)} + ) return [ CellRow(r=row, c=values, links=links.get(row)) for row, values in rows.items() @@ -389,6 +437,7 @@ def extract_formulas_map(self) -> WorkbookFormulasMap: ) def _formulas_map(self, name: str, data: _SheetData) -> SheetFormulasMap: + """Group normalized formula text, translating known shared anchors.""" shared = { f.shared_id: f for f in data.formulas if f.kind == "shared" and f.text } @@ -504,7 +553,10 @@ def read_drawings(self) -> dict[str, SheetDrawingData]: self._ensure_open() if self._drawings is None: - self._drawings = read_sheet_drawings_from_archive(self.archive()) + self._workbook() + self._drawings = read_sheet_drawings_from_archive( + self.archive(), workbook_path=self._workbook_path + ) return self._drawings def close(self) -> None: @@ -522,4 +574,5 @@ def close(self) -> None: self._drawings = None def __exit__(self, *exc_info: object) -> None: + """Release owned resources on normal exit and on extraction errors.""" self.close() diff --git a/tests/core/test_ooxml_extraction_session.py b/tests/core/test_ooxml_extraction_session.py index 1eeb6074..479d9803 100644 --- a/tests/core/test_ooxml_extraction_session.py +++ b/tests/core/test_ooxml_extraction_session.py @@ -19,7 +19,8 @@ from exstruct.core import ooxml_session from exstruct.core.backends.ooxml_backend import OoxmlRichBackend from exstruct.core.backends.openpyxl_backend import OpenpyxlBackend -from exstruct.core.ooxml_session import OoxmlExtractionSession +from exstruct.core.ooxml_session import OoxmlExtractionSession, _SheetData +from exstruct.models import CellRow MAIN = "http://schemas.openxmlformats.org/spreadsheetml/2006/main" REL = "http://schemas.openxmlformats.org/officeDocument/2006/relationships" @@ -27,6 +28,7 @@ def make_book(path: Path) -> None: + """Build a sparse multi-feature archive for current-backend comparisons.""" wb = Workbook() ws = wb.active assert ws is not None @@ -74,6 +76,7 @@ def make_book(path: Path) -> None: def replace_parts(path: Path, parts: dict[str, bytes]) -> None: + """Replace specified ZIP parts while retaining all other fixture members.""" with ZipFile(path) as archive: payload = {name: archive.read(name) for name in archive.namelist()} payload.update(parts) @@ -85,6 +88,7 @@ def replace_parts(path: Path, parts: dict[str, bytes]) -> None: @pytest.mark.parametrize("suffix", [".xlsx", ".xlsm"]) @pytest.mark.parametrize("include_links", [False, True]) def test_core_parity(tmp_path: Path, suffix: str, include_links: bool) -> None: + """Match core artifacts and metadata for both supported archive suffixes.""" path = tmp_path / f"book{suffix}" make_book(path) expected = OpenpyxlBackend(path) @@ -116,6 +120,7 @@ def test_core_parity(tmp_path: Path, suffix: str, include_links: bool) -> None: def test_shared_strings_cached_formulas_and_shared_formula_parity( tmp_path: Path, ) -> None: + """Match rich strings and cached/shared/array formulas against openpyxl.""" path = tmp_path / "book.xlsx" make_book(path) with ZipFile(path) as archive: @@ -167,6 +172,7 @@ def test_shared_strings_cached_formulas_and_shared_formula_parity( def test_archive_and_worksheet_reads_reused_and_rich_parity( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + """Reuse one ZIP and sheet parse while retaining standalone rich output.""" path = tmp_path / "book.xlsx" make_book(path) standalone = OoxmlRichBackend(path) @@ -209,6 +215,7 @@ def read(name: str, *args: object, **kwargs: object) -> object: def test_exception_closes_and_reuse_rejected(tmp_path: Path) -> None: + """Release failed extractions and reject subsequent session access.""" path = tmp_path / "book.xlsx" make_book(path) replace_parts(path, {"xl/worksheets/sheet1.xml": b""}) @@ -229,6 +236,7 @@ def test_exception_closes_and_reuse_rejected(tmp_path: Path) -> None: def test_defused_stream_rejects_entities(tmp_path: Path) -> None: + """Reject entity-bearing XML before it can produce worksheet values.""" path = tmp_path / "book.xlsx" make_book(path) replace_parts( @@ -244,6 +252,7 @@ def test_defused_stream_rejects_entities(tmp_path: Path) -> None: def test_worksheet_stream_never_uses_archive_read( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + """Stream sparse sheets without constructing an openpyxl workbook.""" path = tmp_path / "book.xlsx" make_book(path) with OoxmlExtractionSession(path) as session: @@ -263,6 +272,7 @@ def read(name: str, *args: object, **kwargs: object) -> bytes: def test_sessions_observe_updates_and_reject_biff(tmp_path: Path) -> None: + """Avoid cross-session caching and reject unsupported BIFF input.""" path = tmp_path / "book.xlsx" make_book(path) with OoxmlExtractionSession(path) as first: @@ -282,6 +292,7 @@ def test_sessions_observe_updates_and_reject_biff(tmp_path: Path) -> None: def test_implicit_coordinates_and_out_of_range_date_parity(tmp_path: Path) -> None: + """Match inferred coordinates and error-like date overflow handling.""" path = tmp_path / "implicit.xlsx" make_book(path) with ZipFile(path) as archive: @@ -313,6 +324,7 @@ def test_implicit_coordinates_and_out_of_range_date_parity(tmp_path: Path) -> No def test_mac_epoch_and_iso_date_parity(tmp_path: Path) -> None: + """Honor the workbook's 1904 epoch for date and time cells.""" path = tmp_path / "epoch.xlsx" wb = Workbook() wb.epoch = MAC_EPOCH @@ -331,6 +343,7 @@ def test_mac_epoch_and_iso_date_parity(tmp_path: Path) -> None: def test_foreign_rich_session_rejected_and_unresolved_formula_skipped( tmp_path: Path, ) -> None: + """Skip unresolved shared text and reject a foreign workbook session.""" path = tmp_path / "formulas.xlsx" make_book(path) replace_parts( @@ -346,3 +359,73 @@ def test_foreign_rich_session_rejected_and_unresolved_formula_skipped( assert session.extract_cells()["O'Brien, data"][0].c == {"0": 1} with pytest.raises(ValueError, match="different workbook"): OoxmlRichBackend(tmp_path / "foreign.xlsx", session=session) + + +@pytest.mark.parametrize("kind", ["s", "inlineStr"]) +def test_plain_and_rich_segments_keep_document_order(tmp_path: Path, kind: str) -> None: + """Retain accepted XML segment order while omitting phonetic annotations.""" + path = tmp_path / "strings.xlsx" + text = 'first middle ignoredlast' + cell = "0" if kind == "s" else f"{text}" + with ZipFile(path, "w") as archive: + archive.writestr( + "xl/workbook.xml", + f'', + ) + archive.writestr( + "xl/_rels/workbook.xml.rels", + f'', + ) + archive.writestr( + "xl/sharedStrings.xml", f'{text}' + ) + archive.writestr( + "xl/worksheets/sheet1.xml", + f'{cell}', + ) + with OoxmlExtractionSession(path) as session: + assert session.extract_cells() == { + "Sheet": [CellRow(r=1, c={"0": "first middle last"})] + } + + +def test_sparse_merged_coverage_and_range_link_semantics(tmp_path: Path) -> None: + """Avoid expanding a sheet-wide merge and retain link override/filter rules.""" + data = _SheetData( + values={ + (1, 1): "first", + (1, 2): "covered", + (1, 5): "outside", + (2, 1): "covered", + (3, 3): "third", + (3, 4): "covered", + (4, 1): "fourth", + (500000, 16384): "covered", + (1000000, 1): "covered", + }, + merged=["A1:B2", "C3:D3", "A4:XFD1048576"], + links=[ + ("B1:F3", "https://example.com/range"), + ("E1", "https://example.com/override"), + ("A2", "https://example.com/hidden"), + ], + ) + session = OoxmlExtractionSession(tmp_path / "unused.xlsx") + plain = session._cell_rows(data, include_links=False) + linked = session._cell_rows(data, include_links=True) + assert [(r.r, r.c) for r in plain] == [ + (1, {"0": "first", "4": "outside"}), + (3, {"2": "third"}), + (4, {"0": "fourth"}), + ] + assert [(r.r, r.c) for r in linked] == [(r.r, r.c) for r in plain] + assert linked[0].links == { + "1": "https://example.com/range", + "2": "https://example.com/range", + "3": "https://example.com/range", + "4": "https://example.com/override", + "5": "https://example.com/range", + } + assert linked[1].links == {str(c): "https://example.com/range" for c in range(1, 6)} + assert linked[2].links is None + assert session._cell_rows(data, include_links=True) == linked diff --git a/tests/core/test_ooxml_relocated_workbook.py b/tests/core/test_ooxml_relocated_workbook.py new file mode 100644 index 00000000..313eb97e --- /dev/null +++ b/tests/core/test_ooxml_relocated_workbook.py @@ -0,0 +1,175 @@ +"""Regression coverage for OOXML workbooks outside the conventional xl path.""" + +from pathlib import Path +import posixpath +from xml.etree import ElementTree as ET +from zipfile import ZipFile + +from openpyxl import Workbook +from openpyxl.chart import BarChart, Reference +import pytest + +from exstruct.core import ooxml_session +from exstruct.core.backends.ooxml_backend import OoxmlRichBackend +from exstruct.core.ooxml_session import OoxmlExtractionSession + +MAIN = "http://schemas.openxmlformats.org/spreadsheetml/2006/main" +REL = "http://schemas.openxmlformats.org/officeDocument/2006/relationships" +PKG = "http://schemas.openxmlformats.org/package/2006/relationships" +CONTENT_TYPES = "http://schemas.openxmlformats.org/package/2006/content-types" +DRAWING = "http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing" +DRAWINGML = "http://schemas.openxmlformats.org/drawingml/2006/main" + + +def _make_chart_workbook(path: Path) -> None: + """Build identical chart-bearing workbooks for layout comparisons.""" + workbook = Workbook() + sheet = workbook.active + assert sheet is not None + sheet.title = "Data" + sheet.append(["Category", "Value"]) + sheet.append(["A", 7]) + sheet.append(["B", 12]) + + chart = BarChart() + chart.add_data( + Reference(sheet, min_col=2, min_row=1, max_row=3), titles_from_data=True + ) + chart.set_categories(Reference(sheet, min_col=1, min_row=2, max_row=3)) + sheet.add_chart(chart, "D1") + workbook.save(path) + workbook.close() + + +def _append_shape(drawing_xml: bytes) -> bytes: + """Add a deterministic shape anchor beside the generated chart.""" + ET.register_namespace("xdr", DRAWING) + ET.register_namespace("a", DRAWINGML) + root = ET.fromstring(drawing_xml) + root.append( + ET.fromstring( + f""" + + + 00 + 40 + + + 20 + 60 + + + + + + + + + + + + Relocated shape + + + + + """ + ) + ) + return ET.tostring(root, encoding="utf-8", xml_declaration=True) + + +def _add_shape_to_workbook(path: Path) -> None: + """Inject the same shape into each workbook before relocating its parts.""" + with ZipFile(path) as archive: + parts = {name: archive.read(name) for name in archive.namelist()} + parts["xl/drawings/drawing1.xml"] = _append_shape(parts["xl/drawings/drawing1.xml"]) + with ZipFile(path, "w") as archive: + for name, data in parts.items(): + archive.writestr(name, data) + + +def _relocate_workbook_part(path: Path) -> None: + """Move the workbook and repair root, part and content-type references.""" + with ZipFile(path) as archive: + parts = {name: archive.read(name) for name in archive.namelist()} + + parts["custom/book.xml"] = parts.pop("xl/workbook.xml") + + workbook_rels = ET.fromstring(parts.pop("xl/_rels/workbook.xml.rels")) + for relationship in workbook_rels.findall(f"{{{PKG}}}Relationship"): + if relationship.get("TargetMode") == "External": + continue + target = relationship.get("Target") + if not target: + continue + old_part_path = ( + target.lstrip("/") + if target.startswith("/") + else posixpath.normpath(posixpath.join("xl", target)) + ) + relationship.set("Target", posixpath.relpath(old_part_path, "custom")) + parts["custom/_rels/book.xml.rels"] = ET.tostring(workbook_rels) + + root_rels = ET.fromstring(parts["_rels/.rels"]) + for relationship in root_rels.findall(f"{{{PKG}}}Relationship"): + if relationship.get("Type") == f"{REL}/officeDocument": + relationship.set("Target", "custom/book.xml") + break + parts["_rels/.rels"] = ET.tostring(root_rels) + + content_types = ET.fromstring(parts["[Content_Types].xml"]) + for override in content_types.findall(f"{{{CONTENT_TYPES}}}Override"): + if override.get("PartName") == "/xl/workbook.xml": + override.set("PartName", "/custom/book.xml") + break + parts["[Content_Types].xml"] = ET.tostring(content_types) + + with ZipFile(path, "w") as archive: + for name, data in parts.items(): + archive.writestr(name, data) + + +def _read_outputs(path: Path) -> tuple[object, object, object, object]: + """Exercise drawings first, then core and rich consumers of the same ZIP.""" + with OoxmlExtractionSession(path) as session: + drawings = session.read_drawings() + cells = session.extract_cells() + rich = OoxmlRichBackend(path, session=session) + charts = rich.extract_charts(mode="light") + shapes = rich.extract_shapes(mode="light") + return cells, charts, shapes, drawings + + +def test_relocated_workbook_drawings_first_matches_standard_layout( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Retain cells, charts and shapes with one archive per relocated session.""" + standard_path = tmp_path / "standard.xlsx" + relocated_path = tmp_path / "relocated.xlsx" + _make_chart_workbook(standard_path) + _make_chart_workbook(relocated_path) + _add_shape_to_workbook(standard_path) + _add_shape_to_workbook(relocated_path) + _relocate_workbook_part(relocated_path) + + opened: list[ZipFile] = [] + real_zip = ZipFile + + def track(path: Path) -> ZipFile: + archive = real_zip(path) + opened.append(archive) + return archive + + monkeypatch.setattr(ooxml_session, "ZipFile", track) + standard_outputs = _read_outputs(standard_path) + relocated_outputs = _read_outputs(relocated_path) + + assert relocated_outputs == standard_outputs + assert standard_outputs[1]["Data"] + assert standard_outputs[2]["Data"] + assert standard_outputs[3]["Data"].charts + assert standard_outputs[3]["Data"].shapes + assert len(opened) == 2 + assert all(archive.fp is None for archive in opened)