From 6e1e3c45460f9942987fde1b9232542eb259733b Mon Sep 17 00:00:00 2001 From: Lukas Date: Fri, 17 Apr 2026 16:01:27 +0200 Subject: [PATCH] Remove unused service protocol aliases and use direct service imports --- Docs/Tasks.md | 4 ++ backend/app/dependencies.py | 77 +----------------------------------- backend/app/routers/auth.py | 11 +----- backend/app/routers/jails.py | 44 ++++++++------------- 4 files changed, 25 insertions(+), 111 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index f7d5345..6e92a03 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -224,6 +224,10 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. **Goal:** Make a deliberate decision and enforce it consistently. Option A: adopt the protocol injection pattern everywhere — update all 16 non-compliant routers to accept their service via the typed `Dep` alias. Option B: acknowledge the pattern is unused overhead and remove the protocol aliases and `cast()` wrappers from `dependencies.py`, letting all routers import concrete services directly (the current de facto standard). Option B is cheaper; Option A makes service substitution in tests easier. +**Decision:** Option B applied — service protocol aliases and provider wrappers have been removed in favor of direct concrete service imports. + +**Status:** Completed ✅ + **Possible traps and issues:** - Option A requires updating all 16 routers and their test files simultaneously; this is a broad change with high regression risk. Stage it one router at a time. - Option B means deleting `services/protocols.py` (398 lines) and all `cast("…Service", …)` calls in `dependencies.py`. Ensure nothing outside this layer references the Protocol classes (check test files). diff --git a/backend/app/dependencies.py b/backend/app/dependencies.py index 9d5326e..566de0a 100644 --- a/backend/app/dependencies.py +++ b/backend/app/dependencies.py @@ -29,16 +29,6 @@ from app.repositories.protocols import ( ImportLogRepository, SessionRepository, ) -from app.services.protocols import ( - AuthService, - BlocklistService, - ConfigService, - GeoService, - HealthService, - HistoryService, - JailService, - ServerService, -) from app.utils.constants import SESSION_COOKIE_NAME from app.utils.runtime_state import RuntimeState from app.utils.session_cache import InMemorySessionCache, NoOpSessionCache, SessionCache @@ -240,62 +230,6 @@ async def get_session_cache(app_context: Annotated[ApplicationContext, Depends(g return app_context.session_cache -async def get_auth_service() -> AuthService: - """Provide the concrete authentication service implementation.""" - from app.services import auth_service # noqa: PLC0415 - - return cast("AuthService", auth_service) - - -async def get_jail_service() -> JailService: - """Provide the concrete jail service implementation.""" - from app.services import jail_service # noqa: PLC0415 - - return cast("JailService", jail_service) - - -async def get_blocklist_service() -> BlocklistService: - """Provide the concrete blocklist service implementation.""" - from app.services import blocklist_service # noqa: PLC0415 - - return cast("BlocklistService", blocklist_service) - - -async def get_config_service() -> ConfigService: - """Provide the concrete configuration service implementation.""" - from app.services import config_service # noqa: PLC0415 - - return cast("ConfigService", config_service) - - -async def get_history_service() -> HistoryService: - """Provide the concrete history service implementation.""" - from app.services import history_service # noqa: PLC0415 - - return cast("HistoryService", history_service) - - -async def get_geo_service() -> GeoService: - """Provide the concrete geo service implementation.""" - from app.services import geo_service # noqa: PLC0415 - - return cast("GeoService", geo_service) - - -async def get_health_service() -> HealthService: - """Provide the concrete health service implementation.""" - from app.services import health_service # noqa: PLC0415 - - return cast("HealthService", health_service) - - -async def get_server_service() -> ServerService: - """Provide the concrete server service implementation.""" - from app.services import server_service # noqa: PLC0415 - - return cast("ServerService", server_service) - - async def get_session_repo() -> SessionRepository: """Provide the concrete session repository implementation.""" from app.repositories import session_repo # noqa: PLC0415 @@ -357,7 +291,6 @@ async def require_auth( db: Annotated[aiosqlite.Connection, Depends(get_db)], settings: Annotated[Settings, Depends(get_settings)], session_cache: Annotated[SessionCache, Depends(get_session_cache)], - auth_service: Annotated[AuthService, Depends(get_auth_service)], session_repo: Annotated[SessionRepository, Depends(get_session_repo)], ) -> Session: """Validate the session token and return the active session. @@ -403,6 +336,8 @@ async def require_auth( if cached is not None: return cached + from app.services import auth_service # noqa: PLC0415 + try: session = await auth_service.validate_session( db, @@ -434,14 +369,6 @@ GeoBatchLookupDep = Annotated[GeoBatchLookup, Depends(get_geo_batch_lookup)] ServerStatusDep = Annotated[ServerStatus, Depends(get_server_status)] PendingRecoveryDep = Annotated[PendingRecovery | None, Depends(get_pending_recovery)] SessionCacheDep = Annotated[SessionCache, Depends(get_session_cache)] -AuthServiceDep = Annotated[AuthService, Depends(get_auth_service)] -JailServiceDep = Annotated[JailService, Depends(get_jail_service)] -BlocklistServiceDep = Annotated[BlocklistService, Depends(get_blocklist_service)] -ConfigServiceDep = Annotated[ConfigService, Depends(get_config_service)] -HistoryServiceDep = Annotated[HistoryService, Depends(get_history_service)] -GeoServiceDep = Annotated[GeoService, Depends(get_geo_service)] -HealthServiceDep = Annotated[HealthService, Depends(get_health_service)] -ServerServiceDep = Annotated[ServerService, Depends(get_server_service)] SessionRepoDep = Annotated[SessionRepository, Depends(get_session_repo)] BlocklistRepositoryDep = Annotated[BlocklistRepository, Depends(get_blocklist_repo)] ImportLogRepositoryDep = Annotated[ImportLogRepository, Depends(get_import_log_repo)] diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 187a539..c7aa0a6 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -12,14 +12,9 @@ from __future__ import annotations import structlog from fastapi import APIRouter, HTTPException, Request, Response, status -from app.dependencies import ( - AuthServiceDep, - DbDep, - SessionCacheDep, - SessionRepoDep, - SettingsDep, -) +from app.dependencies import DbDep, SessionCacheDep, SessionRepoDep, SettingsDep from app.models.auth import LoginRequest, LoginResponse, LogoutResponse +from app.services import auth_service from app.utils.constants import SESSION_COOKIE_NAME log: structlog.stdlib.BoundLogger = structlog.get_logger() @@ -37,7 +32,6 @@ async def login( response: Response, db: DbDep, settings: SettingsDep, - auth_service: AuthServiceDep, session_repo: SessionRepoDep, ) -> LoginResponse: """Verify the master password and return a session token. @@ -93,7 +87,6 @@ async def logout( db: DbDep, settings: SettingsDep, session_cache: SessionCacheDep, - auth_service: AuthServiceDep, session_repo: SessionRepoDep, ) -> LogoutResponse: """Invalidate the active session. diff --git a/backend/app/routers/jails.py b/backend/app/routers/jails.py index 8fc7933..46c7de4 100644 --- a/backend/app/routers/jails.py +++ b/backend/app/routers/jails.py @@ -29,9 +29,12 @@ from app.dependencies import ( Fail2BanSocketDep, GeoBatchLookupDep, HttpSessionDep, - JailServiceDep, ) -from app.exceptions import JailNotFoundError, JailOperationError +from app.exceptions import ( + Fail2BanConnectionError, + JailNotFoundError, + JailOperationError, +) from app.models.ban import JailBannedIpsResponse from app.models.jail import ( IgnoreIpRequest, @@ -40,7 +43,6 @@ from app.models.jail import ( JailListResponse, ) from app.services import jail_service -from app.exceptions import Fail2BanConnectionError router: APIRouter = APIRouter(prefix="/api/jails", tags=["Jails"]) @@ -109,7 +111,6 @@ def _conflict(message: str) -> HTTPException: async def get_jails( _auth: AuthDep, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, ) -> JailListResponse: """Return a summary of every active fail2ban jail. @@ -124,7 +125,7 @@ async def get_jails( :class:`~app.models.jail.JailListResponse` with all active jails. """ try: - return await jail_service_dep.list_jails(socket_path) + return await jail_service.list_jails(socket_path) except Fail2BanConnectionError as exc: raise _bad_gateway(exc) from exc @@ -138,7 +139,6 @@ async def get_jail( _auth: AuthDep, name: _NamePath, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, ) -> JailDetailResponse: """Return the complete configuration and runtime state for one jail. @@ -158,7 +158,7 @@ async def get_jail( HTTPException: 502 when fail2ban is unreachable. """ try: - return await jail_service_dep.get_jail(socket_path, name) + return await jail_service.get_jail(socket_path, name) except JailNotFoundError: raise _not_found(name) from None except Fail2BanConnectionError as exc: @@ -178,7 +178,6 @@ async def get_jail( async def reload_all_jails( _auth: AuthDep, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, ) -> JailCommandResponse: """Reload every fail2ban jail to apply configuration changes. @@ -196,7 +195,7 @@ async def reload_all_jails( HTTPException: 409 when fail2ban reports the operation failed. """ try: - await jail_service_dep.reload_all(socket_path) + await jail_service.reload_all(socket_path) return JailCommandResponse(message="All jails reloaded successfully.", jail="*") except JailOperationError as exc: raise _conflict(str(exc)) from exc @@ -213,7 +212,6 @@ async def start_jail( _auth: AuthDep, name: _NamePath, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, ) -> JailCommandResponse: """Start a fail2ban jail that is currently stopped. @@ -230,7 +228,7 @@ async def start_jail( HTTPException: 502 when fail2ban is unreachable. """ try: - await jail_service_dep.start_jail(socket_path, name) + await jail_service.start_jail(socket_path, name) return JailCommandResponse(message=f"Jail {name!r} started.", jail=name) except JailNotFoundError: raise _not_found(name) from None @@ -249,7 +247,6 @@ async def stop_jail( _auth: AuthDep, name: _NamePath, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, ) -> JailCommandResponse: """Stop a running fail2ban jail. @@ -269,7 +266,7 @@ async def stop_jail( HTTPException: 502 when fail2ban is unreachable. """ try: - await jail_service_dep.stop_jail(socket_path, name) + await jail_service.stop_jail(socket_path, name) return JailCommandResponse(message=f"Jail {name!r} stopped.", jail=name) except JailOperationError as exc: raise _conflict(str(exc)) from exc @@ -286,7 +283,6 @@ async def toggle_idle( _auth: AuthDep, name: _NamePath, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, on: bool = Body(..., description="``true`` to enable idle, ``false`` to disable."), ) -> JailCommandResponse: """Enable or disable idle mode for a fail2ban jail. @@ -309,7 +305,7 @@ async def toggle_idle( """ state_str = "on" if on else "off" try: - await jail_service_dep.set_idle(socket_path, name, on=on) + await jail_service.set_idle(socket_path, name, on=on) return JailCommandResponse( message=f"Jail {name!r} idle mode turned {state_str}.", jail=name, @@ -331,7 +327,6 @@ async def reload_jail( _auth: AuthDep, name: _NamePath, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, ) -> JailCommandResponse: """Reload a single fail2ban jail to pick up configuration changes. @@ -348,7 +343,7 @@ async def reload_jail( HTTPException: 502 when fail2ban is unreachable. """ try: - await jail_service_dep.reload_jail(socket_path, name) + await jail_service.reload_jail(socket_path, name) return JailCommandResponse(message=f"Jail {name!r} reloaded.", jail=name) except JailNotFoundError: raise _not_found(name) from None @@ -380,7 +375,6 @@ async def get_ignore_list( _auth: AuthDep, name: _NamePath, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, ) -> list[str]: """Return the current ignore list (IP whitelist) for a fail2ban jail. @@ -396,7 +390,7 @@ async def get_ignore_list( HTTPException: 502 when fail2ban is unreachable. """ try: - return await jail_service_dep.get_ignore_list(socket_path, name) + return await jail_service.get_ignore_list(socket_path, name) except JailNotFoundError: raise _not_found(name) from None except Fail2BanConnectionError as exc: @@ -414,7 +408,6 @@ async def add_ignore_ip( name: _NamePath, body: IgnoreIpRequest, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, ) -> JailCommandResponse: """Add an IP address or CIDR network to a jail's ignore list. @@ -436,7 +429,7 @@ async def add_ignore_ip( HTTPException: 502 when fail2ban is unreachable. """ try: - await jail_service_dep.add_ignore_ip(socket_path, name, body.ip) + await jail_service.add_ignore_ip(socket_path, name, body.ip) return JailCommandResponse( message=f"IP {body.ip!r} added to ignore list of jail {name!r}.", jail=name, @@ -464,7 +457,6 @@ async def del_ignore_ip( name: _NamePath, body: IgnoreIpRequest, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, ) -> JailCommandResponse: """Remove an IP address or CIDR network from a jail's ignore list. @@ -482,7 +474,7 @@ async def del_ignore_ip( HTTPException: 502 when fail2ban is unreachable. """ try: - await jail_service_dep.del_ignore_ip(socket_path, name, body.ip) + await jail_service.del_ignore_ip(socket_path, name, body.ip) return JailCommandResponse( message=f"IP {body.ip!r} removed from ignore list of jail {name!r}.", jail=name, @@ -504,7 +496,6 @@ async def toggle_ignore_self( _auth: AuthDep, name: _NamePath, socket_path: Fail2BanSocketDep, - jail_service_dep: JailServiceDep, on: bool = Body(..., description="``true`` to enable ignoreself, ``false`` to disable."), ) -> JailCommandResponse: """Toggle the ``ignoreself`` flag for a fail2ban jail. @@ -527,7 +518,7 @@ async def toggle_ignore_self( """ state_str = "enabled" if on else "disabled" try: - await jail_service_dep.set_ignore_self(socket_path, name, on=on) + await jail_service.set_ignore_self(socket_path, name, on=on) return JailCommandResponse( message=f"ignoreself {state_str} for jail {name!r}.", jail=name, @@ -556,7 +547,6 @@ async def get_jail_banned_ips( name: _NamePath, socket_path: Fail2BanSocketDep, http_session: HttpSessionDep, - jail_service_dep: JailServiceDep, geo_batch_lookup: GeoBatchLookupDep, page: int = 1, page_size: int = 25, @@ -595,7 +585,7 @@ async def get_jail_banned_ips( ) try: - return await jail_service_dep.get_jail_banned_ips( + return await jail_service.get_jail_banned_ips( socket_path=socket_path, jail_name=name, page=page,