diff --git a/CHANGELOG.md b/CHANGELOG.md index c316011e..e7ad0406 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. @@ -26,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/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..1a47d316 --- /dev/null +++ b/dev-docs/specs/ooxml-core-extraction.md @@ -0,0 +1,157 @@ +# 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. +- 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; + 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. +- 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, + 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. +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 + +- 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..103f9a30 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,41 +179,70 @@ 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, + *, + 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, + 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, workbook_path): + 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 -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, "xl/_rels/workbook.xml.rels") + 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") @@ -227,7 +250,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 +262,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 +282,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 +446,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 +893,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..1db426f7 --- /dev/null +++ b/src/exstruct/core/ooxml_session.py @@ -0,0 +1,578 @@ +"""Extraction-scoped, streaming access to core OOXML workbook parts (#149).""" + +from __future__ import annotations + +from bisect import bisect_left, bisect_right +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: + """Stored formula text and optional shared-formula anchor identity.""" + + coordinate: str + text: str | None + kind: str + shared_id: str | None + + +@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.""" + 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]: + """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: + """Initialize lazy archive ownership and extraction-local caches.""" + 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: + """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") + + 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: + """Resolve the package officeDocument relationship and cache its XML.""" + 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: + """Find a workbook-owned internal relationship of the requested kind.""" + 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]: + """Stream the shared-string dictionary once without retaining XML.""" + 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]: + """Cache scalar number formats from built-in and custom style records.""" + 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: + """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": + 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: + """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] + 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: + """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) + 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]: + """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]]: + """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]: + """Normalize sparse values and restrict links to emitted rows in range.""" + rows: dict[int, dict[str, int | float | str]] = {} + covered = _covered_followers(data) + for (row, col), raw in sorted(data.values.items()): + 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) + 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() + ] + + 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: + """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 + } + 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._workbook() + self._drawings = read_sheet_drawings_from_archive( + self.archive(), workbook_path=self._workbook_path + ) + 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: + """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 new file mode 100644 index 00000000..479d9803 --- /dev/null +++ b/tests/core/test_ooxml_extraction_session.py @@ -0,0 +1,431 @@ +"""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, _SheetData +from exstruct.models import CellRow + +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: + """Build a sparse multi-feature archive for current-backend comparisons.""" + 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: + """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) + 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: + """Match core artifacts and metadata for both supported archive suffixes.""" + 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: + """Match rich strings and cached/shared/array formulas against openpyxl.""" + 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: + """Reuse one ZIP and sheet parse while retaining standalone rich output.""" + 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: + """Release failed extractions and reject subsequent session access.""" + 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: + """Reject entity-bearing XML before it can produce worksheet values.""" + 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: + """Stream sparse sheets without constructing an openpyxl workbook.""" + 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: + """Avoid cross-session caching and reject unsupported BIFF input.""" + 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: + """Match inferred coordinates and error-like date overflow handling.""" + 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: + """Honor the workbook's 1904 epoch for date and time cells.""" + 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: + """Skip unresolved shared text and reject a foreign workbook session.""" + 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) + + +@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_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 == [] 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)