diff --git a/docx_editor/__init__.py b/docx_editor/__init__.py index c1cbfb9..3fec10e 100644 --- a/docx_editor/__init__.py +++ b/docx_editor/__init__.py @@ -53,7 +53,7 @@ WorkspaceSyncError, XMLError, ) -from .track_changes import EditOperation, Revision +from .track_changes import EditOperation, EditValidationResult, Revision from .xml_editor import ( ParagraphInfo, ParagraphLocation, @@ -71,6 +71,7 @@ # Main classes "Document", "EditOperation", + "EditValidationResult", "Revision", "Comment", # Exceptions diff --git a/docx_editor/document.py b/docx_editor/document.py index 4e035bf..86af3db 100644 --- a/docx_editor/document.py +++ b/docx_editor/document.py @@ -8,11 +8,12 @@ import shutil from collections.abc import Iterator from pathlib import Path +from typing import Literal, overload from xml.dom.minidom import Element from .comments import Comment, CommentManager from .exceptions import HashMismatchError, ParagraphIndexError -from .track_changes import EditOperation, Revision, RevisionManager +from .track_changes import EditOperation, EditValidationResult, Revision, RevisionManager from .workspace import Workspace from .xml_editor import ( DocxXMLEditor, @@ -496,7 +497,15 @@ def insert_before(self, anchor: str, text: str, *, paragraph: str, occurrence: i self._revision_manager.insert_text_before(anchor, text, occurrence=occurrence, paragraph=paragraph) return self._compute_new_ref(paragraph) - def batch_edit(self, operations: list[EditOperation]) -> list[str]: + @overload + def batch_edit(self, operations: list[EditOperation], *, dry_run: Literal[False] = ...) -> list[str]: ... + + @overload + def batch_edit(self, operations: list[EditOperation], *, dry_run: Literal[True]) -> list[EditValidationResult]: ... + + def batch_edit( + self, operations: list[EditOperation], *, dry_run: bool = False + ) -> list[str] | list[EditValidationResult]: """Apply multiple edits atomically with upfront hash validation. All paragraph hashes are validated before any edits are applied. @@ -506,17 +515,34 @@ def batch_edit(self, operations: list[EditOperation]) -> list[str]: Args: operations: List of EditOperation objects + dry_run: If True, validate every operation without applying any + edits and return a list of EditValidationResult (one per + operation, in input order). The document is left unchanged. + Each operation is validated independently against the current + document; sequential effects between multiple operations on the + same paragraph are not simulated (see + RevisionManager.validate_batch). Returns: - List of new paragraph references with updated hashes, in input order. + When dry_run is False: list of new paragraph references with updated + hashes, in input order. + When dry_run is True: list of EditValidationResult, one per operation. Example: new_refs = doc.batch_edit([ EditOperation(action="replace", find="old", replace_with="new", paragraph="P20#a7b2"), EditOperation(action="delete", text="remove", paragraph="P15#f3c1"), ]) + + # Pre-flight the batch (note: same-paragraph sequential effects are + # not simulated — see the dry_run note above): + results = doc.batch_edit(ops, dry_run=True) + if all(r.valid for r in results): + doc.batch_edit(ops) """ self._ensure_open() + if dry_run: + return self._revision_manager.validate_batch(operations) self._revision_manager.batch_edit(operations) return [self._compute_new_ref(op.paragraph) for op in operations] diff --git a/docx_editor/track_changes.py b/docx_editor/track_changes.py index 47f6470..164cca2 100644 --- a/docx_editor/track_changes.py +++ b/docx_editor/track_changes.py @@ -44,6 +44,16 @@ class EditOperation: occurrence: int = 0 +@dataclass +class EditValidationResult: + """Outcome of validating one EditOperation in a dry-run batch.""" + + index: int # 0-based position in the input operations list + paragraph: str | None # the operation's paragraph ref (None if it was missing) + valid: bool # True if the op would apply cleanly + error: str | None = None # human-readable reason when not valid + + @dataclass class Revision: """Represents a tracked change (insertion or deletion).""" @@ -169,50 +179,129 @@ def batch_edit(self, operations: list[EditOperation]) -> list[int]: pass raise - def _apply_single_edit(self, op: EditOperation) -> int: - """Apply a single edit operation. Paragraph hash was already validated.""" - ref = ParagraphRef.parse(op.paragraph) - p = self.editor.dom.getElementsByTagName("w:p")[ref.index - 1] + def _resolve_action_target(self, op: EditOperation) -> str: + """Validate op's required args and return the text this op must locate. + + Shared by ``_apply_single_edit`` and ``_validate_single`` so the two + paths cannot drift out of sync. Rejects a negative ``occurrence`` up + front (the one non-well-formed input ``_find_in_paragraph`` chokes on) + so both paths fail cleanly before the search. + + Raises: + ValueError: If ``occurrence`` is negative, required arguments for + op.action are missing, or the action is unrecognized. + """ + if op.occurrence < 0: + raise ValueError(f"occurrence must be >= 0, got {op.occurrence}") if op.action == "replace": if not op.find or op.replace_with is None: raise ValueError("replace requires 'find' and 'replace_with'") - match = self._find_in_paragraph(p, op.find, op.occurrence) - if match is None: - raise TextNotFoundError( - op.find, - paragraph_ref=op.paragraph, - paragraph_preview=self._paragraph_preview(p), - ) - return self._replace_across_nodes(match, op.replace_with) - + return op.find elif op.action == "delete": if not op.text: raise ValueError("delete requires 'text'") - match = self._find_in_paragraph(p, op.text, op.occurrence) - if match is None: - raise TextNotFoundError( - op.text, - paragraph_ref=op.paragraph, - paragraph_preview=self._paragraph_preview(p), - ) - return self._delete_across_nodes(match) - + return op.text elif op.action in ("insert_after", "insert_before"): if not op.anchor or op.text is None: raise ValueError(f"{op.action} requires 'anchor' and 'text'") - match = self._find_in_paragraph(p, op.anchor, op.occurrence) - if match is None: - raise TextNotFoundError( - op.anchor, - paragraph_ref=op.paragraph, - paragraph_preview=self._paragraph_preview(p), - ) + return op.anchor + else: + raise ValueError(f"Unknown action: {op.action}") + + def _apply_single_edit(self, op: EditOperation) -> int: + """Apply a single edit operation. Paragraph hash was already validated.""" + ref = ParagraphRef.parse(op.paragraph) + p = self.editor.dom.getElementsByTagName("w:p")[ref.index - 1] + + target = self._resolve_action_target(op) + match = self._find_in_paragraph(p, target, op.occurrence) + if match is None: + raise TextNotFoundError( + target, + paragraph_ref=op.paragraph, + paragraph_preview=self._paragraph_preview(p), + ) + + if op.action == "replace": + assert op.replace_with is not None # guaranteed by _resolve_action_target + return self._replace_across_nodes(match, op.replace_with) + elif op.action == "delete": + return self._delete_across_nodes(match) + else: # insert_after / insert_before + assert op.text is not None # guaranteed by _resolve_action_target position = "after" if op.action == "insert_after" else "before" return self._insert_near_match(match, op.text, position) - else: - raise ValueError(f"Unknown action: {op.action}") + def validate_batch(self, operations: list[EditOperation]) -> list[EditValidationResult]: + """Validate a batch of edits without applying any of them. + + Mirrors the checks in ``batch_edit`` / ``_apply_single_edit`` (paragraph + ref format, hash freshness, per-action argument requirements, and target + text existence) but never raises and never mutates the document. Each + operation gets its own result so the caller sees the full picture even + when some ops are valid and others are not. + + Limitation: each operation is validated independently against the + *current* document state; sequential effects are not simulated. A batch + with multiple operations on the same paragraph (where one op's edit + changes what a later op would see) may validate differently than it + applies. Cross-paragraph batches are unaffected, since edits never + change the paragraph count. + + Args: + operations: List of EditOperation objects (each should have paragraph set) + + Returns: + One EditValidationResult per operation, in input order. + """ + results = [] + for i, op in enumerate(operations): + error = self._validate_single(op) + results.append( + EditValidationResult( + index=i, + paragraph=op.paragraph, + valid=error is None, + error=error, + ) + ) + return results + + def _validate_single(self, op: EditOperation) -> str | None: + """Return an error message if ``op`` would fail, or None if it is valid. + + Reuses ``_resolve_paragraph``, ``_resolve_action_target``, and + ``_find_in_paragraph`` — the same helpers ``_apply_single_edit`` uses — + so dry-run validation cannot drift from real application semantics. + Reads only. + """ + if not op.paragraph: + return "paragraph reference is required for batch mode" + + try: + ref = ParagraphRef.parse(op.paragraph) + except ValueError as e: + return str(e) + + try: + p = self._resolve_paragraph(ref) + except (ParagraphIndexError, HashMismatchError) as e: + return str(e) + + # Resolve required args + the text this op must locate via the same + # helper _apply_single_edit uses (which also rejects a negative + # occurrence), so validation cannot drift from application semantics and + # the find below never raises. + try: + target = self._resolve_action_target(op) + except ValueError as e: + return str(e) + + if self._find_in_paragraph(p, target, op.occurrence) is None: + return f"text {target!r} not found in paragraph {op.paragraph} ({self._paragraph_preview(p)!r})" + + return None def batch_rewrite(self, rewrites: list[tuple[str, str]]) -> None: """Rewrite multiple paragraphs with upfront hash validation.""" diff --git a/tests/test_batch_edit.py b/tests/test_batch_edit.py index 4c1d4cc..1b33a26 100644 --- a/tests/test_batch_edit.py +++ b/tests/test_batch_edit.py @@ -10,6 +10,7 @@ BatchOperationError, Document, EditOperation, + EditValidationResult, HashMismatchError, TextNotFoundError, ) @@ -326,6 +327,239 @@ def test_batch_unknown_action(self, multi_para_doc): doc.batch_edit(ops) +class TestBatchEditDryRun: + """dry_run=True validates every op without mutating the document.""" + + def test_all_valid(self, multi_para_doc): + """All-matching ops report valid=True, in input order, doc unchanged.""" + doc, _ = multi_para_doc + refs = doc.list_paragraphs() + before = doc.get_visible_text() + + ops = [ + EditOperation( + action="replace", + find="item 3", + replace_with="ITEM_THREE", + paragraph=refs[2].split("|")[0], + ), + EditOperation( + action="delete", + text="item 5", + paragraph=refs[4].split("|")[0], + ), + EditOperation( + action="insert_after", + anchor="item 8", + text=" [APPENDED]", + paragraph=refs[7].split("|")[0], + ), + ] + + results = doc.batch_edit(ops, dry_run=True) + + assert len(results) == len(ops) + assert all(isinstance(r, EditValidationResult) for r in results) + assert all(r.valid for r in results) + assert all(r.error is None for r in results) + assert [r.index for r in results] == [0, 1, 2] + assert [r.paragraph for r in results] == [op.paragraph for op in ops] + + # No edits applied. + assert doc.get_visible_text() == before + + def test_stale_hash(self, multi_para_doc): + """A stale-hash op reports invalid with a hash-mismatch error; doc unchanged.""" + doc, _ = multi_para_doc + refs = doc.list_paragraphs() + + # Make P5's hash stale by editing it. + p5_ref = refs[4].split("|")[0] + doc.replace("item 5", "CHANGED", paragraph=p5_ref) + before = doc.get_visible_text() + + ops = [ + EditOperation( + action="replace", + find="CHANGED", + replace_with="EDIT_5", + paragraph=p5_ref, # STALE hash + ), + ] + + results = doc.batch_edit(ops, dry_run=True) + + assert len(results) == 1 + assert results[0].valid is False + assert "hash" in results[0].error.lower() + assert doc.get_visible_text() == before + + def test_missing_text(self, multi_para_doc): + """A valid ref but non-existent find/text reports invalid; doc unchanged.""" + doc, _ = multi_para_doc + ref = doc.list_paragraphs()[0].split("|")[0] + before = doc.get_visible_text() + + ops = [ + EditOperation( + action="replace", + find="NONEXISTENT", + replace_with="x", + paragraph=ref, + ), + ] + + results = doc.batch_edit(ops, dry_run=True) + + assert results[0].valid is False + assert "not found" in results[0].error.lower() + assert doc.get_visible_text() == before + + def test_mixed_valid_invalid(self, multi_para_doc): + """Valid + stale-hash + missing-text ops report per-op; order preserved; doc unchanged.""" + doc, _ = multi_para_doc + refs = doc.list_paragraphs() + + # Make P5 stale. + p5_ref = refs[4].split("|")[0] + doc.replace("item 5", "CHANGED", paragraph=p5_ref) + before = doc.get_visible_text() + + ops = [ + EditOperation( + action="replace", + find="item 3", + replace_with="ITEM_THREE", + paragraph=refs[2].split("|")[0], # valid + ), + EditOperation( + action="replace", + find="CHANGED", + replace_with="x", + paragraph=p5_ref, # stale hash + ), + EditOperation( + action="delete", + text="NONEXISTENT", + paragraph=refs[7].split("|")[0], # missing text + ), + ] + + results = doc.batch_edit(ops, dry_run=True) + + assert [r.index for r in results] == [0, 1, 2] + assert results[0].valid is True + assert results[0].error is None + assert results[1].valid is False + assert "hash" in results[1].error.lower() + assert results[2].valid is False + assert "not found" in results[2].error.lower() + + assert doc.get_visible_text() == before + + def test_empty_batch(self, multi_para_doc): + """Empty dry-run batch returns an empty list.""" + doc, _ = multi_para_doc + assert doc.batch_edit([], dry_run=True) == [] + + def test_not_found_error_includes_preview(self, multi_para_doc): + """A not-found error carries the paragraph preview for easier debugging.""" + doc, _ = multi_para_doc + ref = doc.list_paragraphs()[0].split("|")[0] + + ops = [ + EditOperation(action="replace", find="NONEXISTENT", replace_with="x", paragraph=ref), + ] + + results = doc.batch_edit(ops, dry_run=True) + + assert results[0].valid is False + # The paragraph's real text ("item 1") should appear in the error preview. + assert "item 1" in results[0].error + + def test_bad_occurrence_reports_instead_of_raising(self, multi_para_doc): + """A malformed op (negative occurrence) is reported, never raised; doc unchanged.""" + doc, _ = multi_para_doc + ref = doc.list_paragraphs()[0].split("|")[0] + before = doc.get_visible_text() + + ops = [ + EditOperation( + action="replace", + find="item 1", + replace_with="x", + paragraph=ref, + occurrence=-1, # malformed + ), + ] + + # Must not raise despite the malformed occurrence. + results = doc.batch_edit(ops, dry_run=True) + + assert results[0].valid is False + assert "occurrence" in results[0].error + assert doc.get_visible_text() == before + + def test_missing_paragraph_ref(self, multi_para_doc): + """An op without a paragraph ref is invalid, with paragraph=None on the result.""" + doc, _ = multi_para_doc + + ops = [EditOperation(action="replace", find="item 1", replace_with="x", paragraph="")] + + results = doc.batch_edit(ops, dry_run=True) + + assert results[0].valid is False + assert results[0].paragraph == "" + assert "paragraph" in results[0].error.lower() + + def test_malformed_paragraph_ref(self, multi_para_doc): + """An unparseable paragraph ref is reported invalid, never raised.""" + doc, _ = multi_para_doc + + ops = [EditOperation(action="replace", find="x", replace_with="y", paragraph="not-a-ref")] + + results = doc.batch_edit(ops, dry_run=True) + + assert results[0].valid is False + assert results[0].error + + def test_missing_action_arguments(self, multi_para_doc): + """Each action reports its missing-argument error without raising.""" + doc, _ = multi_para_doc + refs = doc.list_paragraphs() + r0 = refs[0].split("|")[0] + r1 = refs[1].split("|")[0] + r2 = refs[2].split("|")[0] + + ops = [ + # replace without find/replace_with + EditOperation(action="replace", paragraph=r0), + # delete without text + EditOperation(action="delete", paragraph=r1), + # insert_after without anchor/text + EditOperation(action="insert_after", paragraph=r2), + ] + + results = doc.batch_edit(ops, dry_run=True) + + assert all(r.valid is False for r in results) + assert "replace requires" in results[0].error + assert "delete requires" in results[1].error + assert "insert_after requires" in results[2].error + + def test_unknown_action(self, multi_para_doc): + """An unrecognized action is reported invalid, never raised.""" + doc, _ = multi_para_doc + ref = doc.list_paragraphs()[0].split("|")[0] + + ops = [EditOperation(action="frobnicate", paragraph=ref)] # type: ignore[arg-type] + + results = doc.batch_edit(ops, dry_run=True) + + assert results[0].valid is False + assert "Unknown action" in results[0].error + + class TestStructuredBatchOperationError: def test_predispatch_missing_paragraph_has_operation_index(self, multi_para_doc): doc, _ = multi_para_doc @@ -382,6 +616,28 @@ def test_inner_unknown_action_has_operation_index(self, multi_para_doc): assert err.operation_index == 1 assert "Unknown action" in err.reason + def test_inner_negative_occurrence_has_operation_index(self, multi_para_doc): + """A negative occurrence fails cleanly on the apply path (no internal error).""" + doc, _ = multi_para_doc + refs = doc.list_paragraphs() + before = doc.get_visible_text() + ops = [ + EditOperation( + action="replace", + find="item 1", + replace_with="x", + paragraph=refs[0].split("|")[0], + occurrence=-1, + ), + ] + with pytest.raises(BatchOperationError) as exc: + doc.batch_edit(ops) + err = exc.value + assert err.operation_index == 0 + assert "occurrence" in err.reason + # Rejected before any mutation. + assert doc.get_visible_text() == before + def test_non_batch_value_error_unchanged(self): """Malformed paragraph refs outside batch paths still raise plain ValueError.""" from docx_editor import ParagraphRef