diff --git a/Docs/Tasks.md b/Docs/Tasks.md index e977bce..2ce2fa6 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -81,6 +81,8 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. **Docs changes needed:** None beyond `Refactoring.md`. +**Status:** Completed ✅ + **Why this is needed:** Raw SQL in the service layer bypasses the repository abstraction. It makes the service harder to test (requires a real DB schema) and harder to maintain (schema changes must be tracked in the service, not just the repository). --- diff --git a/backend/app/repositories/history_archive_repo.py b/backend/app/repositories/history_archive_repo.py index 184e7d4..d971055 100644 --- a/backend/app/repositories/history_archive_repo.py +++ b/backend/app/repositories/history_archive_repo.py @@ -36,6 +36,15 @@ async def archive_ban_event( return inserted +async def get_max_timeofban(db: aiosqlite.Connection) -> int | None: + """Return the latest archived ban timestamp or ``None`` when empty.""" + async with db.execute("SELECT MAX(timeofban) FROM history_archive") as cursor: + row = await cursor.fetchone() + if row is None or row[0] is None: + return None + return int(row[0]) + + async def get_archived_history( db: aiosqlite.Connection, since: int | None = None, diff --git a/backend/app/services/history_service.py b/backend/app/services/history_service.py index d2ba99d..7e89772 100644 --- a/backend/app/services/history_service.py +++ b/backend/app/services/history_service.py @@ -28,7 +28,7 @@ from app.models.history import ( IpTimelineEvent, ) from app.repositories import fail2ban_db_repo -from app.repositories.history_archive_repo import archive_ban_event +from app.repositories.history_archive_repo import archive_ban_event, get_max_timeofban from app.services.fail2ban_metadata_service import default_fail2ban_metadata_service from app.utils.fail2ban_db_utils import parse_data_json, ts_to_iso @@ -67,11 +67,7 @@ _HISTORY_SYNC_BACKFILL_WINDOW: int = 648000 async def _get_last_archive_ts(db: aiosqlite.Connection) -> int | None: """Return the most recent archived ban timestamp, or ``None`` if empty.""" - async with db.execute("SELECT MAX(timeofban) FROM history_archive") as cur: - row = await cur.fetchone() - if row is None or row[0] is None: - return None - return int(row[0]) + return await get_max_timeofban(db) async def sync_from_fail2ban_db( diff --git a/backend/tests/test_repositories/test_history_archive_repo.py b/backend/tests/test_repositories/test_history_archive_repo.py index f9b5373..1a86cc5 100644 --- a/backend/tests/test_repositories/test_history_archive_repo.py +++ b/backend/tests/test_repositories/test_history_archive_repo.py @@ -9,7 +9,12 @@ import aiosqlite import pytest from app.db import init_db -from app.repositories.history_archive_repo import archive_ban_event, get_archived_history, purge_archived_history +from app.repositories.history_archive_repo import ( + archive_ban_event, + get_archived_history, + get_max_timeofban, + purge_archived_history, +) @pytest.fixture @@ -52,6 +57,25 @@ async def test_get_archived_history_filtering_and_pagination(app_db: str) -> Non assert rows[0]["ip"] == "2.2.2.2" +@pytest.mark.asyncio +async def test_get_max_timeofban_returns_latest_timestamp(app_db: str) -> None: + async with aiosqlite.connect(app_db) as db: + await archive_ban_event(db, "sshd", "1.1.1.1", 1000, 1, "{}", "ban") + await archive_ban_event(db, "sshd", "1.1.1.2", 2000, 1, "{}", "ban") + + max_ts = await get_max_timeofban(db) + + assert max_ts == 2000 + + +@pytest.mark.asyncio +async def test_get_max_timeofban_returns_none_for_empty_archive(app_db: str) -> None: + async with aiosqlite.connect(app_db) as db: + max_ts = await get_max_timeofban(db) + + assert max_ts is None + + @pytest.mark.asyncio async def test_purge_archived_history(app_db: str) -> None: now = int(time.time())