From 1e39e5a1d62ea06ebd72cba811308dacaa870cdc Mon Sep 17 00:00:00 2001 From: Lukas Date: Tue, 7 Apr 2026 20:39:56 +0200 Subject: [PATCH] Refactor app helpers and use AppStateDep in config router Move service-dependent helper wrappers from app.utils to app.helpers and update config router activation/rollback to use explicit AppState dependency. --- Docs/Tasks.md | 2 +- backend/app/helpers/__init__.py | 6 +++++ backend/app/helpers/config_file_helpers.py | 19 +++++++++++++++ backend/app/helpers/jail_helpers.py | 23 +++++++++++++++++++ backend/app/helpers/log_helpers.py | 21 +++++++++++++++++ backend/app/routers/config.py | 10 ++++---- backend/app/services/action_config_service.py | 13 +++++------ backend/app/services/config_file_service.py | 4 ++-- backend/app/services/config_service.py | 4 ++-- backend/app/services/filter_config_service.py | 10 ++++---- backend/app/services/jail_config_service.py | 16 ++++++------- 11 files changed, 98 insertions(+), 30 deletions(-) create mode 100644 backend/app/helpers/__init__.py create mode 100644 backend/app/helpers/config_file_helpers.py create mode 100644 backend/app/helpers/jail_helpers.py create mode 100644 backend/app/helpers/log_helpers.py diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 1e4b73f..6676977 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -17,7 +17,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. - 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. -- Several utility modules under `backend/app/utils/` import service layer code (`app.services.*`). Utilities should remain low-level helpers and not depend on higher-level service logic. +- Status: **done** — Several utility modules under `backend/app/utils/` import service layer code (`app.services.*`). Utilities should remain low-level helpers and not depend on higher-level service logic. - Fix: move service-dependent helpers into `app/services/` or extract shared logic into a new `app/helpers/` layer, keeping `app/utils/` purely independent. - Expected outcome: lower coupling between utility and service layers, cleaner dependency direction, and better maintainability. diff --git a/backend/app/helpers/__init__.py b/backend/app/helpers/__init__.py new file mode 100644 index 0000000..99ac55f --- /dev/null +++ b/backend/app/helpers/__init__.py @@ -0,0 +1,6 @@ +"""Cross-service helpers and shared abstractions. + +Modules in ``app.helpers`` are allowed to depend on the service layer when they +implement shared business logic used by multiple service modules. This keeps +``app.utils`` independent and low-level. +""" diff --git a/backend/app/helpers/config_file_helpers.py b/backend/app/helpers/config_file_helpers.py new file mode 100644 index 0000000..42f5f71 --- /dev/null +++ b/backend/app/helpers/config_file_helpers.py @@ -0,0 +1,19 @@ +"""Shared config-file helpers reused across service modules.""" + +from __future__ import annotations + +from app.services.config_file_service import ( + _build_inactive_jail, + _get_active_jail_names, + _ordered_config_files, + _parse_jails_sync, + _validate_jail_config_sync, +) + +__all__ = [ + "_ordered_config_files", + "_parse_jails_sync", + "_build_inactive_jail", + "_get_active_jail_names", + "_validate_jail_config_sync", +] diff --git a/backend/app/helpers/jail_helpers.py b/backend/app/helpers/jail_helpers.py new file mode 100644 index 0000000..c6ba9a0 --- /dev/null +++ b/backend/app/helpers/jail_helpers.py @@ -0,0 +1,23 @@ +"""Shared jail management helpers for service-layer code.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from app.services.jail_service import reload_all + +if TYPE_CHECKING: + from collections.abc import Sequence + + +async def reload_jails( + socket_path: str, + include_jails: Sequence[str] | None = None, + exclude_jails: Sequence[str] | None = None, +) -> None: + """Reload fail2ban jails using the shared jail service helper.""" + await reload_all( + socket_path, + include_jails=list(include_jails) if include_jails is not None else None, + exclude_jails=list(exclude_jails) if exclude_jails is not None else None, + ) diff --git a/backend/app/helpers/log_helpers.py b/backend/app/helpers/log_helpers.py new file mode 100644 index 0000000..3e5bf6f --- /dev/null +++ b/backend/app/helpers/log_helpers.py @@ -0,0 +1,21 @@ +"""Shared log-related helpers for service-layer code.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from app.services.log_service import preview_log as _preview_log +from app.services.log_service import test_regex as _test_regex + +if TYPE_CHECKING: + from app.models.config import LogPreviewRequest, LogPreviewResponse, RegexTestRequest, RegexTestResponse + + +def preview_log(req: LogPreviewRequest) -> LogPreviewResponse: + """Return log preview results using the shared log service.""" + return _preview_log(req) + + +def test_regex(req: RegexTestRequest) -> RegexTestResponse: + """Validate a regex pattern using the shared log service.""" + return _test_regex(req) diff --git a/backend/app/routers/config.py b/backend/app/routers/config.py index 049d1aa..2249280 100644 --- a/backend/app/routers/config.py +++ b/backend/app/routers/config.py @@ -678,6 +678,7 @@ async def update_map_color_thresholds( summary="Activate an inactive jail", ) async def activate_jail( + app_state: AppStateDep, app: AppDep, _auth: AuthDep, config_dir: Fail2BanConfigDirDep, @@ -692,6 +693,7 @@ async def activate_jail( the jail starts immediately. Args: + app_state: FastAPI application state. app: FastAPI application instance. _auth: Validated session. name: Name of the jail to activate. @@ -730,7 +732,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,9 +740,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: - 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), ) @@ -932,7 +934,6 @@ async def get_pending_recovery( summary="Disable a bad jail config and restart fail2ban", ) async def rollback_jail( - request: Request, _auth: AuthDep, app_state: AppStateDep, config_dir: Fail2BanConfigDirDep, @@ -949,7 +950,6 @@ async def rollback_jail( On success, clears the :class:`~app.models.config.PendingRecovery` record. Args: - request: FastAPI request object. _auth: Validated session. name: Jail name to disable and roll back. diff --git a/backend/app/services/action_config_service.py b/backend/app/services/action_config_service.py index 7b5f7e2..480101d 100644 --- a/backend/app/services/action_config_service.py +++ b/backend/app/services/action_config_service.py @@ -17,6 +17,12 @@ from pathlib import Path import structlog +from app.exceptions import ConfigWriteError, JailNotFoundInConfigError +from app.helpers.config_file_helpers import ( + _get_active_jail_names, + _parse_jails_sync, +) +from app.helpers.jail_helpers import reload_jails from app.models.config import ( ActionConfig, ActionConfigUpdate, @@ -25,14 +31,7 @@ from app.models.config import ( ActionUpdateRequest, AssignActionRequest, ) -from app.exceptions import JailNotFoundError -from app.utils.config_file_utils import ( - _parse_jails_sync, - _get_active_jail_names, -) -from app.exceptions import ConfigWriteError, JailNotFoundInConfigError from app.utils import conffile_parser -from app.utils.jail_utils import reload_jails log: structlog.stdlib.BoundLogger = structlog.get_logger() diff --git a/backend/app/services/config_file_service.py b/backend/app/services/config_file_service.py index a4c19a2..c319aff 100644 --- a/backend/app/services/config_file_service.py +++ b/backend/app/services/config_file_service.py @@ -32,6 +32,8 @@ from typing import cast import structlog +from app.exceptions import FilterInvalidRegexError, JailNotFoundError +from app.helpers.jail_helpers import reload_jails from app.models.config import ( ActionConfig, ActionConfigUpdate, @@ -54,9 +56,7 @@ from app.models.config import ( JailValidationResult, RollbackResponse, ) -from app.exceptions import FilterInvalidRegexError, JailNotFoundError from app.utils import conffile_parser -from app.utils.jail_utils import reload_jails from app.utils.fail2ban_client import ( Fail2BanClient, Fail2BanConnectionError, diff --git a/backend/app/services/config_service.py b/backend/app/services/config_service.py index 9b57c16..5609538 100644 --- a/backend/app/services/config_service.py +++ b/backend/app/services/config_service.py @@ -29,6 +29,8 @@ if TYPE_CHECKING: from app import __version__ from app.exceptions import ConfigOperationError, ConfigValidationError, JailNotFoundError +from app.helpers.log_helpers import preview_log as util_preview_log +from app.helpers.log_helpers import test_regex as util_test_regex from app.models.config import ( AddLogPathRequest, BantimeEscalation, @@ -48,8 +50,6 @@ from app.models.config import ( ServiceStatusResponse, ) from app.utils.fail2ban_client import Fail2BanClient -from app.utils.log_utils import preview_log as util_preview_log -from app.utils.log_utils import test_regex as util_test_regex from app.utils.setup_utils import ( get_map_color_thresholds as util_get_map_color_thresholds, ) diff --git a/backend/app/services/filter_config_service.py b/backend/app/services/filter_config_service.py index 4ad2cac..cff8a6b 100644 --- a/backend/app/services/filter_config_service.py +++ b/backend/app/services/filter_config_service.py @@ -18,6 +18,11 @@ from pathlib import Path import structlog from app.exceptions import FilterInvalidRegexError +from app.helpers.config_file_helpers import ( + _get_active_jail_names, + _parse_jails_sync, +) +from app.helpers.jail_helpers import reload_jails from app.models.config import ( AssignFilterRequest, FilterConfig, @@ -28,11 +33,6 @@ from app.models.config import ( ) from app.services.config_file_service import _TRUE_VALUES, ConfigWriteError, JailNotFoundInConfigError from app.utils import conffile_parser -from app.utils.config_file_utils import ( - _get_active_jail_names, - _parse_jails_sync, -) -from app.utils.jail_utils import reload_jails log: structlog.stdlib.BoundLogger = structlog.get_logger() diff --git a/backend/app/services/jail_config_service.py b/backend/app/services/jail_config_service.py index 55c1d79..398f2d4 100644 --- a/backend/app/services/jail_config_service.py +++ b/backend/app/services/jail_config_service.py @@ -21,6 +21,13 @@ from typing import cast import structlog from app.exceptions import JailNotFoundError +from app.helpers.config_file_helpers import ( + _build_inactive_jail, + _get_active_jail_names, + _parse_jails_sync, + _validate_jail_config_sync, +) +from app.helpers.jail_helpers import reload_jails from app.models.config import ( ActivateJailRequest, InactiveJail, @@ -29,14 +36,7 @@ from app.models.config import ( JailValidationResult, RollbackResponse, ) -from app.utils.config_file_utils import ( - _build_inactive_jail, - _get_active_jail_names, - _parse_jails_sync, - _validate_jail_config_sync, -) from app.utils.fail2ban_client import Fail2BanClient -from app.utils.jail_utils import reload_jails log: structlog.stdlib.BoundLogger = structlog.get_logger() @@ -461,7 +461,7 @@ async def start_daemon(start_cmd_parts: list[str]) -> bool: return False -# Shared functions from config_file_service are imported from app.utils.config_file_utils +# Shared functions from config_file_service are imported from app.helpers.config_file_helpers # ---------------------------------------------------------------------------