diff --git a/py/services/persistent_recipe_cache.py b/py/services/persistent_recipe_cache.py index 0da9d2c6..f0a03b98 100644 --- a/py/services/persistent_recipe_cache.py +++ b/py/services/persistent_recipe_cache.py @@ -170,20 +170,30 @@ class PersistentRecipeCache: recipes: List[Dict[str, Any]], json_paths: Optional[Dict[str, str]] = None, image_id_map: Optional[Dict[str, str]] = None, - ) -> None: + skip_if_empty: bool = False, + ) -> bool: """Save all recipes to SQLite cache. Args: recipes: List of recipe dictionaries to persist. json_paths: Optional mapping of recipe_id -> json_path for file stats. image_id_map: Optional precomputed civitai image_id → recipe_id mapping. + skip_if_empty: When True, refuse to replace a non-empty cache with an + empty one. This is the storage-level backstop against a scan that + silently loses every recipe (unavailable drive / mis-resolved + recipes directory): overwriting both deletes the user's data and + destroys their only record of it. Intentional full clears (manual + rebuild) must pass ``skip_if_empty=False``. + + Returns: + ``True`` when the write happened, ``False`` when it was skipped. """ if not self.is_enabled(): - return + return False if not self._schema_initialized: self._initialize_schema() if not self._schema_initialized: - return + return False try: with self._db_lock: @@ -192,6 +202,23 @@ class PersistentRecipeCache: conn.execute("PRAGMA foreign_keys = ON") conn.execute("BEGIN") + if skip_if_empty and not recipes: + existing = conn.execute( + "SELECT COUNT(*) FROM recipes" + ).fetchone() + if existing and existing[0]: + conn.rollback() + logger.warning( + "Refusing to persist an empty recipe cache: the " + "stored cache still holds %d recipe(s). The scan " + "found nothing, which usually means the recipes " + "path was unavailable or resolved elsewhere; " + "keeping the stored cache so the data stays " + "recoverable.", + existing[0], + ) + return False + # Clear existing data conn.execute("DELETE FROM recipes") @@ -225,10 +252,12 @@ class PersistentRecipeCache: conn.commit() logger.debug("Persisted %d recipes to cache", len(recipe_rows)) + return True finally: conn.close() except Exception as exc: logger.warning("Failed to persist recipe cache: %s", exc) + return False def get_file_stats(self) -> Dict[str, Tuple[float, int]]: """Return stored file stats for all cached recipes. diff --git a/py/services/recipe_scanner.py b/py/services/recipe_scanner.py index 4c93863f..bdb3e036 100644 --- a/py/services/recipe_scanner.py +++ b/py/services/recipe_scanner.py @@ -116,6 +116,12 @@ class RecipeScanner: self._persistent_cache: Optional[PersistentRecipeCache] = None self._civitai_client: Any = None # Lazily initialized from registry self._json_path_map: Dict[str, str] = {} # recipe_id -> json_path + # True when the last scan refused to prune the stored cache because + # every recorded recipe file was missing (see + # :meth:`_initialize_recipe_cache_sync`). Keeps dependent background + # work (FTS index) aligned with the stored rows instead of the + # intentionally out-of-sync in-memory view. + self._prune_skipped: bool = False if lora_scanner: self._lora_scanner = lora_scanner if checkpoint_scanner: @@ -1651,8 +1657,12 @@ class RecipeScanner: 'pageType': 'recipes', }) self._schedule_post_scan_enrichment() - # Schedule FTS index build in background (non-blocking) - self._schedule_fts_index_build() + # Schedule FTS index build in background (non-blocking). When the + # prune was skipped the in-memory cache is intentionally out of sync + # with the stored rows, so leave the existing index alone instead of + # rebuilding it from the empty view. + if not self._prune_skipped: + self._schedule_fts_index_build() except Exception as e: logger.error(f"Recipe Scanner: Error initializing cache in background: {e}") # Ensure the cache is never None so the page stops showing the @@ -1723,6 +1733,7 @@ class RecipeScanner: """ loop = None scan_start_time: Optional[float] = None + self._prune_skipped = False try: # Ensure cache exists to avoid None reference errors if self._cache is None: @@ -1749,14 +1760,38 @@ class RecipeScanner: logger.warning(f"Recipes directory not found: {recipes_dir}") return self._cache + # Record which directory the scan actually used. When the Recipes + # Storage Path is empty this falls back to the first LoRA root, and + # a support reader needs that path to tell a real wipe apart from a + # scan that looked somewhere else (see the prune guard below). + logger.info(f"Recipe scan directory: {recipes_dir}") + # Try to load from persistent cache first persisted = self._persistent_cache.load_cache() if persisted: - recipes, changed, json_paths = self._reconcile_recipe_cache( - persisted, recipes_dir - ) + ( + recipes, + changed, + json_paths, + skipped_prune_reason, + ) = self._reconcile_recipe_cache(persisted, recipes_dir) self._json_path_map = json_paths + if skipped_prune_reason: + # Every persisted recipe file vanished at once. That is not a + # reliable deletion signal: a drive that did not mount, a + # recipes_path that silently fell back to another root, or a + # shared cache touched by a second instance all look exactly + # like this. Keep the stored cache and skip the prune, so the + # only copy of the user's recipes is not destroyed. + logger.warning( + f"Recipe cache prune skipped: {skipped_prune_reason}. " + f"Keeping {len(persisted.raw_data)} stored recipe(s); this " + "session reports no recipes until the files are found again." + ) + self._prune_skipped = True + return self._cache + if not changed: # Fast path: use cached data directly logger.info( @@ -1770,7 +1805,10 @@ class RecipeScanner: if self._backfill_source_path_if_needed(recipes, json_paths): self._cache.image_id_map = self._build_image_id_map() self._persistent_cache.save_cache( - recipes, json_paths, self._cache.image_id_map + recipes, + json_paths, + self._cache.image_id_map, + skip_if_empty=True, ) else: # Use persisted map, or rebuild if empty (e.g. first startup @@ -1798,7 +1836,10 @@ class RecipeScanner: self._cache.image_id_map = self._build_image_id_map() # Persist updated cache self._persistent_cache.save_cache( - recipes, json_paths, self._cache.image_id_map + recipes, + json_paths, + self._cache.image_id_map, + skip_if_empty=True, ) return self._cache @@ -1825,7 +1866,10 @@ class RecipeScanner: # Persist for next startup self._persistent_cache.save_cache( - recipes, json_paths, self._cache.image_id_map + recipes, + json_paths, + self._cache.image_id_map, + skip_if_empty=True, ) if report_progress: @@ -1862,7 +1906,7 @@ class RecipeScanner: self, persisted: PersistedRecipeData, recipes_dir: str, - ) -> Tuple[List[Dict[str, Any]], bool, Dict[str, str]]: + ) -> Tuple[List[Dict[str, Any]], bool, Dict[str, str], Optional[str]]: """Reconcile persisted cache with current filesystem state. Args: @@ -1870,7 +1914,11 @@ class RecipeScanner: recipes_dir: Path to the recipes directory. Returns: - Tuple of (recipes list, changed flag, json_paths dict). + Tuple of (recipes list, changed flag, json_paths dict, + skipped_prune_reason). The last element is ``None`` on a normal + reconcile. When it is a string, the scan saw every persisted recipe + file disappear at once; the caller must then keep the persisted + cache instead of overwriting it. The reason text is user-facing. """ recipes: List[Dict[str, Any]] = [] json_paths: Dict[str, str] = {} @@ -1951,12 +1999,67 @@ class RecipeScanner: time.sleep(0) # Check for deleted files - for json_path in persisted.file_stats.keys(): - if json_path not in current_files: - changed = True - logger.debug("Recipe file deleted: %s", json_path) + orphaned_stats = [ + json_path + for json_path in persisted.file_stats.keys() + if json_path not in current_files + ] + if orphaned_stats: + changed = True + # This single line plus the resolved scan directory logged by the + # caller are the evidence a support reader gets for a recipes path + # that moved; the per-file lines stay at debug to avoid flooding. + if len(orphaned_stats) > 10: + logger.info( + f"Recipe reconcile: {len(orphaned_stats)} of " + f"{len(persisted.file_stats)} cached recipe file(s) are not in " + f"{recipes_dir} (first: {orphaned_stats[0]}, " + f"last: {orphaned_stats[-1]})" + ) + else: + for json_path in orphaned_stats: + logger.debug("Recipe file deleted: %s", json_path) - return recipes, changed, json_paths + skipped_prune_reason: Optional[str] = None + if not current_files and persisted.file_stats: + metadata_is_coherent = self._persisted_metadata_is_coherent(persisted) + if metadata_is_coherent: + skipped_prune_reason = ( + f"every recipe file recorded in the cache " + f"({len(persisted.file_stats)}) is missing from {recipes_dir}" + ) + else: + # The stored row set and its recorded file stats disagree, so + # this cache is stale rather than a faithful record of recipes + # that have just gone missing. Pruning it is safe. + logger.info( + f"Recipe reconcile: stored cache is inconsistent " + f"({len(persisted.raw_data)} row(s) vs " + f"{len(persisted.file_stats)} file record(s)); falling back " + "to a normal prune." + ) + + return recipes, changed, json_paths, skipped_prune_reason + + @staticmethod + def _persisted_metadata_is_coherent(persisted: PersistedRecipeData) -> bool: + """Return True when the stored rows and their file stats describe one set. + + The prune guard treats "no recipe files found" as a signal that the + directory moved out from under us, which is only meaningful when the + stored cache is a faithful record of recipes that exist on disk. A cache + whose row set and file-stat set have diverged (left behind by an older + reconcile) carries recipes that were already orphaned, so it is not + evidence of a fresh disappearance. + """ + stats_ids = { + os.path.basename(json_path)[: -len(".recipe.json")] + for json_path in persisted.file_stats + if os.path.basename(json_path).lower().endswith(".recipe.json") + } + rows_ids = {str(recipe.get("id", "")) for recipe in persisted.raw_data} + rows_ids.discard("") + return bool(rows_ids) and rows_ids == stats_ids # Metadata key recording that the one-shot source_path backfill has run. _SOURCE_PATH_BACKFILL_MARKER = "source_path_backfilled" @@ -2656,7 +2759,8 @@ class RecipeScanner: # Schedule non-blocking background work self._schedule_post_scan_enrichment() - self._schedule_fts_index_build() + if not self._prune_skipped: + self._schedule_fts_index_build() return cast(RecipeCache, self._cache) diff --git a/tests/services/test_recipe_cache_prune_guard.py b/tests/services/test_recipe_cache_prune_guard.py new file mode 100644 index 00000000..12f44dde --- /dev/null +++ b/tests/services/test_recipe_cache_prune_guard.py @@ -0,0 +1,274 @@ +"""Regression tests for the recipe empty-prune guard (issue #1116). + +A scan that finds no recipe files at all is not a trustworthy deletion signal: +an unmounted drive, a ``recipes_path`` that silently fell back to another LoRA +root, or a cache shared with a second instance all look identical to a real +wipe. Before this guard, such a scan overwrote the persistent cache with an +empty one, destroying the user's only record of their recipes. + +Covered contracts: + +1. ``_reconcile_recipe_cache`` reports the "every persisted file vanished" + condition and does not treat an empty directory as a trustworthy prune. +2. ``_initialize_recipe_cache_sync`` keeps the stored cache in that case + instead of persisting the empty result. +3. A partial orphan (some files still present) still prunes normally, so + ordinary manual deletions keep working. +4. ``PersistentRecipeCache.save_cache(skip_if_empty=True)`` is the + storage-level backstop and a manual rebuild can still clear the cache. +""" + +from __future__ import annotations + +import json +import logging +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from py.config import config +from py.services import recipe_scanner as recipe_scanner_module +from py.services import settings_manager as settings_manager_module +from py.services.persistent_recipe_cache import ( + PersistedRecipeData, + PersistentRecipeCache, +) +from py.services.recipe_cache import RecipeCache +from py.services.recipe_scanner import RecipeScanner + + +def _write_recipe_json(path: Path, recipe_id: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps( + { + "id": recipe_id, + "file_path": str(path.with_suffix(".png")), + "title": f"Recipe {recipe_id}", + "modified": 0.0, + "created_date": 0.0, + "loras": [], + } + ), + encoding="utf-8", + ) + + +def _persisted_for(paths: list[Path]) -> PersistedRecipeData: + """Build persisted cache state describing *paths* as known recipe files.""" + raw_data = [] + file_stats = {} + for path in paths: + recipe_id = path.name[: -len(".recipe.json")] + raw_data.append({"id": recipe_id, "title": f"Recipe {recipe_id}"}) + stat = path.stat() + file_stats[str(path)] = (stat.st_mtime, stat.st_size) + return PersistedRecipeData( + raw_data=raw_data, file_stats=file_stats, image_id_map={} + ) + + +@pytest.fixture +def guard_scanner(tmp_path: Path, monkeypatch): + """RecipeScanner wired to a real persistent cache, without a ComfyUI app.""" + RecipeScanner._instance = None + settings_manager_module.reset_settings_manager() + monkeypatch.setattr(config, "loras_roots", [str(tmp_path / "loras-root")]) + + scanner = RecipeScanner.__new__(RecipeScanner) + scanner._persistent_cache = PersistentRecipeCache( + db_path=str(tmp_path / "recipe_cache.sqlite") + ) + scanner._cache = None + scanner._json_path_map = {} + scanner._lora_scanner = SimpleNamespace() + + yield scanner, scanner._persistent_cache + + RecipeScanner._instance = None + settings_manager_module.reset_settings_manager() + + +def test_reconcile_flags_prune_when_every_persisted_file_is_gone( + guard_scanner, tmp_path: Path +): + """An empty recipes dir must not be reported as a trustworthy prune.""" + scanner, _cache = guard_scanner + recipes_dir = tmp_path / "recipes" + recipes_dir.mkdir() + + # The files used to live at another root (a changed recipes_path) and are + # all gone from the directory the scanner resolved this time. + old_files = [tmp_path / "elsewhere" / f"r{idx}.recipe.json" for idx in range(3)] + for path in old_files: + _write_recipe_json(path, path.name[: -len(".recipe.json")]) + persisted = _persisted_for(old_files) + for path in old_files: + path.unlink() + + recipes, changed, json_paths, skipped_prune_reason = ( + scanner._reconcile_recipe_cache(persisted, str(recipes_dir)) + ) + + assert recipes == [] + assert json_paths == {} + assert changed is True + assert skipped_prune_reason is not None + assert str(recipes_dir) in skipped_prune_reason + assert "3" in skipped_prune_reason + + +def test_reconcile_prunes_normally_when_only_some_files_disappear( + guard_scanner, tmp_path: Path +): + """A partial orphan is an ordinary deletion and keeps its old behaviour.""" + scanner, _cache = guard_scanner + recipes_dir = tmp_path / "recipes" + + survivor = recipes_dir / "survivor.recipe.json" + _write_recipe_json(survivor, "survivor") + vanished = recipes_dir / "vanished.recipe.json" + _write_recipe_json(vanished, "vanished") + persisted = _persisted_for([survivor, vanished]) + vanished.unlink() + + recipes, changed, _json_paths, skipped_prune_reason = ( + scanner._reconcile_recipe_cache(persisted, str(recipes_dir)) + ) + + assert skipped_prune_reason is None + assert changed is True + assert [recipe["id"] for recipe in recipes] == ["survivor"] + + +def test_reconcile_ignores_empty_persisted_cache(guard_scanner, tmp_path: Path): + """A genuinely empty cache has nothing to lose and must not be guarded.""" + scanner, _cache = guard_scanner + recipes_dir = tmp_path / "recipes" + recipes_dir.mkdir() + + persisted = PersistedRecipeData(raw_data=[], file_stats={}, image_id_map={}) + + _recipes, changed, _json_paths, skipped_prune_reason = ( + scanner._reconcile_recipe_cache(persisted, str(recipes_dir)) + ) + + assert changed is False + assert skipped_prune_reason is None + + +def test_reconcile_prunes_when_stored_metadata_is_inconsistent( + guard_scanner, tmp_path: Path +): + """A stale row set must not masquerade as a fresh mass disappearance. + + Leftover rows (rows without a recorded file stat) mean the stored cache is + already out of date; guarding them would preserve orphans forever. + """ + scanner, _cache = guard_scanner + recipes_dir = tmp_path / "recipes" + recipes_dir.mkdir() + + gone = tmp_path / "old-location" / "kept.recipe.json" + _write_recipe_json(gone, "kept") + persisted = _persisted_for([gone]) + gone.unlink() + # A row with no matching file record: the cache diverged at some point. + persisted.raw_data.append({"id": "orphan-row", "title": "Orphan"}) + + recipes, changed, _json_paths, skipped_prune_reason = ( + scanner._reconcile_recipe_cache(persisted, str(recipes_dir)) + ) + + assert recipes == [] + assert changed is True + assert skipped_prune_reason is None + + +def test_sync_init_keeps_stored_cache_when_scan_finds_nothing( + guard_scanner, tmp_path: Path, caplog: pytest.LogCaptureFixture +): + """The startup path must not overwrite the stored cache with an empty one.""" + scanner, cache = guard_scanner + recipes_dir = Path(config.loras_roots[0]) / "recipes" + gone = tmp_path / "old-location" / "kept.recipe.json" + _write_recipe_json(gone, "kept") + + assert cache.save_cache( + [{"id": "kept", "title": "Recipe kept"}], {"kept": str(gone)} + ) + gone.unlink() + + with caplog.at_level(logging.WARNING, logger=recipe_scanner_module.__name__): + scanner._initialize_recipe_cache_sync() + + assert "Recipe cache prune skipped" in caplog.text + assert scanner._prune_skipped is True + # The stored cache survived, so the recipes remain recoverable. + persisted = cache.load_cache() + assert persisted is not None + assert [recipe["id"] for recipe in persisted.raw_data] == ["kept"] + + +def test_skipped_prune_leaves_fts_index_untouched(guard_scanner, tmp_path: Path): + """A skipped prune must not rebuild the FTS index from the empty view.""" + scanner, cache = guard_scanner + gone = tmp_path / "old-location" / "kept.recipe.json" + _write_recipe_json(gone, "kept") + assert cache.save_cache( + [{"id": "kept", "title": "Recipe kept"}], {"kept": str(gone)} + ) + gone.unlink() + + schedule_calls = [] + scanner._schedule_fts_index_build = lambda: schedule_calls.append(True) + + scanner._initialize_recipe_cache_sync() + + assert scanner._prune_skipped is True + assert schedule_calls == [] + + +def test_sync_init_persists_when_recipes_are_found(guard_scanner, tmp_path: Path): + """The guard must not block a normal successful scan.""" + scanner, cache = guard_scanner + recipes_dir = Path(config.loras_roots[0]) / "recipes" + _write_recipe_json(recipes_dir / "fresh.recipe.json", "fresh") + + scanner._initialize_recipe_cache_sync() + + persisted = cache.load_cache() + assert persisted is not None + assert [recipe["id"] for recipe in persisted.raw_data] == ["fresh"] + + +def test_save_cache_skip_if_empty_preserves_existing_rows(tmp_path: Path): + """The storage-level backstop refuses to empty a populated cache.""" + cache = PersistentRecipeCache(db_path=str(tmp_path / "recipe_cache.sqlite")) + assert cache.save_cache([{"id": "r1", "title": "One"}], {"r1": "/tmp/r1.json"}) + + written = cache.save_cache([], {}, skip_if_empty=True) + + assert written is False + persisted = cache.load_cache() + assert persisted is not None + assert [recipe["id"] for recipe in persisted.raw_data] == ["r1"] + + +def test_save_cache_skip_if_empty_allows_clearing_an_empty_cache(tmp_path: Path): + """Nothing to protect: an already-empty cache still returns success.""" + cache = PersistentRecipeCache(db_path=str(tmp_path / "recipe_cache.sqlite")) + + assert cache.save_cache([], {}, skip_if_empty=True) is True + + +def test_save_cache_default_still_allows_intentional_full_clear(tmp_path: Path): + """A manual rebuild passes skip_if_empty=False and must clear the cache.""" + cache = PersistentRecipeCache(db_path=str(tmp_path / "recipe_cache.sqlite")) + assert cache.save_cache([{"id": "r1", "title": "One"}], {"r1": "/tmp/r1.json"}) + + assert cache.save_cache([], {}) is True + + persisted = cache.load_cache() + assert persisted is None or persisted.raw_data == []