From ec004933a5bea23bde7498382e223f014be0f67b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 21 May 2026 16:02:36 -0400 Subject: [PATCH] fc3d: cleanup_old_download_events daily task (terminal-only, configurable retention) Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/app/tasks/maintenance.py | 28 +++++- tests/test_cleanup_old_download_events.py | 108 ++++++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 tests/test_cleanup_old_download_events.py diff --git a/backend/app/tasks/maintenance.py b/backend/app/tasks/maintenance.py index be6c932..ced907f 100644 --- a/backend/app/tasks/maintenance.py +++ b/backend/app/tasks/maintenance.py @@ -11,7 +11,7 @@ from sqlalchemy.orm import sessionmaker from ..celery_app import celery from ..config import get_config -from ..models import ImageRecord, ImportTask +from ..models import DownloadEvent, ImageRecord, ImportSettings, ImportTask from ..utils.phash import compute_phash log = logging.getLogger(__name__) @@ -196,3 +196,29 @@ def verify_integrity() -> int: last_id = rows[-1].id log.info("verify_integrity verdicts: %s (total %d)", counts, total) return total + + +@celery.task(name="backend.app.tasks.maintenance.cleanup_old_download_events") +def cleanup_old_download_events() -> int: + """FC-3d: delete terminal DownloadEvent rows older than the configured + retention window. Never touches pending/running rows. + + Why terminal-only: pending/running rows represent in-flight work whose + owning task may still be alive; deleting them would orphan the task. + Retention days comes from ImportSettings.download_event_retention_days + so the operator can tune without a code change. + """ + SessionLocal = _sync_session_factory() + with SessionLocal() as session: + settings = session.execute( + select(ImportSettings).where(ImportSettings.id == 1) + ).scalar_one() + retention_days = settings.download_event_retention_days + cutoff = datetime.now(UTC) - timedelta(days=retention_days) + result = session.execute( + delete(DownloadEvent) + .where(DownloadEvent.status.in_(["ok", "error", "skipped"])) + .where(DownloadEvent.started_at < cutoff) + ) + session.commit() + return result.rowcount or 0 diff --git a/tests/test_cleanup_old_download_events.py b/tests/test_cleanup_old_download_events.py new file mode 100644 index 0000000..cba325d --- /dev/null +++ b/tests/test_cleanup_old_download_events.py @@ -0,0 +1,108 @@ +"""FC-3d: cleanup_old_download_events task tests. + +Asserts: (1) old terminal events are deleted, (2) recent terminal +events survive, (3) pending/running events survive regardless of age. +""" +from datetime import UTC, datetime, timedelta + +import pytest +from sqlalchemy import create_engine, select +from sqlalchemy.orm import sessionmaker + +import backend.app.tasks.maintenance # noqa: F401 — side-effect: register task +from backend.app.celery_app import celery +from backend.app.config import get_config +from backend.app.models import Artist, DownloadEvent, ImportSettings, Source + +pytestmark = pytest.mark.integration + + +def _sync_session(): + cfg = get_config() + engine = create_engine(cfg.database_url_sync, future=True, pool_pre_ping=True) + return sessionmaker(engine, expire_on_commit=False)() + + +def test_cleanup_deletes_old_terminal_events_only(): + """The maintenance task opens its own sync session and commits, so we + seed via a separate sync session and rely on the autouse + _reset_db_after_integration TRUNCATE to clean up afterward. + """ + from backend.app.tasks.maintenance import cleanup_old_download_events + + s = _sync_session() + try: + # Force retention to 30 days for the test. + settings = s.execute( + select(ImportSettings).where(ImportSettings.id == 1) + ).scalar_one() + settings.download_event_retention_days = 30 + s.commit() + + artist = Artist(name="cleanup-a", slug="cleanup-a") + s.add(artist) + s.flush() + source = Source( + artist_id=artist.id, platform="patreon", + url="https://cleanup-x", enabled=True, + ) + s.add(source) + s.flush() + + old_cutoff = datetime.now(UTC) - timedelta(days=60) + recent = datetime.now(UTC) - timedelta(days=5) + + old_ok = DownloadEvent( + source_id=source.id, status="ok", + started_at=old_cutoff, finished_at=old_cutoff, + ) + old_err = DownloadEvent( + source_id=source.id, status="error", + started_at=old_cutoff, finished_at=old_cutoff, + error="x", + ) + old_skip = DownloadEvent( + source_id=source.id, status="skipped", + started_at=old_cutoff, finished_at=old_cutoff, + ) + old_pending = DownloadEvent( + source_id=source.id, status="pending", + started_at=old_cutoff, + ) + old_running = DownloadEvent( + source_id=source.id, status="running", + started_at=old_cutoff, + ) + recent_ok = DownloadEvent( + source_id=source.id, status="ok", + started_at=recent, finished_at=recent, + ) + for ev in (old_ok, old_err, old_skip, old_pending, old_running, recent_ok): + s.add(ev) + s.commit() + ids = { + "old_ok": old_ok.id, + "old_err": old_err.id, + "old_skip": old_skip.id, + "old_pending": old_pending.id, + "old_running": old_running.id, + "recent_ok": recent_ok.id, + } + finally: + s.close() + + deleted = cleanup_old_download_events() + assert deleted == 3 + + s2 = _sync_session() + try: + survivors = set(s2.execute( + select(DownloadEvent.id).where(DownloadEvent.id.in_(list(ids.values()))) + ).scalars().all()) + finally: + s2.close() + assert survivors == {ids["old_pending"], ids["old_running"], ids["recent_ok"]} + + +def test_cleanup_task_is_registered(): + assert "backend.app.tasks.maintenance.cleanup_old_download_events" in celery.tasks