diff --git a/README.md b/README.md index 1dfd35b..3ca4ed4 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,18 @@ with state.snapshot.locked(reload=True): should go through this pattern. It makes `state.save()` raise unless it is called inside `state.snapshot.locked(...)`. +By default, snapclass writes snapshot files in place. This is friendlier to +active Windows app folders where another process may briefly have the file open +for reading. If a model or stash should preserve the old complete file until a +new complete file is ready, opt into same-directory temp-file replacement: + +```python +safe_runs = Stash("./runs", write_strategy="atomic") +``` + +Use locks to coordinate cooperative writers. Use `write_strategy="atomic"` when +the file-integrity tradeoff matters more than compatibility with active readers. + ## FAQ ### Why use `snapclass` over `datafiles`? diff --git a/pyproject.toml b/pyproject.toml index fd023d0..73c9025 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "snapclass" -version = "0.1.3" +version = "0.1.4" description = "Human-readable file persistence for Python dataclasses." readme = "README.md" requires-python = ">=3.11" diff --git a/src/snapclass/collections.py b/src/snapclass/collections.py index b075a1d..6dbc6cc 100644 --- a/src/snapclass/collections.py +++ b/src/snapclass/collections.py @@ -43,7 +43,7 @@ def get_or_none(self, *args: Any, **kwargs: Any) -> Any | None: return None def get_or_create(self, *args: Any, **kwargs: Any) -> Any: - from .schemas import _attach_snapshot, _mark_snapshot_ready, _write_lock_for + from .schemas import _attach_snapshot, _mark_snapshot_ready __tracebackhide__ = sessions.HIDDEN_TRACEBACK instance = self._empty_instance(*args, **kwargs, include_defaults=True) @@ -52,7 +52,7 @@ def get_or_create(self, *args: Any, **kwargs: Any) -> Any: lock = ( instance.snapshot.locked() if getattr(instance.snapshot, "require_lock", False) - else _write_lock_for(initial_path) + else instance.snapshot._write_lock_for_path(initial_path) ) with lock: if instance.snapshot.exists: diff --git a/src/snapclass/schemas.py b/src/snapclass/schemas.py index c49f6d0..0c3422c 100644 --- a/src/snapclass/schemas.py +++ b/src/snapclass/schemas.py @@ -27,7 +27,7 @@ from .collections import Collection, CollectionDescriptor from .paths import safe_path_placeholder from .formatters import FileFormatter -from .stash import Stash, _is_home_relative +from .stash import Stash, _WriteStrategy, _is_home_relative, _normalize_write_strategy Missing = dataclasses.MISSING _MISSING_TYPE = type(dataclasses.MISSING) @@ -66,6 +66,7 @@ class Config: formatter: type[FileFormatter] | None = None minimal_diffs: bool | None = None write_delay: float | None = None + write_strategy: _WriteStrategy | None = None unknown: str = "ignore" extras_field: str | None = None migrate: Callable[..., Mapping[str, Any] | None] | None = None @@ -85,6 +86,7 @@ class Meta: snapshot_formatter: Any = None snapshot_minimal_diffs: bool | None = None snapshot_write_delay: float | None = None + snapshot_write_strategy: _WriteStrategy | None = None snapshot_unknown: str = "ignore" snapshot_extras_field: str | None = None snapshot_migrate: Callable[..., Mapping[str, Any] | None] | None = None @@ -104,6 +106,7 @@ def snapclass( formatter: type[FileFormatter] | None = None, minimal_diffs: bool | None = None, write_delay: float | None = None, + write_strategy: _WriteStrategy | None = None, unknown: str = "ignore", extras_field: str | None = None, migrate: Callable[..., Mapping[str, Any] | None] | None = None, @@ -126,6 +129,7 @@ def decorate(cls: type): cls = _dataclass_with_sidecars(cls, **dataclass_kwargs) unknown_policy = _normalize_unknown_policy(unknown, extras_field) conflict_policy = _normalize_conflict_policy(conflict) + write_strategy_policy = _normalize_write_strategy(write_strategy) _validate_snapshot_pattern(pattern) _validate_require_lock(pattern, manual, require_lock) _validate_extras_field(cls, unknown_policy, extras_field) @@ -140,6 +144,7 @@ def decorate(cls: type): formatter=formatter, minimal_diffs=minimal_diffs, write_delay=write_delay, + write_strategy=write_strategy_policy, unknown=unknown_policy, extras_field=extras_field, migrate=migrate, @@ -163,6 +168,7 @@ def create_model( infer: bool | None = None, minimal_diffs: bool | None = None, write_delay: float | None = None, + write_strategy: _WriteStrategy | None = None, migrate: Callable[..., Mapping[str, Any] | None] | None = None, conflict: str | None = None, require_lock: bool | None = None, @@ -198,6 +204,10 @@ def create_model( resolved_write_delay = write_delay if write_delay is not None else ( getattr(meta, "snapshot_write_delay", None) if meta is not None else None ) + resolved_write_strategy = write_strategy if write_strategy is not None else ( + getattr(meta, "snapshot_write_strategy", None) if meta is not None else None + ) + resolved_write_strategy = _normalize_write_strategy(resolved_write_strategy) unknown = getattr(meta, "snapshot_unknown", "ignore") if meta is not None else "ignore" extras_field = getattr(meta, "snapshot_extras_field", None) if meta is not None else None resolved_migrate = migrate if migrate is not None else ( @@ -220,6 +230,7 @@ def create_model( formatter=formatter, minimal_diffs=resolved_minimal_diffs, write_delay=resolved_write_delay, + write_strategy=resolved_write_strategy, unknown=unknown, extras_field=extras_field, migrate=resolved_migrate, @@ -236,6 +247,7 @@ def create_model( snapshot_formatter=formatter, snapshot_minimal_diffs=resolved_minimal_diffs, snapshot_write_delay=resolved_write_delay, + snapshot_write_strategy=resolved_write_strategy, snapshot_unknown=_normalize_unknown_policy(unknown, extras_field), snapshot_extras_field=extras_field, snapshot_migrate=resolved_migrate, @@ -257,6 +269,7 @@ def _install_model_config( formatter: type[FileFormatter] | None = None, minimal_diffs: bool | None = None, write_delay: float | None = None, + write_strategy: _WriteStrategy | None = None, unknown: str = "ignore", extras_field: str | None = None, migrate: Callable[..., Mapping[str, Any] | None] | None = None, @@ -265,6 +278,7 @@ def _install_model_config( ) -> None: unknown_policy = _normalize_unknown_policy(unknown, extras_field) conflict_policy = _normalize_conflict_policy(conflict) + write_strategy_policy = _normalize_write_strategy(write_strategy) resolved_manual = True if pattern is None else manual _validate_snapshot_pattern(pattern) _validate_require_lock(pattern, resolved_manual, require_lock) @@ -280,6 +294,7 @@ def _install_model_config( formatter=formatter, minimal_diffs=minimal_diffs, write_delay=write_delay, + write_strategy=write_strategy_policy, unknown=unknown_policy, extras_field=extras_field, migrate=migrate, @@ -330,6 +345,7 @@ def __init_subclass__(cls, **kwargs: Any) -> None: formatter=getattr(meta, "snapshot_formatter", None), minimal_diffs=getattr(meta, "snapshot_minimal_diffs", None), write_delay=getattr(meta, "snapshot_write_delay", None), + write_strategy=getattr(meta, "snapshot_write_strategy", None), unknown=getattr(meta, "snapshot_unknown", "ignore"), extras_field=getattr(meta, "snapshot_extras_field", None), migrate=getattr(meta, "snapshot_migrate", None), @@ -350,6 +366,7 @@ def sync( formatter: type[FileFormatter] | None = None, minimal_diffs: bool | None = None, write_delay: float | None = None, + write_strategy: _WriteStrategy | None = None, unknown: str = "ignore", extras_field: str | None = None, migrate: Callable[..., Mapping[str, Any] | None] | None = None, @@ -359,6 +376,7 @@ def sync( cls = instance.__class__ unknown_policy = _normalize_unknown_policy(unknown, extras_field) conflict_policy = _normalize_conflict_policy(conflict) + write_strategy_policy = _normalize_write_strategy(write_strategy) _validate_snapshot_pattern(pattern) _validate_require_lock(pattern, manual, require_lock) _validate_extras_field(cls, unknown_policy, extras_field) @@ -373,6 +391,7 @@ def sync( formatter=formatter, minimal_diffs=minimal_diffs, write_delay=write_delay, + write_strategy=write_strategy_policy, unknown=unknown_policy, extras_field=extras_field, migrate=migrate, @@ -732,6 +751,7 @@ def __init__( infer: bool | None = None, minimal_diffs: bool | None = None, write_delay: float | None = None, + write_strategy: _WriteStrategy | None = None, require_lock: bool | None = None, root: "Snapshot | None" = None, ) -> None: @@ -746,6 +766,7 @@ def __init__( fields=fields or {}, minimal_diffs=minimal_diffs, write_delay=write_delay, + write_strategy=_normalize_write_strategy(write_strategy), require_lock=False if require_lock is None else require_lock, ) config.type_hints = _safe_type_hints(instance.__class__) @@ -871,6 +892,12 @@ def require_lock(self) -> bool: return self._root.require_lock return self._config.require_lock + @property + def write_strategy(self) -> _WriteStrategy: + if self._root is not None: + return self._root.write_strategy + return _effective_write_strategy(self._config, self.stash) + @property def stash(self) -> Stash | None: return self._stash or self._config.stash @@ -900,13 +927,14 @@ def text(self) -> str: def text(self, value: str) -> None: __tracebackhide__ = sessions.HIDDEN_TRACEBACK path = self._require_path() - with _write_lock_for(path): + with self._write_lock_for_path(path): self._check_required_lock(path) self._check_write_conflict(path) - _write_text_atomic( + _write_text( path, value, write_delay=_effective_write_delay(self._config, self.stash), + write_strategy=self.write_strategy, ) self.load() @@ -920,7 +948,7 @@ def save( if path is not None: self.path = path current_path = self._require_path() - with _write_lock_for(current_path): + with self._write_lock_for_path(current_path): self._check_required_lock(current_path) self._check_write_conflict(current_path) sidecar.reconcile_before_save(self._instance, current_path) @@ -933,10 +961,11 @@ def save( template = self._loaded_data if self._loaded_path == current_path else None rendered_data = _data_for_dump(template, data) text = _dump_data(current_path, rendered_data, self._config, self.stash) - _write_text_atomic( + _write_text( current_path, text, write_delay=_effective_write_delay(self._config, self.stash), + write_strategy=self.write_strategy, ) self._loaded_data = rendered_data self._loaded_path = current_path @@ -983,7 +1012,7 @@ def locked(self, *, reload: bool = False) -> Iterator["Snapshot"]: "Snapshot path changed while locked; keep path fields stable inside " "snapshot.locked()" ) - with _locked_path(current_path): + with _locked_path(_lock_target_for_write(current_path, self.write_strategy)): previous_path = self._locked_path self._locked_path = current_path self._lock_depth += 1 @@ -1019,6 +1048,9 @@ def _check_required_lock(self, path: Path) -> None: "coordinating shared writers" ) + def _write_lock_for_path(self, path: Path) -> Any: + return _write_lock_for(path, write_strategy=self.write_strategy) + def _check_write_conflict(self, path: Path) -> None: if self._config.conflict != "raise" or not path.exists(): return @@ -1810,6 +1842,16 @@ def _effective_write_delay(config: Config | None, stash: Stash | None) -> float: return sessions.WRITE_DELAY +def _effective_write_strategy(config: Config | None, stash: Stash | None) -> _WriteStrategy: + if config is not None and config.write_strategy is not None: + return config.write_strategy + if stash is not None: + value = stash.effective_write_strategy() + if value is not None: + return value + return "in_place" + + def _load_data(path: Path, text: str, config: Config, stash: Stash | None) -> dict[str, Any]: formatter = formatters.formatter_for( path, @@ -2777,30 +2819,58 @@ def _module_dir_for(cls: type) -> Path | None: return Path(filename).resolve().parent -def _write_text_atomic(path: Path, text: str, *, write_delay: float | None = None) -> None: - with _write_lock_for(path): - path.parent.mkdir(parents=True, exist_ok=True) - fd, temp_name = tempfile.mkstemp( - prefix=f".{path.name}.", suffix=".tmp", dir=os.fspath(path.parent), text=True - ) - temp_path = Path(temp_name) +def _write_text( + path: Path, + text: str, + *, + write_delay: float | None = None, + write_strategy: _WriteStrategy = "in_place", +) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + if write_strategy == "atomic": + _write_text_atomic(path, text) + elif write_strategy == "in_place": + with path.open("w", encoding="utf-8", newline="") as handle: + handle.write(text) + else: + raise ValueError("write_strategy must be 'in_place' or 'atomic'") + + if write_delay is None: + write_delay = sessions.WRITE_DELAY + if write_delay: + time.sleep(write_delay) + + +def _write_text_atomic(path: Path, text: str) -> None: + fd, temp_name = tempfile.mkstemp( + prefix=f".{path.name}.", suffix=".tmp", dir=os.fspath(path.parent), text=True + ) + temp_path = Path(temp_name) + try: + with os.fdopen(fd, "w", encoding="utf-8", newline="") as handle: + handle.write(text) + _replace_path_atomic(temp_path, path) + except Exception: try: - with os.fdopen(fd, "w", encoding="utf-8", newline="") as handle: - handle.write(text) - _replace_path_atomic(temp_path, path) - if write_delay is None: - write_delay = sessions.WRITE_DELAY - if write_delay: - time.sleep(write_delay) - except Exception: - try: - temp_path.unlink(missing_ok=True) - finally: - raise + temp_path.unlink(missing_ok=True) + finally: + raise + + +def _write_lock_for( + path: Path, + *, + write_strategy: _WriteStrategy = "atomic", +) -> Any: + return _shared_write_lock_for(_lock_target_for_write(path, write_strategy)) -def _write_lock_for(path: Path) -> Any: - return _shared_write_lock_for(path) +def _lock_target_for_write(path: Path, write_strategy: _WriteStrategy) -> Path: + if write_strategy == "in_place": + return path.resolve(strict=False) + if write_strategy == "atomic": + return path + raise ValueError("write_strategy must be 'in_place' or 'atomic'") def _replace_path_atomic(temp_path: Path, path: Path) -> None: diff --git a/src/snapclass/sidecar.py b/src/snapclass/sidecar.py index 90ba867..8da1a89 100644 --- a/src/snapclass/sidecar.py +++ b/src/snapclass/sidecar.py @@ -240,9 +240,7 @@ def write( ) if needs_lock_check or (save_metadata and self._field): metadata_path = snapshot._require_path() - from .schemas import _write_lock_for - - parent_lock = _write_lock_for(metadata_path) + parent_lock = snapshot._write_lock_for_path(metadata_path) if parent_lock is not None: with parent_lock: self._check_parent_snapshot_before_write( diff --git a/src/snapclass/stash.py b/src/snapclass/stash.py index 5ce5da2..f0c270b 100644 --- a/src/snapclass/stash.py +++ b/src/snapclass/stash.py @@ -6,7 +6,7 @@ from dataclasses import dataclass, field from pathlib import Path from types import MappingProxyType -from typing import Any +from typing import Any, Literal from . import formatters as _formatters from . import serializers as _serializers @@ -15,6 +15,8 @@ _FormatterClass = type[_formatters.FileFormatter] | type[_formatters.Formatter] _FormatterPolicy = Mapping[str, _FormatterClass] _SerializerPolicy = Mapping[type | str, type[_serializers.Serializer]] +_WriteStrategy = Literal["in_place", "atomic"] +_WRITE_STRATEGIES = {"in_place", "atomic"} @dataclass(frozen=True) @@ -32,6 +34,7 @@ class Stash: serializers: _SerializerPolicy | None = field(default=None, compare=False) minimal_diffs: bool | None = field(default=None, compare=False) write_delay: float | None = field(default=None, compare=False) + write_strategy: _WriteStrategy | None = field(default=None, compare=False) _parent: "Stash | None" = field(default=None, repr=False, compare=False) _bindings: dict[str, Any] = field(default_factory=dict, repr=False, compare=False) _resolved: _Resolved | None = field(default=None, init=False, repr=False, compare=False) @@ -47,6 +50,11 @@ def __post_init__(self) -> None: "serializers", MappingProxyType(_serializers.normalize_serializers(self.serializers)), ) + object.__setattr__( + self, + "write_strategy", + _normalize_write_strategy(self.write_strategy), + ) def __truediv__(self, child: str | os.PathLike[str] | "Stash") -> "Stash": if isinstance(child, Stash): @@ -95,10 +103,12 @@ def with_options( *, minimal_diffs: bool | None = None, write_delay: float | None = None, + write_strategy: _WriteStrategy | None = None, ) -> "Stash": return self._copy( minimal_diffs=self.minimal_diffs if minimal_diffs is None else minimal_diffs, write_delay=self.write_delay if write_delay is None else write_delay, + write_strategy=self.write_strategy if write_strategy is None else write_strategy, ) def effective_formatters(self) -> dict[str, type[_formatters.FileFormatter]]: @@ -121,6 +131,11 @@ def effective_write_delay(self) -> float | None: return self.write_delay return self._parent.effective_write_delay() if self._parent else None + def effective_write_strategy(self) -> _WriteStrategy | None: + if self.write_strategy is not None: + return self.write_strategy + return self._parent.effective_write_strategy() if self._parent else None + def _reparent(self, parent: "Stash") -> "Stash": if self._parent is None: return self._copy(_parent=parent, _bindings=dict(self._bindings)) @@ -137,6 +152,7 @@ def _copy(self, **overrides: Any) -> "Stash": "serializers": self.serializers, "minimal_diffs": self.minimal_diffs, "write_delay": self.write_delay, + "write_strategy": self.write_strategy, "_parent": self._parent, "_bindings": dict(self._bindings), } @@ -245,6 +261,14 @@ def _is_home_relative(path: Path) -> bool: return bool(path.parts) and path.parts[0] == "~" +def _normalize_write_strategy(value: str | None) -> _WriteStrategy | None: + if value is None: + return None + if value not in _WRITE_STRATEGIES: + raise ValueError("write_strategy must be 'in_place' or 'atomic'") + return value # type: ignore[return-value] + + def _missing_placeholders(pattern: str, bindings: dict[str, Any]) -> list[str]: names: list[str] = [] for _, field_name, _, _ in string.Formatter().parse(pattern): diff --git a/tests/test_conversion_and_writes.py b/tests/test_conversion_and_writes.py index 40b5559..a887100 100644 --- a/tests/test_conversion_and_writes.py +++ b/tests/test_conversion_and_writes.py @@ -589,7 +589,12 @@ class Item: def test_atomic_replace_failure_preserves_existing_file_and_cleans_temp(tmp_path, monkeypatch): - @snapclass("{self.name}.yml", stash=Stash(tmp_path), manual=True) + @snapclass( + "{self.name}.yml", + stash=Stash(tmp_path), + manual=True, + write_strategy="atomic", + ) class Item: name: str value: str diff --git a/tests/test_model_meta.py b/tests/test_model_meta.py index af2fa29..d71ae9c 100644 --- a/tests/test_model_meta.py +++ b/tests/test_model_meta.py @@ -1,6 +1,7 @@ from __future__ import annotations from dataclasses import dataclass, field +from pathlib import Path import pytest @@ -16,6 +17,7 @@ def test_model_exposes_default_meta_configuration(): assert Model.Meta.snapshot_stash is None assert Model.Meta.snapshot_minimal_diffs is None assert Model.Meta.snapshot_write_delay is None + assert Model.Meta.snapshot_write_strategy is None assert Model.Meta.snapshot_unknown == "ignore" assert Model.Meta.snapshot_conflict == "overwrite" assert Model.Meta.snapshot_require_lock is False @@ -49,6 +51,29 @@ class Meta: assert Item.snapshots.get("Alpha").tags == ["fixture"] +def test_model_meta_write_strategy_beats_stash_policy(tmp_path, monkeypatch): + def fail_replace(self: Path, target: Path) -> Path: + raise AssertionError("model write_strategy should override stash policy") + + monkeypatch.setattr(Path, "replace", fail_replace) + + class Item(Model): + name: str + value: str = "" + + class Meta: + snapshot_pattern = "{self.name}.yml" + snapshot_stash = Stash(tmp_path, write_strategy="atomic") + snapshot_manual = True + snapshot_write_strategy = "in_place" + + item = Item("a", "one") + item.snapshot.save() + + assert item.Meta.snapshot_write_strategy == "in_place" + assert (tmp_path / "a.yml").read_text(encoding="utf-8") == "value: one\n" + + def test_dataclass_model_meta_keeps_outer_dataclass_decorator_compatible(tmp_path): root = Stash(tmp_path / "notes") @@ -244,6 +269,36 @@ class Prompt: assert Prompt.snapshots.get("a").text == "hello" +def test_create_model_accepts_write_strategy(tmp_path, monkeypatch): + replaced: list[tuple[Path, Path]] = [] + original_replace = Path.replace + + def record_replace(self: Path, target: Path) -> Path: + replaced.append((self, target)) + return original_replace(self, target) + + monkeypatch.setattr(Path, "replace", record_replace) + + @dataclass + class Prompt: + name: str + text: str = "" + + create_model( + Prompt, + pattern=str(tmp_path / "{self.name}.yml"), + manual=True, + write_strategy="atomic", + ) + + Prompt("a", "hello").snapshot.save() + + assert Prompt.Meta.snapshot_write_strategy == "atomic" + assert len(replaced) == 1 + assert replaced[0][1] == tmp_path / "a.yml" + assert (tmp_path / "a.yml").read_text(encoding="utf-8") == "text: hello\n" + + def test_create_model_accepts_direct_stash_binding(tmp_path): @dataclass class Prompt: diff --git a/tests/test_sessions_and_frozen.py b/tests/test_sessions_and_frozen.py index 98ce2c6..ac7ed96 100644 --- a/tests/test_sessions_and_frozen.py +++ b/tests/test_sessions_and_frozen.py @@ -2,6 +2,7 @@ from dataclasses import FrozenInstanceError, dataclass, field import json +from pathlib import Path import pytest @@ -418,6 +419,140 @@ class Item: assert (tmp_path / "a.yml").read_text(encoding="utf-8") == "value: direct\n" +def test_default_write_strategy_writes_in_place_without_replace(tmp_path, monkeypatch): + def fail_replace(self: Path, target: Path) -> Path: + raise AssertionError("default writes should not replace the target file") + + monkeypatch.setattr(Path, "replace", fail_replace) + + @snapclass("{self.name}.yml", stash=Stash(tmp_path), manual=True) + class Item: + name: str + value: str = "" + + item = Item("a", "one") + item.snapshot.save() + item.value = "two" + item.snapshot.save() + + assert (tmp_path / "a.yml").read_text(encoding="utf-8") == "value: two\n" + assert not list(tmp_path.glob("*.tmp")) + + +def test_snapshot_text_setter_uses_in_place_by_default(tmp_path, monkeypatch): + def fail_replace(self: Path, target: Path) -> Path: + raise AssertionError("default text setter writes should not replace the target file") + + monkeypatch.setattr(Path, "replace", fail_replace) + + @snapclass("{self.name}.yml", stash=Stash(tmp_path), manual=True) + class Item: + name: str + value: str = "" + + item = Item("a", "one") + item.snapshot.text = "value: direct\n" + + assert (tmp_path / "a.yml").read_text(encoding="utf-8") == "value: direct\n" + assert not list(tmp_path.glob("*.tmp")) + + +def test_snapshot_text_setter_can_use_atomic_write_strategy(tmp_path, monkeypatch): + replaced: list[tuple[Path, Path]] = [] + original_replace = Path.replace + + def record_replace(self: Path, target: Path) -> Path: + replaced.append((self, target)) + return original_replace(self, target) + + monkeypatch.setattr(Path, "replace", record_replace) + + @snapclass( + "{self.name}.yml", + stash=Stash(tmp_path), + manual=True, + write_strategy="atomic", + ) + class Item: + name: str + value: str = "" + + item = Item("a", "one") + item.snapshot.text = "value: direct\n" + + assert len(replaced) == 1 + assert replaced[0][0].name.startswith(".a.yml.") + assert replaced[0][0].suffix == ".tmp" + assert replaced[0][1] == tmp_path / "a.yml" + assert (tmp_path / "a.yml").read_text(encoding="utf-8") == "value: direct\n" + assert not list(tmp_path.glob("*.tmp")) + + +def test_stash_write_strategy_is_scoped_to_that_stash(tmp_path, monkeypatch): + replaced: list[tuple[Path, Path]] = [] + original_replace = Path.replace + + def record_replace(self: Path, target: Path) -> Path: + replaced.append((self, target)) + return original_replace(self, target) + + monkeypatch.setattr(Path, "replace", record_replace) + + atomic_stash = Stash(tmp_path / "atomic", write_strategy="atomic") + in_place_stash = Stash(tmp_path / "in-place", write_strategy="in_place") + + @snapclass("{self.name}.yml", stash=atomic_stash, manual=True) + class AtomicItem: + name: str + value: str = "" + + @snapclass("{self.name}.yml", stash=in_place_stash, manual=True) + class InPlaceItem: + name: str + value: str = "" + + AtomicItem("a", "one").snapshot.save() + InPlaceItem("a", "two").snapshot.save() + + assert len(replaced) == 1 + assert replaced[0][1] == tmp_path / "atomic" / "a.yml" + assert (tmp_path / "atomic" / "a.yml").read_text(encoding="utf-8") == "value: one\n" + assert (tmp_path / "in-place" / "a.yml").read_text(encoding="utf-8") == "value: two\n" + + +def test_model_write_strategy_beats_stash_policy(tmp_path, monkeypatch): + def fail_replace(self: Path, target: Path) -> Path: + raise AssertionError("model write_strategy should override stash policy") + + monkeypatch.setattr(Path, "replace", fail_replace) + + stash = Stash(tmp_path, write_strategy="atomic") + + @snapclass( + "{self.name}.yml", + stash=stash, + manual=True, + write_strategy="in_place", + ) + class Item: + name: str + value: str = "" + + Item("a", "one").snapshot.save() + + assert (tmp_path / "a.yml").read_text(encoding="utf-8") == "value: one\n" + + +def test_invalid_write_strategy_is_rejected(tmp_path): + with pytest.raises(ValueError, match="write_strategy"): + Stash(tmp_path, write_strategy="sometimes") # type: ignore[arg-type] + + with pytest.raises(ValueError, match="write_strategy"): + @snapclass("{self.name}.yml", stash=Stash(tmp_path), write_strategy="sometimes") + class Item: + name: str + + def test_hidden_traceback_marks_patched_save_frames(): @dataclass class Sample(Model): diff --git a/tests/test_snapshot_locking.py b/tests/test_snapshot_locking.py index 58f3123..99dc297 100644 --- a/tests/test_snapshot_locking.py +++ b/tests/test_snapshot_locking.py @@ -84,7 +84,12 @@ def test_snapshot_save_keeps_leaf_symlink_lock_aligned_with_replaced_path(tmp_pa alias = tmp_path / "alias.yml" _symlink_or_skip(alias, target) - @snapclass("alias.yml", stash=Stash(tmp_path), manual=True) + @snapclass( + "alias.yml", + stash=Stash(tmp_path), + manual=True, + write_strategy="atomic", + ) class State: steps: list[str] = field(default_factory=list) @@ -106,6 +111,32 @@ class State: } +def test_snapshot_save_resolves_leaf_symlink_lock_for_in_place_writes(tmp_path): + real = tmp_path / "real" + real.mkdir() + target = real / "state.yml" + target.write_text("steps:\n - target\n", encoding="utf-8") + alias = tmp_path / "alias.yml" + _symlink_or_skip(alias, target) + + @snapclass("alias.yml", stash=Stash(tmp_path), manual=True) + class State: + steps: list[str] = field(default_factory=list) + + state = State.snapshots.get() + state.steps.append("alias") + + with state.snapshot.locked(): + state.snapshot.save() + assert (real / "state.yml.lock").exists() + assert not (tmp_path / "alias.yml.lock").exists() + + assert alias.is_symlink() + assert YAMLFormatter.loads(target.read_text(encoding="utf-8")) == { + "steps": ["target", "alias"], + } + + def test_windows_lock_retry_classifier_only_accepts_lock_contention(): msvcrt_lock_contention = OSError(errno.EACCES, "permission denied") lock_violation = OSError(errno.EACCES, "locked") diff --git a/tests/test_snapshot_magic_and_serializers.py b/tests/test_snapshot_magic_and_serializers.py index a1a9e4f..bc53bbd 100644 --- a/tests/test_snapshot_magic_and_serializers.py +++ b/tests/test_snapshot_magic_and_serializers.py @@ -102,6 +102,7 @@ def test_conflict_raise_serializes_concurrent_stale_instance_saves(tmp_path, mon stash=Stash(tmp_path), manual=True, conflict="raise", + write_strategy="atomic", ) class Item: name: str diff --git a/tests/test_stash_binding_and_api.py b/tests/test_stash_binding_and_api.py index 4de43f3..8cb634d 100644 --- a/tests/test_stash_binding_and_api.py +++ b/tests/test_stash_binding_and_api.py @@ -719,13 +719,18 @@ class OtherSerializer(TokenSerializer): .with_formatters({".two": TwoFormatter}) .with_serializer(Token, TokenSerializer) .with_serializers({"OtherToken": OtherSerializer}) - .with_options(minimal_diffs=False, write_delay=0.125) + .with_options( + minimal_diffs=False, + write_delay=0.125, + write_strategy="atomic", + ) ) assert base.effective_formatters() == {} assert base.effective_serializers() == {} assert base.effective_minimal_diffs() is None assert base.effective_write_delay() is None + assert base.effective_write_strategy() is None assert updated.effective_formatters() == { ".one": OneFormatter, ".two": TwoFormatter, @@ -735,6 +740,7 @@ class OtherSerializer(TokenSerializer): assert updated.effective_serializers()["OtherToken"] is OtherSerializer assert updated.effective_minimal_diffs() is False assert updated.effective_write_delay() == 0.125 + assert updated.effective_write_strategy() == "atomic" def test_collection_bound_stash_uses_bound_formatter_policy(tmp_path): diff --git a/tests/test_sync.py b/tests/test_sync.py index e92e2c0..5213722 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -2,6 +2,7 @@ import concurrent.futures from dataclasses import dataclass, field +from pathlib import Path import threading import pytest @@ -31,6 +32,35 @@ class Workflow: assert "pending" in text +def test_sync_accepts_write_strategy(tmp_path, monkeypatch): + replaced: list[tuple[Path, Path]] = [] + original_replace = Path.replace + + def record_replace(self: Path, target: Path) -> Path: + replaced.append((self, target)) + return original_replace(self, target) + + monkeypatch.setattr(Path, "replace", record_replace) + + @dataclass + class Workflow: + id: str + status: str = "pending" + + workflow = Workflow("wf-atomic", "running") + sync( + workflow, + str(tmp_path / "{self.id}.yml"), + manual=True, + write_strategy="atomic", + ) + workflow.snapshot.save() + + assert len(replaced) == 1 + assert replaced[0][1] == tmp_path / "wf-atomic.yml" + assert (tmp_path / "wf-atomic.yml").read_text(encoding="utf-8") == "status: running\n" + + def test_sync_workflow_lifecycle_snapshot_updates(tmp_path): @dataclass class Step: