diff --git a/py/api/admin.py b/py/api/admin.py index f4a6af9..af16fbc 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -26,6 +26,12 @@ from .jobs import ( JobBusy, ) +from ..config_store import ( + ConfigUnreadable, + RevisionConflict, + get_config_store, +) + try: from ...utils.parallel import map_parallel from ...utils.validators import validate_result_timeout, validate_worker_threads @@ -133,6 +139,45 @@ def _thumbnail_rel_path(rel_path, thumbnail_ext): return f"thumbnails/{rel_path.with_suffix('').as_posix()}_thumb{thumbnail_ext}" +# Settings owned by POST /settings: config attribute -> (section, key). +_SETTINGS_FILE_KEYS = { + "RESULT_TIMEOUT": ("web_ui", "result_timeout"), + "WEBUI_DISPLAY_MODE": ("web_ui", "webui_display_mode"), + "INFINITE_SCROLL": ("web_ui", "infinite_scroll"), + "WORKER_THREADS": ("performance", "worker_threads"), +} + + +def _section(config, name): + """``config[name]`` as a fresh dict (a non-object section starts empty).""" + value = config.get(name) + return dict(value) if isinstance(value, dict) else {} + + +def _settings_mutation(updates, new_roots): + """The config commit for a settings save: only the submitted keys change. + + Every other section and key (integrations and the CivitAI key, database, + unknown sections) is kept as it is on disk. + """ + + def mutate(config): + for name, value in updates.items(): + section, key = _SETTINGS_FILE_KEYS[name] + merged = _section(config, section) + merged[key] = value + config[section] = merged + if new_roots is not None: + gallery = _section(config, "gallery") + monitoring = _section(gallery, "monitoring") + monitoring["directories"] = list(new_roots) + gallery["monitoring"] = monitoring + config["gallery"] = gallery + return config + + return mutate + + async def _read_json(request): """Parse a JSON object body through the package's capped reader. @@ -890,9 +935,11 @@ class AdminRoutesMixin: root_paths = [ self._public_path(d) for d in GalleryConfig.MONITORING_DIRECTORIES ] + revision = await self._run_in_executor(self._config_revision) return web.json_response( { "success": True, + **revision, "settings": { "result_timeout": PromptManagerConfig.RESULT_TIMEOUT, "webui_display_mode": PromptManagerConfig.WEBUI_DISPLAY_MODE, @@ -924,14 +971,25 @@ class AdminRoutesMixin: return list(GalleryConfig.MONITORING_DIRECTORIES) async def save_settings(self, request): - """Save settings: the whole payload is validated before anything changes.""" - try: - from ..config import PromptManagerConfig + """Save settings: validate, commit to config.json, then publish (#220). + Nothing changes at runtime until config.json holds the new values, so + a save that cannot be written reports ``persisted: false`` and leaves + the running settings as they were. ``expected_revision`` (the + ``revision`` from GET) refuses to overwrite a file changed meanwhile; + without it the payload merges into what is on disk (legacy page). + """ + try: data, error_response = await _read_json(request) if error_response is not None: return error_response + expected_revision = data.get("expected_revision") + if expected_revision is not None and not isinstance(expected_revision, str): + return web.json_response( + {"success": False, "error": "expected_revision must be a string"}, + status=400, + ) try: updates = self._validate_settings(data) except ValueError as ve: @@ -943,19 +1001,20 @@ class AdminRoutesMixin: if error: return web.json_response({"success": False, "error": error}, status=400) - for name, value in updates.items(): - setattr(PromptManagerConfig, name, value) - restart_required = False - if new_roots is not None: - restart_required = self._apply_gallery_roots(new_roots) - - self._persist_settings() + try: + _saved, revision = await self._run_in_executor( + get_config_store().commit, + _settings_mutation(updates, new_roots), + expected_revision=expected_revision, + ) + except (ConfigUnreadable, RevisionConflict, OSError) as exc: + return self._config_commit_failure(exc) return web.json_response( { "success": True, "message": "Settings saved successfully", - "restart_required": restart_required, + **self._publish_settings(updates, new_roots, revision), } ) except Exception as e: @@ -965,6 +1024,82 @@ class AdminRoutesMixin: status=500, ) + def _publish_settings(self, updates, new_roots, revision): + """Install committed settings at runtime; the ConfigCommit fields. + + The values are already durable. A publication that fails is reported + as ``runtime_applied: false`` with the fields that need a restart; + nothing is retried or rolled back. + """ + from ..config import PromptManagerConfig + + for name, value in updates.items(): + setattr(PromptManagerConfig, name, value) + restart_fields = [] + runtime_applied = True + if new_roots is not None: + try: + if self._apply_gallery_roots(new_roots): + restart_fields.append("gallery_root_paths") + except Exception as e: + self.logger.error( + f"Saved gallery roots could not be applied: {e}", exc_info=True + ) + runtime_applied = False + restart_fields.append("gallery_root_paths") + return { + "restart_required": bool(restart_fields), + "persisted": True, + "revision": revision, + "runtime_applied": runtime_applied, + "restart_required_fields": restart_fields, + } + + def _config_commit_failure(self, exc): + """The response for a config commit that wrote nothing.""" + not_saved = {"success": False, "persisted": False, "runtime_applied": False} + if isinstance(exc, ConfigUnreadable): + self.logger.warning(f"Settings not saved: {exc}") + return web.json_response( + { + **not_saved, + "code": "config-unreadable", + "error": ( + "config.json cannot be read, so nothing was saved. " + "Fix or remove the file, then save again." + ), + }, + status=409, + ) + if isinstance(exc, RevisionConflict): + return web.json_response( + { + **not_saved, + "code": "settings-changed", + "error": "Settings changed since they were loaded. Reload them.", + "revision": exc.current, + }, + status=409, + ) + self.logger.error(f"Could not write config.json: {exc}") + return web.json_response( + { + **not_saved, + "code": "config-write-failed", + "error": "Settings could not be written to config.json.", + }, + status=500, + ) + + @staticmethod + def _config_revision(): + """``{"revision": ...}``, or a null revision with ``config_error``.""" + try: + _data, revision = get_config_store().read() + except ConfigUnreadable: + return {"revision": None, "config_error": "config-unreadable"} + return {"revision": revision} + @staticmethod def _validate_settings(data): """Scalar settings of a payload as ``{CONFIG_ATTR: value}``. @@ -1041,49 +1176,6 @@ class AdminRoutesMixin: self.reset_gallery_roots_cache() return True - def _persist_settings(self): - """Merge the user-editable settings into the configured config.json. - - Only the keys this endpoint owns are rewritten; every other section - (integrations, database, performance, ...) and every other key in - ``web_ui``/``gallery`` is kept as it was. A config file that cannot - be parsed is replaced. - """ - from ..config import GalleryConfig, PromptManagerConfig, write_private_json - - config_file = PromptManagerConfig.get_config_path() - existing = self._read_config_file(config_file) - web_ui = dict(existing.get("web_ui") or {}) - web_ui["result_timeout"] = PromptManagerConfig.RESULT_TIMEOUT - web_ui["webui_display_mode"] = PromptManagerConfig.WEBUI_DISPLAY_MODE - web_ui["infinite_scroll"] = PromptManagerConfig.INFINITE_SCROLL - gallery = dict(existing.get("gallery") or {}) - monitoring = dict(gallery.get("monitoring") or {}) - monitoring["directories"] = list(GalleryConfig.MONITORING_DIRECTORIES) - gallery["monitoring"] = monitoring - performance = dict(existing.get("performance") or {}) - performance["worker_threads"] = PromptManagerConfig.WORKER_THREADS - config_data = { - **existing, - "web_ui": web_ui, - "gallery": gallery, - "performance": performance, - } - try: - write_private_json(config_file, config_data) - self.logger.info(f"Settings saved to {config_file}") - except OSError as save_err: - self.logger.warning(f"Could not save config file: {save_err}") - - def _read_config_file(self, config_file): - """Current contents of config.json as a dict, or {} when unusable.""" - try: - with open(config_file, "r", encoding="utf-8") as f: - data = json.load(f) - except (OSError, ValueError): - return {} - return data if isinstance(data, dict) else {} - async def run_diagnostics(self, request): """Run comprehensive system diagnostics and health checks.""" try: diff --git a/py/config_store.py b/py/config_store.py new file mode 100644 index 0000000..b05bb0d --- /dev/null +++ b/py/config_store.py @@ -0,0 +1,159 @@ +"""The one serialized writer of config.json (#220). + +Every save that changes config.json goes through ``ConfigStore.commit``: the +settings page, the LoRA integration and, later, every System section. A +commit runs under one process-wide lock, re-reads the file inside it, applies +the caller's change to a copy and replaces the file atomically. So two saves +never overwrite each other's keys, sections this process does not know are +kept byte-for-byte in meaning, and a write that fails leaves the old file in +place and raises instead of being logged and reported as saved. + +A revision names the file content a client last saw: ``"sha256:"`` of +the file bytes, or ``"absent"`` when there is no file. A commit that names a +revision refuses to write over different content (``RevisionConflict``). + +config.json can hold the CivitAI key, so the file is written owner-only. +""" + +import copy +import hashlib +import json +import os +import tempfile +import threading +from typing import Callable, Optional, Tuple + +ABSENT_REVISION = "absent" + +# One lock for the process: every ConfigStore writes the same file. +_COMMIT_LOCK = threading.Lock() + + +class ConfigUnreadable(Exception): + """config.json exists but is not a readable JSON object; it is left alone.""" + + +class RevisionConflict(Exception): + """config.json changed since the revision the caller saw.""" + + def __init__(self, current: str): + super().__init__("config.json changed since it was read") + self.current = current + + +def revision_of(data: Optional[bytes]) -> str: + """The revision of a file's bytes; ``None`` means the file is absent.""" + if data is None: + return ABSENT_REVISION + return "sha256:" + hashlib.sha256(data).hexdigest() + + +def _default_path() -> str: + from .config import PromptManagerConfig + + return PromptManagerConfig.get_config_path() + + +def _write_atomically(path: str, payload: bytes) -> None: + """Replace ``path`` with ``payload``: temp file beside it, fsync, replace. + + The descriptor is closed before ``os.replace`` (Windows refuses to replace + over an open handle), and on any failure the temp file is closed and + removed, so a failed write leaves the old file and no stray temp file. + """ + parent = os.path.dirname(path) or "." + os.makedirs(parent, exist_ok=True) + fd, temp_path = tempfile.mkstemp(dir=parent, prefix=".config-", suffix=".tmp") + try: + try: + handle = os.fdopen(fd, "wb") + except BaseException: + os.close(fd) + raise + with handle: # closed before the replace, also when a write fails + handle.write(payload) + handle.flush() + os.fsync(handle.fileno()) + if os.name == "posix": + os.chmod(temp_path, 0o600) + os.replace(temp_path, path) + except BaseException: + try: + os.unlink(temp_path) + except OSError: + pass + raise + + +class ConfigStore: + """Serialized, atomic reads and commits of config.json.""" + + def __init__(self, path_getter: Optional[Callable[[], str]] = None): + self._path_getter = path_getter or _default_path + + @property + def path(self) -> str: + """The config file this store reads and writes (resolved per call).""" + return self._path_getter() + + def read(self) -> Tuple[dict, str]: + """``(config, revision)``; a missing file is ``({}, "absent")``. + + Raises ConfigUnreadable when the file cannot be read, is not JSON, or + is not a JSON object. + """ + return self._read(self.path) + + @staticmethod + def _read(path: str) -> Tuple[dict, str]: + try: + with open(path, "rb") as handle: + raw = handle.read() + except (FileNotFoundError, NotADirectoryError): + # No file there (a parent that is a file cannot hold one either); + # a commit then fails on the write, as config-write-failed. + return {}, ABSENT_REVISION + except OSError as exc: + raise ConfigUnreadable(f"config.json cannot be read: {exc}") from exc + try: + data = json.loads(raw.decode("utf-8")) + except (UnicodeDecodeError, ValueError) as exc: + raise ConfigUnreadable("config.json is not valid JSON") from exc + if not isinstance(data, dict): + raise ConfigUnreadable("config.json is not a JSON object") + return data, revision_of(raw) + + def commit( + self, + mutate: Callable[[dict], dict], + *, + expected_revision: Optional[str], + ) -> Tuple[dict, str]: + """Apply ``mutate`` to the current config and write the result. + + ``mutate`` receives a deep copy of the file's content and returns the + whole new config (an object). ``expected_revision`` None merges into + whatever is on disk (legacy partial updates); a revision refuses to + write over different content. Returns ``(config, revision)`` as + written. Raises ConfigUnreadable, RevisionConflict, TypeError (a + non-object result) or OSError (the write failed; the old file stays). + """ + with _COMMIT_LOCK: + path = self.path + current, revision = self._read(path) + if expected_revision is not None and expected_revision != revision: + raise RevisionConflict(revision) + candidate = mutate(copy.deepcopy(current)) + if not isinstance(candidate, dict): + raise TypeError("A config commit must produce a JSON object") + payload = json.dumps(candidate, indent=2).encode("utf-8") + _write_atomically(path, payload) + return candidate, revision_of(payload) + + +_store = ConfigStore() + + +def get_config_store() -> ConfigStore: + """The process's config store (on ``PromptManagerConfig.get_config_path``).""" + return _store diff --git a/tests/test_api_admin.py b/tests/test_api_admin.py index bddcc39..39afd5f 100644 --- a/tests/test_api_admin.py +++ b/tests/test_api_admin.py @@ -5,6 +5,7 @@ database and a stub ``folder_paths`` module, so the tests run without ComfyUI. """ import asyncio +import hashlib import json import os import shutil @@ -1059,14 +1060,29 @@ class TestSettingsMisc(AdminAPITestCase): saved["gallery"]["monitoring"]["directories"], [self._root(self.output_dir)] ) - async def test_save_replaces_a_corrupt_config_file(self): - with open(self.config_path, "w") as f: - f.write("{not json") + async def test_save_refuses_a_corrupt_config_file(self): + # #220: a config.json that cannot be parsed is the user's file, never + # silently replaced; the save is refused and nothing is published. + Path(self.config_path).write_bytes(b"{not json") + before_timeout = PromptManagerConfig.RESULT_TIMEOUT resp = await self.client.request( "POST", "/prompt_manager/settings", json={"result_timeout": 4} ) - self.assertEqual(resp.status, 200) - self.assertEqual(self._read_config()["web_ui"]["result_timeout"], 4) + body = await resp.json() + self.assertEqual((resp.status, body["code"]), (409, "config-unreadable")) + self.assertIs(body["success"], False) + self.assertIs(body["persisted"], False) + self.assertEqual(PromptManagerConfig.RESULT_TIMEOUT, before_timeout) + self.assertEqual(Path(self.config_path).read_bytes(), b"{not json") + + async def test_get_settings_reports_an_unreadable_config(self): + Path(self.config_path).write_bytes(b"{not json") + resp = await self.client.request("GET", "/prompt_manager/settings") + body = await resp.json() + self.assertEqual(resp.status, 200) # the legacy page still loads + self.assertIsNone(body["revision"]) + self.assertEqual(body["config_error"], "config-unreadable") + self.assertIn("result_timeout", body["settings"]) async def test_saved_config_file_is_private_to_the_user(self): import stat @@ -1107,12 +1123,156 @@ class TestSettingsMisc(AdminAPITestCase): self.assertEqual(resp.status, 200) self.assertFalse(data["restart_required"]) - async def test_unwritable_config_path_still_succeeds(self): - os.environ[CONFIG_PATH_ENV] = os.path.join(self.tmpdir, "missing", "c.json") + async def test_unwritable_config_path_reports_failure_without_runtime_change(self): + # #220: the parent is a file, so config.json cannot be written. The + # save reports the failure and publishes nothing. + blocker = Path(self.tmpdir) / "not-a-dir" + blocker.write_text("x") + os.environ[CONFIG_PATH_ENV] = str(blocker / "c.json") + GalleryConfig.MONITORING_DIRECTORIES = [] + before_timeout = PromptManagerConfig.RESULT_TIMEOUT + with self.assertLogs("prompt_manager.api", level="ERROR"): + resp = await self.client.request( + "POST", + "/prompt_manager/settings", + json={ + "result_timeout": before_timeout + 1, + "gallery_root_paths": [self._root(self.output_dir)], + }, + ) + body = await resp.json() + self.assertEqual( + (resp.status, body["code"], body["persisted"]), + (500, "config-write-failed", False), + ) + self.assertIs(body["success"], False) + self.assertIs(body["runtime_applied"], False) + self.assertEqual(PromptManagerConfig.RESULT_TIMEOUT, before_timeout) + self.assertEqual(GalleryConfig.MONITORING_DIRECTORIES, []) + self.assertEqual(blocker.read_text(), "x") + + async def test_save_reports_the_commit(self): resp = await self.client.request( "POST", "/prompt_manager/settings", json={"result_timeout": 3} ) + body = await resp.json() self.assertEqual(resp.status, 200) + self.assertEqual(body["message"], "Settings saved successfully") + self.assertIs(body["persisted"], True) + self.assertIs(body["runtime_applied"], True) + self.assertIs(body["restart_required"], False) + self.assertEqual(body["restart_required_fields"], []) + digest = hashlib.sha256(Path(self.config_path).read_bytes()).hexdigest() + self.assertEqual(body["revision"], "sha256:" + digest) + + async def test_changed_roots_name_the_restart_field(self): + resp = await self.client.request( + "POST", + "/prompt_manager/settings", + json={"gallery_root_paths": [self._root(self.output_dir)]}, + ) + body = await resp.json() + self.assertEqual(resp.status, 200) + self.assertIs(body["restart_required"], True) + self.assertEqual(body["restart_required_fields"], ["gallery_root_paths"]) + + async def test_runtime_publication_failure_after_commit_is_reported(self): + # The values are durable; the response must not claim they are live. + self.api.reset_gallery_roots_cache = _raise + with self.assertLogs("prompt_manager.api", level="ERROR"): + resp = await self.client.request( + "POST", + "/prompt_manager/settings", + json={"gallery_root_paths": [self._root(self.output_dir)]}, + ) + body = await resp.json() + self.assertEqual(resp.status, 200) + self.assertIs(body["persisted"], True) + self.assertIs(body["runtime_applied"], False) + self.assertIs(body["restart_required"], True) + self.assertEqual(body["restart_required_fields"], ["gallery_root_paths"]) + self.assertEqual( + self._read_config()["gallery"]["monitoring"]["directories"], + [self._root(self.output_dir)], + ) + + async def test_get_settings_reports_revision(self): + resp = await self.client.request("GET", "/prompt_manager/settings") + body = await resp.json() + self.assertEqual(body["revision"], "absent") + self.assertNotIn("config_error", body) + + self._write_config({"web_ui": {"result_timeout": 2}}) + body = await ( + await self.client.request("GET", "/prompt_manager/settings") + ).json() + digest = hashlib.sha256(Path(self.config_path).read_bytes()).hexdigest() + self.assertEqual(body["revision"], "sha256:" + digest) + + async def test_native_save_with_stale_revision_is_409(self): + self._write_config({"web_ui": {"result_timeout": 2}}) + body = await ( + await self.client.request("GET", "/prompt_manager/settings") + ).json() + seen = body["revision"] + self._write_config({"web_ui": {"result_timeout": 6}}) # another writer + before = Path(self.config_path).read_bytes() + before_timeout = PromptManagerConfig.RESULT_TIMEOUT + + resp = await self.client.request( + "POST", + "/prompt_manager/settings", + json={"result_timeout": 9, "expected_revision": seen}, + ) + body = await resp.json() + self.assertEqual((resp.status, body["code"]), (409, "settings-changed")) + self.assertIs(body["persisted"], False) + digest = hashlib.sha256(before).hexdigest() + self.assertEqual(body["revision"], "sha256:" + digest) + self.assertEqual(Path(self.config_path).read_bytes(), before) + self.assertEqual(PromptManagerConfig.RESULT_TIMEOUT, before_timeout) + + async def test_native_save_with_current_revision_commits(self): + body = await ( + await self.client.request("GET", "/prompt_manager/settings") + ).json() + resp = await self.client.request( + "POST", + "/prompt_manager/settings", + json={"result_timeout": 9, "expected_revision": body["revision"]}, + ) + self.assertEqual(resp.status, 200) + self.assertEqual(self._read_config()["web_ui"]["result_timeout"], 9) + + async def test_expected_revision_must_be_a_string(self): + resp = await self.client.request( + "POST", + "/prompt_manager/settings", + json={"result_timeout": 9, "expected_revision": 5}, + ) + self.assertEqual(resp.status, 400) + self.assertFalse(os.path.exists(self.config_path)) + + async def test_legacy_partial_update_without_revision_still_merges(self): + self._write_config( + { + "integrations": {"lora_manager": {"civitai_api_key": "secret"}}, + "web_ui": {"result_timeout": 1, "webui_display_mode": "popup"}, + "custom_section": {"keep": True}, + } + ) + resp = await self.client.request( + "POST", "/prompt_manager/settings", json={"result_timeout": 8} + ) + self.assertEqual(resp.status, 200) + self.assertNotIn("secret", await resp.text()) + saved = self._read_config() + self.assertEqual( + saved["integrations"]["lora_manager"]["civitai_api_key"], "secret" + ) + self.assertEqual(saved["custom_section"], {"keep": True}) + self.assertEqual(saved["web_ui"]["result_timeout"], 8) + self.assertEqual(saved["web_ui"]["webui_display_mode"], "popup") # not sent async def test_save_unexpected_error_is_500(self): self.api._parse_gallery_roots = _raise diff --git a/tests/test_config_store.py b/tests/test_config_store.py new file mode 100644 index 0000000..a9833cf --- /dev/null +++ b/tests/test_config_store.py @@ -0,0 +1,272 @@ +"""Tests for the serialized config store (py/config_store.py, #220). + +Every test works on a config.json inside its own temporary directory; the +repository's config.json (live user data) is never read or written. +""" + +import hashlib +import json +import os +import shutil +import sys +import tempfile +import threading +import unittest +from pathlib import Path +from unittest.mock import MagicMock, patch + +_mock_server = MagicMock() +_mock_server.PromptServer.instance.routes = MagicMock() +sys.modules.setdefault("server", _mock_server) + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from py import config_store # noqa: E402 +from py.config_store import ( # noqa: E402 + ABSENT_REVISION, + ConfigStore, + ConfigUnreadable, + RevisionConflict, + get_config_store, + revision_of, +) + +try: + from tests.open_handles import assert_closed # noqa: E402 +except ImportError: # discovered with tests/ as the top-level directory + from open_handles import assert_closed # noqa: E402 + + +class ConfigStoreTests(unittest.TestCase): + def setUp(self): + self.tmpdir = os.path.realpath(tempfile.mkdtemp()) + self.path = os.path.join(self.tmpdir, "config.json") + self.store = ConfigStore(lambda: self.path) + + def tearDown(self): + try: + assert_closed(self, self.tmpdir) + finally: + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def _write(self, data): + Path(self.path).write_text(json.dumps(data), encoding="utf-8") + + def _bytes(self): + return Path(self.path).read_bytes() + + def _leftovers(self): + return sorted(name for name in os.listdir(self.tmpdir) if name != "config.json") + + # ── read ────────────────────────────────────────────────────────── + + def test_read_of_a_missing_file_is_empty_and_absent(self): + self.assertEqual(self.store.read(), ({}, ABSENT_REVISION)) + + def test_read_reports_the_sha256_of_the_file_bytes(self): + self._write({"web_ui": {"result_timeout": 3}}) + data, revision = self.store.read() + self.assertEqual(data, {"web_ui": {"result_timeout": 3}}) + expected = "sha256:" + hashlib.sha256(self._bytes()).hexdigest() + self.assertEqual(revision, expected) + self.assertEqual(revision_of(self._bytes()), expected) + + def test_non_object_config_is_unreadable(self): + self._write([1, 2]) + with self.assertRaises(ConfigUnreadable): + self.store.read() + + def test_unreadable_path_is_unreadable(self): + os.mkdir(self.path) # a directory where the file should be + with self.assertRaises(ConfigUnreadable): + self.store.read() + + # ── commit ──────────────────────────────────────────────────────── + + def test_commit_preserves_unknown_sections_and_secret(self): + self._write( + { + "custom_section": {"keep": True}, + "integrations": {"lora_manager": {"civitai_api_key": "secret"}}, + "web_ui": {"result_timeout": 1, "show_test_button": True}, + } + ) + + def mutate(config): + config.setdefault("web_ui", {})["result_timeout"] = 7 + return config + + saved, revision = self.store.commit(mutate, expected_revision=None) + + on_disk = json.loads(self._bytes()) + self.assertEqual(saved, on_disk) + self.assertEqual(on_disk["custom_section"], {"keep": True}) + self.assertEqual( + on_disk["integrations"]["lora_manager"]["civitai_api_key"], "secret" + ) + self.assertEqual( + on_disk["web_ui"], {"result_timeout": 7, "show_test_button": True} + ) + self.assertEqual(revision, revision_of(self._bytes())) + self.assertEqual(self.store.read(), (on_disk, revision)) + + def test_commit_creates_a_missing_file_and_its_parent(self): + self.path = os.path.join(self.tmpdir, "nested", "config.json") + saved, revision = self.store.commit( + lambda config: {**config, "a": 1}, expected_revision=ABSENT_REVISION + ) + self.assertEqual(saved, {"a": 1}) + self.assertEqual(json.loads(Path(self.path).read_text()), {"a": 1}) + self.assertNotEqual(revision, ABSENT_REVISION) + + def test_mutate_works_on_a_copy(self): + self._write({"web_ui": {"result_timeout": 1}}) + seen = {} + + def mutate(config): + seen["config"] = config + config["web_ui"]["result_timeout"] = 2 + raise RuntimeError("validation failed late") + + before = self._bytes() + with self.assertRaises(RuntimeError): + self.store.commit(mutate, expected_revision=None) + self.assertEqual(self._bytes(), before) + self.assertEqual(self.store.read()[0], {"web_ui": {"result_timeout": 1}}) + + def test_mutate_must_return_an_object(self): + self._write({"a": 1}) + before = self._bytes() + with self.assertRaises(TypeError): + self.store.commit(lambda config: None, expected_revision=None) + self.assertEqual(self._bytes(), before) + + def test_malformed_existing_config_is_refused_and_untouched(self): + Path(self.path).write_bytes(b"{not json") + calls = [] + with self.assertRaises(ConfigUnreadable): + self.store.commit( + lambda config: calls.append(config) or config, expected_revision=None + ) + self.assertEqual(calls, []) + self.assertEqual(self._bytes(), b"{not json") + self.assertEqual(self._leftovers(), []) + + def test_write_failure_leaves_bytes_and_raises(self): + self._write({"a": 1}) + before = self._bytes() + with patch.object(config_store.os, "fsync", side_effect=OSError("disk full")): + with self.assertRaises(OSError): + self.store.commit(lambda c: {**c, "b": 2}, expected_revision=None) + self.assertEqual(self._bytes(), before) + self.assertEqual(self._leftovers(), []) + + def test_unwritable_directory_raises_oserror(self): + blocker = os.path.join(self.tmpdir, "afile") + Path(blocker).write_text("x") + self.path = os.path.join(blocker, "config.json") # parent is a file + with self.assertRaises(OSError): + self.store.commit(lambda c: {"a": 1}, expected_revision=None) + + def test_stale_revision_conflicts_without_write(self): + self._write({"a": 1}) + _data, revision = self.store.read() + self._write({"a": 2}) # another writer got there first + before = self._bytes() + calls = [] + with self.assertRaises(RevisionConflict) as caught: + self.store.commit( + lambda c: calls.append(c) or c, expected_revision=revision + ) + self.assertEqual(caught.exception.current, revision_of(before)) + self.assertEqual(calls, []) + self.assertEqual(self._bytes(), before) + + def test_matching_revision_commits(self): + self._write({"a": 1}) + _data, revision = self.store.read() + saved, _ = self.store.commit( + lambda c: {**c, "b": 2}, expected_revision=revision + ) + self.assertEqual(saved, {"a": 1, "b": 2}) + + def test_temp_file_removed_after_replace_failure(self): + self._write({"a": 1}) + before = self._bytes() + with patch.object( + config_store.os, "replace", side_effect=PermissionError("locked") + ): + with self.assertRaises(OSError): + self.store.commit(lambda c: {**c, "b": 2}, expected_revision=None) + self.assertEqual(self._bytes(), before) + self.assertEqual(self._leftovers(), []) + + @unittest.skipUnless(os.name == "posix", "POSIX file modes") + def test_committed_file_is_private_to_the_user(self): + import stat + + self._write({"a": 1}) + os.chmod(self.path, 0o644) + self.store.commit(lambda c: c, expected_revision=None) + self.assertEqual(stat.S_IMODE(os.stat(self.path).st_mode), 0o600) + + def test_concurrent_commits_serialize(self): + self._write({"keep": True}) + inside = threading.Event() + release = threading.Event() + errors = [] + + def slow(config): + inside.set() + release.wait(10) # hold the lock while the second commit waits + config["first"] = 1 + return config + + def fast(config): + config["second"] = 2 + return config + + def run(mutate): + try: + self.store.commit(mutate, expected_revision=None) + except Exception as exc: # pragma: no cover - reported below + errors.append(exc) + + first = threading.Thread(target=run, args=(slow,)) + first.start() + self.assertTrue(inside.wait(10)) + second = threading.Thread(target=run, args=(fast,)) + second.start() + release.set() + first.join(10) + second.join(10) + + self.assertEqual(errors, []) + self.assertEqual( + json.loads(self._bytes()), {"keep": True, "first": 1, "second": 2} + ) + + def test_handles_closed_after_commit(self): + self._write({"a": 1}) + self.store.read() + self.store.commit(lambda c: {**c, "b": 2}, expected_revision=None) + assert_closed(self, self.tmpdir) + os.unlink(self.path) # Windows refuses this while a handle is open + self.assertEqual(self._leftovers(), []) + + +class SingletonTests(unittest.TestCase): + def test_get_config_store_is_one_process_store_on_the_configured_path(self): + store = get_config_store() + self.assertIs(store, get_config_store()) + tmpdir = tempfile.mkdtemp() + try: + custom = os.path.join(tmpdir, "c.json") + with patch.dict(os.environ, {"PROMPT_MANAGER_CONFIG_PATH": custom}): + self.assertEqual(store.path, custom) + finally: + shutil.rmtree(tmpdir, ignore_errors=True) + + +if __name__ == "__main__": + unittest.main()