From ed3aa61c35f4d2c4ffc602aa1196730eef437f57 Mon Sep 17 00:00:00 2001 From: Lukas Date: Tue, 7 Apr 2026 20:27:06 +0200 Subject: [PATCH] Refactor routers to use explicit FastAPI app dependencies --- Docs/Tasks.md | 2 +- backend/app/dependencies.py | 8 +++++++- backend/app/routers/blocklist.py | 16 ++++++---------- backend/app/routers/config.py | 20 ++++++++++---------- backend/app/routers/setup.py | 10 +++++----- 5 files changed, 29 insertions(+), 27 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index e70df2b..1e4b73f 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -13,7 +13,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. - 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. +- Status: **done** — `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. - Expected outcome: routers become easier to unit test, composition is more explicit, and shared state access is only available through documented FastAPI dependencies. diff --git a/backend/app/dependencies.py b/backend/app/dependencies.py index 85ae7f3..2502930 100644 --- a/backend/app/dependencies.py +++ b/backend/app/dependencies.py @@ -15,7 +15,7 @@ import aiohttp import aiosqlite import structlog from apscheduler.schedulers.asyncio import AsyncIOScheduler # type: ignore[import-untyped] -from fastapi import Depends, HTTPException, Request, status +from fastapi import Depends, FastAPI, HTTPException, Request, status from app.config import Settings from app.models.auth import Session @@ -181,6 +181,11 @@ async def get_app_state(request: Request) -> AppState: return cast("AppState", request.app.state) +async def get_app(request: Request) -> FastAPI: + """Provide the FastAPI application instance for the current request.""" + return request.app + + async def get_server_status(request: Request) -> ServerStatus: """Return the cached fail2ban server status snapshot from app state.""" state = cast("AppState", request.app.state) @@ -264,4 +269,5 @@ 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)] +AppDep = Annotated[FastAPI, Depends(get_app)] AuthDep = Annotated[Session, Depends(require_auth)] diff --git a/backend/app/routers/blocklist.py b/backend/app/routers/blocklist.py index 245b2a4..ad6f13c 100644 --- a/backend/app/routers/blocklist.py +++ b/backend/app/routers/blocklist.py @@ -22,17 +22,14 @@ registered *before* the ``/{id}`` routes so FastAPI resolves them correctly. from __future__ import annotations -from typing import TYPE_CHECKING, Annotated +from typing import Annotated import aiosqlite - -if TYPE_CHECKING: - import aiohttp -from fastapi import APIRouter, Depends, HTTPException, Query, Request, status +from fastapi import APIRouter, Depends, HTTPException, Query, status from app.dependencies import ( + AppDep, AuthDep, - DbDep, Fail2BanSocketDep, HttpSessionDep, SchedulerDep, @@ -137,7 +134,6 @@ async def run_import_now( :class:`~app.models.blocklist.ImportRunResult` with per-source results and aggregated counters. """ - from app.services import jail_service return await blocklist_service.import_all( db, @@ -189,13 +185,13 @@ async def update_schedule( db: DbDep, _auth: AuthDep, scheduler: SchedulerDep, - request: Request, + app: AppDep, ) -> ScheduleInfo: """Persist a new schedule configuration and reschedule the import job. Args: payload: New :class:`~app.models.blocklist.ScheduleConfig`. - request: Incoming request (used to access the scheduler). + app: FastAPI application instance used to reschedule the import job. db: Application database connection (injected). _auth: Validated session — enforces authentication. @@ -204,7 +200,7 @@ async def update_schedule( """ await blocklist_service.set_schedule(db, payload) # Reschedule the background job immediately. - blocklist_import_task.reschedule(request.app) + blocklist_import_task.reschedule(app) job = scheduler.get_job(blocklist_import_task.JOB_ID) next_run_at: str | None = None diff --git a/backend/app/routers/config.py b/backend/app/routers/config.py index 62f7930..049d1aa 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 ( + AppDep, AppStateDep, AuthDep, DbDep, @@ -677,9 +678,8 @@ async def update_map_color_thresholds( summary="Activate an inactive jail", ) async def activate_jail( - request: Request, + app: AppDep, _auth: AuthDep, - app_state: AppStateDep, config_dir: Fail2BanConfigDirDep, socket_path: Fail2BanSocketDep, name: _NamePath, @@ -692,7 +692,7 @@ async def activate_jail( the jail starts immediately. Args: - request: FastAPI request object. + app: FastAPI application instance. _auth: Validated session. name: Name of the jail to activate. body: Optional override values (bantime, findtime, maxretry, port, @@ -730,7 +730,7 @@ async def activate_jail( # Record this activation so the health-check task can attribute a # subsequent fail2ban crash to it. - app_state.last_activation = { + app.state.last_activation = { "jail_name": name, "at": datetime.datetime.now(tz=datetime.UTC), } @@ -738,15 +738,15 @@ 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: - app_state.pending_recovery = PendingRecovery( + app.state.pending_recovery = PendingRecovery( jail_name=name, - activated_at=app_state.last_activation["at"], + activated_at=app.state.last_activation["at"], detected_at=datetime.datetime.now(tz=datetime.UTC), ) # Force an immediate health probe so the cached status reflects the current # fail2ban state without waiting for the next scheduled check. - await _run_probe(request.app) + await _run_probe(app) return result @@ -757,7 +757,7 @@ async def activate_jail( summary="Deactivate an active jail", ) async def deactivate_jail( - request: Request, + app: AppDep, _auth: AuthDep, config_dir: Fail2BanConfigDirDep, socket_path: Fail2BanSocketDep, @@ -769,7 +769,7 @@ async def deactivate_jail( full fail2ban reload so the jail stops immediately. Args: - request: FastAPI request object. + app: FastAPI application instance. _auth: Validated session. name: Name of the jail to deactivate. @@ -805,7 +805,7 @@ async def deactivate_jail( # Force an immediate health probe so the cached status reflects the current # fail2ban state (reload changes the active-jail count) without waiting for # the next scheduled background check (up to 30 seconds). - await _run_probe(request.app) + await _run_probe(app) return result diff --git a/backend/app/routers/setup.py b/backend/app/routers/setup.py index 36aa6f9..cd4f9cb 100644 --- a/backend/app/routers/setup.py +++ b/backend/app/routers/setup.py @@ -8,9 +8,9 @@ return ``409 Conflict``. from __future__ import annotations import structlog -from fastapi import APIRouter, HTTPException, Request, status +from fastapi import APIRouter, HTTPException, status -from app.dependencies import DbDep +from app.dependencies import AppDep, DbDep from app.models.setup import SetupRequest, SetupResponse, SetupStatusResponse, SetupTimezoneResponse from app.services import setup_service from app.utils.setup_state import set_setup_complete_cache @@ -43,14 +43,14 @@ async def get_setup_status(db: DbDep) -> SetupStatusResponse: summary="Run the initial setup wizard", ) async def post_setup( - request: Request, + app: AppDep, body: SetupRequest, db: DbDep, ) -> SetupResponse: """Persist the initial BanGUI configuration. Args: - request: The incoming HTTP request. + app: The FastAPI application instance. body: Setup request payload validated by Pydantic. db: Injected aiosqlite connection. @@ -74,7 +74,7 @@ async def post_setup( timezone=body.timezone, session_duration_minutes=body.session_duration_minutes, ) - set_setup_complete_cache(request.app, True) + set_setup_complete_cache(app, True) return SetupResponse()