From d868745194dc813ed1e989cd4ffa5fde14f48a7e Mon Sep 17 00:00:00 2001 From: Vito Date: Tue, 27 Oct 2026 05:00:55 -0700 Subject: [PATCH] fix(lora): cache LoraManager's layout and close the #355 follow-ups (#362) (#364) * feat(lora): cache LoraManager's layout behind file signatures (#362, phase 1) A new py/lora_layout_cache.py keeps one (install, layout) entry and reuses it while the configured path, the plugin settings file's and the settings file's signatures (st_mtime_ns, st_size), the install and every media root's real path are unchanged and the entry is younger than 30 s. A load whose watched file changed within 3 s (git's racy rule), could not be checked, or whose settings were unreadable serves its request only. clear() bumps a generation so a load that started earlier stores nothing. Hits take no lock. Not wired yet; phase 2 puts it behind the image hot paths. * fix(lora): treat an offline Windows share as unvalidated and make cache tests portable (#362) Review findings on phase 1: Windows raises FileNotFoundError with winerror 53/67 for an offline share, which is a temporary failure, not a missing file. Test times are whole milliseconds (NTFS keeps 100 ns steps) and a file system that rounds them skips; cleanup no longer ignores errors; tests added for ValueError, NotADirectoryError, a failed reload, resolve failures and the watched-file set. * perf(lora): serve the image hot paths from the layout cache (#362, phase 2) _lora_image_dirs takes (install, layout) from lora_utils.cached_lora_manager_layout instead of resolving the install and reading LoraManager's settings files on every Library, inspect, reveal and serve request. serve_image builds the allow-list in the executor instead of on the event loop. A finished import (System job or legacy scan, both through replace_lora_imports) clears the cache, so files linked under a folder LoraManager just created are served at once. The integration-off check and the configured-path rule still run before the cache. * fix(lora): fingerprint every scanned sidecar, kept or skipped (#362, phase 3) Discovery keeps every metadata file it saw (scanned_files, resolved and sorted; scanned is now its length). The fingerprint hashes each one's path, size, modification time and whether it was kept, so a skipped sidecar added, removed or edited between the review and Start refuses Start with plan-changed, as the review's counts promised. * fix(lora): warn about refused default LoRA folders and log what failed (#362, phase 4) ComfyUI's default LoRA folders already pass the folder guard; a refused one that exists is now listed in the layout's refused_defaults, and the layout cache warns once per newly refused folder. The cache writes one INFO line when a stored layout differs from the last one logged. BAD_SIDECAR_ROOT and the module docstring name a folder that contains the home folder. A relative preview_url writes a DEBUG line before the suffix search; a non-OSError failure while loading trigger words logs its traceback. Review follow-ups harden two serve tests. * fix(lora): skip a sidecar that is not UTF-8 instead of failing the load (#362) Review follow-up on phase 4: read_lora_metadata caught JSONDecodeError but not UnicodeDecodeError, so one non-UTF-8 sidecar failed the whole trigger-word load (now logged with a traceback every retry) and the import review. It is skipped like any unreadable sidecar. The relative preview_url DEBUG line uses lazy formatting. * docs(testing): the LoRA linked-only gate, layout cache and fingerprint scope (#362, phase 5) TESTING.md's LoRA reference import contract now states what #358 and #362 shipped: the linked-only gate and _media_roots, linked_only media, the cover link order, the guard on ComfyUI's default LoRA folders and refused_defaults, the folder-unreadable code, the trigger cache's catch and 60 s retry, the fingerprint over every scanned sidecar, and the layout cache's key, signatures, racy window, maximum age, root re-check and clears. The targeted command includes tests.test_lora_layout_cache and tests.test_api_images. * fix(lora): a cached layout needs the same home folder; give the rebuild poll its time (#362) Final review follow-ups: the folder guard compares against the home folder, so a cache hit now also needs Path.home() unchanged. TESTING.md says the configured-path rule runs on the cache's result and that a non-UTF-8 sidecar is skipped. CI: 'Rebuild all is explicit...' (webkit) polled the job for the default 5 s while it regenerated 24 previews; the failure screenshot shows 12 of 24 done at the deadline on the busy runner (0.17 s locally). The poll now allows 30 s for that real work. --- TESTING.md | 66 +- py/api/images.py | 25 +- py/api/lora_import_routes.py | 11 +- py/lora_import.py | 23 +- py/lora_layout_cache.py | 217 +++++++ py/lora_manager_layout.py | 79 ++- py/lora_utils.py | 46 +- .../browser/workspace-system-sources.spec.mjs | 4 +- tests/test_api_images.py | 111 +++- tests/test_api_lora.py | 12 + tests/test_lora_import.py | 95 +++ tests/test_lora_layout_cache.py | 590 ++++++++++++++++++ tests/test_lora_manager_layout.py | 117 +++- tests/test_lora_utils.py | 36 ++ 14 files changed, 1353 insertions(+), 79 deletions(-) create mode 100644 py/lora_layout_cache.py create mode 100644 tests/test_lora_layout_cache.py diff --git a/TESTING.md b/TESTING.md index 1988085..16fb848 100644 --- a/TESTING.md +++ b/TESTING.md @@ -3496,7 +3496,9 @@ workflow (README changelog). ### LoRA reference import contract (#221, #249 System S10) The LoRA import is staged (`py/lora_import.py`): discover the LoraManager -`*.metadata.json` files and fingerprint them (path, size, modification time); stage each +`*.metadata.json` files and fingerprint every scanned one, kept or skipped (path, size, +modification time and whether it was kept, #362; a skipped sidecar added, removed or edited +after the review refuses Start with `plan-changed`, `test_start_refused_when_a_skipped_sidecar_changed`); stage each record (CivitAI example images go into the image cache first, best effort, counted in `downloaded` / `download_errors`; each image is written to a temp file beside its cache entry and moved in place, so a half-written image is never seen as cached); the files are @@ -3562,25 +3564,67 @@ the kept, readable LoRAs and `skipped` the rest. Media per LoRA, without repeats preview (`preview_url`, else LoraManager's `PREVIEW_EXTENSIONS` order), then every file in `[/]//` in natural order; a leading video yields to the first still. A file counts only with a servable extension and inside the guarded media -roots. CivitAI downloads run only for a LoRA without local media. The folder guard keeps a -settings folder only when it exists and is neither a filesystem root (`/`, `C:\`, a UNC -share root) nor the home folder or one of its ancestors (realpath + `normcase`). An -unusable layout (no install, unreadable / non-JSON / non-object / oversized settings, no -`platformdirs`, a missing or refused centralized sidecar root) gives a review without a -token ("LoraManager settings: not usable: . Nothing was changed."), a legacy scan -error with that reason, the default LoRA roots for serving, and trigger words from the -default roots in alongside mode. `tests/run_suite.py` and `tests/browser/server.py` point +roots. Only an absolute `preview_url` is used; a relative one is skipped with a DEBUG line +naming the sidecar. A sidecar that is not valid UTF-8 JSON is skipped like any unreadable +one (#362). CivitAI downloads run only for a LoRA without local media; a record's +images are linked in reverse order, so its first image (the cover) is the newest link and +the card image. The folder guard keeps a folder from the settings, and each of ComfyUI's +default LoRA folders (`models/loras`, `extra_model_paths.yaml`, `folder_paths`), only when +it exists and is neither a filesystem root (`/`, `C:\`, a UNC share root) nor the home +folder or a folder that contains it (realpath + `normcase`); a refused default folder that +exists is listed in the layout's `refused_defaults` and logged as one warning each time the +refused set changes (`RefusedDefaultsTests`). An unusable layout (no install, unreadable / +non-JSON / non-object / oversized settings, no `platformdirs`, a missing or refused +centralized sidecar root, "the sidecar folder is a filesystem root, the home folder or a +folder that contains it") gives a review without a token ("LoraManager settings: not +usable: . Nothing was changed."), a legacy scan error with that reason, the default +LoRA roots for serving (no example-images root), and trigger words from the default roots +in alongside mode. A folder discovery cannot list answers the review with 500, code +`folder-unreadable` ("A LoRA folder could not be listed (...). Nothing was changed."). +`TriggerWordCache.load` never raises: an `OSError` is a warning, anything else is logged +with its traceback (#362), both keep the previous map, and a running node retries a failed +load only after 60 s (`TRIGGER_RETRY_SECONDS`). `tests/run_suite.py` and `tests/browser/server.py` point `LORA_MANAGER_SETTINGS_DIR` at a disposable folder, so no test reads the developer's real LoraManager settings; tests that call `load_layout` inject their own folder too. +**Linked-only gate and the layout cache (#362).** While the integration is on, the LoRA +allow-list (`ImagesMixin._lora_image_dirs`) is the layout's `media_roots` plus the CivitAI +download cache, and `_media_roots(outputs)` appends it after the output folders for Library +previews, the Library prompt detail, the per-prompt images page, `serve_output_image` for a +root past the outputs, `inspect_image` and `reveal_image`. Under such a root only a file the +database records as a linked image is served, inspected or revealed (`_linked_only_error`, +by its given and its real path); anything else answers 403. Library responses mark that +media `linked_only`: no preview file, no favorite, no Tag button. The allow-list's layout +comes from `py/lora_layout_cache.py` (`tests/test_lora_layout_cache.py`, which injects the +clocks, `stat` and `ntpath` and never sleeps): one entry keyed by the stripped configured +path, reused while the plugin settings file's and the settings file's `(st_mtime_ns, +st_size)` (or "missing") are unchanged, the install and every media root still resolve to +themselves (a folder swapped for a symlink reloads and the guard runs again), and the entry +is under `MAX_AGE_SECONDS` (30 s, monotonic clock) old. A load is stored only when no +watched file was modified within `RACY_SECONDS` (3 s) of the load (git's racy rule: a write +during the load, LoraManager's in-place write, FAT/HFS+ coarse times), every watched file +could be checked (any `stat` error but not-found, and an offline Windows share, is "cannot +validate"), the settings were readable, and no `clear()` ran meanwhile (generation +counter). Hits take no lock; concurrent misses each load. A finished import (System job or +legacy scan, both through `replace_lora_imports`) clears it. The integration-off check runs +before it, and the configured-path rule (a configured path that is not an install under +`custom_nodes` adds no folders) on its result; a hit also needs the same home folder, which +the guard compares against. `serve_image` builds the allow-list in the +executor. The cache writes one INFO line, "LoraManager layout changed: settings, + storage, media folders", when a stored layout differs from the last one logged. +The review, Start, the legacy scan and the trigger cache read the layout fresh. Image route +tests clear the per-process cache in `ImageRouteCoverageCase.setUp`; `write_settings` in +`TestLoraManagerMediaServing` back-dates its file 10 s so the first load is stored. + `tests/browser/workspace_integrations_fixture.py` (`/__test__/integrations/*`) makes a fake LoraManager install and LoRA folder under the server's temp root, holds or fails the import, saves a blocking pair and answers CivitAI downloads with a stub opener, so no test reaches the network. ```sh -python3 -m unittest tests.test_lora_manager_layout tests.test_lora_import tests.test_api_lora \ - tests.test_lora_database tests.test_lora_utils -v +python3 -m unittest tests.test_lora_layout_cache tests.test_lora_manager_layout \ + tests.test_lora_import tests.test_api_lora tests.test_lora_database tests.test_lora_utils \ + tests.test_api_images -v node --test tests/js/workspace-system-integrations.test.js tests/js/prompt-bridge-adapter.test.js npm run --prefix tools/vendor test:workflows -- workspace-system-integrations.spec.mjs \ system-integrations-concept.spec.mjs \ diff --git a/py/api/images.py b/py/api/images.py index 45196f0..bfafcca 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -2272,7 +2272,8 @@ class ImageRoutesMixin: ) image_path = Path(image["image_path"]) - allowed_dirs = list(self._get_all_output_dirs()) + self._lora_image_dirs() + lora_dirs = await self._run_in_executor(self._lora_image_dirs) + allowed_dirs = list(self._get_all_output_dirs()) + lora_dirs denied = _media_access_error(image_path, allowed_dirs) if denied is not None: @@ -2320,7 +2321,10 @@ class ImageRoutesMixin: LoraManager's guarded LoRA roots, sidecar root and example-images root (``lora_manager_layout.media_roots``, #355) and PromptManager's CivitAI download cache. An unusable LoraManager layout gives the - default LoRA roots and the cache. + default LoRA roots and the cache. The layout comes from the layout + cache (``lora_utils.cached_lora_manager_layout``, #362), which reloads + when LoraManager's settings files change; blocking, so the routes + call it in the executor. """ try: from ..config import IntegrationConfig @@ -2328,25 +2332,18 @@ class ImageRoutesMixin: if not IntegrationConfig.LORA_MANAGER_ENABLED: return [] from ..lora_utils import ( + cached_lora_manager_layout, get_lora_image_cache_dir, - layout_for_install, - lora_manager_layout, ) - lora_root = IntegrationConfig.LORA_MANAGER_PATH or "" - if lora_root.strip(): + configured = IntegrationConfig.LORA_MANAGER_PATH or "" + install, layout = cached_lora_manager_layout(configured) + if configured.strip() and install is None: # A configured path comes from config.json, which can be hand # edited: only a LoraManager install under custom_nodes may # widen the serving allow-list. An empty path keeps the # auto-detection, which itself only scans custom_nodes. - from ..lora_utils import resolve_lora_manager_path - - lora_root = resolve_lora_manager_path(lora_root) - if not lora_root: - return [] - layout = layout_for_install(lora_root) - else: - layout = lora_manager_layout("") + return [] dirs = [Path(d) for d in layout.media_roots] # Named, not made: listing and serving never create the cache. diff --git a/py/api/lora_import_routes.py b/py/api/lora_import_routes.py index cb31341..a09ec68 100644 --- a/py/api/lora_import_routes.py +++ b/py/api/lora_import_routes.py @@ -194,8 +194,13 @@ class LoraImportRoutesMixin: return False async def replace_lora_imports(self, db, staged, *, expected_ids, allow_empty): - """Swap the collection for ``staged.records`` in one transaction.""" - return await self._run_in_executor( + """Swap the collection for ``staged.records`` in one transaction. + + Then the image routes' layout cache is dropped (#362): the import may + have linked files under a folder LoraManager created after the cache + stored its layout (its first example-image download creates one). + """ + result = await self._run_in_executor( lambda: db.replace_category_prompts( CATEGORY, staged.records, @@ -203,6 +208,8 @@ class LoraImportRoutesMixin: allow_empty=allow_empty, ) ) + lora_utils.clear_layout_cache() + return result # ── preview ─────────────────────────────────────────────────────── diff --git a/py/lora_import.py b/py/lora_import.py index e405d9d..23f053c 100644 --- a/py/lora_import.py +++ b/py/lora_import.py @@ -12,9 +12,10 @@ last writes to the database: the sidecars (``discover``). In ``alongside`` mode a sidecar beside its model is kept when ``.safetensors`` exists; in ``centralized`` mode every sidecar under the sidecar root is kept when its ``file_path`` names - a model inside a LoRA root. ``fingerprint`` covers the kept sidecars (path, - size and modification time) and the layout; ``preview`` adds the counts a - review shows. Nothing is written and nothing is requested. + a model inside a LoRA root. ``fingerprint`` covers every scanned sidecar, + kept or skipped (path, size, modification time, kept or not, #362) and the + layout; ``preview`` adds the counts a review shows. Nothing is written + and nothing is requested. 2. **stage**: read each sidecar and build its record. Its LoraManager preview and example images are linked where they are (``local_media``); only a LoRA without any local image downloads its CivitAI example images into the @@ -69,9 +70,10 @@ def discover(lora_manager_path: Optional[str]) -> Discovery: def fingerprint(discovery: Discovery) -> str: - """What a review saw: the layout, then each kept sidecar's path, size and - modification time (a LoraManager save that changes unrelated settings - leaves it as it was).""" + """What a review saw: the layout, then every scanned sidecar, kept or + skipped, with its path, size, modification time and whether it was kept + (#362). A LoraManager save that changes unrelated settings leaves it as + it was.""" layout = discovery.layout digest = hashlib.sha256() for value in ( @@ -87,13 +89,14 @@ def fingerprint(discovery: Discovery) -> str: digest.update(f"{value}\n".encode("utf-8", "surrogatepass")) for name in discovery.unreadable: digest.update(f"unreadable\0{name}\n".encode("utf-8", "surrogatepass")) - for sidecar in discovery.sidecars: - path = sidecar.metadata_path + kept = {str(sidecar.metadata_path) for sidecar in discovery.sidecars} + for path in discovery.scanned_files: + flag = 1 if str(path) in kept else 0 try: stat = path.stat() - entry = f"{path}\0{stat.st_size}\0{stat.st_mtime_ns}\n" + entry = f"{path}\0{stat.st_size}\0{stat.st_mtime_ns}\0{flag}\n" except OSError: - entry = f"{path}\0missing\n" + entry = f"{path}\0missing\0{flag}\n" digest.update(entry.encode("utf-8", "surrogatepass")) return "sha256:" + digest.hexdigest() diff --git a/py/lora_layout_cache.py b/py/lora_layout_cache.py new file mode 100644 index 0000000..ce35dbb --- /dev/null +++ b/py/lora_layout_cache.py @@ -0,0 +1,217 @@ +"""A per-process cache of LoraManager's layout for the image hot paths (#362). + +Every Library list, prompt detail, inspect, reveal and serve request needs the +LoRA allow-list (``ImagesMixin._lora_image_dirs``). Building it means finding +the LoraManager install, reading its plugin settings file and its settings +file, and resolving every folder. ``LayoutCache`` keeps the last result and +reuses it while all of these hold: + +- the key (the configured install path, stripped) and the home folder the + folder guard compares against are the same; +- the entry is younger than ``MAX_AGE_SECONDS`` (monotonic clock); +- the plugin settings file and the settings file have the same signature + (``st_mtime_ns``, ``st_size``, or ``MISSING``) as when it was stored; +- the install and every stored media root still resolve to themselves (a + folder swapped for a symlink reloads, so the folder guard runs again). + +A result is stored only when no watched file is "racy" (modified within +``RACY_SECONDS`` of the load, git's rule for racily clean index entries), +every watched file could be checked, the settings file was readable, and no +``clear()`` ran during the load. What each rule covers: signatures catch a +settings change on the next request; the racy rule catches a write during the +load, a half-written file and coarse timestamps; the maximum age bounds the +inputs no signature sees (``extra_model_paths.yaml``, environment, temporary +failures); ``clear()`` makes a finished import's new folders servable at once. + +Hits and loads take no lock: a slow ``os.stat`` on a network share holds up +only its own request, and two threads that miss together both load (both +loads only read). A short lock guards the compare-and-store and the logging +state; no file I/O runs under it. Standard library only, so it is tested +without ComfyUI or LoraManager. +""" + +import logging +import os +import threading +import time +from dataclasses import dataclass +from pathlib import Path +from typing import Callable, Optional, Tuple + +from .lora_manager_layout import UNREADABLE, LoraManagerLayout, _canonical + +# A watched file modified this recently is not trusted: LoraManager writes its +# settings in place, and FAT/exFAT store modification times in 2 s steps. +RACY_SECONDS = 3 +# Bounds what no signature sees (unwatched inputs, temporary failures). +MAX_AGE_SECONDS = 30 + +# The signature of a watched file that does not exist (a valid state). +MISSING = object() +# ERROR_BAD_NETPATH and ERROR_BAD_NET_NAME, which Windows raises as +# FileNotFoundError. +_OFFLINE_SHARE_ERRORS = frozenset({53, 67}) + +logger = logging.getLogger("prompt_manager.lora_layout_cache") + + +@dataclass(frozen=True) +class _Entry: + key: str + home: str + install: Optional[str] + layout: LoraManagerLayout + signatures: Tuple[Tuple[str, object], ...] + stored_at: float + + +class _CannotValidate(Exception): + """A watched file could not be checked (not merely missing).""" + + +class LayoutCache: + """At most one ``(install, layout)`` for the active LoraManager config.""" + + def __init__( + self, + resolve: Callable[[str], Optional[str]], + load: Callable[[Optional[str]], LoraManagerLayout], + *, + wall_ns: Callable[[], int] = time.time_ns, + monotonic: Callable[[], float] = time.monotonic, + stat=os.stat, + pathmod=os.path, + home: Callable[[], str] = lambda: str(Path.home()), + ): + self._resolve = resolve + self._load = load + self._wall_ns = wall_ns + self._monotonic = monotonic + self._stat = stat + self._pathmod = pathmod + self._home = home + self._entry: Optional[_Entry] = None + self._generation = 0 + self._lock = threading.Lock() + # What was last logged; survives clear() so a reload of the same + # layout stays quiet. + self._logged_layout: Optional[LoraManagerLayout] = None + self._logged_refused: Tuple[str, ...] = () + + def get(self, configured: str) -> Tuple[Optional[str], LoraManagerLayout]: + """``(install, layout)`` for the configured path ("" auto-detects).""" + key = (configured or "").strip() + home = self._home() + entry = self._entry + if entry is not None and self._valid(entry, key, home): + return entry.install, entry.layout + with self._lock: + if entry is not None and self._entry is entry: + self._entry = None + generation = self._generation + install = self._resolve(key) + layout = self._load(install) + self._log_refused(layout) + try: + signatures = tuple( + (path, self._signature(path)) for path in self._watched(install, layout) + ) + except _CannotValidate: + return install, layout + if layout.error == UNREADABLE or self._racy(signatures, self._wall_ns()): + return install, layout + stored = _Entry(key, home, install, layout, signatures, self._monotonic()) + changed = False + with self._lock: + if self._generation == generation: + self._entry = stored + changed = layout != self._logged_layout + if changed: + self._logged_layout = layout + if changed: + logger.info( + f"LoraManager layout changed: {layout.source or 'no'} settings, " + f"{layout.mode} storage, {len(layout.media_roots)} media folders" + ) + return install, layout + + def clear(self) -> None: + """Drop the entry; a load that started before this stores nothing.""" + with self._lock: + self._entry = None + self._generation += 1 + + def _log_refused(self, layout: LoraManagerLayout) -> None: + """One warning per default LoRA folder newly refused by the guard.""" + refused = tuple(layout.refused_defaults) + with self._lock: + if refused == self._logged_refused: + return + new = [folder for folder in refused if folder not in self._logged_refused] + self._logged_refused = refused + for folder in new: + logger.warning( + f"ComfyUI LoRA folder '{folder}' is neither scanned nor served: " + "it is a filesystem root, the home folder or a folder that " + "contains it" + ) + + # ── validation ────────────────────────────────────────────────── + + def _valid(self, entry: _Entry, key: str, home: str) -> bool: + if entry.key != key or entry.home != home: + return False + if self._monotonic() - entry.stored_at >= MAX_AGE_SECONDS: + return False + try: + for path, signature in entry.signatures: + if self._signature(path) != signature: + return False + except _CannotValidate: + return False + folders = entry.layout.media_roots + if entry.install is not None: + folders = (entry.install,) + tuple(folders) + for folder in folders: + try: + if _canonical(folder, self._pathmod) != folder: + return False + except (OSError, ValueError): + return False + return True + + def _watched(self, install, layout) -> Tuple[str, ...]: + """The plugin settings file, then the settings file, each once.""" + if install is None: + return () + paths = [self._pathmod.join(install, "settings.json")] + if layout.settings_path is not None: + paths.append(str(layout.settings_path)) + watched, seen = [], set() + for path in paths: + identity = self._pathmod.normcase(self._pathmod.abspath(path)) + if identity not in seen: + seen.add(identity) + watched.append(path) + return tuple(watched) + + def _signature(self, path: str): + try: + result = self._stat(path) + except (FileNotFoundError, NotADirectoryError) as error: + # Windows reports an offline share as "not found": that is a + # temporary failure, not a missing file. + if getattr(error, "winerror", None) in _OFFLINE_SHARE_ERRORS: + raise _CannotValidate(path) from error + return MISSING + except (OSError, ValueError) as error: + raise _CannotValidate(path) from error + return (result.st_mtime_ns, result.st_size) + + @staticmethod + def _racy(signatures, now_ns: int) -> bool: + threshold = now_ns - RACY_SECONDS * 10**9 + return any( + signature is not MISSING and signature[0] > threshold + for _path, signature in signatures + ) diff --git a/py/lora_manager_layout.py b/py/lora_manager_layout.py index dc3716e..d05abe5 100644 --- a/py/lora_manager_layout.py +++ b/py/lora_manager_layout.py @@ -12,8 +12,10 @@ server share one definition: plugin file, or the user config folder, in LoraManager's order), keeps only the seven keys the import needs, and resolves the LoRA roots, the sidecar root (centralized storage), the example-images root and the active - LoraManager library. Every folder from the settings passes the folder - guard (exists, not a filesystem root, not the home folder itself). + LoraManager library. Every folder from the settings, and each of + ComfyUI's default LoRA folders, passes the folder guard: it must exist and + must not be a filesystem root, the home folder or a folder that contains + it. Default folders the guard refuses are listed in ``refused_defaults``. - ``discover_sidecars`` finds every sidecar whose model file exists inside a LoRA root: beside the models in ``alongside`` mode, under the sidecar root (matched to its model by ``file_path``) in ``centralized`` mode. @@ -25,6 +27,7 @@ it is testable outside ComfyUI. """ import json +import logging import os import re from dataclasses import dataclass, replace @@ -98,7 +101,12 @@ NOT_OBJECT = "the settings file is not a JSON object" TOO_LARGE = "the settings file is larger than 5 MB" NO_PLATFORMDIRS = "platformdirs is not available" NO_SIDECAR_ROOT = "the sidecar folder does not exist" -BAD_SIDECAR_ROOT = "the sidecar folder is a filesystem root or the home folder" +BAD_SIDECAR_ROOT = ( + "the sidecar folder is a filesystem root, the home folder or a folder " + "that contains it" +) + +logger = logging.getLogger("prompt_manager.lora_manager_layout") @dataclass(frozen=True) @@ -114,6 +122,9 @@ class LoraManagerLayout: example_images_root: Optional[str] library: str media_roots: Tuple[str, ...] + # ComfyUI's default LoRA folders that exist but that the folder guard + # refused (canonical): the layout cache logs them (#362). + refused_defaults: Tuple[str, ...] = () @dataclass(frozen=True) @@ -126,13 +137,20 @@ class Sidecar: @dataclass(frozen=True) class Discovery: - """The sidecars kept, how many metadata files were seen, and those unread.""" + """The sidecars kept, every metadata file seen, and those unread.""" layout: LoraManagerLayout sidecars: List[Sidecar] - scanned: int + # Every ``*.metadata.json`` seen, kept or skipped: resolved, once each, + # sorted by ``str(path)``. The import's fingerprint covers all of them. + scanned_files: Tuple[Path, ...] unreadable: List[str] + @property + def scanned(self) -> int: + """How many metadata files were seen.""" + return len(self.scanned_files) + class _SettingsError(Exception): """The settings file cannot be used, for the reason it carries.""" @@ -315,6 +333,23 @@ def _guarded(paths, home: str) -> Tuple[str, ...]: return tuple(kept) +def _refused(paths, home: str) -> Tuple[str, ...]: + """The folders in ``paths`` that exist but that the guard refuses.""" + refused = [] + for path in paths: + absolute = _absolute(path) + if not absolute or not os.path.isdir(absolute): + continue + if _guard_folder(absolute, home) is None: + try: + canonical = _canonical(absolute) + except (OSError, ValueError): + continue + if canonical not in refused: + refused.append(canonical) + return tuple(refused) + + def _example_images_root( settings: Mapping, library: str, libraries: int, home: str ) -> Optional[str]: @@ -351,7 +386,9 @@ def load_layout( default LoRA roots, so serving and trigger words keep working. """ home = str(Path.home()) - defaults = _guarded(default_roots(), home) + default_list = list(default_roots()) + defaults = _guarded(default_list, home) + refused_defaults = _refused(default_list, home) fallback = LoraManagerLayout( settings_path=None, source=None, @@ -362,6 +399,7 @@ def load_layout( example_images_root=None, library="default", media_roots=defaults, + refused_defaults=refused_defaults, ) if not lora_manager_path: return fallback @@ -377,10 +415,14 @@ def load_layout( except _SettingsError as error: return replace(fallback, error=str(error)) settings = _settings_subset(raw or {}) - return _layout_from_settings(settings, settings_path, source, defaults, home) + return _layout_from_settings( + settings, settings_path, source, defaults, home, refused_defaults + ) -def _layout_from_settings(settings, settings_path, source, defaults, home): +def _layout_from_settings( + settings, settings_path, source, defaults, home, refused_defaults=() +): library, entry, libraries = _active_library(settings) lora_roots = _media_roots( defaults, @@ -400,6 +442,7 @@ def _layout_from_settings(settings, settings_path, source, defaults, home): example_images_root=example_root, library=library, media_roots=_media_roots(lora_roots, [example_root]), + refused_defaults=refused_defaults, ) if mode != MODE_CENTRALIZED: return layout @@ -482,13 +525,13 @@ def discover_sidecars(layout: LoraManagerLayout) -> Discovery: cannot see every folder must not replace the collection with part of it. """ if layout.error is not None: - return Discovery(layout, [], 0, []) + return Discovery(layout, [], (), []) if layout.mode == MODE_CENTRALIZED: folders = [layout.sidecar_root] else: folders = list(layout.lora_roots) seen = set() - scanned = 0 + scanned: List[Path] = [] unreadable: List[str] = [] sidecars = [] for folder in folders: @@ -498,12 +541,13 @@ def discover_sidecars(layout: LoraManagerLayout) -> Discovery: if key in seen: continue seen.add(key) - scanned += 1 + scanned.append(resolved) model = _model_for(resolved, layout, unreadable) if model is not None: sidecars.append(Sidecar(resolved, Path(os.path.realpath(model)))) sidecars.sort(key=lambda sidecar: str(sidecar.metadata_path)) - return Discovery(layout, sidecars, scanned, unreadable) + scanned.sort(key=str) + return Discovery(layout, sidecars, tuple(scanned), unreadable) # ── images ────────────────────────────────────────────────────────── @@ -526,8 +570,15 @@ def _example_files(folder: str) -> List[str]: def _preview_candidates(sidecar: Sidecar, metadata: Dict) -> Iterator[str]: """The paths a preview is looked for at, in LoraManager's order.""" url = metadata.get("preview_url") - if isinstance(url, str) and url.strip() and os.path.isabs(url.strip()): - yield url.strip() + if isinstance(url, str) and url.strip(): + if os.path.isabs(url.strip()): + yield url.strip() + else: + # LoraManager writes absolute paths; it would resolve a relative + # one against its working directory, not the sidecar folder. + logger.debug( + "Relative preview_url skipped in %s", sidecar.metadata_path.name + ) base = sidecar.metadata_path.name[: -len(METADATA_SUFFIX)] for suffix in PREVIEW_EXTENSIONS: yield str(sidecar.metadata_path.parent / f"{base}{suffix}") diff --git a/py/lora_utils.py b/py/lora_utils.py index 5d33489..da71a3c 100644 --- a/py/lora_utils.py +++ b/py/lora_utils.py @@ -30,6 +30,7 @@ except ImportError: from dataclasses import replace +from .lora_layout_cache import LayoutCache from .lora_manager_layout import ( MODE_ALONGSIDE, METADATA_SUFFIX, @@ -295,10 +296,35 @@ def layout_for_install(install: Optional[str]) -> LoraManagerLayout: def lora_manager_layout(lora_manager_path: Optional[str]) -> LoraManagerLayout: - """Where LoraManager keeps its files (``py/lora_manager_layout.py``).""" + """Where LoraManager keeps its files (``py/lora_manager_layout.py``). + + A fresh read: the import review, Start, the legacy scan and the trigger + word cache use it. The image hot paths use ``cached_lora_manager_layout``. + """ return layout_for_install(lora_manager_install(lora_manager_path)) +# The image hot paths' layout cache (#362). The lambdas look the functions up +# when called, so a test that patches them here is seen by the cache. +_layout_cache = LayoutCache( + lambda path: lora_manager_install(path), + lambda install: layout_for_install(install), +) + + +def cached_lora_manager_layout( + configured: Optional[str], +) -> Tuple[Optional[str], LoraManagerLayout]: + """``(install, layout)`` for the configured path, from the layout cache + while LoraManager's settings files are unchanged (``py/lora_layout_cache.py``).""" + return _layout_cache.get(configured or "") + + +def clear_layout_cache() -> None: + """Drop the cached layout (after an import linked files in new folders).""" + _layout_cache.clear() + + def _get_extra_lora_paths(comfyui_root: Path) -> List[Path]: """Parse extra_model_paths.yaml for additional LoRA directories.""" results = [] @@ -342,7 +368,7 @@ def read_lora_metadata(metadata_path: Path) -> Optional[Dict]: try: with open(metadata_path, "r", encoding="utf-8") as f: return json.load(f) - except (json.JSONDecodeError, OSError) as e: + except (ValueError, OSError) as e: # JSONDecodeError, UnicodeDecodeError logger.debug(f"Failed to read {metadata_path}: {e}") return None @@ -857,11 +883,13 @@ class TriggerWordCache: """ try: new_cache = self._build(lora_manager_path) - except Exception as error: + except OSError as error: logger.warning(f"Trigger word cache not refreshed: {error}") - with self._lock: - self._failed_at = time.monotonic() - return len(self._cache) + return self._failed() + except Exception: + # Not a disk error: keep the traceback (#362). + logger.exception("Trigger word cache not refreshed") + return self._failed() with self._lock: self._cache = new_cache @@ -873,6 +901,12 @@ class TriggerWordCache: ) return len(new_cache) + def _failed(self) -> int: + """Keep the previous map and note when this load failed.""" + with self._lock: + self._failed_at = time.monotonic() + return len(self._cache) + @staticmethod def _build(lora_manager_path: str) -> Dict[str, List[str]]: layout = lora_manager_layout(lora_manager_path) diff --git a/tests/browser/workspace-system-sources.spec.mjs b/tests/browser/workspace-system-sources.spec.mjs index 80cbb56..556ff4e 100644 --- a/tests/browser/workspace-system-sources.spec.mjs +++ b/tests/browser/workspace-system-sources.spec.mjs @@ -429,7 +429,9 @@ test('Rebuild all is explicit and the plan line says every preview is regenerate await expect(page).toHaveURL(/#\/activity\/[0-9a-f]{32}$/); expect(seen.starts).toEqual([{ mode: 'rebuild' }]); const id = page.url().split('/').pop(); - await expect.poll(async () => (await serverJson(request, `/prompt_manager/jobs/${id}`)).job.state).toBe('finished'); + // Rebuild regenerates all 24 previews with real image work: on a busy CI + // runner that took over 5 s (12 of 24 done at the default poll budget). + await expect.poll(async () => (await serverJson(request, `/prompt_manager/jobs/${id}`)).job.state, { timeout: 30000 }).toBe('finished'); }); test('a plan that cannot be read says so; Start still starts', async ({ page }) => { diff --git a/tests/test_api_images.py b/tests/test_api_images.py index 5b0fa3e..08713dd 100644 --- a/tests/test_api_images.py +++ b/tests/test_api_images.py @@ -9,9 +9,11 @@ import asyncio import json import os import pathlib +import shutil import sys import tempfile import threading +import time import types import unittest from contextlib import ExitStack @@ -36,6 +38,8 @@ from PIL import Image # noqa: E402 from database.operations import PromptDatabase # noqa: E402 import py.api.images as images_module from py.api import gallery_listing # noqa: E402 +from py import lora_utils # noqa: E402 +from py import lora_manager_layout as layout_module # noqa: E402 from py.api.thumbnail_settings import ThumbnailSettings # noqa: E402 from py.api import PromptManagerAPI # noqa: E402 from py.api.prompts import MAX_PAGE_LIMIT # noqa: E402 @@ -111,13 +115,25 @@ class ImageAPITestCase(AioHTTPTestCase): self.assertGreater(image_id, 0, "fixture: image link failed") return image_id - def _make_symlink(self, target, link): + def _make_symlink(self, target, link, target_is_directory=False): """Create a symlink or skip the test when the platform refuses.""" try: - os.symlink(str(target), str(link)) + os.symlink(str(target), str(link), target_is_directory=target_is_directory) except (OSError, NotImplementedError, AttributeError) as exc: self.skipTest(f"symlinks unavailable: {exc}") + def _spy_executor(self): + """Record the __name__ of every callable sent to _run_in_executor.""" + original = self.api._run_in_executor + calls = [] + + async def spy(func, *args, **kwargs): + calls.append(getattr(func, "__name__", repr(func))) + return await original(func, *args, **kwargs) + + self.api._run_in_executor = spy + return calls + class TestServeOutputImage(ImageAPITestCase): """GET /prompt_manager/images/serve/{filepath}""" @@ -388,18 +404,6 @@ class ThumbnailTestCase(ImageAPITestCase): PROGRESS_URL = "/prompt_manager/images/generate-thumbnails/progress" - def _spy_executor(self): - """Record the __name__ of every callable sent to _run_in_executor.""" - original = self.api._run_in_executor - calls = [] - - async def spy(func, *args, **kwargs): - calls.append(getattr(func, "__name__", repr(func))) - return await original(func, *args, **kwargs) - - self.api._run_in_executor = spy - return calls - def _thumb_path(self, name, suffix=".webp"): return self.output_dir / "thumbnails" / f"{name}_thumb{suffix}" @@ -1216,6 +1220,13 @@ def _raise(*_args, **_kwargs): class ImageRouteCoverageCase(ImageAPITestCase): """Helpers for driving success and failure branches of each route.""" + def setUp(self): + # #362: the LoraManager layout cache is per process; every test here + # starts and ends with it empty (runs before the subclasses' setUpAsync). + super().setUp() + lora_utils.clear_layout_cache() + self.addCleanup(lora_utils.clear_layout_cache) + def _break_db(self, db_method): setattr(self.api.db, db_method, _raise) @@ -1327,6 +1338,9 @@ class TestServeErrorBranches(ImageRouteCoverageCase): ) self.assertEqual(resp.status, 200) + # The cache does not watch default_lora_roots (#362): without a + # clear the next request would reuse the folders above. + lora_utils.clear_layout_cache() with ( patch.object(IntegrationConfig, "LORA_MANAGER_ENABLED", True), patch.object(IntegrationConfig, "LORA_MANAGER_PATH", ""), @@ -1419,32 +1433,39 @@ class TestLoraManagerMediaServing(ImageRouteCoverageCase): self.cache = root / "cache" for folder in (self.sidecars, self.examples, self.cache): folder.mkdir() + self.root = root self.write_settings(str(self.sidecars)) self.enabled = patch.object(IntegrationConfig, "LORA_MANAGER_ENABLED", True) + self.default_roots = patch("py.lora_utils.default_lora_roots", return_value=[]) for item in ( self.enabled, patch.object(IntegrationConfig, "LORA_MANAGER_PATH", str(self.lm)), patch( "py.lora_utils.custom_nodes_directories", return_value=[custom_nodes] ), - patch("py.lora_utils.default_lora_roots", return_value=[]), + self.default_roots, patch("py.lora_utils.get_lora_image_cache_dir", return_value=self.cache), patch.dict(os.environ, {"LORA_MANAGER_SETTINGS_DIR": str(self.settings)}), ): item.start() self.addCleanup(item.stop) - def write_settings(self, sidecar_root): - (self.settings / "settings.json").write_text( + def write_settings(self, sidecar_root, examples=None): + path = self.settings / "settings.json" + path.write_text( json.dumps( { "sidecar_storage_mode": "centralized", "sidecar_storage_path": sidecar_root, - "example_images_path": str(self.examples), + "example_images_path": str(examples or self.examples), "civitai_api_key": "never-served", } ) ) + # Ten seconds old, so the layout cache (real clock here) stores the + # first load instead of treating the file as being written (#362). + stamp = time.time_ns() - 10 * 10**9 + os.utime(path, ns=(stamp, stamp)) async def _status(self, image_id): resp = await self.client.request( @@ -1464,11 +1485,65 @@ class TestLoraManagerMediaServing(ImageRouteCoverageCase): for image_id in ids: self.assertEqual(await self._status(image_id), 200) + # A warm layout cache does not outlive the integration (#362). + self.assertIsNotNone(lora_utils._layout_cache._entry) self.enabled.stop() for image_id in ids: self.assertEqual(await self._status(image_id), 403) self.enabled.start() + def _example(self, folder=None, name="1.mp4"): + path = (folder or self.examples) / "ab12" / name + path.parent.mkdir(exist_ok=True) + path.write_bytes(b"video") + return path + + async def test_unchanged_settings_reuse_the_cached_layout(self): + image_id = self._link_image(self._example()) + self.assertEqual(await self._status(image_id), 200) + self.assertIsNotNone(lora_utils._layout_cache._entry) + self.assertEqual(await self._status(image_id), 200) + self.assertEqual(lora_utils.default_lora_roots.call_count, 1) + + async def test_a_settings_change_is_seen_without_a_clear(self): + image_id = self._link_image(self._example()) + self.assertEqual(await self._status(image_id), 200) + self.assertIsNotNone(lora_utils._layout_cache._entry) + moved = self.root / "examples-moved" + moved.mkdir() + self.write_settings(str(self.sidecars), examples=moved) + self.assertEqual(await self._status(image_id), 403) + + async def test_a_cached_folder_swapped_for_a_symlink_to_home_is_refused(self): + # CUJ 7: the example-images folder becomes a symlink to a folder that + # holds the home folder; the same recorded file is refused. + outer = self.root / "outer" + (outer / "home").mkdir(parents=True) + self._example(outer) + image_id = self._link_image(self._example()) + with patch.object(layout_module.Path, "home", return_value=outer / "home"): + self.assertEqual(await self._status(image_id), 200) + self.assertIsNotNone(lora_utils._layout_cache._entry) + # A skip after the rmtree is safe: setUp builds a fresh tree. + shutil.rmtree(self.examples) + self._make_symlink(outer, self.examples, target_is_directory=True) + self.assertEqual(await self._status(image_id), 403) + + async def test_a_configured_path_outside_custom_nodes_adds_nothing(self): + from py.config import IntegrationConfig + + self.assertEqual(await self._status(self._link_image(self._example())), 200) + self.assertIsNotNone(lora_utils._layout_cache._entry) + with patch.object(IntegrationConfig, "LORA_MANAGER_PATH", str(self.sidecars)): + self.assertEqual(self.api._lora_image_dirs(), []) + # The warm entry for the real install was never handed out for it. + self.assertIsNone(lora_utils.cached_lora_manager_layout(str(self.sidecars))[0]) + + async def test_serve_image_builds_the_allow_list_in_the_executor(self): + calls = self._spy_executor() + self.assertEqual(await self._status(self._link_image(self._example())), 200) + self.assertIn("_lora_image_dirs", calls) + async def test_a_filesystem_root_as_sidecar_folder_adds_nothing(self): self.write_settings(os.path.abspath(os.sep)) dirs = self.api._lora_image_dirs() diff --git a/tests/test_api_lora.py b/tests/test_api_lora.py index 6b7ec49..8fda056 100644 --- a/tests/test_api_lora.py +++ b/tests/test_api_lora.py @@ -876,6 +876,18 @@ class TestLoraScanRoute(LoraAPITestCase): resp = await self.client.request("POST", "/prompt_manager/lora/scan") self.assertEqual(resp.status, 400) + async def test_a_finished_scan_clears_the_layout_cache(self): + # #362: the legacy scan replaces through the same method. + self._enable() + loras = self.tmp_root / "loras" + loras.mkdir() + _write_metadata(loras, "plain") + (loras / "plain.png").write_bytes(_png_bytes()) + with patch("py.lora_utils.clear_layout_cache") as clear: + resp, events, _calls = await self._scan(loras) + self.assertEqual(events[-1]["type"], "complete", events) + clear.assert_called_once_with() + async def test_scan_imports_loras_with_images_and_links_existing(self): self._enable() loras = self.tmp_root / "loras" diff --git a/tests/test_lora_import.py b/tests/test_lora_import.py index f5ff24d..6aaee2e 100644 --- a/tests/test_lora_import.py +++ b/tests/test_lora_import.py @@ -529,6 +529,16 @@ class LoraImportRouteTests(LoraImportAPITestCase): resp, body = await self._post(APPLY, {"token": plan["token"]}) self.assertEqual((resp.status, body["code"]), (409, "plan-used")) + async def test_a_finished_import_clears_the_layout_cache(self): + # #362: the import may link files under a folder LoraManager created + # after the image routes cached its layout. + self._two_loras() + with patch("py.lora_utils.clear_layout_cache") as clear: + _plan, job = await self._apply_reviewed() + await job.task + self.assertEqual(job.state, "finished", job.log) + clear.assert_called_once_with() + async def test_discovery_failure_keeps_previous_imports(self): self._two_loras() before = _snapshot(self.api.db) @@ -936,6 +946,35 @@ class LoraImportRefusalTests(LoraImportAPITestCase): self.assertEqual((job.state, job.scope["failure"]), ("failed", "plan-changed")) self.assertEqual(_snapshot(self.api.db), before) + async def test_a_refused_import_keeps_the_layout_cache(self): + self._two_loras() + resp, plan = await self._post(PREVIEW) + self.api.db.save_prompt( + text="late", category=CATEGORY, prompt_hash=generate_prompt_hash("late") + ) + with patch("py.lora_utils.clear_layout_cache") as clear: + resp, body = await self._post(APPLY, {"token": plan["token"]}) + job = self.api._jobs.get(body["job"]["id"]) + await job.task + self.assertEqual(job.scope["failure"], "plan-changed") + clear.assert_not_called() + + async def test_start_refused_when_a_skipped_sidecar_changed(self): + # #362 CUJ 4: the review counted the orphan as skipped. + self._two_loras() + orphan = self.loras / "orphan.metadata.json" + orphan.write_text(json.dumps({"model_name": "orphan"})) + resp, plan = await self._post(PREVIEW) + self.assertEqual(resp.status, 200, plan) + self.assertEqual((plan["lora_files"], plan["skipped"]), (3, 1)) + before = _snapshot(self.api.db) + orphan.write_text(orphan.read_text() + " ") + resp, body = await self._post(APPLY, {"token": plan["token"]}) + self.assertEqual(resp.status, 409, body) + self.assertEqual(body["code"], "plan-changed") + self.assertEqual(self.api._jobs.list(), []) + self.assertEqual(_snapshot(self.api.db), before) + async def test_a_pair_saved_after_the_review_blocks_the_job(self): self._two_loras() resp, plan = await self._post(PREVIEW) @@ -1187,6 +1226,62 @@ class LoraImportStageTests(unittest.TestCase): ) self.assertNotEqual(lora_import.fingerprint(self.discover()), first) + # #362: a skipped sidecar is part of what the review counted. + + def _assert_fingerprint_follows(self, change): + first = lora_import.fingerprint(self.discover()) + self.assertEqual(lora_import.fingerprint(self.discover()), first) + change() + self.assertNotEqual(lora_import.fingerprint(self.discover()), first) + + def _assert_add_edit_remove_change_it(self, path, text): + self._assert_fingerprint_follows(lambda: path.write_text(text)) + self._assert_fingerprint_follows( + lambda: path.write_text(path.read_text() + " ") + ) + self._assert_fingerprint_follows(path.unlink) + + def _centralized(self): + sidecars = self.tmp / "sidecars" + sidecars.mkdir() + self.settings_file( + { + "sidecar_storage_mode": "centralized", + "sidecar_storage_path": str(sidecars), + } + ) + model = self.loras / "kept.safetensors" + model.write_bytes(b"m") + (sidecars / "kept.metadata.json").write_text( + json.dumps({"file_path": str(model)}) + ) + return sidecars + + def test_fingerprint_follows_a_centralized_sidecar_whose_model_is_missing(self): + sidecars = self._centralized() + gone = self.loras / "gone.safetensors" + self._assert_add_edit_remove_change_it( + sidecars / "gone.metadata.json", json.dumps({"file_path": str(gone)}) + ) + self.assertEqual(len(self.discover().sidecars), 1) + + def test_fingerprint_follows_a_checkpoint_sidecar_outside_the_lora_roots(self): + sidecars = self._centralized() + checkpoints = self.tmp / "checkpoints" + checkpoints.mkdir() + ckpt = checkpoints / "base.safetensors" + ckpt.write_bytes(b"c") + self._assert_add_edit_remove_change_it( + sidecars / "base.metadata.json", json.dumps({"file_path": str(ckpt)}) + ) + + def test_fingerprint_follows_an_alongside_sidecar_without_its_model(self): + _write_metadata(self.loras, "kept") + self._assert_add_edit_remove_change_it( + self.loras / "orphan.metadata.json", json.dumps({"model_name": "orphan"}) + ) + self.assertEqual(len(self.discover().sidecars), 1) + def test_fingerprint_follows_a_missing_file_between_stat_calls(self): meta = _write_metadata(self.loras, "one") discovery = self.discover() diff --git a/tests/test_lora_layout_cache.py b/tests/test_lora_layout_cache.py new file mode 100644 index 0000000..bbe2305 --- /dev/null +++ b/tests/test_lora_layout_cache.py @@ -0,0 +1,590 @@ +"""The LoraManager layout cache for the image hot paths (#362). + +``py/lora_layout_cache.py``. Every test injects the install resolver, the +layout loader (both counting calls), the wall clock, the monotonic clock and, +where needed, ``stat`` and ``pathmod``, over a temporary folder. No test +sleeps: modification times are set with ``os.utime`` and the clocks are +plain attributes the test moves. +""" + +import ntpath +import os +import shutil +import sys +import tempfile +import threading +import time +import unittest +from dataclasses import replace +from pathlib import Path, PureWindowsPath +from types import SimpleNamespace +from unittest.mock import patch + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from py import lora_layout_cache as cache_module +from py.lora_layout_cache import ( + MAX_AGE_SECONDS, + MISSING, + RACY_SECONDS, + LayoutCache, +) +from py.lora_manager_layout import UNREADABLE, LoraManagerLayout, _canonical + +SECOND_NS = 10**9 +# NTFS keeps modification times in 100 ns steps and FAT in 2 s steps: every +# time a test sets is a whole millisecond, and a test whose file system rounds +# it anyway skips (``set_mtime``). +TICK_NS = 10**6 + + +def make_layout(settings_path=None, media_roots=(), error=None, source="user config"): + return LoraManagerLayout( + settings_path=settings_path, + source=source, + error=error, + mode="alongside", + lora_roots=tuple(media_roots), + sidecar_root=None, + example_images_root=None, + library="default", + media_roots=tuple(media_roots), + ) + + +class CacheTestCase(unittest.TestCase): + """An install, its plugin settings file, a settings file and a LoRA root.""" + + def setUp(self): + self.tmp = Path(tempfile.mkdtemp()).resolve() + self.addCleanup(shutil.rmtree, self.tmp) + self.install_dir = self.tmp / "custom_nodes" / "ComfyUI-Lora-Manager" + self.install_dir.mkdir(parents=True) + self.plugin_settings = self.install_dir / "settings.json" + self.settings = self.tmp / "user-config" / "settings.json" + self.settings.parent.mkdir() + self.root_dir = self.tmp / "loras" + self.root_dir.mkdir() + # Files are written ten minutes before the injected wall clock, so + # nothing is racy unless a test sets a recent modification time. + self.now_ns = time.time_ns() // TICK_NS * TICK_NS + self.mono = 1000.0 + self.resolved = [] + self.loaded = [] + self.install = _canonical(str(self.install_dir)) + self.layout = make_layout( + settings_path=self.settings, + media_roots=(_canonical(str(self.root_dir)),), + ) + self.write(self.settings, "{}") + + def write(self, path, text, mtime_ns=None): + Path(path).write_text(text, encoding="utf-8") + stamp = mtime_ns if mtime_ns is not None else self.now_ns - 600 * SECOND_NS + self.set_mtime(path, stamp) + + def set_mtime(self, path, stamp): + os.utime(path, ns=(stamp, stamp)) + if os.stat(path).st_mtime_ns != stamp: + self.skipTest("this file system rounds modification times") + + def resolve(self, key): + self.resolved.append(key) + return self.install + + def load(self, install): + self.loaded.append(install) + return self.layout + + def cache(self, **kwargs): + kwargs.setdefault("wall_ns", lambda: self.now_ns) + kwargs.setdefault("monotonic", lambda: self.mono) + return LayoutCache( + kwargs.pop("resolve", self.resolve), kwargs.pop("load", self.load), **kwargs + ) + + +class ValidationTests(CacheTestCase): + def test_hit_reuses_the_entry_without_resolving_or_loading(self): + cache = self.cache() + first = cache.get("") + second = cache.get("") + self.assertEqual(len(self.resolved), 1) + self.assertEqual(len(self.loaded), 1) + self.assertIs(first[1], second[1]) + self.assertIs(first[0], second[0]) + self.assertEqual(first, (self.install, self.layout)) + + def test_settings_size_change_reloads_once(self): + cache = self.cache() + cache.get("") + stamp = self.settings.stat().st_mtime_ns + self.write(self.settings, '{"a": 1}', mtime_ns=stamp) + cache.get("") + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_settings_mtime_change_reloads_once(self): + cache = self.cache() + cache.get("") + self.set_mtime(self.settings, self.settings.stat().st_mtime_ns + TICK_NS) + cache.get("") + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_settings_deletion_reloads_once(self): + cache = self.cache() + cache.get("") + self.settings.unlink() + cache.get("") + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_settings_creation_reloads_once(self): + self.settings.unlink() + cache = self.cache() + cache.get("") + self.write(self.settings, "{}") + cache.get("") + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_plugin_settings_change_reloads_once(self): + cache = self.cache() + cache.get("") + self.write(self.plugin_settings, '{"use_portable_settings": true}') + cache.get("") + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_a_different_configured_path_reloads(self): + cache = self.cache() + cache.get("") + cache.get("other") + self.assertEqual(self.resolved, ["", "other"]) + self.assertEqual(len(self.loaded), 2) + + def test_a_different_home_folder_reloads(self): + # The folder guard compares against the home folder: a cached layout + # must not outlive a change of it. + home = {"value": str(self.tmp / "home-a")} + cache = self.cache(home=lambda: home["value"]) + cache.get("") + cache.get("") + home["value"] = str(self.tmp / "home-b") + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_configured_path_is_stripped_for_the_key_and_the_resolver(self): + cache = self.cache() + cache.get(" x ") + cache.get("x") + self.assertEqual(self.resolved, ["x"]) + self.assertEqual(len(self.loaded), 1) + + def test_a_recently_modified_file_is_not_stored(self): + self.write(self.settings, "{}", mtime_ns=self.now_ns - SECOND_NS) + cache = self.cache() + cache.get("") + cache.get("") + self.assertEqual(len(self.loaded), 2) + self.assertIsNone(cache._entry) + self.now_ns += RACY_SECONDS * SECOND_NS + cache.get("") + self.assertIsNotNone(cache._entry) + cache.get("") + self.assertEqual(len(self.loaded), 3) + + def test_racy_boundary(self): + # Exactly RACY_SECONDS old is trusted; anything newer is not. + boundary = self.now_ns - RACY_SECONDS * SECOND_NS + self.write(self.settings, "{}", mtime_ns=boundary + TICK_NS) + cache = self.cache() + cache.get("") + self.assertIsNone(cache._entry) + self.set_mtime(self.settings, boundary) + cache.get("") + self.assertIsNotNone(cache._entry) + + def test_a_modification_time_in_the_future_is_racy(self): + self.write(self.plugin_settings, "{}", mtime_ns=self.now_ns + 60 * SECOND_NS) + cache = self.cache() + cache.get("") + cache.get("") + self.assertEqual(len(self.loaded), 2) + self.assertIsNone(cache._entry) + + def test_a_missing_file_is_not_racy(self): + self.assertFalse(self.plugin_settings.exists()) + self.settings.unlink() + cache = self.cache() + cache.get("") + self.assertIsNotNone(cache._entry) + signatures = dict(cache._entry.signatures) + self.assertTrue(all(value is MISSING for value in signatures.values())) + self.assertEqual(len(signatures), 2) + + def test_entry_is_reused_below_the_maximum_age_and_reloaded_at_it(self): + cache = self.cache() + cache.get("") + stored_at = self.mono + self.mono = stored_at + MAX_AGE_SECONDS - 0.5 + cache.get("") + self.assertEqual(len(self.loaded), 1) + self.mono = stored_at + MAX_AGE_SECONDS + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_a_wall_clock_moved_backward_does_not_extend_the_age(self): + cache = self.cache() + cache.get("") + self.now_ns -= 3600 * SECOND_NS + self.mono += MAX_AGE_SECONDS + # The old entry is too old; the new load sees files "from the future" + # (racy) and is not stored, but it does load. + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_stat_failure_on_a_hit_reloads(self): + failing = {"on": False} + + def stat(path): + if failing["on"]: + raise PermissionError(13, "sharing violation", path) + return os.stat(path) + + cache = self.cache(stat=stat) + cache.get("") + failing["on"] = True + cache.get("") + self.assertEqual(len(self.loaded), 2) + self.assertIsNone(cache._entry) + + def test_stat_failure_on_a_miss_leaves_the_layout_unstored(self): + def stat(path): + raise PermissionError(13, "sharing violation", path) + + cache = self.cache(stat=stat) + self.assertEqual(cache.get(""), (self.install, self.layout)) + self.assertIsNone(cache._entry) + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_an_unreadable_settings_layout_is_not_stored(self): + self.layout = make_layout(settings_path=self.settings, error=UNREADABLE) + cache = self.cache() + self.assertEqual(cache.get("")[1].error, UNREADABLE) + cache.get("") + self.assertEqual(len(self.loaded), 2) + self.assertIsNone(cache._entry) + + def test_other_layout_errors_are_stored(self): + self.layout = make_layout( + settings_path=self.settings, error="the settings file is not valid JSON" + ) + cache = self.cache() + cache.get("") + cache.get("") + self.assertEqual(len(self.loaded), 1) + + def _symlink(self, link, target): + try: + os.symlink(target, link, target_is_directory=True) + except (OSError, NotImplementedError) as error: + self.skipTest(f"symlinks are not available here: {error}") + + def test_a_media_root_swapped_for_a_symlink_reloads(self): + elsewhere = self.tmp / "elsewhere" + elsewhere.mkdir() + cache = self.cache() + cache.get("") + shutil.rmtree(self.root_dir) + self._symlink(self.root_dir, elsewhere) + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_an_install_swapped_for_a_symlink_reloads(self): + elsewhere = self.tmp / "elsewhere-lm" + elsewhere.mkdir() + cache = self.cache() + cache.get("") + moved = self.tmp / "moved-lm" + self.install_dir.rename(moved) + self._symlink(self.install_dir, elsewhere) + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_a_root_whose_realpath_fails_reloads(self): + cache = self.cache() + cache.get("") + with patch.object( + cache_module, "_canonical", side_effect=OSError("realpath failed") + ): + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_a_stat_value_error_cannot_validate(self): + def stat(path): + raise ValueError("embedded null character in path") + + cache = self.cache(stat=stat) + cache.get("") + self.assertIsNone(cache._entry) + + def test_not_a_directory_is_missing(self): + def stat(path): + raise NotADirectoryError(20, "Not a directory", path) + + cache = self.cache(stat=stat) + cache.get("") + self.assertTrue( + all(value is MISSING for _path, value in cache._entry.signatures) + ) + cache.get("") + self.assertEqual(len(self.loaded), 1) + + def test_an_offline_windows_share_cannot_validate(self): + def stat(path): + error = FileNotFoundError(2, "The network path was not found", path) + error.winerror = 53 + raise error + + cache = self.cache(stat=stat) + cache.get("") + self.assertIsNone(cache._entry) + + def test_a_failed_reload_leaves_no_stale_entry(self): + failing = {"on": False} + + def load(install): + self.loaded.append(install) + if failing["on"]: + raise RuntimeError("boom") + return self.layout + + cache = self.cache(load=load) + cache.get("") + self.mono += MAX_AGE_SECONDS + failing["on"] = True + with self.assertRaises(RuntimeError): + cache.get("") + self.assertIsNone(cache._entry) + + def test_a_resolve_failure_propagates_and_stores_nothing(self): + def resolve(key): + raise RuntimeError("boom") + + cache = self.cache(resolve=resolve) + with self.assertRaises(RuntimeError): + cache.get("") + self.assertIsNone(cache._entry) + + def test_without_a_settings_path_only_the_plugin_file_is_watched(self): + self.layout = make_layout(media_roots=self.layout.media_roots) + cache = self.cache() + cache.get("") + self.assertEqual( + [path for path, _sig in cache._entry.signatures], + [os.path.join(self.install, "settings.json")], + ) + + def test_portable_settings_are_watched_once(self): + self.layout = make_layout(settings_path=self.install_dir / "settings.json") + cache = self.cache() + cache.get("") + self.assertEqual(len(cache._entry.signatures), 1) + + def test_a_load_failure_propagates_and_stores_nothing(self): + calls = [] + + def load(install): + calls.append(install) + if len(calls) == 1: + raise RuntimeError("boom") + return self.layout + + cache = self.cache(load=load) + with self.assertRaises(RuntimeError): + cache.get("") + self.assertIsNone(cache._entry) + self.assertEqual(cache.get(""), (self.install, self.layout)) + self.assertEqual(len(calls), 2) + + def test_without_an_install_nothing_is_watched_and_the_entry_ages_out(self): + stats = [] + + def stat(path): + stats.append(path) + return os.stat(path) + + self.layout = make_layout(media_roots=self.layout.media_roots, source=None) + cache = self.cache(resolve=lambda key: self.resolved.append(key), stat=stat) + self.assertEqual(cache.get(""), (None, self.layout)) + self.assertEqual(cache._entry.signatures, ()) + cache.get("") + self.assertEqual(len(self.loaded), 1) + self.assertEqual(stats, []) + self.mono += MAX_AGE_SECONDS + cache.get("") + self.assertEqual(len(self.loaded), 2) + + def test_windows_paths_differing_only_in_case_are_watched_once(self): + stats = [] + + def stat(path): + stats.append(path) + return SimpleNamespace(st_mtime_ns=self.now_ns - 600 * SECOND_NS, st_size=2) + + install = _canonical("C:\\ComfyUI\\custom_nodes\\LM", ntpath) + # The hit check needs ntpath's realpath to be idempotent here. + self.assertEqual(install, _canonical(install, ntpath)) + self.layout = make_layout( + settings_path=PureWindowsPath( + "C:\\ComfyUI\\custom_nodes\\lm\\SETTINGS.JSON" + ) + ) + cache = self.cache(resolve=lambda key: install, stat=stat, pathmod=ntpath) + cache.get("") + self.assertEqual(len(cache._entry.signatures), 1) + self.assertEqual(len(stats), 1) + cache.get("") + self.assertEqual(len(self.loaded), 1) + self.assertEqual(len(stats), 2) + + +class ConcurrencyTests(CacheTestCase): + def test_a_load_that_started_before_clear_stores_nothing(self): + started = threading.Event() + release = threading.Event() + + def load(install): + self.loaded.append(install) + if len(self.loaded) == 1: + started.set() + self.assertTrue(release.wait(10)) + return self.layout + + cache = self.cache(load=load) + results = [] + worker = threading.Thread(target=lambda: results.append(cache.get(""))) + worker.start() + self.assertTrue(started.wait(10)) + cache.clear() + release.set() + worker.join(10) + self.assertFalse(worker.is_alive()) + self.assertEqual(results, [(self.install, self.layout)]) + self.assertIsNone(cache._entry) + cache.get("") + self.assertEqual(len(self.loaded), 2) + self.assertIsNotNone(cache._entry) + + def test_concurrent_misses_all_get_a_layout_and_one_entry_is_stored(self): + cache = self.cache() + count = 6 + barrier = threading.Barrier(count) + results = [] + lock = threading.Lock() + + def worker(): + barrier.wait(10) + result = cache.get("") + with lock: + results.append(result) + + threads = [threading.Thread(target=worker) for _ in range(count)] + for thread in threads: + thread.start() + for thread in threads: + thread.join(10) + self.assertEqual(results, [(self.install, self.layout)] * count) + self.assertIsNotNone(cache._entry) + loads = len(self.loaded) + cache.get("") + self.assertEqual(len(self.loaded), loads) + + def test_a_valid_hit_takes_no_lock(self): + cache = self.cache() + cache.get("") + results = [] + with cache._lock: + worker = threading.Thread(target=lambda: results.append(cache.get(""))) + worker.start() + worker.join(10) + alive = worker.is_alive() + worker.join(10) + self.assertFalse(alive) + self.assertEqual(results, [(self.install, self.layout)]) + + def test_clear_forces_a_reload(self): + cache = self.cache() + cache.get("") + cache.clear() + self.assertIsNone(cache._entry) + cache.get("") + self.assertEqual(len(self.loaded), 2) + + +class LoggingTests(CacheTestCase): + LOGGER = "prompt_manager.lora_layout_cache" + + def get(self, cache): + with self.assertLogs(self.LOGGER, "DEBUG") as logs: + cache.get("") + # assertLogs needs one record; this marker is not counted. + cache_module.logger.debug("marker") + return [r for r in logs.records if r.getMessage() != "marker"] + + def test_a_new_layout_logs_one_info_line(self): + cache = self.cache() + records = self.get(cache) + self.assertEqual([r.levelname for r in records], ["INFO"]) + self.assertEqual( + records[0].getMessage(), + "LoraManager layout changed: user config settings, alongside storage, " + "1 media folders", + ) + self.mono += MAX_AGE_SECONDS + self.assertEqual(self.get(cache), []) # an equal layout, reloaded + cache.clear() + self.assertEqual(self.get(cache), []) + self.layout = replace(self.layout, media_roots=(), lora_roots=()) + self.mono += MAX_AGE_SECONDS + self.assertEqual(len(self.get(cache)), 1) + + def test_no_settings_source_prints_no(self): + self.layout = replace(self.layout, source=None) + records = self.get(self.cache()) + self.assertIn("layout changed: no settings,", records[0].getMessage()) + + def test_an_unstored_load_logs_no_info_line(self): + self.write(self.settings, "{}", mtime_ns=self.now_ns) + self.assertEqual( + [r for r in self.get(self.cache()) if r.levelname == "INFO"], [] + ) + + def test_a_newly_refused_default_folder_logs_one_warning(self): + self.layout = replace(self.layout, refused_defaults=("/home",)) + cache = self.cache() + records = self.get(cache) + warnings = [r for r in records if r.levelname == "WARNING"] + self.assertEqual(len(warnings), 1) + self.assertIn("/home", warnings[0].getMessage()) + self.mono += MAX_AGE_SECONDS + self.assertEqual(self.get(cache), []) + cache.clear() + self.assertEqual(self.get(cache), []) + self.layout = replace(self.layout, refused_defaults=("/home", "/")) + self.mono += MAX_AGE_SECONDS + warnings = [r for r in self.get(cache) if r.levelname == "WARNING"] + self.assertEqual([w.getMessage().count("'/'") for w in warnings], [1]) + + +class ModuleTests(unittest.TestCase): + def test_constants(self): + self.assertEqual(cache_module.RACY_SECONDS, 3) + self.assertEqual(cache_module.MAX_AGE_SECONDS, 30) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_lora_manager_layout.py b/tests/test_lora_manager_layout.py index 52003fa..16c94cd 100644 --- a/tests/test_lora_manager_layout.py +++ b/tests/test_lora_manager_layout.py @@ -33,6 +33,10 @@ MEDIA = frozenset( {".png", ".jpg", ".jpeg", ".webp", ".gif", ".mp4", ".webm", ".mov", ".bmp"} ) MARKER = "marker-from-unkept-settings-0123" +BAD_SIDECAR_REASON = ( + "the sidecar folder is a filesystem root, the home folder or a folder " + "that contains it" +) class LayoutTestCase(unittest.TestCase): @@ -261,11 +265,26 @@ class UnusableLayoutTests(LayoutTestCase): {"sidecar_storage_mode": "centralized", "sidecar_storage_path": "/"}, ) layout = self.load() - self.assertEqual( - layout.error, "the sidecar folder is a filesystem root or the home folder" - ) + self.assertEqual(layout.error, BAD_SIDECAR_REASON) self.assertNotIn(os.path.normcase(os.path.realpath("/")), layout.media_roots) + def test_a_centralized_sidecar_root_that_contains_home(self): + home = self.tmp / "outer" / "me" + home.mkdir(parents=True) + self.write_settings( + self.user_config, + { + "sidecar_storage_mode": "centralized", + "sidecar_storage_path": str(self.tmp / "outer"), + }, + ) + with patch.object(layout_module.Path, "home", return_value=home): + layout = self.load() + self.assertEqual(layout.error, BAD_SIDECAR_REASON) + self.assertEqual( + layout.media_roots, (os.path.normcase(str(self.default_loras)),) + ) + class SecretTests(LayoutTestCase): @@ -501,6 +520,31 @@ class CentralizedDiscoveryTests(LayoutTestCase): self.assertEqual(discovery.scanned, 3) self.assertEqual(discovery.sidecars, []) + def test_every_scanned_file_is_listed_resolved_and_sorted(self): + # #362: the fingerprint covers every scanned sidecar, kept or skipped. + ckpt = self.model(self.checkpoints, "base") + alpha = self.model(self.loras, "alpha") + self.sidecar(self.sidecars / "z", "base", {"file_path": str(ckpt)}) + self.sidecar(self.sidecars / "a", "alpha", {"file_path": str(alpha)}) + self.sidecar(self.sidecars / "m", "nopath", {"model_name": "x"}) + discovery = discover_sidecars(self.load()) + files = discovery.scanned_files + self.assertIsInstance(files, tuple) + self.assertEqual(len(files), 3) + self.assertEqual(discovery.scanned, len(files)) + self.assertEqual([str(f) for f in files], sorted(str(f) for f in files)) + self.assertEqual( + [f.name for f in files], + [ + f"alpha{METADATA_SUFFIX}", + f"nopath{METADATA_SUFFIX}", + f"base{METADATA_SUFFIX}", + ], + ) + for path in files: + self.assertEqual(str(path), os.path.realpath(path)) + self.assertEqual(len(discovery.sidecars), 1) + def test_leftover_sidecars_beside_models_are_not_scanned(self): model = self.model(self.loras, "alpha") self.sidecar(self.loras, "alpha", {"file_path": str(model)}) @@ -611,6 +655,16 @@ class LocalMediaTests(LayoutTestCase): self.touch(self.default_loras / "alpha.webp") self.assertEqual(self.media({"preview_url": preview}), [preview]) + def test_a_relative_preview_url_is_logged_and_skipped(self): + fallback = self.touch(self.default_loras / "alpha.webp") + self.touch(self.default_loras / "relative.png") + with self.assertLogs("prompt_manager.lora_manager_layout", "DEBUG") as logs: + media = self.media({"preview_url": "relative.png"}) + self.assertEqual(media, [fallback]) + self.assertEqual(len(logs.records), 1) + self.assertEqual(logs.records[0].levelname, "DEBUG") + self.assertIn(self.meta.name, logs.output[0]) + def test_preview_url_outside_media_roots_or_not_servable_is_ignored(self): outside = self.touch(self.tmp / "elsewhere" / "p.png") avif = self.touch(self.default_loras / "p.avif") @@ -810,6 +864,63 @@ class FolderGuardTests(unittest.TestCase): self.assertFalse(layout_module._inside(str(self.tmp / "other.png"), (root,))) +class RefusedDefaultsTests(LayoutTestCase): + """#362: ComfyUI's default LoRA folders pass the same guard; the refused + ones that exist are listed so the layout cache can warn about them.""" + + def setUp(self): + super().setUp() + self.outer = self.tmp / "outer" + self.home = self.outer / "me" + self.home.mkdir(parents=True) + self.fs_root = os.path.abspath(os.sep) + self.defaults = [ + str(self.home), + str(self.outer), + self.fs_root, + str(self.tmp / "missing"), + str(self.default_loras), + ] + self.expected = ( + layout_module._canonical(str(self.home)), + layout_module._canonical(str(self.outer)), + layout_module._canonical(self.fs_root), + ) + home_patch = patch.object(layout_module.Path, "home", return_value=self.home) + home_patch.start() + self.addCleanup(home_patch.stop) + + def load_defaults(self, path="__plugin__"): + return self.load(path, default_roots=lambda: list(self.defaults)) + + def test_refused_existing_defaults_are_listed_on_a_usable_layout(self): + self.write_settings(self.user_config, {}) + layout = self.load_defaults() + self.assertIsNone(layout.error) + self.assertEqual(layout.refused_defaults, self.expected) + self.assertEqual( + layout.lora_roots, (os.path.normcase(str(self.default_loras)),) + ) + + def test_they_are_listed_on_an_unusable_centralized_layout(self): + self.write_settings( + self.user_config, + {"sidecar_storage_mode": "centralized", "sidecar_storage_path": "/"}, + ) + layout = self.load_defaults() + self.assertEqual(layout.error, BAD_SIDECAR_REASON) + self.assertEqual(layout.refused_defaults, self.expected) + + def test_they_are_listed_without_an_install(self): + layout = self.load_defaults(None) + self.assertEqual(layout.error, "no LoraManager install found") + self.assertEqual(layout.refused_defaults, self.expected) + + def test_nothing_is_listed_when_every_default_is_accepted(self): + self.write_settings(self.user_config, {}) + self.assertEqual(self.load().refused_defaults, ()) + + class SummaryTests(LayoutTestCase): def test_summary_names_what_the_import_read(self): diff --git a/tests/test_lora_utils.py b/tests/test_lora_utils.py index 33f3668..d271c77 100644 --- a/tests/test_lora_utils.py +++ b/tests/test_lora_utils.py @@ -224,6 +224,14 @@ class TestReadLoraMetadata(unittest.TestCase): result = read_lora_metadata(Path("/nonexistent/file.metadata.json")) self.assertIsNone(result) + def test_returns_none_for_a_file_that_is_not_utf8(self): + # #362: one malformed sidecar is skipped; it must not fail the whole + # trigger-word load (now logged with a traceback) or the review. + with tempfile.TemporaryDirectory() as folder: + path = Path(folder) / "bad.metadata.json" + path.write_bytes(b'{"model_name": "\xff\xfe"}') + self.assertIsNone(read_lora_metadata(path)) + class TestGetLoraImageCacheDir(unittest.TestCase): """Test get_lora_image_cache_dir — returns and creates cache path.""" @@ -1791,6 +1799,34 @@ class TestTriggerWordCacheLayout(unittest.TestCase): self.assertTrue(cache.is_loaded) self.assertTrue(cache.retry_due()) + def test_an_os_error_is_a_warning_without_a_traceback(self): + cache = TriggerWordCache() + cache._cache = {"old": ["word"]} + with ( + patch.object(TriggerWordCache, "_build", side_effect=OSError("denied")), + self.assertLogs("prompt_manager.lora_utils", level="WARNING") as logs, + ): + self.assertEqual(cache.load(str(self.lm)), 1) + self.assertEqual([r.levelname for r in logs.records], ["WARNING"]) + self.assertIsNone(logs.records[0].exc_info) + self.assertEqual(cache.get_trigger_words("old"), ["word"]) + self.assertFalse(cache.retry_due()) + + def test_an_unexpected_failure_logs_its_traceback(self): + # #362: a programming error must not look like a disk error. + cache = TriggerWordCache() + cache._cache = {"old": ["word"]} + with ( + patch.object(TriggerWordCache, "_build", side_effect=ValueError("bug")), + self.assertLogs("prompt_manager.lora_utils", level="WARNING") as logs, + ): + self.assertEqual(cache.load(str(self.lm)), 1) + self.assertEqual(len(logs.records), 1) + self.assertIsNotNone(logs.records[0].exc_info) + self.assertIs(logs.records[0].exc_info[0], ValueError) + self.assertEqual(cache.get_trigger_words("old"), ["word"]) + self.assertFalse(cache.retry_due()) + def test_any_other_failure_never_raises(self): cache = TriggerWordCache() with (