diff --git a/Docs/Refactoring.md b/Docs/Refactoring.md index 4f7dd59..b7dd8dd 100644 --- a/Docs/Refactoring.md +++ b/Docs/Refactoring.md @@ -7,4 +7,5 @@ This document catalogues architecture violations, code smells, and structural is ## Completed Refactors - Moved `Fail2BanConnectionError` and `Fail2BanProtocolError` from `backend/app/utils/fail2ban_client.py` into `backend/app/exceptions.py`. Updated all router, service, and test call sites to import these domain exceptions from `app.exceptions` and retained backward compatibility through re-exporting in `app.utils.fail2ban_client`. +- Moved config file exceptions (`ConfigDirError`, `ConfigFileNotFoundError`, `ConfigFileExistsError`, `ConfigFileWriteError`, `ConfigFileNameError`) from `backend/app/services/raw_config_io_service.py` into `backend/app/exceptions.py`. Updated router and tests to import the shared domain exceptions from `app.exceptions`. diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 9d89a1b..67aee8c 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -44,6 +44,8 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. **Why this is needed:** Routers should not reach into service internals to import exception types. All domain exceptions belong in the central registry so `main.py` global handlers and any future middleware can catch them uniformly. +**Status:** Completed ✅ + --- ### Task 3 — Eliminate the `config_file_service.py` delegation façade diff --git a/backend/app/exceptions.py b/backend/app/exceptions.py index 0db055a..d764e29 100644 --- a/backend/app/exceptions.py +++ b/backend/app/exceptions.py @@ -23,6 +23,44 @@ class ConfigOperationError(Exception): """Raised when a config payload update or command fails.""" +class ConfigDirError(Exception): + """Raised when the fail2ban config directory is missing or inaccessible.""" + + +class ConfigFileNotFoundError(Exception): + """Raised when a requested config file does not exist.""" + + def __init__(self, filename: str) -> None: + """Initialize with the filename that was not found. + + Args: + filename: The filename that could not be located. + """ + self.filename = filename + super().__init__(f"Config file not found: {filename!r}") + + +class ConfigFileExistsError(Exception): + """Raised when trying to create a file that already exists.""" + + def __init__(self, filename: str) -> None: + """Initialize with the filename that already exists. + + Args: + filename: The filename that conflicts. + """ + self.filename = filename + super().__init__(f"Config file already exists: {filename!r}") + + +class ConfigFileWriteError(Exception): + """Raised when a file cannot be written (permissions, disk full, etc.).""" + + +class ConfigFileNameError(Exception): + """Raised when a supplied filename is invalid or unsafe.""" + + class ServerOperationError(Exception): """Raised when a server control command (e.g. refresh) fails.""" diff --git a/backend/app/routers/file_config.py b/backend/app/routers/file_config.py index d538dd3..5086a68 100644 --- a/backend/app/routers/file_config.py +++ b/backend/app/routers/file_config.py @@ -52,7 +52,7 @@ from app.models.file_config import ( JailConfigFilesResponse, ) from app.services import raw_config_io_service -from app.services.raw_config_io_service import ( +from app.exceptions import ( ConfigDirError, ConfigFileExistsError, ConfigFileNameError, diff --git a/backend/app/services/raw_config_io_service.py b/backend/app/services/raw_config_io_service.py index 95d1c3e..d773008 100644 --- a/backend/app/services/raw_config_io_service.py +++ b/backend/app/services/raw_config_io_service.py @@ -17,6 +17,13 @@ from __future__ import annotations import asyncio from app.utils.async_utils import run_blocking +from app.exceptions import ( + ConfigDirError, + ConfigFileExistsError, + ConfigFileNameError, + ConfigFileNotFoundError, + ConfigFileWriteError, +) import configparser import re from pathlib import Path @@ -63,44 +70,6 @@ _SAFE_NAME_RE: re.Pattern[str] = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$ # --------------------------------------------------------------------------- -class ConfigDirError(Exception): - """Raised when the fail2ban config directory is missing or inaccessible.""" - - -class ConfigFileNotFoundError(Exception): - """Raised when a requested config file does not exist.""" - - def __init__(self, filename: str) -> None: - """Initialise with the filename that was not found. - - Args: - filename: The filename that could not be located. - """ - self.filename = filename - super().__init__(f"Config file not found: {filename!r}") - - -class ConfigFileExistsError(Exception): - """Raised when trying to create a file that already exists.""" - - def __init__(self, filename: str) -> None: - """Initialise with the filename that already exists. - - Args: - filename: The filename that conflicts. - """ - self.filename = filename - super().__init__(f"Config file already exists: {filename!r}") - - -class ConfigFileWriteError(Exception): - """Raised when a file cannot be written (permissions, disk full, etc.).""" - - -class ConfigFileNameError(Exception): - """Raised when a supplied filename is invalid or unsafe.""" - - # --------------------------------------------------------------------------- # Internal path helpers # --------------------------------------------------------------------------- diff --git a/backend/tests/test_routers/test_file_config.py b/backend/tests/test_routers/test_file_config.py index e8cbed8..610c143 100644 --- a/backend/tests/test_routers/test_file_config.py +++ b/backend/tests/test_routers/test_file_config.py @@ -26,7 +26,7 @@ from app.models.file_config import ( JailConfigFileContent, JailConfigFilesResponse, ) -from app.services.raw_config_io_service import ( +from app.exceptions import ( ConfigDirError, ConfigFileExistsError, ConfigFileNameError, diff --git a/backend/tests/test_services/test_file_config_service.py b/backend/tests/test_services/test_file_config_service.py index 8062f0c..10a8084 100644 --- a/backend/tests/test_services/test_file_config_service.py +++ b/backend/tests/test_services/test_file_config_service.py @@ -8,12 +8,14 @@ import pytest from app.models.config import ActionConfigUpdate, FilterConfigUpdate, JailFileConfigUpdate from app.models.file_config import ConfFileCreateRequest, ConfFileUpdateRequest -from app.services.raw_config_io_service import ( +from app.exceptions import ( ConfigDirError, ConfigFileExistsError, ConfigFileNameError, ConfigFileNotFoundError, ConfigFileWriteError, +) +from app.services.raw_config_io_service import ( _parse_enabled, _set_enabled_in_content, _validate_new_name,