diff --git a/services/search/cache.py b/services/search/cache.py index 222682c7b..a3134abf0 100644 --- a/services/search/cache.py +++ b/services/search/cache.py @@ -40,24 +40,37 @@ def generate_cache_key(data: str) -> str: def cleanup_cache(cache_dir: Path, cache_index: Dict[str, datetime], max_age: timedelta): """Remove expired cache entries and enforce LRU policy.""" current_time = datetime.now() - files_in_dir = {f.name.split(".")[0]: f for f in cache_dir.glob("*.cache")} + files_in_dir = {f.stem: f for f in cache_dir.glob("*.cache")} - to_remove = [] - for key, timestamp in list(cache_index.items()): - if current_time - timestamp > max_age or key not in files_in_dir: - to_remove.append(key) - if key in files_in_dir: - files_in_dir[key].unlink(missing_ok=True) + live_entries = [] + for key, cache_file in list(files_in_dir.items()): + timestamp = cache_index.get(key) + if timestamp is None: + try: + timestamp = datetime.fromtimestamp(cache_file.stat().st_mtime) + except OSError: + continue + if current_time - timestamp > max_age: + try: + cache_file.unlink(missing_ok=True) + cache_metrics["evictions"] += 1 + cache_index.pop(key, None) + except OSError as e: + logger.debug("Failed to remove expired cache file %s: %s", cache_file, e) + else: + live_entries.append((key, timestamp, cache_file)) - for key in to_remove: - cache_index.pop(key, None) - cache_metrics["evictions"] += 1 - - if len(cache_index) > CACHE_MAX_ENTRIES: - sorted_items = sorted(cache_index.items(), key=lambda x: x[1]) - excess_count = len(cache_index) - CACHE_MAX_ENTRIES - for key, _ in sorted_items[:excess_count]: + for key in list(cache_index): + if key not in files_in_dir: cache_index.pop(key, None) - cache_file = cache_dir / f"{key}.cache" - cache_file.unlink(missing_ok=True) cache_metrics["evictions"] += 1 + + if len(live_entries) > CACHE_MAX_ENTRIES: + excess_count = len(live_entries) - CACHE_MAX_ENTRIES + for key, _, cache_file in sorted(live_entries, key=lambda x: x[1])[:excess_count]: + try: + cache_file.unlink(missing_ok=True) + cache_metrics["evictions"] += 1 + cache_index.pop(key, None) + except OSError as e: + logger.debug("Failed to remove excess cache file %s: %s", cache_file, e) diff --git a/tests/test_search_cache_cleanup.py b/tests/test_search_cache_cleanup.py new file mode 100644 index 000000000..286eb8295 --- /dev/null +++ b/tests/test_search_cache_cleanup.py @@ -0,0 +1,69 @@ +import os +import time +from datetime import datetime, timedelta +from pathlib import Path + +from services.search import cache as cache_module + + +def _write_cache_file(cache_dir, key, age_seconds): + path = cache_dir / f"{key}.cache" + path.write_text("{}", encoding="utf-8") + mtime = time.time() - age_seconds + os.utime(path, (mtime, mtime)) + return path + + +def test_cleanup_cache_removes_expired_disk_files_missing_from_index(tmp_path): + expired = _write_cache_file(tmp_path, "expired", age_seconds=7200) + + cache_module.cleanup_cache(tmp_path, {}, timedelta(hours=1)) + + assert not expired.exists() + + +def test_cleanup_cache_keeps_fresh_disk_files_missing_from_index(tmp_path): + fresh = _write_cache_file(tmp_path, "fresh", age_seconds=60) + + cache_module.cleanup_cache(tmp_path, {}, timedelta(hours=1)) + + assert fresh.exists() + + +def test_cleanup_cache_enforces_max_entries_against_disk_files(tmp_path, monkeypatch): + monkeypatch.setattr(cache_module, "CACHE_MAX_ENTRIES", 2) + oldest = _write_cache_file(tmp_path, "oldest", age_seconds=30) + newer = _write_cache_file(tmp_path, "newer", age_seconds=20) + newest = _write_cache_file(tmp_path, "newest", age_seconds=10) + + cache_module.cleanup_cache(tmp_path, {}, timedelta(hours=1)) + + assert not oldest.exists() + assert newer.exists() + assert newest.exists() + + +def test_cleanup_cache_removes_index_entries_for_missing_files(tmp_path): + cache_index = {"missing": datetime.now()} + + cache_module.cleanup_cache(tmp_path, cache_index, timedelta(hours=1)) + + assert cache_index == {} + + +def test_cleanup_cache_keeps_index_when_delete_fails(tmp_path, monkeypatch): + expired = _write_cache_file(tmp_path, "expired", age_seconds=7200) + cache_index = {"expired": datetime.fromtimestamp(expired.stat().st_mtime)} + original_unlink = Path.unlink + + def fail_expired_unlink(self, missing_ok=False): + if self == expired: + raise OSError("delete failed") + return original_unlink(self, missing_ok=missing_ok) + + monkeypatch.setattr(Path, "unlink", fail_expired_unlink) + + cache_module.cleanup_cache(tmp_path, cache_index, timedelta(hours=1)) + + assert expired.exists() + assert "expired" in cache_index