diff --git a/onenote_export/parser/one_store.py b/onenote_export/parser/one_store.py index 62016a8..4e00381 100644 --- a/onenote_export/parser/one_store.py +++ b/onenote_export/parser/one_store.py @@ -33,7 +33,25 @@ def _patch_pyonenote() -> None: cProperties : uint32 — number of child PropertySets prid : PropertyID (4 bytes) — only if cProperties > 0 Data : cProperties consecutive PropertySet structures + + 3. ``FileNode.__init__`` crashes on a FileNodeListFragment + *terminator* node. A terminator has ``file_node_id == 0`` (or + 255), which is not in pyOneNote's ``_FileNodeIDs`` map, so its + type resolves to ``"Invalid"`` and the dispatch chain's silent + ``else`` branch never sets ``self.data``. The constructor then + unconditionally recurses for reference nodes + (``if baseType == 2: FileNodeList(..., self.data.ref)``) and + dereferences the missing ``self.data``. The fragment loop *does* + break on ``id in (0, 255)`` — but only *after* the node is + constructed, so the crash beats the guard. Files whose root + fragment ends on a "dirty" terminator (id 0 with non-zero upper + bits, e.g. large/heavily-revised sections) fail to parse at all. + + We wrap the constructor so a terminator node that raises during + construction is treated as empty (``data = None``, no children); + the existing fragment-loop break then ends the fragment cleanly. """ + from pyOneNote.FileNode import FileNode as _FileNode from pyOneNote.FileNode import ObjectSpaceObjectStreamOfIDs _original_read = ObjectSpaceObjectStreamOfIDs.read @@ -121,6 +139,26 @@ def _patched_init( PropertySet.__init__ = _patched_init + _original_filenode_init = _FileNode.__init__ + + def _patched_filenode_init(self, file, document): + try: + _original_filenode_init(self, file, document) + except AttributeError: + # Only swallow the terminator case (id 0/255); anything else + # is a genuine parse error and must propagate. + header = getattr(self, "file_node_header", None) + fnid = getattr(header, "file_node_id", None) + if fnid in (0, 255): + if not hasattr(self, "data"): + self.data = None + if not hasattr(self, "children"): + self.children = [] + else: + raise + + _FileNode.__init__ = _patched_filenode_init + # Apply patches at import time _patch_pyonenote() @@ -350,12 +388,19 @@ def _build_pages(self, objects: list[ExtractedObject]) -> list[ExtractedPage]: pages.append(ExtractedPage(objects=all_content)) return pages - # Build a lookup: GUID -> page metadata + # Build a lookup: GUID -> page metadata. + # + # A page's object space contains MULTIPLE metadata revisions. The + # first one encountered in traversal order is the current revision; + # later ones are stale copies carrying an outdated PageLevel and/or + # title (from before the page was renamed or promoted/demoted to a + # subpage). Keep the first and never overwrite — overwriting with a + # stale copy corrupts both the subpage level and the title. meta_by_guid: dict[str, ExtractedObject] = {} for meta in page_metas: guid = _extract_guid(meta.identity) - # Later entries (newer revisions) overwrite earlier ones - meta_by_guid[guid] = meta + if guid not in meta_by_guid: + meta_by_guid[guid] = meta # Orphan metas: metadata GUIDs with no page node (old revisions) orphan_metas: dict[str, ExtractedObject] = { @@ -375,8 +420,13 @@ def _build_pages(self, objects: list[ExtractedObject]) -> list[ExtractedPage]: _NUMBER_LIST, } - # Build one page per content GUID (GUID that has a PageNode) - seen_titles: dict[str, int] = {} + # Build one page per content GUID (GUID that has a PageNode). + # Each unique page-node GUID is a distinct page; revisions of the + # *same* page already share a GUID and are collapsed by the + # ``page_node_guids`` uniqueness above. We deliberately do NOT + # deduplicate by title — distinct pages legitimately share titles + # (e.g. repeated "Example Output" subpages), and collapsing them + # silently drops real content. for content_guid in page_node_guids: objs = guid_objects.get(content_guid, []) @@ -418,15 +468,7 @@ def _build_pages(self, objects: list[ExtractedObject]) -> list[ExtractedPage]: objects=content, ) - # Deduplicate by title — keep the version with more content - key = title.lower().strip() - if key in seen_titles: - idx = seen_titles[key] - if len(content) > len(pages[idx].objects): - pages[idx] = page - else: - seen_titles[key] = len(pages) - pages.append(page) + pages.append(page) return pages diff --git a/tests/test_data/ExporterEval/DupTitles.one b/tests/test_data/ExporterEval/DupTitles.one new file mode 100644 index 0000000..0c7be18 Binary files /dev/null and b/tests/test_data/ExporterEval/DupTitles.one differ diff --git a/tests/test_data/ExporterEval/Hierarchy.one b/tests/test_data/ExporterEval/Hierarchy.one new file mode 100644 index 0000000..a44f031 Binary files /dev/null and b/tests/test_data/ExporterEval/Hierarchy.one differ diff --git a/tests/test_eval_notebook.py b/tests/test_eval_notebook.py new file mode 100644 index 0000000..86b0eaf --- /dev/null +++ b/tests/test_eval_notebook.py @@ -0,0 +1,52 @@ +"""Parser tests against the shareable ``ExporterEval`` fixture notebook. + +``ExporterEval`` is a small, synthetic (dummy-content) OneNote notebook built to +exercise these parser fixes, so it can be committed as public test data: + + * ``DupTitles`` — distinct pages that share a title must all survive (no + title-based collapse). + * ``Hierarchy`` — page display order and subpage levels (1-3) come straight + from the ``.one`` binary. +""" + +from pathlib import Path + +import pytest + +from onenote_export.parser.one_store import OneStoreParser + +EVAL = Path(__file__).parent / "test_data" / "ExporterEval" +_HAS_EVAL = EVAL.exists() and any(EVAL.glob("*.one")) + +pytestmark = pytest.mark.skipif( + not _HAS_EVAL, reason="ExporterEval fixture not available" +) + + +def _parse(name: str): + return OneStoreParser(EVAL / name).parse() + + +class TestDupTitles: + def test_all_same_titled_pages_retained(self): + sec = _parse("DupTitles.one") + titles = [p.title for p in sec.pages] + assert len(sec.pages) == 4 + assert titles.count("Note") == 3 # three distinct pages titled "Note" + assert "Unique" in titles + + +class TestHierarchy: + EXPECTED = [ + ("Zeta", 1), + ("Zeta-Sub-1", 2), + ("Zeta-Sub-2", 2), + ("Zeta-Sub-2a", 3), # level-3 subpage + ("Alpha", 1), + ("Alpha-Sub-1", 2), + ("Mike", 1), # leaf; last despite "M" — order is display order, not alphabetical + ] + + def test_order_and_levels(self): + sec = _parse("Hierarchy.one") + assert [(p.title, p.level) for p in sec.pages] == self.EXPECTED