From 4754f1407e37a2936f7e10c575c404dcda17bbc9 Mon Sep 17 00:00:00 2001 From: Lukas Date: Fri, 17 Apr 2026 17:04:09 +0200 Subject: [PATCH] Mark task 19 done and centralize map-color thresholds in settings_service --- Docs/Tasks.md | 2 + backend/app/services/setup_service.py | 33 --------- .../test_services/test_settings_service.py | 72 +++++++++++++++++++ .../tests/test_services/test_setup_service.py | 72 +------------------ 4 files changed, 76 insertions(+), 103 deletions(-) create mode 100644 backend/tests/test_services/test_settings_service.py diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 659fcf0..38d53c3 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -365,6 +365,8 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. **Docs changes needed:** Update `Docs/Refactoring.md`. +**Status:** Completed ✅ + **Why this is needed:** Using a first-run setup service for ongoing runtime operations misleads developers into modifying the wrong service when behaviour needs to change. It also creates an implicit dependency between setup state and runtime configuration. --- diff --git a/backend/app/services/setup_service.py b/backend/app/services/setup_service.py index f02c4bc..c1c0185 100644 --- a/backend/app/services/setup_service.py +++ b/backend/app/services/setup_service.py @@ -15,12 +15,6 @@ import structlog from app.db import init_db, open_db from app.repositories import settings_repo -from app.services.settings_service import ( - get_map_color_thresholds as util_get_map_color_thresholds, -) -from app.services.settings_service import ( - set_map_color_thresholds as util_set_map_color_thresholds, -) from app.utils.async_utils import run_blocking if TYPE_CHECKING: @@ -195,30 +189,3 @@ async def get_timezone(db: aiosqlite.Connection) -> str: return tz if tz else "UTC" -async def get_map_color_thresholds( - db: aiosqlite.Connection, -) -> tuple[int, int, int]: - """Return the configured map color thresholds (high, medium, low).""" - return await util_get_map_color_thresholds(db) - - -async def set_map_color_thresholds( - db: aiosqlite.Connection, - *, - threshold_high: int, - threshold_medium: int, - threshold_low: int, -) -> None: - """Update the map color threshold configuration.""" - await util_set_map_color_thresholds( - db, - threshold_high=threshold_high, - threshold_medium=threshold_medium, - threshold_low=threshold_low, - ) - log.info( - "map_color_thresholds_updated", - high=threshold_high, - medium=threshold_medium, - low=threshold_low, - ) diff --git a/backend/tests/test_services/test_settings_service.py b/backend/tests/test_services/test_settings_service.py new file mode 100644 index 0000000..3109e6f --- /dev/null +++ b/backend/tests/test_services/test_settings_service.py @@ -0,0 +1,72 @@ +"""Tests for settings_service runtime settings helpers.""" + +from __future__ import annotations + +from pathlib import Path + +import aiosqlite +import pytest + +from app.db import init_db +from app.services import settings_service + + +@pytest.fixture +async def db(tmp_path: Path) -> aiosqlite.Connection: # type: ignore[misc] + """Provide an initialised aiosqlite connection for settings service tests.""" + conn: aiosqlite.Connection = await aiosqlite.connect(str(tmp_path / "test.db")) + conn.row_factory = aiosqlite.Row + await init_db(conn) + yield conn + await conn.close() + + +class TestMapColorThresholds: + async def test_get_map_color_thresholds_returns_defaults_on_fresh_db( + self, db: aiosqlite.Connection + ) -> None: + """get_map_color_thresholds() returns default values on a fresh database.""" + high, medium, low = await settings_service.get_map_color_thresholds(db) + + assert high == 100 + assert medium == 50 + assert low == 20 + + async def test_set_map_color_thresholds_persists_values( + self, db: aiosqlite.Connection + ) -> None: + """set_map_color_thresholds() stores and retrieves custom values.""" + await settings_service.set_map_color_thresholds( + db, threshold_high=200, threshold_medium=80, threshold_low=30 + ) + + high, medium, low = await settings_service.get_map_color_thresholds(db) + assert high == 200 + assert medium == 80 + assert low == 30 + + async def test_set_map_color_thresholds_rejects_non_positive( + self, db: aiosqlite.Connection + ) -> None: + """set_map_color_thresholds() raises ValueError for non-positive thresholds.""" + with pytest.raises(ValueError, match="positive integers"): + await settings_service.set_map_color_thresholds( + db, threshold_high=100, threshold_medium=50, threshold_low=0 + ) + with pytest.raises(ValueError, match="positive integers"): + await settings_service.set_map_color_thresholds( + db, threshold_high=-10, threshold_medium=50, threshold_low=20 + ) + + async def test_set_map_color_thresholds_rejects_invalid_order( + self, db: aiosqlite.Connection + ) -> None: + """set_map_color_thresholds() rejects invalid ordering.""" + with pytest.raises(ValueError, match="high > medium > low"): + await settings_service.set_map_color_thresholds( + db, threshold_high=50, threshold_medium=50, threshold_low=20 + ) + with pytest.raises(ValueError, match="high > medium > low"): + await settings_service.set_map_color_thresholds( + db, threshold_high=100, threshold_medium=30, threshold_low=50 + ) diff --git a/backend/tests/test_services/test_setup_service.py b/backend/tests/test_services/test_setup_service.py index 59a359a..bf7f775 100644 --- a/backend/tests/test_services/test_setup_service.py +++ b/backend/tests/test_services/test_setup_service.py @@ -12,7 +12,7 @@ import pytest from app.db import init_db from app.repositories import settings_repo -from app.services import setup_service +from app.services import settings_service, setup_service @pytest.fixture @@ -171,7 +171,7 @@ class TestRunSetup: timezone="UTC", session_duration_minutes=60, ) - high, medium, low = await setup_service.get_map_color_thresholds(db) + high, medium, low = await settings_service.get_map_color_thresholds(db) assert high == 100 assert medium == 50 assert low == 20 @@ -197,74 +197,6 @@ class TestGetTimezone: assert await setup_service.get_timezone(db) == "America/New_York" -class TestMapColorThresholds: - async def test_get_map_color_thresholds_returns_defaults_on_fresh_db( - self, db: aiosqlite.Connection - ) -> None: - """get_map_color_thresholds() returns default values on a fresh database.""" - high, medium, low = await setup_service.get_map_color_thresholds(db) - assert high == 100 - assert medium == 50 - assert low == 20 - - async def test_set_map_color_thresholds_persists_values( - self, db: aiosqlite.Connection - ) -> None: - """set_map_color_thresholds() stores and retrieves custom values.""" - await setup_service.set_map_color_thresholds( - db, threshold_high=200, threshold_medium=80, threshold_low=30 - ) - high, medium, low = await setup_service.get_map_color_thresholds(db) - assert high == 200 - assert medium == 80 - assert low == 30 - - async def test_set_map_color_thresholds_rejects_non_positive( - self, db: aiosqlite.Connection - ) -> None: - """set_map_color_thresholds() raises ValueError for non-positive thresholds.""" - with pytest.raises(ValueError, match="positive integers"): - await setup_service.set_map_color_thresholds( - db, threshold_high=100, threshold_medium=50, threshold_low=0 - ) - with pytest.raises(ValueError, match="positive integers"): - await setup_service.set_map_color_thresholds( - db, threshold_high=-10, threshold_medium=50, threshold_low=20 - ) - - async def test_set_map_color_thresholds_rejects_invalid_order( - self, db: aiosqlite.Connection - ) -> None: - """ - set_map_color_thresholds() rejects invalid ordering. - """ - with pytest.raises(ValueError, match="high > medium > low"): - await setup_service.set_map_color_thresholds( - db, threshold_high=50, threshold_medium=50, threshold_low=20 - ) - with pytest.raises(ValueError, match="high > medium > low"): - await setup_service.set_map_color_thresholds( - db, threshold_high=100, threshold_medium=30, threshold_low=50 - ) - - async def test_run_setup_initializes_default_thresholds( - self, db: aiosqlite.Connection, tmp_path: Path - ) -> None: - """run_setup() initializes map color thresholds with defaults.""" - await setup_service.run_setup( - db, - master_password="mypassword1", - database_path=str(tmp_path / "test.db"), - fail2ban_socket="/var/run/fail2ban/fail2ban.sock", - timezone="UTC", - session_duration_minutes=60, - ) - high, medium, low = await setup_service.get_map_color_thresholds(db) - assert high == 100 - assert medium == 50 - assert low == 20 - - class TestRunSetupAsync: """Verify the async/non-blocking bcrypt behavior of run_setup."""