diff --git a/docs/api.md b/docs/api.md index f554ee4..d8649a0 100644 --- a/docs/api.md +++ b/docs/api.md @@ -10,7 +10,7 @@ from docx_editor import Document ### Opening Documents -#### `Document.open(path, author=None, force_recreate=False)` +#### `Document.open(path, author=None, force_recreate=False, workspace_dir=None)` Open a Word document for editing. @@ -18,13 +18,15 @@ Open a Word document for editing. - `path` (str | Path): Path to the .docx file - `author` (str, optional): Author name for tracked changes. Defaults to system username. -- `force_recreate` (bool): If True, delete any existing workspace (stale or in-sync) before opening — any unsaved edits in the workspace are discarded. Use this to recover from `WorkspaceSyncError`. Defaults to False. +- `force_recreate` (bool): If True, delete any existing workspace (stale or in-sync) before opening — whatever XML it holds is discarded — and re-unpack from the current source. Use this to recover from `WorkspaceSyncError`. Defaults to False. +- `workspace_dir` (str | Path, optional): Base directory for the workspace. Overrides the `DOCX_EDITOR_WORKSPACE_DIR` environment variable and the platform cache default (see [Workspace location](#workspace-location)). A relative path resolves against the document's directory, so `workspace_dir=".docx"` keeps the workspace next to the file. Defaults to None. **Returns:** Document instance ready for editing **Raises:** -- `WorkspaceSyncError`: If the source `.docx` was modified since the workspace was created. Pass `force_recreate=True` to discard the stale workspace and re-unpack from the current source. The workspace is never deleted silently. +- `WorkspaceSyncError`: If the source `.docx` was modified since the workspace was created. Pass `force_recreate=True` to discard the stale workspace and re-unpack from the current source. The workspace is never deleted silently. The error message includes the workspace path. +- `WorkspaceError`: If the workspace directory cannot be created (e.g. the base is not writable), the home directory backing the default cache cannot be determined, or an existing workspace was unpacked from a different document. The message names the override to set. **Example:** @@ -33,6 +35,29 @@ doc = Document.open("contract.docx") doc = Document.open("contract.docx", author="Legal Team") ``` +#### Workspace location + +When you open a document, its unpacked OOXML contents are stored in a workspace directory. By default this lives under the platform user cache, in a subfolder `docx-editor/` where `` is derived from the document's absolute path: + +| Platform | Default base directory | +| --- | --- | +| Linux | `$XDG_CACHE_HOME/docx-editor` (falls back to `~/.cache/docx-editor`) | +| macOS | `~/Library/Caches/docx-editor` | +| Windows | `%LOCALAPPDATA%\docx-editor` (falls back to `~\AppData\Local\docx-editor`) | + +To override the location: + +- Set the `DOCX_EDITOR_WORKSPACE_DIR` environment variable to a base directory, or +- Pass `workspace_dir=` to `Document.open()` (takes precedence over the environment variable). + +Both overrides are tilde-expanded, and an empty value counts as unset. A **relative** override resolves against the document's directory, so `workspace_dir=".docx"` reproduces the old next-to-file layout (handy for debugging). An **absolute** override is used as-is. (A relative `XDG_CACHE_HOME` / `%LOCALAPPDATA%` is ignored, per the XDG spec.) + +Cleanup semantics are unchanged: the workspace persists until `close()` is called, and `close(cleanup=False)` preserves it for inspection. `close()` removes only the document's own workspace folder — the base directory is shared and is never deleted, so it is safe to point `workspace_dir` at a directory you also use for other things. + +The workspace directory is created with owner-only permissions (`0o700`), since it holds the document's plaintext in a shared cache location. Use `doc.workspace_path` to locate it. + +> **Note:** The default location moved from the old `.docx//` folder next to the document to the platform cache. Workspaces created by older versions are no longer found and are simply ignored. Delete leftover `.docx/` folders, or pass `workspace_dir=".docx"` to keep using the old layout. + ### Properties #### `author` @@ -51,6 +76,14 @@ Get the path to the source document. print(doc.source_path) # Path("/path/to/contract.docx") ``` +#### `workspace_path` + +Get the path to this document's workspace folder. Since the workspace lives in the user cache by default, this is how you locate the unpacked XML — for example after `close(cleanup=False)`, or when a workspace was preserved because an exception was raised. + +```python +print(doc.workspace_path) # Path("/home/you/.cache/docx-editor/0bebafb463a87cfa") +``` + ### Track Changes Methods #### `paragraph_count()` diff --git a/docs/quickstart.md b/docs/quickstart.md index fbbae7e..4e5e38d 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -23,12 +23,26 @@ with Document.open("contract.docx", author="Legal Team") as doc: doc.save() ``` -If the source `.docx` was modified outside this library since the workspace was created, `Document.open()` raises `WorkspaceSyncError` instead of silently discarding the workspace. Pass `force_recreate=True` to acknowledge the divergence and re-unpack from the current source: +If the source `.docx` was modified outside this library since the workspace was created, `Document.open()` raises `WorkspaceSyncError` instead of silently discarding the workspace. The error message includes the workspace path. Pass `force_recreate=True` to acknowledge the divergence and re-unpack from the current source: ```python doc = Document.open("contract.docx", force_recreate=True) ``` +## Workspace location + +Opening a document unpacks it into a workspace directory. By default this lives under the platform user cache — `~/.cache/docx-editor/` on Linux (or `$XDG_CACHE_HOME`), `~/Library/Caches/docx-editor/` on macOS, `%LOCALAPPDATA%\docx-editor\` on Windows — in a subfolder named after a hash of the document's absolute path. + +Override the base directory with the `DOCX_EDITOR_WORKSPACE_DIR` environment variable, or per-call with `workspace_dir=`: + +```python +# Keep the workspace next to the document (relative path → resolved against +# the document's directory); useful for inspecting the unpacked XML. +doc = Document.open("contract.docx", workspace_dir=".docx") +``` + +The default location moved from the old `.docx//` folder next to the document. Leftover `.docx/` folders from older versions are no longer used and can be deleted. + ## Paragraph References Tracked edit methods are scoped to a hash-anchored paragraph reference. Start by listing paragraphs: @@ -203,7 +217,7 @@ print(f"Rejected {count} revisions") doc.save() # Save to original path doc.save("contract_v2.docx") # Save to a new path doc.close() # Delete workspace -doc.close(cleanup=False) # Keep workspace for inspection +doc.close(cleanup=False) # Keep workspace (in the cache dir) for inspection ``` ## Complete Example diff --git a/docx_editor/document.py b/docx_editor/document.py index 86af3db..e8ab983 100644 --- a/docx_editor/document.py +++ b/docx_editor/document.py @@ -83,26 +83,43 @@ def open( path: str | Path, author: str | None = None, force_recreate: bool = False, + workspace_dir: str | Path | None = None, ) -> "Document": """Open a Word document for editing. - Creates a workspace in .docx/{document_stem}/ alongside the document. - The workspace persists until close() is called. + Creates a workspace holding the document's unpacked contents. By + default the workspace lives under the platform user cache directory + (Linux: ``$XDG_CACHE_HOME`` or ``~/.cache``; macOS: + ``~/Library/Caches``; Windows: ``%LOCALAPPDATA%``), in a subfolder + named ``docx-editor/`` where ```` is derived from the + document's absolute path. The workspace persists until close() is + called. Args: path: Path to the .docx file author: Author name for tracked changes (defaults to system username) force_recreate: If True, delete any existing workspace (stale or - in-sync) before opening. Any unsaved edits in the workspace - are discarded. Use this to recover from WorkspaceSyncError. + in-sync) before opening, discarding whatever XML it holds, and + re-unpack from the current source. Use this to recover from + WorkspaceSyncError. + workspace_dir: Base directory for the workspace. Overrides the + DOCX_EDITOR_WORKSPACE_DIR environment variable and the platform + cache default. Tilde-expanded; an empty value counts as unset. + A relative path resolves against the document's directory, so + ``workspace_dir=".docx"`` keeps the workspace next to the file + (handy for debugging). Returns: Document instance ready for editing Raises: + WorkspaceError: If the workspace cannot be created (unwritable base, + undeterminable home directory) or an existing workspace was + unpacked from a different document. WorkspaceSyncError: If the source document was modified since the - workspace was created. Pass force_recreate=True to discard the - stale workspace and re-unpack from the current source. + workspace was created. The message includes the workspace path. + Pass force_recreate=True to discard the stale workspace and + re-unpack from the current source. Example: doc = Document.open("contract.docx") @@ -111,9 +128,9 @@ def open( path = Path(path).resolve() if force_recreate: - Workspace.delete(path) + Workspace.delete(path, workspace_dir=workspace_dir) - workspace = Workspace(path, author=author, create=True) + workspace = Workspace(path, author=author, create=True, workspace_dir=workspace_dir) return cls(workspace) @property @@ -126,6 +143,16 @@ def source_path(self) -> Path: """Get the path to the source document.""" return self._workspace.source_path + @property + def workspace_path(self) -> Path: + """Get the path to this document's workspace folder. + + The workspace lives under the user cache by default, so this is the + only way to locate the unpacked XML — e.g. after close(cleanup=False) + or when a workspace is preserved because an exception was raised. + """ + return self._workspace.workspace_path + # ==================== Track Changes API ==================== def find_text(self, text: str, occurrence: int = 0): diff --git a/docx_editor/workspace.py b/docx_editor/workspace.py index a36f891..23baaae 100644 --- a/docx_editor/workspace.py +++ b/docx_editor/workspace.py @@ -1,10 +1,18 @@ """Workspace management for docx_editor. -Manages the .docx/ workspace folder that contains unpacked document contents. +Manages the workspace folder that holds a document's unpacked OOXML contents. +By default the workspace lives under the platform user cache directory (see +:func:`_default_cache_dir`); the location can be overridden with the +``DOCX_EDITOR_WORKSPACE_DIR`` environment variable or an explicit +``workspace_dir`` argument. """ import getpass +import hashlib import json +import os +import shutil +import sys from datetime import datetime, timezone from pathlib import Path from typing import Any @@ -19,19 +27,67 @@ from .ooxml import pack_document, unpack_document +def _cache_root_from_env(name: str) -> Path | None: + """Return ``$name`` as a cache root, or None if it is unusable. + + Per the XDG base directory spec, a relative value "must be ignored and a + default equivalent [...] used instead" — the same rule is applied to + %LOCALAPPDATA%. Without it a relative value would later be joined to the + source document's directory, silently scattering caches next to documents. + """ + value = os.environ.get(name, "").strip() + if not value: + return None + root = Path(os.path.expanduser(value)) + return root if root.is_absolute() else None + + +def _home() -> Path: + """Return the user's home directory as a WorkspaceError-domain failure.""" + try: + return Path.home() + except RuntimeError as exc: # no HOME and no passwd entry (slim containers) + raise WorkspaceError( + "Cannot determine the home directory for the default workspace cache. " + "Set DOCX_EDITOR_WORKSPACE_DIR to an absolute path, or pass workspace_dir=." + ) from exc + + +def _default_cache_dir() -> Path: + """Return the platform-appropriate user cache base for workspaces. + + - Windows: ``%LOCALAPPDATA%\\docx-editor`` (``~/AppData/Local`` fallback) + - macOS: ``~/Library/Caches/docx-editor`` + - Linux/other: ``$XDG_CACHE_HOME/docx-editor`` (``~/.cache`` fallback) + + A relative ``%LOCALAPPDATA%``/``$XDG_CACHE_HOME`` is ignored (see + :func:`_cache_root_from_env`). + """ + if os.name == "nt": + root = _cache_root_from_env("LOCALAPPDATA") or _home() / "AppData" / "Local" + return root / "docx-editor" + if sys.platform == "darwin": + return _home() / "Library" / "Caches" / "docx-editor" + root = _cache_root_from_env("XDG_CACHE_HOME") or _home() / ".cache" + return root / "docx-editor" + + class Workspace: - """Manages the .docx/ workspace folder for a document. + """Manages the per-document workspace folder. - The workspace is stored in a hidden .docx/ folder in the same directory - as the source document, with a subfolder named after the document stem. + The workspace holds the unpacked OOXML contents of a single document. It is + stored in a subdirectory named ``sha256(source_path)[:16]`` under a base + directory resolved (in order) from an explicit ``workspace_dir`` argument, + the ``DOCX_EDITOR_WORKSPACE_DIR`` environment variable, or the platform user + cache directory. Pass ``workspace_dir=".docx"`` to keep the workspace next + to the source file for debugging. Attributes: source_path: Path to the original .docx file - workspace_path: Path to the workspace folder (.docx/{stem}/) + workspace_path: Path to this document's workspace folder meta: Dictionary containing workspace metadata """ - WORKSPACE_DIR = ".docx" # If you rename META_FILE, also update EXCLUDED_PATHS in ooxml/pack.py — # otherwise the renamed file will be packed into the .docx and Word will # flag it as "unreadable content" (issue #8). @@ -44,6 +100,7 @@ def __init__( source_path: str | Path, author: str | None = None, create: bool = True, + workspace_dir: str | Path | None = None, ): """Initialize workspace for a document. @@ -51,6 +108,10 @@ def __init__( source_path: Path to the .docx file author: Author name for tracked changes (defaults to system user) create: If True, create workspace if it doesn't exist + workspace_dir: Base directory for the workspace. Overrides the + DOCX_EDITOR_WORKSPACE_DIR environment variable and the platform + cache default. A relative path resolves against the source + document's directory (e.g. ".docx" keeps it next to the file). Raises: DocumentNotFoundError: If the source document doesn't exist @@ -66,8 +127,7 @@ def __init__( raise InvalidDocumentError(f"Not a .docx file: {source_path}") # Determine workspace path - workspace_dir = self.source_path.parent / self.WORKSPACE_DIR - self.workspace_path = workspace_dir / self.source_path.stem + self.workspace_path = self._resolve_workspace_path(self.source_path, workspace_dir) # Set author (default to system user) self._author = author or getpass.getuser() @@ -77,6 +137,7 @@ def __init__( # Check if it's stale or matches current document existing_meta = self._load_meta() if existing_meta: + self._check_provenance(existing_meta) source_mtime = self.source_path.stat().st_mtime if existing_meta.get("source_mtime") != source_mtime: raise WorkspaceSyncError( @@ -97,8 +158,73 @@ def __init__( loaded_meta = self._load_meta() if not loaded_meta: raise WorkspaceError(f"Invalid workspace (no meta.json): {self.workspace_path}") + self._check_provenance(loaded_meta) self.meta = loaded_meta + def _check_provenance(self, meta: dict[str, Any]) -> None: + """Refuse to adopt a workspace that was unpacked from a different document. + + The workspace directory is keyed by a hash of the source path, so a + mismatch here means either a hash collision or a directory planted by + something else. Adopting it would let a later save() pack that other + document's XML over this one's source file. + """ + recorded = meta.get("source_path") + # Compare with the same folding the workspace key uses (see + # _resolve_workspace_path), or two spellings that map to one workspace on + # a case-insensitive filesystem would be rejected as different documents. + if recorded is None or os.path.normcase(recorded) != os.path.normcase(str(self.source_path)): + raise WorkspaceError( + f"Workspace {self.workspace_path} belongs to a different document " + f"({recorded!r}, expected {str(self.source_path)!r}). " + f"Delete it or use force_recreate=True." + ) + + @classmethod + def _resolve_workspace_path(cls, source_path: Path, workspace_dir: str | Path | None) -> Path: + """Resolve the workspace directory for a source document. + + Precedence for the base directory: + 1. the explicit ``workspace_dir`` argument + 2. the ``DOCX_EDITOR_WORKSPACE_DIR`` environment variable + 3. the platform user cache (see :func:`_default_cache_dir`) + + Both overrides are tilde-expanded, and an empty/whitespace-only value + counts as unset. A relative base resolves against ``source_path.parent`` + (so ``workspace_dir=".docx"`` reproduces the old next-to-file layout for + debugging); an absolute base is used as-is. + + The per-document subdirectory is ``sha256(normcase(source_path))[:16]``, + hashed via :func:`os.fsencode` so undecodable filename bytes (which + ``str(Path)`` carries as surrogates) do not raise UnicodeEncodeError. + ``normcase`` folds case on Windows, so one file cannot map to two + workspaces there; it is a no-op on POSIX. + + Args: + source_path: Resolved (absolute) path to the .docx file. + workspace_dir: Explicit base override, or None. + """ + base = None + for override in (workspace_dir, os.environ.get("DOCX_EDITOR_WORKSPACE_DIR")): + if override is None: + continue + # A str override (including the env var) is stripped — stray whitespace + # there is config noise, not part of a real path. A Path is used + # verbatim, so an exotic path really ending in a space still works. + text = override.strip() if isinstance(override, str) else os.fspath(override) + if not text: # empty/whitespace override falls through to the next level + continue + base = Path(os.path.expanduser(text)) + break + if base is None: + base = _default_cache_dir() + + if not base.is_absolute(): + base = source_path.parent / base + + key = os.fsencode(os.path.normcase(source_path)) + return base / hashlib.sha256(key).hexdigest()[:16] + @property def author(self) -> str: """Get the author name for tracked changes.""" @@ -126,8 +252,19 @@ def document_xml_path(self) -> Path: def _create_workspace(self) -> None: """Create the workspace by unpacking the document.""" - # Create workspace directory - self.workspace_path.mkdir(parents=True, exist_ok=True) + # Create workspace directory. The workspace holds the document's + # plaintext in a shared, predictably-named cache base, so restrict it to + # the owner. mkdir(parents=True, mode=...) applies the mode to the leaf + # only, so the base is created separately to keep it 0o700 too. + try: + self.workspace_path.parent.mkdir(parents=True, mode=0o700, exist_ok=True) + self.workspace_path.mkdir(mode=0o700, exist_ok=True) + except OSError as exc: + raise WorkspaceError( + f"Cannot create workspace at {self.workspace_path}: {exc}. " + f"Set DOCX_EDITOR_WORKSPACE_DIR to a writable absolute path, " + f"or pass workspace_dir=." + ) from exc # Unpack document rsid = unpack_document(self.source_path, self.workspace_path) @@ -206,16 +343,13 @@ def close(self, cleanup: bool = True) -> None: Args: cleanup: If True, delete the workspace folder """ + # Only this document's workspace is removed. The base directory is + # shared (and may be a caller-supplied one via workspace_dir=/ + # DOCX_EDITOR_WORKSPACE_DIR), so it is left alone even when empty — + # _create_workspace() recreates it on demand. if cleanup and self.workspace_path.exists(): - import shutil - shutil.rmtree(self.workspace_path) - # Remove .docx parent if empty - workspace_dir = self.workspace_path.parent - if workspace_dir.exists() and not any(workspace_dir.iterdir()): - workspace_dir.rmdir() - def get_xml_path(self, relative_path: str) -> Path: """Get the full path to an XML file in the workspace. @@ -243,43 +377,37 @@ def sync_check(self) -> bool: ) @classmethod - def exists(cls, source_path: str | Path) -> bool: + def exists(cls, source_path: str | Path, workspace_dir: str | Path | None = None) -> bool: """Check if a workspace exists for a document. Args: source_path: Path to the .docx file + workspace_dir: Base directory override (see __init__) Returns: True if workspace exists """ source_path = Path(source_path).resolve() - workspace_dir = source_path.parent / cls.WORKSPACE_DIR - workspace_path = workspace_dir / source_path.stem + workspace_path = cls._resolve_workspace_path(source_path, workspace_dir) return workspace_path.exists() @classmethod - def delete(cls, source_path: str | Path) -> bool: + def delete(cls, source_path: str | Path, workspace_dir: str | Path | None = None) -> bool: """Delete workspace for a document if it exists. Args: source_path: Path to the .docx file + workspace_dir: Base directory override (see __init__) Returns: True if workspace was deleted, False if it didn't exist """ - import shutil - source_path = Path(source_path).resolve() - workspace_dir = source_path.parent / cls.WORKSPACE_DIR - workspace_path = workspace_dir / source_path.stem + workspace_path = cls._resolve_workspace_path(source_path, workspace_dir) if not workspace_path.exists(): return False + # As in close(), the shared base directory is left in place. shutil.rmtree(workspace_path) - - # Remove .docx parent if empty - if workspace_dir.exists() and not any(workspace_dir.iterdir()): - workspace_dir.rmdir() - return True diff --git a/openspec/project.md b/openspec/project.md index afb410b..f778667 100644 --- a/openspec/project.md +++ b/openspec/project.md @@ -32,7 +32,7 @@ docx_editor/ ├── xml_editor.py # Core XML manipulation (XMLEditor, DocxXMLEditor) ├── track_changes.py # RevisionManager - track changes operations ├── comments.py # CommentManager - comment operations -├── workspace.py # Workspace management (.docx/{stem}/ folders) +├── workspace.py # Workspace management (user cache dir, / folders) ├── exceptions.py # Custom exception hierarchy └── ooxml/ # OOXML utilities (pack/unpack) ``` diff --git a/skills/docx/SKILL.md b/skills/docx/SKILL.md index 3ef5e39..4096099 100644 --- a/skills/docx/SKILL.md +++ b/skills/docx/SKILL.md @@ -188,7 +188,8 @@ with Document.open("contract.docx", author=author) as doc: doc.save() # Overwrites original # or doc.save("reviewed.docx") # Save to new file # Workspace is cleaned up automatically on normal exit -# On exception, workspace is preserved for inspection +# On exception, the workspace is preserved for inspection in the user cache +# dir (~/.cache/docx-editor// on Linux; error messages print the exact path) ``` Without context manager: @@ -606,7 +607,7 @@ Benefits: If unsure, ask the user: "Should I use Opus (best), Sonnet (recommended) or Haiku (faster/cheaper) for this task?" -**Editing in parallel**: NOT safe for the same document. docx_editor uses a shared workspace - concurrent edits will overwrite each other. Edit documents sequentially, or use different files. +**Editing in parallel**: NOT safe for the same document. The workspace is keyed by the document's absolute path in the user cache dir, so two processes editing the same file share one workspace and will overwrite each other. Edit the same document sequentially. Different files never collide (each gets its own workspace), so editing distinct documents in parallel is fine. ### Limitations diff --git a/tests/conftest.py b/tests/conftest.py index 8a4897e..ae42b26 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -45,6 +45,20 @@ def temp_dir(): shutil.rmtree(temp, ignore_errors=True) +@pytest.fixture(autouse=True) +def isolated_workspace_base(monkeypatch): + """Isolate every test's workspace base from the real user cache. + + Points DOCX_EDITOR_WORKSPACE_DIR at a throwaway per-test directory so tests + never write to ~/.cache/docx-editor/ and implicitly exercise the env-var + resolution path. + """ + base = tempfile.mkdtemp(prefix="docx_editor_ws_") + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", base) + yield Path(base) + shutil.rmtree(base, ignore_errors=True) + + @pytest.fixture def temp_docx(simple_docx, temp_dir) -> Path: """Copy simple.docx to a temp location for testing.""" @@ -55,11 +69,9 @@ def temp_docx(simple_docx, temp_dir) -> Path: @pytest.fixture def clean_workspace(temp_docx): - """Ensure no workspace exists for the test document.""" - workspace_dir = temp_docx.parent / ".docx" - if workspace_dir.exists(): - shutil.rmtree(workspace_dir) - yield temp_docx - # Cleanup after test - if workspace_dir.exists(): - shutil.rmtree(workspace_dir) + """Alias for temp_docx, kept for backwards compatibility. + + Workspace isolation is handled by the autouse isolated_workspace_base + fixture, so no manual cleanup is needed here. + """ + return temp_docx diff --git a/tests/test_document.py b/tests/test_document.py index 0c3ae92..6b1b88a 100644 --- a/tests/test_document.py +++ b/tests/test_document.py @@ -39,6 +39,26 @@ def test_open_force_recreate(self, clean_workspace): doc2 = Document.open(clean_workspace, force_recreate=True) doc2.close() + def test_open_with_workspace_dir(self, clean_workspace, tmp_path): + """Test that workspace_dir is threaded through to the Workspace.""" + doc = Document.open(clean_workspace, workspace_dir=tmp_path) + + assert doc._workspace.workspace_path.parent == tmp_path + assert doc._workspace.workspace_path.exists() + assert Workspace.exists(clean_workspace, workspace_dir=tmp_path) + + doc.close() + + def test_open_force_recreate_with_workspace_dir(self, clean_workspace, tmp_path): + """Test force_recreate deletes the workspace under the given workspace_dir.""" + doc1 = Document.open(clean_workspace, workspace_dir=tmp_path) + doc1.close(cleanup=False) + assert Workspace.exists(clean_workspace, workspace_dir=tmp_path) + + doc2 = Document.open(clean_workspace, force_recreate=True, workspace_dir=tmp_path) + assert doc2._workspace.workspace_path.parent == tmp_path + doc2.close() + class TestDocumentSave: """Tests for saving documents.""" @@ -276,7 +296,7 @@ def test_force_recreate_with_persistent_sync_error(self, clean_workspace): # Make Workspace always raise WorkspaceSyncError with patch("docx_editor.document.Workspace") as mock_workspace_cls: mock_workspace_cls.side_effect = WorkspaceSyncError("Persistent error") - mock_workspace_cls.delete = lambda p: True + mock_workspace_cls.delete = lambda p, workspace_dir=None: True with pytest.raises(WorkspaceSyncError): Document.open(clean_workspace, force_recreate=True) diff --git a/tests/test_workspace.py b/tests/test_workspace.py index 0964a06..875a396 100644 --- a/tests/test_workspace.py +++ b/tests/test_workspace.py @@ -1,15 +1,22 @@ """Tests for workspace management.""" +import hashlib import json +import os +import re +import stat +import sys +from pathlib import Path import pytest from docx_editor.exceptions import ( DocumentNotFoundError, InvalidDocumentError, + WorkspaceError, WorkspaceSyncError, ) -from docx_editor.workspace import Workspace +from docx_editor.workspace import Workspace, _default_cache_dir class TestWorkspaceCreation: @@ -190,8 +197,8 @@ def test_workspace_exists_no_meta_json(self, clean_workspace, temp_dir): from docx_editor.exceptions import WorkspaceExistsError # Create workspace directory without meta.json - workspace_dir = clean_workspace.parent / ".docx" / clean_workspace.stem - workspace_dir.mkdir(parents=True) + workspace_path = Workspace._resolve_workspace_path(clean_workspace.resolve(), None) + workspace_path.mkdir(parents=True) with pytest.raises(WorkspaceExistsError): Workspace(clean_workspace) @@ -199,7 +206,7 @@ def test_workspace_exists_no_meta_json(self, clean_workspace, temp_dir): # Cleanup import shutil - shutil.rmtree(clean_workspace.parent / ".docx") + shutil.rmtree(workspace_path) def test_workspace_create_false_not_found(self, clean_workspace): """Test error when workspace not found with create=False.""" @@ -213,8 +220,8 @@ def test_workspace_create_false_no_meta(self, clean_workspace): from docx_editor.exceptions import WorkspaceError # Create workspace directory without meta.json - workspace_dir = clean_workspace.parent / ".docx" / clean_workspace.stem - workspace_dir.mkdir(parents=True) + workspace_path = Workspace._resolve_workspace_path(clean_workspace.resolve(), None) + workspace_path.mkdir(parents=True) with pytest.raises(WorkspaceError, match="Invalid workspace"): Workspace(clean_workspace, create=False) @@ -222,7 +229,7 @@ def test_workspace_create_false_no_meta(self, clean_workspace): # Cleanup import shutil - shutil.rmtree(clean_workspace.parent / ".docx") + shutil.rmtree(workspace_path) def test_load_meta_corrupt_json(self, clean_workspace): """Test that corrupt meta.json returns None in _load_meta.""" @@ -285,8 +292,7 @@ def test_sync_check_source_modified(self, clean_workspace): workspace2 = Workspace.__new__(Workspace) workspace2.source_path = clean_workspace.resolve() workspace2._author = "test" - workspace_dir = clean_workspace.parent / ".docx" - workspace2.workspace_path = workspace_dir / clean_workspace.stem + workspace2.workspace_path = Workspace._resolve_workspace_path(clean_workspace.resolve(), None) workspace2.meta = workspace2._load_meta() assert workspace2.sync_check() is False @@ -294,26 +300,30 @@ def test_sync_check_source_modified(self, clean_workspace): # Cleanup Workspace.delete(clean_workspace) - def test_close_removes_empty_parent_dir(self, clean_workspace): - """Test that close removes empty .docx parent directory.""" + def test_close_keeps_shared_base_dir(self, clean_workspace): + """close() removes only this workspace, never the shared base directory. + + The base may be a caller-supplied directory (workspace_dir= / + DOCX_EDITOR_WORKSPACE_DIR), so the library must not delete it. + """ workspace = Workspace(clean_workspace) - parent_dir = workspace.workspace_path.parent + base_dir = workspace.workspace_path.parent workspace.close(cleanup=True) - # Parent .docx dir should be removed if empty - assert not parent_dir.exists() + assert not workspace.workspace_path.exists() + assert base_dir.exists() - def test_delete_removes_empty_parent_dir(self, clean_workspace): - """Test that delete removes empty .docx parent directory.""" + def test_delete_keeps_shared_base_dir(self, clean_workspace): + """delete() removes only this workspace, never the shared base directory.""" workspace = Workspace(clean_workspace) - parent_dir = workspace.workspace_path.parent + base_dir = workspace.workspace_path.parent workspace.close(cleanup=False) - Workspace.delete(clean_workspace) + assert Workspace.delete(clean_workspace) is True - # Parent .docx dir should be removed if empty - assert not parent_dir.exists() + assert not workspace.workspace_path.exists() + assert base_dir.exists() def test_load_meta_returns_existing(self, clean_workspace): """Test that existing valid meta.json is returned by _load_meta. @@ -367,3 +377,225 @@ def test_workspace_create_false_with_valid_meta(self, clean_workspace): assert workspace2.meta is not None workspace2.close() + + +class TestDefaultCacheDir: + """Tests for the _default_cache_dir platform helper.""" + + def test_linux_with_xdg_cache_home(self, monkeypatch): + monkeypatch.setattr(os, "name", "posix") + monkeypatch.setattr(sys, "platform", "linux") + monkeypatch.setenv("XDG_CACHE_HOME", "/custom/cache") + assert _default_cache_dir() == Path("/custom/cache") / "docx-editor" + + def test_linux_without_xdg_cache_home(self, monkeypatch): + monkeypatch.setattr(os, "name", "posix") + monkeypatch.setattr(sys, "platform", "linux") + monkeypatch.delenv("XDG_CACHE_HOME", raising=False) + assert _default_cache_dir() == Path.home() / ".cache" / "docx-editor" + + def test_macos(self, monkeypatch): + monkeypatch.setattr(os, "name", "posix") + monkeypatch.setattr(sys, "platform", "darwin") + assert _default_cache_dir() == Path.home() / "Library" / "Caches" / "docx-editor" + + # The Windows branch constructs pathlib.Path, which resolves to WindowsPath + # only when os.name == "nt". Patching os.name on a POSIX host makes Path + # raise UnsupportedOperation, so these run for real only on Windows. + @pytest.mark.skipif(os.name != "nt", reason="requires a Windows host") + def test_windows_with_localappdata(self, monkeypatch): + monkeypatch.setenv("LOCALAPPDATA", r"C:\Users\x\AppData\Local") + assert _default_cache_dir() == Path(r"C:\Users\x\AppData\Local") / "docx-editor" + + @pytest.mark.skipif(os.name != "nt", reason="requires a Windows host") + def test_windows_without_localappdata(self, monkeypatch): + monkeypatch.delenv("LOCALAPPDATA", raising=False) + assert _default_cache_dir() == Path.home() / "AppData" / "Local" / "docx-editor" + + +class TestWorkspaceLocation: + """Tests for workspace location resolution (cache dir, env var, override).""" + + @staticmethod + def _expected_subdir(source_path): + return hashlib.sha256(str(source_path.resolve()).encode()).hexdigest()[:16] + + def test_default_lands_in_platform_cache(self, temp_docx, monkeypatch, tmp_path): + """With no override, workspace lands under the platform cache base.""" + monkeypatch.delenv("DOCX_EDITOR_WORKSPACE_DIR", raising=False) + monkeypatch.setattr(os, "name", "posix") + monkeypatch.setattr(sys, "platform", "linux") + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + + workspace = Workspace(temp_docx) + expected = tmp_path / "docx-editor" / self._expected_subdir(temp_docx) + assert workspace.workspace_path == expected + assert workspace.workspace_path.exists() + workspace.close() + + def test_env_var_used_when_no_param(self, temp_docx, tmp_path, monkeypatch): + """DOCX_EDITOR_WORKSPACE_DIR is used as the base when no param is given.""" + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", str(tmp_path)) + workspace = Workspace(temp_docx) + assert workspace.workspace_path == tmp_path / self._expected_subdir(temp_docx) + workspace.close() + + def test_explicit_param_overrides_env_var(self, temp_docx, tmp_path, monkeypatch): + """The workspace_dir param wins over the env var.""" + env_base = tmp_path / "env_base" + param_base = tmp_path / "param_base" + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", str(env_base)) + + workspace = Workspace(temp_docx, workspace_dir=param_base) + assert workspace.workspace_path == param_base / self._expected_subdir(temp_docx) + # The env-var base must not have been touched. + assert not (env_base / self._expected_subdir(temp_docx)).exists() + workspace.close() + + def test_relative_workspace_dir_next_to_source(self, temp_docx): + """A relative workspace_dir resolves against the source directory.""" + workspace = Workspace(temp_docx, workspace_dir=".docx") + expected = temp_docx.parent / ".docx" / self._expected_subdir(temp_docx) + assert workspace.workspace_path == expected + assert workspace.workspace_path.exists() + workspace.close() + + def test_distinct_sources_get_distinct_dirs(self, simple_docx, temp_dir, tmp_path, monkeypatch): + """Two different source files map to different hash subdirs, same base.""" + import shutil + + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", str(tmp_path)) + doc_a = temp_dir / "a.docx" + doc_b = temp_dir / "b.docx" + shutil.copy(simple_docx, doc_a) + shutil.copy(simple_docx, doc_b) + + ws_a = Workspace(doc_a) + ws_b = Workspace(doc_b) + assert ws_a.workspace_path != ws_b.workspace_path + assert ws_a.workspace_path.parent == tmp_path + assert ws_b.workspace_path.parent == tmp_path + + ws_a.close() + ws_b.close() + + def test_sync_error_message_contains_workspace_path(self, temp_docx, tmp_path, monkeypatch): + """WorkspaceSyncError from the cache location prints the workspace path.""" + import time + + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", str(tmp_path)) + workspace = Workspace(temp_docx) + workspace_path = workspace.workspace_path + workspace.close(cleanup=False) + + # Modify the source so the mtime check fails on reopen. + time.sleep(0.1) + temp_docx.write_bytes(temp_docx.read_bytes() + b"\x00") + + with pytest.raises(WorkspaceSyncError, match=re.escape(str(workspace_path))): + Workspace(temp_docx) + + Workspace.delete(temp_docx) + + +class TestWorkspaceLocationHardening: + """Regression tests for workspace path resolution edge cases.""" + + def test_non_utf8_source_filename(self, simple_docx, temp_dir, tmp_path, monkeypatch): + """A source path with undecodable bytes must not raise UnicodeEncodeError. + + str(Path) carries such bytes as surrogates, which strict UTF-8 .encode() + rejects; the hash key goes through os.fsencode instead. + """ + import shutil + + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", str(tmp_path)) + # "café.docx" in latin-1 — undecodable as UTF-8, so it round-trips as a surrogate. + weird = temp_dir / os.fsdecode(b"caf\xe9.docx") + shutil.copy(simple_docx, weird) + + workspace = Workspace(weird) + assert workspace.workspace_path.exists() + assert workspace.workspace_path.parent == tmp_path + workspace.close() + + def test_env_var_is_tilde_expanded(self, temp_docx, tmp_path, monkeypatch): + """A ~ in the env var is expanded, not taken as a literal directory name.""" + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", "~/ws") + + workspace = Workspace(temp_docx) + assert workspace.workspace_path.parent == tmp_path / "ws" + assert "~" not in str(workspace.workspace_path) + workspace.close() + + def test_empty_workspace_dir_is_treated_as_unset(self, temp_docx, tmp_path, monkeypatch): + """workspace_dir="" must not rebase the workspace onto the document's own dir.""" + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", str(tmp_path)) + + workspace = Workspace(temp_docx, workspace_dir="") + assert workspace.workspace_path.parent == tmp_path + assert workspace.workspace_path.parent != temp_docx.parent + workspace.close() + + def test_relative_xdg_cache_home_is_ignored(self, monkeypatch, tmp_path): + """Per the XDG spec, a relative XDG_CACHE_HOME is ignored, not honored.""" + monkeypatch.setattr(os, "name", "posix") + monkeypatch.setattr(sys, "platform", "linux") + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("XDG_CACHE_HOME", "relative-cache") + + assert _default_cache_dir() == tmp_path / ".cache" / "docx-editor" + + def test_workspace_dir_is_owner_only(self, temp_docx, tmp_path, monkeypatch): + """The workspace holds document plaintext — it must not be group/world readable.""" + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", str(tmp_path / "base")) + + workspace = Workspace(temp_docx) + assert stat.S_IMODE(workspace.workspace_path.stat().st_mode) == 0o700 + assert stat.S_IMODE(workspace.workspace_path.parent.stat().st_mode) == 0o700 + workspace.close() + + # NB: a skipif condition is evaluated at collection time on every platform, + # so it must not touch os.geteuid (absent on Windows) unless already guarded + # by the short-circuiting os.name check. + @pytest.mark.skipif( + os.name != "posix" or os.geteuid() == 0, + reason="POSIX only; root ignores directory permissions", + ) + def test_unwritable_base_raises_workspace_error(self, temp_docx, tmp_path, monkeypatch): + """A filesystem failure surfaces as WorkspaceError, not a raw PermissionError.""" + readonly = tmp_path / "readonly" + readonly.mkdir(mode=0o500) + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", str(readonly / "nested")) + + with pytest.raises(WorkspaceError, match="DOCX_EDITOR_WORKSPACE_DIR"): + Workspace(temp_docx) + + def test_adopting_another_documents_workspace_is_refused(self, simple_docx, temp_dir, tmp_path, monkeypatch): + """A workspace whose meta names a different source must not be adopted. + + The workspace dir is keyed by a hash of the source path, so a mismatch + means a collision or a planted directory; adopting it would let save() + pack the other document's XML over this one's source. + """ + import shutil + + monkeypatch.setenv("DOCX_EDITOR_WORKSPACE_DIR", str(tmp_path)) + victim = temp_dir / "victim.docx" + shutil.copy(simple_docx, victim) + + workspace = Workspace(victim) + workspace_path = workspace.workspace_path + workspace.close(cleanup=False) + + # Rewrite meta.json so it claims to belong to a different document. + meta_path = workspace_path / Workspace.META_FILE + meta = json.loads(meta_path.read_text()) + meta["source_path"] = str(temp_dir / "other.docx") + meta_path.write_text(json.dumps(meta)) + + with pytest.raises(WorkspaceError, match="different document"): + Workspace(victim) + + Workspace.delete(victim)