Use explicit AppState dependency in config router and update task status
This commit is contained in:
@@ -11,6 +11,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
|
|||||||
### Backend Architecture Review Findings
|
### 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 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.
|
- `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.
|
||||||
@@ -34,7 +35,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
|
|||||||
|
|
||||||
### Recommended refactors for an AI agent
|
### 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.
|
- 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.
|
- 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.
|
- 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.
|
||||||
|
|||||||
@@ -6,12 +6,15 @@ Routers import directly from this module — never from ``app.state``
|
|||||||
directly — to keep coupling explicit and testable.
|
directly — to keep coupling explicit and testable.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import datetime
|
||||||
import time
|
import time
|
||||||
from collections.abc import AsyncGenerator
|
from collections.abc import AsyncGenerator
|
||||||
from typing import Annotated, Protocol, cast
|
from typing import Annotated, Protocol, cast
|
||||||
|
|
||||||
|
import aiohttp
|
||||||
import aiosqlite
|
import aiosqlite
|
||||||
import structlog
|
import structlog
|
||||||
|
from apscheduler.schedulers.asyncio import AsyncIOScheduler # type: ignore[import-untyped]
|
||||||
from fastapi import Depends, HTTPException, Request, status
|
from fastapi import Depends, HTTPException, Request, status
|
||||||
|
|
||||||
from app.config import Settings
|
from app.config import Settings
|
||||||
@@ -20,9 +23,6 @@ from app.models.config import PendingRecovery
|
|||||||
from app.models.server import ServerStatus
|
from app.models.server import ServerStatus
|
||||||
from app.utils.time_utils import utc_now
|
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()
|
log: structlog.stdlib.BoundLogger = structlog.get_logger()
|
||||||
|
|
||||||
|
|
||||||
@@ -32,6 +32,9 @@ class AppState(Protocol):
|
|||||||
settings: Settings
|
settings: Settings
|
||||||
http_session: aiohttp.ClientSession
|
http_session: aiohttp.ClientSession
|
||||||
scheduler: AsyncIOScheduler
|
scheduler: AsyncIOScheduler
|
||||||
|
server_status: ServerStatus
|
||||||
|
pending_recovery: PendingRecovery | None
|
||||||
|
last_activation: dict[str, datetime.datetime] | None
|
||||||
|
|
||||||
|
|
||||||
_COOKIE_NAME = "bangui_session"
|
_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."""
|
"""Provide the configured fail2ban start command."""
|
||||||
return settings.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:
|
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)
|
||||||
@@ -255,4 +263,5 @@ Fail2BanConfigDirDep = Annotated[str, Depends(get_fail2ban_config_dir)]
|
|||||||
Fail2BanStartCommandDep = Annotated[str, Depends(get_fail2ban_start_command)]
|
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)]
|
||||||
AuthDep = Annotated[Session, Depends(require_auth)]
|
AuthDep = Annotated[Session, Depends(require_auth)]
|
||||||
|
|||||||
@@ -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 (
|
||||||
|
AppStateDep,
|
||||||
AuthDep,
|
AuthDep,
|
||||||
DbDep,
|
DbDep,
|
||||||
Fail2BanConfigDirDep,
|
Fail2BanConfigDirDep,
|
||||||
@@ -678,6 +679,7 @@ async def update_map_color_thresholds(
|
|||||||
async def activate_jail(
|
async def activate_jail(
|
||||||
request: Request,
|
request: Request,
|
||||||
_auth: AuthDep,
|
_auth: AuthDep,
|
||||||
|
app_state: AppStateDep,
|
||||||
config_dir: Fail2BanConfigDirDep,
|
config_dir: Fail2BanConfigDirDep,
|
||||||
socket_path: Fail2BanSocketDep,
|
socket_path: Fail2BanSocketDep,
|
||||||
name: _NamePath,
|
name: _NamePath,
|
||||||
@@ -728,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.
|
||||||
request.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),
|
||||||
}
|
}
|
||||||
@@ -736,9 +738,9 @@ 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:
|
||||||
request.app.state.pending_recovery = PendingRecovery(
|
app_state.pending_recovery = PendingRecovery(
|
||||||
jail_name=name,
|
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),
|
detected_at=datetime.datetime.now(tz=datetime.UTC),
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -932,6 +934,7 @@ async def get_pending_recovery(
|
|||||||
async def rollback_jail(
|
async def rollback_jail(
|
||||||
request: Request,
|
request: Request,
|
||||||
_auth: AuthDep,
|
_auth: AuthDep,
|
||||||
|
app_state: AppStateDep,
|
||||||
config_dir: Fail2BanConfigDirDep,
|
config_dir: Fail2BanConfigDirDep,
|
||||||
socket_path: Fail2BanSocketDep,
|
socket_path: Fail2BanSocketDep,
|
||||||
start_cmd: Fail2BanStartCommandDep,
|
start_cmd: Fail2BanStartCommandDep,
|
||||||
@@ -971,8 +974,8 @@ async def rollback_jail(
|
|||||||
|
|
||||||
# Clear pending recovery if fail2ban came back online.
|
# Clear pending recovery if fail2ban came back online.
|
||||||
if result.fail2ban_running:
|
if result.fail2ban_running:
|
||||||
request.app.state.pending_recovery = None
|
app_state.pending_recovery = None
|
||||||
request.app.state.last_activation = None
|
app_state.last_activation = None
|
||||||
|
|
||||||
return result
|
return result
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user