diff --git a/Docs/Tasks.md b/Docs/Tasks.md index bd9124e..e70df2b 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -11,6 +11,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. ### Backend Architecture Review Findings - Status: **done** — `backend/app/routers/config.py` now uses explicit dependency injection for fail2ban settings and no longer reads `request.app.state.settings` directly. +- Status: **done** — `backend/app/routers/config.py` now uses an explicit `AppState` dependency for pending recovery and activation state instead of writing `request.app.state` directly. - `backend/app/routers/*` often reads config directly from `request.app.state.settings` instead of using dependency injection. This bypasses the dependency layer and creates hidden coupling between routers and application state. - Fix: replace direct `request.app.state.settings` access with `SettingsDep` or other explicit dependencies such as `ServerStatusDep` and `PendingRecoveryDep` in router function signatures. @@ -34,7 +35,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. ### Recommended refactors for an AI agent -- Standardise dependency injection in routers by using `SettingsDep`, `ServerStatusDep`, `PendingRecoveryDep`, and other dependency definitions from `backend/app/dependencies.py`. +- Status: **done** — Standardise dependency injection in routers by using `SettingsDep`, `ServerStatusDep`, `PendingRecoveryDep`, and other dependency definitions from `backend/app/dependencies.py`. - Refactor `backend/app/utils/` so it does not import business-layer services. Move cross-layer helpers to the appropriate layer or introduce a shared helper package if needed. - Simplify background task DB management in `backend/app/tasks/`: remove the dead `app.state.db` logic or implement a real shared connection and document its lifecycle. - Document auth cache semantics in the code and project docs. If cluster deployments are intended, replace the process-local cache with a shared cache or remove it. diff --git a/backend/app/dependencies.py b/backend/app/dependencies.py index f4a1dda..85ae7f3 100644 --- a/backend/app/dependencies.py +++ b/backend/app/dependencies.py @@ -6,12 +6,15 @@ Routers import directly from this module — never from ``app.state`` directly — to keep coupling explicit and testable. """ +import datetime import time from collections.abc import AsyncGenerator from typing import Annotated, Protocol, cast +import aiohttp import aiosqlite import structlog +from apscheduler.schedulers.asyncio import AsyncIOScheduler # type: ignore[import-untyped] from fastapi import Depends, HTTPException, Request, status from app.config import Settings @@ -20,9 +23,6 @@ from app.models.config import PendingRecovery from app.models.server import ServerStatus from app.utils.time_utils import utc_now -import aiohttp -from apscheduler.schedulers.asyncio import AsyncIOScheduler # type: ignore[import-untyped] - log: structlog.stdlib.BoundLogger = structlog.get_logger() @@ -32,6 +32,9 @@ class AppState(Protocol): settings: Settings http_session: aiohttp.ClientSession scheduler: AsyncIOScheduler + server_status: ServerStatus + pending_recovery: PendingRecovery | None + last_activation: dict[str, datetime.datetime] | None _COOKIE_NAME = "bangui_session" @@ -173,6 +176,11 @@ async def get_fail2ban_start_command(settings: Settings = Depends(get_settings)) """Provide the configured fail2ban start command.""" return settings.fail2ban_start_command +async def get_app_state(request: Request) -> AppState: + """Provide the application state object for the current request.""" + return cast("AppState", request.app.state) + + async def get_server_status(request: Request) -> ServerStatus: """Return the cached fail2ban server status snapshot from app state.""" state = cast("AppState", request.app.state) @@ -255,4 +263,5 @@ Fail2BanConfigDirDep = Annotated[str, Depends(get_fail2ban_config_dir)] Fail2BanStartCommandDep = Annotated[str, Depends(get_fail2ban_start_command)] ServerStatusDep = Annotated[ServerStatus, Depends(get_server_status)] PendingRecoveryDep = Annotated[PendingRecovery | None, Depends(get_pending_recovery)] +AppStateDep = Annotated[AppState, Depends(get_app_state)] AuthDep = Annotated[Session, Depends(require_auth)] diff --git a/backend/app/routers/config.py b/backend/app/routers/config.py index 4352246..62f7930 100644 --- a/backend/app/routers/config.py +++ b/backend/app/routers/config.py @@ -44,6 +44,7 @@ import structlog from fastapi import APIRouter, HTTPException, Path, Query, Request, status from app.dependencies import ( + AppStateDep, AuthDep, DbDep, Fail2BanConfigDirDep, @@ -678,6 +679,7 @@ async def update_map_color_thresholds( async def activate_jail( request: Request, _auth: AuthDep, + app_state: AppStateDep, config_dir: Fail2BanConfigDirDep, socket_path: Fail2BanSocketDep, name: _NamePath, @@ -728,7 +730,7 @@ async def activate_jail( # Record this activation so the health-check task can attribute a # subsequent fail2ban crash to it. - request.app.state.last_activation = { + app_state.last_activation = { "jail_name": name, "at": datetime.datetime.now(tz=datetime.UTC), } @@ -736,9 +738,9 @@ async def activate_jail( # If fail2ban stopped responding after the reload, create a pending-recovery # record immediately (before the background health task notices). if not result.fail2ban_running: - request.app.state.pending_recovery = PendingRecovery( + app_state.pending_recovery = PendingRecovery( jail_name=name, - activated_at=request.app.state.last_activation["at"], + activated_at=app_state.last_activation["at"], detected_at=datetime.datetime.now(tz=datetime.UTC), ) @@ -932,6 +934,7 @@ async def get_pending_recovery( async def rollback_jail( request: Request, _auth: AuthDep, + app_state: AppStateDep, config_dir: Fail2BanConfigDirDep, socket_path: Fail2BanSocketDep, start_cmd: Fail2BanStartCommandDep, @@ -971,8 +974,8 @@ async def rollback_jail( # Clear pending recovery if fail2ban came back online. if result.fail2ban_running: - request.app.state.pending_recovery = None - request.app.state.last_activation = None + app_state.pending_recovery = None + app_state.last_activation = None return result