Refactor routers to use explicit FastAPI app dependencies
This commit is contained in:
@@ -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 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.
|
- 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.
|
- 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.
|
- Expected outcome: routers become easier to unit test, composition is more explicit, and shared state access is only available through documented FastAPI dependencies.
|
||||||
|
|
||||||
|
|||||||
@@ -15,7 +15,7 @@ import aiohttp
|
|||||||
import aiosqlite
|
import aiosqlite
|
||||||
import structlog
|
import structlog
|
||||||
from apscheduler.schedulers.asyncio import AsyncIOScheduler # type: ignore[import-untyped]
|
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.config import Settings
|
||||||
from app.models.auth import Session
|
from app.models.auth import Session
|
||||||
@@ -181,6 +181,11 @@ async def get_app_state(request: Request) -> AppState:
|
|||||||
return cast("AppState", request.app.state)
|
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:
|
async def get_server_status(request: Request) -> ServerStatus:
|
||||||
"""Return the cached fail2ban server status snapshot from app state."""
|
"""Return the cached fail2ban server status snapshot from app state."""
|
||||||
state = cast("AppState", request.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)]
|
ServerStatusDep = Annotated[ServerStatus, Depends(get_server_status)]
|
||||||
PendingRecoveryDep = Annotated[PendingRecovery | None, Depends(get_pending_recovery)]
|
PendingRecoveryDep = Annotated[PendingRecovery | None, Depends(get_pending_recovery)]
|
||||||
AppStateDep = Annotated[AppState, Depends(get_app_state)]
|
AppStateDep = Annotated[AppState, Depends(get_app_state)]
|
||||||
|
AppDep = Annotated[FastAPI, Depends(get_app)]
|
||||||
AuthDep = Annotated[Session, Depends(require_auth)]
|
AuthDep = Annotated[Session, Depends(require_auth)]
|
||||||
|
|||||||
@@ -22,17 +22,14 @@ registered *before* the ``/{id}`` routes so FastAPI resolves them correctly.
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from typing import TYPE_CHECKING, Annotated
|
from typing import Annotated
|
||||||
|
|
||||||
import aiosqlite
|
import aiosqlite
|
||||||
|
from fastapi import APIRouter, Depends, HTTPException, Query, status
|
||||||
if TYPE_CHECKING:
|
|
||||||
import aiohttp
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, Query, Request, status
|
|
||||||
|
|
||||||
from app.dependencies import (
|
from app.dependencies import (
|
||||||
|
AppDep,
|
||||||
AuthDep,
|
AuthDep,
|
||||||
DbDep,
|
|
||||||
Fail2BanSocketDep,
|
Fail2BanSocketDep,
|
||||||
HttpSessionDep,
|
HttpSessionDep,
|
||||||
SchedulerDep,
|
SchedulerDep,
|
||||||
@@ -137,7 +134,6 @@ async def run_import_now(
|
|||||||
:class:`~app.models.blocklist.ImportRunResult` with per-source
|
:class:`~app.models.blocklist.ImportRunResult` with per-source
|
||||||
results and aggregated counters.
|
results and aggregated counters.
|
||||||
"""
|
"""
|
||||||
from app.services import jail_service
|
|
||||||
|
|
||||||
return await blocklist_service.import_all(
|
return await blocklist_service.import_all(
|
||||||
db,
|
db,
|
||||||
@@ -189,13 +185,13 @@ async def update_schedule(
|
|||||||
db: DbDep,
|
db: DbDep,
|
||||||
_auth: AuthDep,
|
_auth: AuthDep,
|
||||||
scheduler: SchedulerDep,
|
scheduler: SchedulerDep,
|
||||||
request: Request,
|
app: AppDep,
|
||||||
) -> ScheduleInfo:
|
) -> ScheduleInfo:
|
||||||
"""Persist a new schedule configuration and reschedule the import job.
|
"""Persist a new schedule configuration and reschedule the import job.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
payload: New :class:`~app.models.blocklist.ScheduleConfig`.
|
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).
|
db: Application database connection (injected).
|
||||||
_auth: Validated session — enforces authentication.
|
_auth: Validated session — enforces authentication.
|
||||||
|
|
||||||
@@ -204,7 +200,7 @@ async def update_schedule(
|
|||||||
"""
|
"""
|
||||||
await blocklist_service.set_schedule(db, payload)
|
await blocklist_service.set_schedule(db, payload)
|
||||||
# Reschedule the background job immediately.
|
# 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)
|
job = scheduler.get_job(blocklist_import_task.JOB_ID)
|
||||||
next_run_at: str | None = None
|
next_run_at: str | None = None
|
||||||
|
|||||||
@@ -44,6 +44,7 @@ import structlog
|
|||||||
from fastapi import APIRouter, HTTPException, Path, Query, Request, status
|
from fastapi import APIRouter, HTTPException, Path, Query, Request, status
|
||||||
|
|
||||||
from app.dependencies import (
|
from app.dependencies import (
|
||||||
|
AppDep,
|
||||||
AppStateDep,
|
AppStateDep,
|
||||||
AuthDep,
|
AuthDep,
|
||||||
DbDep,
|
DbDep,
|
||||||
@@ -677,9 +678,8 @@ async def update_map_color_thresholds(
|
|||||||
summary="Activate an inactive jail",
|
summary="Activate an inactive jail",
|
||||||
)
|
)
|
||||||
async def activate_jail(
|
async def activate_jail(
|
||||||
request: Request,
|
app: AppDep,
|
||||||
_auth: AuthDep,
|
_auth: AuthDep,
|
||||||
app_state: AppStateDep,
|
|
||||||
config_dir: Fail2BanConfigDirDep,
|
config_dir: Fail2BanConfigDirDep,
|
||||||
socket_path: Fail2BanSocketDep,
|
socket_path: Fail2BanSocketDep,
|
||||||
name: _NamePath,
|
name: _NamePath,
|
||||||
@@ -692,7 +692,7 @@ async def activate_jail(
|
|||||||
the jail starts immediately.
|
the jail starts immediately.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
request: FastAPI request object.
|
app: FastAPI application instance.
|
||||||
_auth: Validated session.
|
_auth: Validated session.
|
||||||
name: Name of the jail to activate.
|
name: Name of the jail to activate.
|
||||||
body: Optional override values (bantime, findtime, maxretry, port,
|
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
|
# Record this activation so the health-check task can attribute a
|
||||||
# subsequent fail2ban crash to it.
|
# subsequent fail2ban crash to it.
|
||||||
app_state.last_activation = {
|
app.state.last_activation = {
|
||||||
"jail_name": name,
|
"jail_name": name,
|
||||||
"at": datetime.datetime.now(tz=datetime.UTC),
|
"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
|
# If fail2ban stopped responding after the reload, create a pending-recovery
|
||||||
# record immediately (before the background health task notices).
|
# record immediately (before the background health task notices).
|
||||||
if not result.fail2ban_running:
|
if not result.fail2ban_running:
|
||||||
app_state.pending_recovery = PendingRecovery(
|
app.state.pending_recovery = PendingRecovery(
|
||||||
jail_name=name,
|
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),
|
detected_at=datetime.datetime.now(tz=datetime.UTC),
|
||||||
)
|
)
|
||||||
|
|
||||||
# Force an immediate health probe so the cached status reflects the current
|
# Force an immediate health probe so the cached status reflects the current
|
||||||
# fail2ban state without waiting for the next scheduled check.
|
# fail2ban state without waiting for the next scheduled check.
|
||||||
await _run_probe(request.app)
|
await _run_probe(app)
|
||||||
|
|
||||||
return result
|
return result
|
||||||
|
|
||||||
@@ -757,7 +757,7 @@ async def activate_jail(
|
|||||||
summary="Deactivate an active jail",
|
summary="Deactivate an active jail",
|
||||||
)
|
)
|
||||||
async def deactivate_jail(
|
async def deactivate_jail(
|
||||||
request: Request,
|
app: AppDep,
|
||||||
_auth: AuthDep,
|
_auth: AuthDep,
|
||||||
config_dir: Fail2BanConfigDirDep,
|
config_dir: Fail2BanConfigDirDep,
|
||||||
socket_path: Fail2BanSocketDep,
|
socket_path: Fail2BanSocketDep,
|
||||||
@@ -769,7 +769,7 @@ async def deactivate_jail(
|
|||||||
full fail2ban reload so the jail stops immediately.
|
full fail2ban reload so the jail stops immediately.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
request: FastAPI request object.
|
app: FastAPI application instance.
|
||||||
_auth: Validated session.
|
_auth: Validated session.
|
||||||
name: Name of the jail to deactivate.
|
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
|
# Force an immediate health probe so the cached status reflects the current
|
||||||
# fail2ban state (reload changes the active-jail count) without waiting for
|
# fail2ban state (reload changes the active-jail count) without waiting for
|
||||||
# the next scheduled background check (up to 30 seconds).
|
# the next scheduled background check (up to 30 seconds).
|
||||||
await _run_probe(request.app)
|
await _run_probe(app)
|
||||||
|
|
||||||
return result
|
return result
|
||||||
|
|
||||||
|
|||||||
@@ -8,9 +8,9 @@ return ``409 Conflict``.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import structlog
|
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.models.setup import SetupRequest, SetupResponse, SetupStatusResponse, SetupTimezoneResponse
|
||||||
from app.services import setup_service
|
from app.services import setup_service
|
||||||
from app.utils.setup_state import set_setup_complete_cache
|
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",
|
summary="Run the initial setup wizard",
|
||||||
)
|
)
|
||||||
async def post_setup(
|
async def post_setup(
|
||||||
request: Request,
|
app: AppDep,
|
||||||
body: SetupRequest,
|
body: SetupRequest,
|
||||||
db: DbDep,
|
db: DbDep,
|
||||||
) -> SetupResponse:
|
) -> SetupResponse:
|
||||||
"""Persist the initial BanGUI configuration.
|
"""Persist the initial BanGUI configuration.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
request: The incoming HTTP request.
|
app: The FastAPI application instance.
|
||||||
body: Setup request payload validated by Pydantic.
|
body: Setup request payload validated by Pydantic.
|
||||||
db: Injected aiosqlite connection.
|
db: Injected aiosqlite connection.
|
||||||
|
|
||||||
@@ -74,7 +74,7 @@ async def post_setup(
|
|||||||
timezone=body.timezone,
|
timezone=body.timezone,
|
||||||
session_duration_minutes=body.session_duration_minutes,
|
session_duration_minutes=body.session_duration_minutes,
|
||||||
)
|
)
|
||||||
set_setup_complete_cache(request.app, True)
|
set_setup_complete_cache(app, True)
|
||||||
return SetupResponse()
|
return SetupResponse()
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user