From 5fecd91cd9897b02787da5a91c754c3606c27863 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Thu, 1 Oct 2026 15:23:59 +0200 Subject: [PATCH 1/4] fix: preserve collection sources and outputs on export failure --- .github/workflows/source-platform-smoke.yml | 7 + core/pdf/collection.py | 38 ++++- tests/test_collection_publication.py | 150 ++++++++++++++++++++ 3 files changed, 191 insertions(+), 4 deletions(-) create mode 100644 tests/test_collection_publication.py diff --git a/.github/workflows/source-platform-smoke.yml b/.github/workflows/source-platform-smoke.yml index 124e605..8a41aa9 100644 --- a/.github/workflows/source-platform-smoke.yml +++ b/.github/workflows/source-platform-smoke.yml @@ -81,6 +81,13 @@ jobs: run: | python -m pytest tests/test_search_bar_platform_open.py tests/test_registry_platform_import.py -v + - name: Run collection export and output protection tests + env: + QT_QPA_PLATFORM: offscreen + PYTHONIOENCODING: utf-8 + run: | + python -m pytest tests/test_pdf_collection.py tests/test_collection_publication.py -v + - name: Run store-materials and readiness tests env: PYTHONIOENCODING: utf-8 diff --git a/core/pdf/collection.py b/core/pdf/collection.py index 209cf41..42d79fb 100644 --- a/core/pdf/collection.py +++ b/core/pdf/collection.py @@ -10,6 +10,7 @@ from dataclasses import dataclass, field from pathlib import Path +import os import tempfile from typing import List, Optional, Sequence, Tuple, Union @@ -76,6 +77,18 @@ def __init__(self): if not PYMUPDF_AVAILABLE: self.logger.warning("PyMuPDF ist nicht verfügbar") + @staticmethod + def _check_output_sources(destination: Path, originals: Sequence[Path]) -> None: + for source in originals: + if destination.resolve() == source.resolve(): + raise ValueError(f"Exportziel ist ein Quelldokument: {source}") + try: + same_file = destination.samefile(source) + except FileNotFoundError: + same_file = False + if same_file: + raise ValueError(f"Exportziel verweist auf ein Quelldokument: {source}") + def export( self, documents: Sequence[Union[str, Path]], @@ -109,18 +122,21 @@ def export( opts = options or CollectionExportOptions() out_p = Path(output_path) - out_p.parent.mkdir(parents=True, exist_ok=True) output_doc = None temp_files: List[Path] = [] skipped: List[str] = [] toc_entries: List[List[Union[int, str]]] = [] doc_count = 0 + published = False try: + originals = tuple(Path(entry) for entry in documents) + self._check_output_sources(out_p, originals) + out_p.parent.mkdir(parents=True, exist_ok=True) output_doc = fitz.open() - for doc_entry in documents: + for doc_entry in originals: p = Path(doc_entry) if not p.exists() or not p.is_file(): self.logger.warning("Datei übersprungen (nicht gefunden): %s", p) @@ -184,7 +200,15 @@ def export( output_doc.set_metadata(meta) # Speichern - output_doc.save(str(out_p), garbage=3, deflate=True) + with tempfile.TemporaryDirectory(prefix=".dokuzen-collection-", dir=out_p.parent) as directory: + staged = Path(directory) / "collection.pdf" + output_doc.save(str(staged), garbage=3, deflate=True) + with fitz.open(str(staged)) as verified: + if verified.is_encrypted or len(verified) != total_pages: + raise ValueError("Die erzeugte Sammel-PDF ist unvollständig.") + self._check_output_sources(out_p, originals) + os.replace(staged, out_p) + published = True self.logger.info( "Sammel-PDF erfolgreich erstellt: %s (%d Seiten aus %d Dokumenten)", out_p, @@ -201,6 +225,12 @@ def export( ) except Exception as exc: + if published: + self.logger.warning("Sammel-PDF veröffentlicht; nachträgliche Bereinigung fehlgeschlagen: %s", exc) + return CollectionExportResult( + success=True, output_path=str(out_p), total_pages=total_pages, + document_count=doc_count, skipped_files=skipped, + ) self.logger.error("Fehler beim Sammel-PDF-Export: %s", exc, exc_info=True) return CollectionExportResult( success=False, @@ -212,7 +242,7 @@ def export( ) finally: - if output_doc: + if output_doc is not None: output_doc.close() # Temporäre Konvertierungsdateien bereinigen for tf in temp_files: diff --git a/tests/test_collection_publication.py b/tests/test_collection_publication.py new file mode 100644 index 0000000..9d9a690 --- /dev/null +++ b/tests/test_collection_publication.py @@ -0,0 +1,150 @@ +import os +from pathlib import Path + +import fitz +import pytest + +from core.pdf import collection + +def create_pdf(path, text='Original'): + with fitz.open() as doc: + page = doc.new_page() + page.insert_text((50, 50), text) + doc.save(path) + +@pytest.mark.parametrize('kind', ['pdf', 'text', 'hardlink', 'late-hardlink']) +def test_collection_preserves_all_originals(tmp_path, monkeypatch, kind): + source = tmp_path / ('original.txt' if kind == 'text' else 'original.pdf') + if kind == 'text': + source.write_text('Bücher und Originaltexte.', encoding='utf-8') + else: + create_pdf(source) + original = source.read_bytes() + target = source if kind in ('pdf', 'text') else tmp_path / 'alias.pdf' + if kind == 'hardlink': + os.link(source, target) + if kind == 'late-hardlink': + real_save = fitz.Document.save + def save(doc, path, *args, **kwargs): + real_save(doc, path, *args, **kwargs) + if not target.exists(): + os.link(source, target) + monkeypatch.setattr(fitz.Document, 'save', save) + result = collection.export_collection_pdf([source], target) + assert not result.success + assert result.error + assert source.read_bytes() == original + assert target.read_bytes() == original + assert set(tmp_path.iterdir()) == ({source} if target == source else {source, target}) + +@pytest.mark.parametrize('stage', ['partial', 'empty', 'truncated', 'publish']) +@pytest.mark.parametrize('existing', [False, True]) +def test_failed_collection_preserves_previous_output(tmp_path, monkeypatch, stage, existing): + source = tmp_path / 'source.pdf' + target = tmp_path / 'report.pdf' + create_pdf(source) + source_bytes = source.read_bytes() + old = b'Previous output bytes' + if existing: + target.write_bytes(old) + real_save = fitz.Document.save + def save(doc, path, *args, **kwargs): + if stage == 'publish': + return real_save(doc, path, *args, **kwargs) + Path(path).write_bytes(b'' if stage == 'empty' else b'%PDF-incomplete') + if stage == 'partial': + raise OSError('Interrupted output write') + monkeypatch.setattr(fitz.Document, 'save', save) + if stage == 'publish': + def fail_replace(*args): + raise PermissionError('Output locked') + monkeypatch.setattr(os, 'replace', fail_replace) + result = collection.export_collection_pdf([source], target) + assert not result.success + assert result.error + assert source.read_bytes() == source_bytes + assert target.read_bytes() == old if existing else not target.exists() + assert set(tmp_path.iterdir()) == ({source, target} if existing else {source}) + +def test_regular_collection_replaces_pdf_and_keeps_bookmarks(tmp_path): + source = tmp_path / 'chapter.pdf' + target = tmp_path / 'report.pdf' + create_pdf(source) + create_pdf(target, 'Previous report') + source_bytes = source.read_bytes() + result = collection.export_collection_pdf([source], target) + assert result.success and result.total_pages == 1 and result.document_count == 1 + assert source.read_bytes() == source_bytes + with fitz.open(target) as doc: + assert len(doc) == 1 + assert 'Original' in doc[0].get_text() + assert doc.get_toc() == [[1, 'chapter', 1]] + assert set(tmp_path.iterdir()) == {source, target} + + +def test_invalid_output_parent_returns_failure(tmp_path): + source = tmp_path / 'source.pdf' + create_pdf(source) + blocked_parent = tmp_path / 'file' + blocked_parent.write_bytes(b'Keep this file') + result = collection.export_collection_pdf([source], blocked_parent / 'report.pdf') + assert not result.success and result.error + assert blocked_parent.read_bytes() == b'Keep this file' + + +def test_empty_collection_document_is_closed(tmp_path, monkeypatch): + source = tmp_path / 'unsupported.xyz' + source.write_bytes(b'Unsupported') + real_open = fitz.open + opened = [] + + def track_open(*args, **kwargs): + doc = real_open(*args, **kwargs) + opened.append(doc) + return doc + + monkeypatch.setattr(fitz, 'open', track_open) + result = collection.export_collection_pdf([source], tmp_path / 'report.pdf') + assert not result.success + assert len(opened) == 1 and opened[0].is_closed + + +def test_cleanup_failure_after_publication_reports_success(tmp_path, monkeypatch): + source = tmp_path / 'source.pdf' + target = tmp_path / 'report.pdf' + create_pdf(source) + real_directory = collection.tempfile.TemporaryDirectory + + class CleanupFailure: + def __init__(self, *args, **kwargs): + self.directory = real_directory(*args, **kwargs) + + def __enter__(self): + return self.directory.__enter__() + + def __exit__(self, *args): + self.directory.__exit__(*args) + raise PermissionError('Cleanup failed after publication') + + monkeypatch.setattr(collection.tempfile, 'TemporaryDirectory', CleanupFailure) + result = collection.export_collection_pdf([source], target) + assert result.success and result.total_pages == 1 + with fitz.open(target) as doc: + assert 'Original' in doc[0].get_text() + assert set(tmp_path.iterdir()) == {source, target} + + +def test_identity_check_permission_failure_preserves_output(tmp_path, monkeypatch): + source = tmp_path / 'source.pdf' + target = tmp_path / 'report.pdf' + create_pdf(source) + target.write_bytes(b'Previous output') + + def denied(*args): + raise PermissionError('Cannot establish file identity') + + monkeypatch.setattr(Path, 'samefile', denied) + result = collection.export_collection_pdf([source], target) + assert not result.success and result.error + assert target.read_bytes() == b'Previous output' + assert set(tmp_path.iterdir()) == {source, target} From f7b8152159a1bbd0db41463c20132a6f3bb3e5aa Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Thu, 1 Oct 2026 15:27:41 +0200 Subject: [PATCH 2/4] fix: require PDF output and repair Windows CI install --- .github/workflows/source-platform-smoke.yml | 13 +------------ core/pdf/collection.py | 2 +- tests/test_collection_publication.py | 6 +++++- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/.github/workflows/source-platform-smoke.yml b/.github/workflows/source-platform-smoke.yml index 2e6a259..9a0510a 100644 --- a/.github/workflows/source-platform-smoke.yml +++ b/.github/workflows/source-platform-smoke.yml @@ -54,18 +54,7 @@ jobs: - name: Install Python deps (CI subset) run: | python -m pip install --upgrade pip - pip install \ - "PySide6>=6.5.0" \ - "PyMuPDF>=1.23.0" \ - "pikepdf>=8.0.0" \ - "pytesseract>=0.3.10" \ - "Pillow>=10.0.0" \ - "python-docx>=1.1.0" \ - "chardet>=5.0.0" \ - "rapidfuzz>=3.0.0" \ - pytest \ - pytest-subtests \ - ruff + python -m pip install "PySide6>=6.5.0" "PyMuPDF>=1.23.0" "pikepdf>=8.0.0" "pytesseract>=0.3.10" "Pillow>=10.0.0" "python-docx>=1.1.0" "chardet>=5.0.0" "rapidfuzz>=3.0.0" pytest pytest-subtests ruff - name: Run ruff linting check run: | diff --git a/core/pdf/collection.py b/core/pdf/collection.py index 42d79fb..b86a1a5 100644 --- a/core/pdf/collection.py +++ b/core/pdf/collection.py @@ -204,7 +204,7 @@ def export( staged = Path(directory) / "collection.pdf" output_doc.save(str(staged), garbage=3, deflate=True) with fitz.open(str(staged)) as verified: - if verified.is_encrypted or len(verified) != total_pages: + if not verified.is_pdf or verified.is_encrypted or len(verified) != total_pages: raise ValueError("Die erzeugte Sammel-PDF ist unvollständig.") self._check_output_sources(out_p, originals) os.replace(staged, out_p) diff --git a/tests/test_collection_publication.py b/tests/test_collection_publication.py index 9d9a690..7e5d6c7 100644 --- a/tests/test_collection_publication.py +++ b/tests/test_collection_publication.py @@ -37,7 +37,7 @@ def save(doc, path, *args, **kwargs): assert target.read_bytes() == original assert set(tmp_path.iterdir()) == ({source} if target == source else {source, target}) -@pytest.mark.parametrize('stage', ['partial', 'empty', 'truncated', 'publish']) +@pytest.mark.parametrize('stage', ['partial', 'empty', 'truncated', 'image', 'publish']) @pytest.mark.parametrize('existing', [False, True]) def test_failed_collection_preserves_previous_output(tmp_path, monkeypatch, stage, existing): source = tmp_path / 'source.pdf' @@ -51,6 +51,10 @@ def test_failed_collection_preserves_previous_output(tmp_path, monkeypatch, stag def save(doc, path, *args, **kwargs): if stage == 'publish': return real_save(doc, path, *args, **kwargs) + if stage == 'image': + from PIL import Image + Image.new('RGB', (10, 10), 'white').save(path, format='PNG') + return Path(path).write_bytes(b'' if stage == 'empty' else b'%PDF-incomplete') if stage == 'partial': raise OSError('Interrupted output write') From 76b835613bdf570590ef2aca334052b9f5c80ee0 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Thu, 1 Oct 2026 15:31:54 +0200 Subject: [PATCH 3/4] fix: release validation file handles and destroy smoke widgets safely --- core/pdf/collection.py | 3 ++- tests/test_source_platform_smoke.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/core/pdf/collection.py b/core/pdf/collection.py index b86a1a5..9090731 100644 --- a/core/pdf/collection.py +++ b/core/pdf/collection.py @@ -203,7 +203,8 @@ def export( with tempfile.TemporaryDirectory(prefix=".dokuzen-collection-", dir=out_p.parent) as directory: staged = Path(directory) / "collection.pdf" output_doc.save(str(staged), garbage=3, deflate=True) - with fitz.open(str(staged)) as verified: + # Parse bytes so a failed native open cannot retain a Windows file handle. + with fitz.open(stream=staged.read_bytes(), filetype="pdf") as verified: if not verified.is_pdf or verified.is_encrypted or len(verified) != total_pages: raise ValueError("Die erzeugte Sammel-PDF ist unvollständig.") self._check_output_sources(out_p, originals) diff --git a/tests/test_source_platform_smoke.py b/tests/test_source_platform_smoke.py index a2b8fa8..c2028e1 100644 --- a/tests/test_source_platform_smoke.py +++ b/tests/test_source_platform_smoke.py @@ -12,7 +12,9 @@ sys.path.insert(0, str(PROJECT_ROOT)) import fitz +from PySide6.QtCore import QCoreApplication, QEvent, QSettings from PySide6.QtWidgets import QApplication +from shiboken6 import isValid import gui.main_window as main_window_module from core.library.persistence import PersistenceManager @@ -30,6 +32,12 @@ def setUp(self): self.state_file = self.tmp_path / "dokuzen_state.json" self.original_state_file = PersistenceManager.DEFAULT_STATE_FILE PersistenceManager.DEFAULT_STATE_FILE = self.state_file + self._settings_patch = mock.patch( + "PySide6.QtCore.QSettings", + side_effect=lambda *args: QSettings(str(self.tmp_path / "settings.ini"), QSettings.Format.IniFormat), + ) + self._settings_patch.start() + self.addCleanup(self._settings_patch.stop) self.text_path = self.tmp_path / "Überblick_äöü.md" self.text_path.write_text( @@ -49,6 +57,10 @@ def setUp(self): def tearDown(self): try: self.window.close() + # Destroy widgets on the GUI thread while QApplication is still alive. + self.window.deleteLater() + QCoreApplication.sendPostedEvents(None, QEvent.Type.DeferredDelete) + self.assertFalse(isValid(self.window)) finally: PersistenceManager.DEFAULT_STATE_FILE = self.original_state_file self._tmp.cleanup() From 95b5b73f16a4b2e9f082611be5db1444b81d8463 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Thu, 1 Oct 2026 15:34:10 +0200 Subject: [PATCH 4/4] test: process deferred deletion only for the owned smoke window --- tests/test_source_platform_smoke.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_source_platform_smoke.py b/tests/test_source_platform_smoke.py index c2028e1..bb7abfe 100644 --- a/tests/test_source_platform_smoke.py +++ b/tests/test_source_platform_smoke.py @@ -59,7 +59,7 @@ def tearDown(self): self.window.close() # Destroy widgets on the GUI thread while QApplication is still alive. self.window.deleteLater() - QCoreApplication.sendPostedEvents(None, QEvent.Type.DeferredDelete) + QCoreApplication.sendPostedEvents(self.window, QEvent.Type.DeferredDelete) self.assertFalse(isValid(self.window)) finally: PersistenceManager.DEFAULT_STATE_FILE = self.original_state_file