From 4c7c51fcb24f23496d27ced1e58014a5c4c0037d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 28 Jul 2026 09:27:45 -0700 Subject: [PATCH] =?UTF-8?q?refactor(gateway):=20one=20media-cache=20cleanu?= =?UTF-8?q?p=20loop=20=E2=80=94=20extend=20pruning=20to=20video=20+=20scre?= =?UTF-8?q?enshot=20caches?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to salvaged PR #56473: dedupe the five cleanup_*_cache bodies into a shared _cleanup_cache_dir() helper, add cleanup_video_cache() and cleanup_screenshot_cache() (with get_screenshot_cache_dir()), and drive all five from a single (name, fn) loop in _start_gateway_housekeeping() instead of one copy-pasted try/except per cache. Covers the video/ screenshot half of #56427. --- gateway/platforms/base.py | 71 ++++++++++++++++++------------- gateway/run.py | 43 ++++++++++--------- tests/gateway/test_audio_cache.py | 63 +++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 51 deletions(-) diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 04d3321bf4b..83820b3a6a0 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -842,15 +842,15 @@ async def cache_image_from_url(url: str, ext: str = ".jpg", retries: int = 2) -> raise -def cleanup_image_cache(max_age_hours: int = 24) -> int: +def _cleanup_cache_dir(cache_dir: Path, max_age_hours: int) -> int: """ - Delete cached images older than *max_age_hours*. + Delete files in *cache_dir* older than *max_age_hours*. - Returns the number of files removed. + Shared implementation behind every ``cleanup_*_cache`` helper — one loop, + not N copies. Returns the number of files removed. """ import time - cache_dir = get_image_cache_dir() cutoff = time.time() - (max_age_hours * 3600) removed = 0 for f in cache_dir.iterdir(): @@ -863,6 +863,15 @@ def cleanup_image_cache(max_age_hours: int = 24) -> int: return removed +def cleanup_image_cache(max_age_hours: int = 24) -> int: + """ + Delete cached images older than *max_age_hours*. + + Returns the number of files removed. + """ + return _cleanup_cache_dir(get_image_cache_dir(), max_age_hours) + + # --------------------------------------------------------------------------- # Audio cache utilities # @@ -981,19 +990,7 @@ def cleanup_audio_cache(max_age_hours: int = 24) -> int: Returns the number of files removed. """ - import time - - cache_dir = get_audio_cache_dir() - cutoff = time.time() - (max_age_hours * 3600) - removed = 0 - for f in cache_dir.iterdir(): - if f.is_file() and f.stat().st_mtime < cutoff: - try: - f.unlink() - removed += 1 - except OSError: - pass - return removed + return _cleanup_cache_dir(get_audio_cache_dir(), max_age_hours) # --------------------------------------------------------------------------- @@ -1031,6 +1028,15 @@ def cache_video_from_bytes(data: bytes, ext: str = ".mp4") -> str: return str(filepath) +def cleanup_video_cache(max_age_hours: int = 24) -> int: + """ + Delete cached videos older than *max_age_hours*. + + Returns the number of files removed. + """ + return _cleanup_cache_dir(get_video_cache_dir(), max_age_hours) + + # --------------------------------------------------------------------------- # Document cache utilities # @@ -1041,6 +1047,23 @@ def cache_video_from_bytes(data: bytes, ext: str = ".mp4") -> str: DOCUMENT_CACHE_DIR = get_hermes_dir("cache/documents", "document_cache") SCREENSHOT_CACHE_DIR = get_hermes_dir("cache/screenshots", "browser_screenshots") + +def get_screenshot_cache_dir() -> Path: + """Return the browser screenshot cache directory, creating it if needed.""" + d = _resolve_cache_dir("SCREENSHOT_CACHE_DIR", "cache/screenshots", "browser_screenshots") + d.mkdir(parents=True, exist_ok=True) + return d + + +def cleanup_screenshot_cache(max_age_hours: int = 24) -> int: + """ + Delete cached browser screenshots older than *max_age_hours*. + + Returns the number of files removed. + """ + return _cleanup_cache_dir(get_screenshot_cache_dir(), max_age_hours) + + # Import-time defaults; _resolve_cache_dir compares against these to tell a # test monkeypatch from an unmodified constant. _CACHE_DIR_IMPORT_DEFAULTS = { @@ -1820,19 +1843,7 @@ def cleanup_document_cache(max_age_hours: int = 24) -> int: Returns the number of files removed. """ - import time - - cache_dir = get_document_cache_dir() - cutoff = time.time() - (max_age_hours * 3600) - removed = 0 - for f in cache_dir.iterdir(): - if f.is_file() and f.stat().st_mtime < cutoff: - try: - f.unlink() - removed += 1 - except OSError: - pass - return removed + return _cleanup_cache_dir(get_document_cache_dir(), max_age_hours) # --------------------------------------------------------------------------- diff --git a/gateway/run.py b/gateway/run.py index d53b2f07382..9ac80d72305 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -24087,14 +24087,16 @@ def _start_gateway_housekeeping(stop_event: threading.Event, adapters=None, loop this housekeeping still wants its hourly cadence — so it owns its own loop. Refreshes the channel directory every 5 minutes and prunes the - image/audio/document cache + expired ``hermes debug share`` pastes once per - hour, and polls the curator hourly (its inner gate enforces the real - weekly cadence). + image/audio/video/document/screenshot caches + expired ``hermes debug + share`` pastes once per hour, and polls the curator hourly (its inner + gate enforces the real weekly cadence). """ from gateway.platforms.base import ( cleanup_audio_cache, cleanup_document_cache, cleanup_image_cache, + cleanup_screenshot_cache, + cleanup_video_cache, ) from hermes_cli.debug import _sweep_expired_pastes @@ -24104,6 +24106,16 @@ def _start_gateway_housekeeping(stop_event: threading.Event, adapters=None, loop CURATOR_EVERY = 60 # ticks — poll hourly (inner gate handles the real cadence) AUTO_ARCHIVE_EVERY = 60 # ticks — poll hourly (state_meta gate owns the real cadence) + # Every platform media cache prunes on the same hourly cadence — one loop + # over (name, cleanup_fn), not a copy-pasted try/except per cache. + MEDIA_CACHE_CLEANUPS = ( + ("Image", cleanup_image_cache), + ("Document", cleanup_document_cache), + ("Audio", cleanup_audio_cache), + ("Video", cleanup_video_cache), + ("Screenshot", cleanup_screenshot_cache), + ) + logger.info("Gateway housekeeping started (interval=%ds)", interval) tick_count = 0 while not stop_event.is_set(): @@ -24128,24 +24140,13 @@ def _start_gateway_housekeeping(stop_event: threading.Event, adapters=None, loop logger.debug("Channel directory refresh error: %s", e) if tick_count % IMAGE_CACHE_EVERY == 0: - try: - removed = cleanup_image_cache(max_age_hours=24) - if removed: - logger.info("Image cache cleanup: removed %d stale file(s)", removed) - except Exception as e: - logger.debug("Image cache cleanup error: %s", e) - try: - removed = cleanup_document_cache(max_age_hours=24) - if removed: - logger.info("Document cache cleanup: removed %d stale file(s)", removed) - except Exception as e: - logger.debug("Document cache cleanup error: %s", e) - try: - removed = cleanup_audio_cache(max_age_hours=24) - if removed: - logger.info("Audio cache cleanup: removed %d stale file(s)", removed) - except Exception as e: - logger.debug("Audio cache cleanup error: %s", e) + for cache_name, cleanup_fn in MEDIA_CACHE_CLEANUPS: + try: + removed = cleanup_fn(max_age_hours=24) + if removed: + logger.info("%s cache cleanup: removed %d stale file(s)", cache_name, removed) + except Exception as e: + logger.debug("%s cache cleanup error: %s", cache_name, e) if tick_count % PASTE_SWEEP_EVERY == 0: try: diff --git a/tests/gateway/test_audio_cache.py b/tests/gateway/test_audio_cache.py index 59bf505fb36..736a4398f10 100644 --- a/tests/gateway/test_audio_cache.py +++ b/tests/gateway/test_audio_cache.py @@ -127,3 +127,66 @@ class TestCleanupAudioCache: def test_empty_cache_returns_zero(self): removed = cleanup_audio_cache(max_age_hours=24) assert removed == 0 + +# --------------------------------------------------------------------------- +# TestUnifiedMediaCacheCleanup — video + screenshot ride the same shared loop +# --------------------------------------------------------------------------- + +class TestUnifiedMediaCacheCleanup: + def test_cleanup_video_cache_removes_old_files(self, tmp_path, monkeypatch): + from gateway.platforms.base import cleanup_video_cache, get_video_cache_dir + + monkeypatch.setattr( + "gateway.platforms.base.VIDEO_CACHE_DIR", tmp_path / "video_cache" + ) + cache_dir = get_video_cache_dir() + old_file = cache_dir / "old.mp4" + old_file.write_text("old") + old_mtime = time.time() - 48 * 3600 + os.utime(old_file, (old_mtime, old_mtime)) + fresh = cache_dir / "fresh.mp4" + fresh.write_text("fresh") + + removed = cleanup_video_cache(max_age_hours=24) + assert removed == 1 + assert not old_file.exists() + assert fresh.exists() + + def test_cleanup_screenshot_cache_removes_old_files(self, tmp_path, monkeypatch): + from gateway.platforms.base import ( + cleanup_screenshot_cache, + get_screenshot_cache_dir, + ) + + monkeypatch.setattr( + "gateway.platforms.base.SCREENSHOT_CACHE_DIR", tmp_path / "screenshots" + ) + cache_dir = get_screenshot_cache_dir() + old_file = cache_dir / "old.png" + old_file.write_text("old") + old_mtime = time.time() - 48 * 3600 + os.utime(old_file, (old_mtime, old_mtime)) + fresh = cache_dir / "fresh.png" + fresh.write_text("fresh") + + removed = cleanup_screenshot_cache(max_age_hours=24) + assert removed == 1 + assert not old_file.exists() + assert fresh.exists() + + def test_housekeeping_loop_covers_all_media_caches(self): + """The housekeeping tick prunes every media cache via one shared loop.""" + import inspect + + from gateway import run as gateway_run + + src = inspect.getsource(gateway_run._start_gateway_housekeeping) + assert "MEDIA_CACHE_CLEANUPS" in src + for fn_name in ( + "cleanup_image_cache", + "cleanup_document_cache", + "cleanup_audio_cache", + "cleanup_video_cache", + "cleanup_screenshot_cache", + ): + assert fn_name in src, f"{fn_name} missing from housekeeping loop"