Dedup _resolve_service_record into shared service_resolution module
Extract the duplicated _resolve_service_record helper (identical in routers/monitoring.py and routers/authentik_users.py) into a shared services/service_resolution.py module. Both routers now import resolve_service_record from the shared module. The authentik router previously hardcoded service_type='authentik' in its local copy; the shared helper takes service_type as a param (same as monitoring's did). Tests updated: test_api.py patches now target the correct module paths (resolve_service_record on the monitoring module where it's imported, build_service_record on the service_resolution module). 283 backend tests pass; ruff clean.
This commit is contained in:
@@ -20,8 +20,9 @@ from media_library_viewer_api.config import get_settings
|
||||
from media_library_viewer_api.dependencies import get_mail_queue, get_settings_store
|
||||
from media_library_viewer_api.services.mail_queue import MailQueue
|
||||
from media_library_viewer_api.services.mailer import validate_smtp_settings
|
||||
from media_library_viewer_api.services.service_resolution import resolve_service_record
|
||||
from media_library_viewer_api.services.settings_store import SettingsStore
|
||||
from media_library_viewer_api.widgets.sources import ServiceRecord, build_service_record
|
||||
from media_library_viewer_api.widgets.sources import ServiceRecord
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -36,29 +37,6 @@ class MessageRequest(BaseModel):
|
||||
html_body: str
|
||||
|
||||
|
||||
def _resolve_service_record(
|
||||
store: SettingsStore,
|
||||
service_id: str | None = None,
|
||||
) -> ServiceRecord | None:
|
||||
"""Return the requested authentik instance, else the first enabled one.
|
||||
|
||||
Returns ``None`` when the instance does not exist / is the wrong type, or
|
||||
when no enabled ``authentik`` instance is configured.
|
||||
"""
|
||||
service_type = "authentik"
|
||||
if service_id:
|
||||
row = store.get_service(service_id)
|
||||
if not row or row.get("service_type") != service_type:
|
||||
return None
|
||||
if not row.get("enabled", True):
|
||||
return None
|
||||
return build_service_record(store, row)
|
||||
for row in store.list_services(service_type):
|
||||
if row.get("enabled", True):
|
||||
return build_service_record(store, row)
|
||||
return None
|
||||
|
||||
|
||||
def _build_client(service: ServiceRecord) -> AuthentikClient:
|
||||
base_url = str(service.config.get("base_url") or "").rstrip("/")
|
||||
api_token = str(service.secrets.get("api_token") or "")
|
||||
@@ -82,7 +60,7 @@ def get_authentik_users(
|
||||
store: SettingsStore = Depends(get_settings_store),
|
||||
) -> dict[str, Any]:
|
||||
"""Paginated Authentik user directory for a specific service instance."""
|
||||
service = _resolve_service_record(store, service_id)
|
||||
service = resolve_service_record(store, "authentik", service_id)
|
||||
if service is None:
|
||||
logger.info("Authentik users requested but no enabled authentik service for id=%s", service_id)
|
||||
return _empty("Authentik service not configured")
|
||||
@@ -102,7 +80,7 @@ def get_authentik_message_status(
|
||||
mail_queue: MailQueue = Depends(get_mail_queue),
|
||||
) -> dict[str, Any]:
|
||||
"""Mail-queue status snapshot for the Authentik messaging tab."""
|
||||
service = _resolve_service_record(store, service_id)
|
||||
service = resolve_service_record(store, "authentik", service_id)
|
||||
if service is None:
|
||||
return {"state": "stopped", "worker_running": False, "error": "Authentik service not configured"}
|
||||
return mail_queue.status()
|
||||
@@ -116,7 +94,7 @@ def post_authentik_message(
|
||||
mail_queue: MailQueue = Depends(get_mail_queue),
|
||||
) -> dict[str, Any]:
|
||||
"""Enqueue an email to Authentik-sourced recipients via the mail queue."""
|
||||
service = _resolve_service_record(store, service_id)
|
||||
service = resolve_service_record(store, "authentik", service_id)
|
||||
if service is None:
|
||||
return {"status": "error", "error": "Authentik service not configured"}
|
||||
|
||||
|
||||
@@ -15,34 +15,14 @@ import requests
|
||||
from fastapi import APIRouter, Body, Depends
|
||||
|
||||
from media_library_viewer_api.dependencies import get_settings_store
|
||||
from media_library_viewer_api.services.service_resolution import resolve_service_record
|
||||
from media_library_viewer_api.services.settings_store import SettingsStore
|
||||
from media_library_viewer_api.services.targets import build_node_exporter_targets
|
||||
from media_library_viewer_api.widgets.sources import ServiceRecord, build_service_record
|
||||
from media_library_viewer_api.widgets.sources import ServiceRecord
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _resolve_service_record(
|
||||
store: SettingsStore, service_type: str, service_id: str | None = None
|
||||
) -> ServiceRecord | None:
|
||||
"""Return the requested service instance, else the first enabled one.
|
||||
|
||||
Returns ``None`` when the instance does not exist / is the wrong type, or
|
||||
when no enabled instance of ``service_type`` is configured.
|
||||
"""
|
||||
if service_id:
|
||||
row = store.get_service(service_id)
|
||||
if not row or row.get("service_type") != service_type:
|
||||
return None
|
||||
if not row.get("enabled", True):
|
||||
return None
|
||||
return build_service_record(store, row)
|
||||
for row in store.list_services(service_type):
|
||||
if row.get("enabled", True):
|
||||
return build_service_record(store, row)
|
||||
return None
|
||||
|
||||
|
||||
def _base_url(service: ServiceRecord) -> str:
|
||||
return str(service.config.get("base_url") or "").rstrip("/")
|
||||
|
||||
@@ -104,7 +84,7 @@ def get_alertmanager_alerts(
|
||||
is configured the endpoint returns an empty summary with an
|
||||
``alertmanager_not_configured`` error so the UI can render a health card.
|
||||
"""
|
||||
service = _resolve_service_record(store, "alertmanager", service_id)
|
||||
service = resolve_service_record(store, "alertmanager", service_id)
|
||||
if service is None:
|
||||
return {"total": 0, "by_severity": {}, "alerts": [], "error": "alertmanager_not_configured"}
|
||||
try:
|
||||
@@ -149,7 +129,7 @@ def get_alertmanager_status(
|
||||
store: SettingsStore = Depends(get_settings_store),
|
||||
) -> dict[str, Any]:
|
||||
"""Return Alertmanager cluster/status for the UI health card."""
|
||||
service = _resolve_service_record(store, "alertmanager", service_id)
|
||||
service = resolve_service_record(store, "alertmanager", service_id)
|
||||
if service is None:
|
||||
return {
|
||||
"up": False,
|
||||
@@ -198,7 +178,7 @@ def get_grafana_status(
|
||||
store: SettingsStore = Depends(get_settings_store),
|
||||
) -> dict[str, Any]:
|
||||
"""Probe a Grafana service instance's ``/api/health`` endpoint."""
|
||||
service = _resolve_service_record(store, "grafana", service_id)
|
||||
service = resolve_service_record(store, "grafana", service_id)
|
||||
if service is None:
|
||||
return _status_response(None, error="no_service_configured")
|
||||
try:
|
||||
@@ -221,7 +201,7 @@ def get_prometheus_status(
|
||||
store: SettingsStore = Depends(get_settings_store),
|
||||
) -> dict[str, Any]:
|
||||
"""Probe a Prometheus service instance's health and build info."""
|
||||
service = _resolve_service_record(store, "prometheus", service_id)
|
||||
service = resolve_service_record(store, "prometheus", service_id)
|
||||
if service is None:
|
||||
return _status_response(None, error="no_service_configured")
|
||||
base = _base_url(service)
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
"""Shared helpers for resolving service instances at request time.
|
||||
|
||||
Extracted from the duplicated ``_resolve_service_record`` helpers that lived
|
||||
in ``routers/monitoring.py`` and ``routers/authentik_users.py``. Both routers
|
||||
need the same logic: return the requested service instance (by id), or fall
|
||||
back to the first enabled instance of the type. Returns ``None`` when the
|
||||
instance does not exist, is the wrong type, is disabled, or when no enabled
|
||||
instance of the type is configured.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from media_library_viewer_api.services.settings_store import SettingsStore
|
||||
from media_library_viewer_api.widgets.sources import ServiceRecord, build_service_record
|
||||
|
||||
|
||||
def resolve_service_record(
|
||||
store: SettingsStore,
|
||||
service_type: str,
|
||||
service_id: str | None = None,
|
||||
) -> ServiceRecord | None:
|
||||
"""Return the requested service instance, else the first enabled one.
|
||||
|
||||
Returns ``None`` when the instance does not exist / is the wrong type, or
|
||||
when no enabled instance of ``service_type`` is configured.
|
||||
"""
|
||||
if service_id:
|
||||
row = store.get_service(service_id)
|
||||
if not row or row.get("service_type") != service_type:
|
||||
return None
|
||||
if not row.get("enabled", True):
|
||||
return None
|
||||
return build_service_record(store, row)
|
||||
for row in store.list_services(service_type):
|
||||
if row.get("enabled", True):
|
||||
return build_service_record(store, row)
|
||||
return None
|
||||
+23
-22
@@ -26,6 +26,7 @@ from media_library_viewer_api.widgets.sources import ServiceRecord
|
||||
|
||||
# Short alias for the monitoring router module under test.
|
||||
_MON = "media_library_viewer_api.routers.monitoring"
|
||||
_SVC = "media_library_viewer_api.services.service_resolution"
|
||||
|
||||
# --- Fixtures ---
|
||||
|
||||
@@ -496,7 +497,7 @@ class TestMonitoring:
|
||||
|
||||
|
||||
class TestResolveServiceRecord:
|
||||
"""Unit tests for _resolve_service_record (service_id + first-enabled paths)."""
|
||||
"""Unit tests for resolve_service_record (service_id + first-enabled paths)."""
|
||||
|
||||
def _store(self, rows):
|
||||
store = MagicMock()
|
||||
@@ -509,31 +510,31 @@ class TestResolveServiceRecord:
|
||||
return store
|
||||
|
||||
def test_service_id_match_returns_record(self):
|
||||
from media_library_viewer_api.routers.monitoring import _resolve_service_record
|
||||
from media_library_viewer_api.services.service_resolution import resolve_service_record
|
||||
|
||||
row = {"id": "am1", "service_type": "alertmanager", "name": "AM", "enabled": True, "config": {}, "secrets": {}}
|
||||
store = self._store([row])
|
||||
with patch(f"{_MON}.build_service_record", return_value="RECORD") as mock_build:
|
||||
result = _resolve_service_record(store, "alertmanager", "am1")
|
||||
with patch(f"{_SVC}.build_service_record", return_value="RECORD") as mock_build:
|
||||
result = resolve_service_record(store, "alertmanager", "am1")
|
||||
assert result == "RECORD"
|
||||
mock_build.assert_called_once_with(store, row)
|
||||
|
||||
def test_service_id_type_mismatch_returns_none(self):
|
||||
from media_library_viewer_api.routers.monitoring import _resolve_service_record
|
||||
from media_library_viewer_api.services.service_resolution import resolve_service_record
|
||||
|
||||
row = {"id": "x1", "service_type": "grafana", "name": "G", "enabled": True, "config": {}, "secrets": {}}
|
||||
store = self._store([row])
|
||||
assert _resolve_service_record(store, "alertmanager", "x1") is None
|
||||
assert resolve_service_record(store, "alertmanager", "x1") is None
|
||||
|
||||
def test_service_id_disabled_returns_none(self):
|
||||
from media_library_viewer_api.routers.monitoring import _resolve_service_record
|
||||
from media_library_viewer_api.services.service_resolution import resolve_service_record
|
||||
|
||||
row = {"id": "am1", "service_type": "alertmanager", "name": "AM", "enabled": False, "config": {}, "secrets": {}}
|
||||
store = self._store([row])
|
||||
assert _resolve_service_record(store, "alertmanager", "am1") is None
|
||||
assert resolve_service_record(store, "alertmanager", "am1") is None
|
||||
|
||||
def test_no_service_id_returns_first_enabled(self):
|
||||
from media_library_viewer_api.routers.monitoring import _resolve_service_record
|
||||
from media_library_viewer_api.services.service_resolution import resolve_service_record
|
||||
|
||||
rows = [
|
||||
{
|
||||
@@ -554,16 +555,16 @@ class TestResolveServiceRecord:
|
||||
},
|
||||
]
|
||||
store = self._store(rows)
|
||||
with patch(f"{_MON}.build_service_record", return_value="RECORD") as mock_build:
|
||||
result = _resolve_service_record(store, "alertmanager", None)
|
||||
with patch(f"{_SVC}.build_service_record", return_value="RECORD") as mock_build:
|
||||
result = resolve_service_record(store, "alertmanager", None)
|
||||
assert result == "RECORD"
|
||||
mock_build.assert_called_once_with(store, rows[1])
|
||||
|
||||
def test_no_service_id_and_none_enabled_returns_none(self):
|
||||
from media_library_viewer_api.routers.monitoring import _resolve_service_record
|
||||
from media_library_viewer_api.services.service_resolution import resolve_service_record
|
||||
|
||||
store = self._store([])
|
||||
assert _resolve_service_record(store, "alertmanager", None) is None
|
||||
assert resolve_service_record(store, "alertmanager", None) is None
|
||||
|
||||
|
||||
class TestSettingsMachines:
|
||||
@@ -624,7 +625,7 @@ class TestAlertmanager:
|
||||
def test_alerts_endpoint_when_unreachable(self, test_client):
|
||||
service = _am_service()
|
||||
with (
|
||||
patch(f"{_MON}._resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.requests.get", side_effect=Exception("connection refused")),
|
||||
):
|
||||
response = test_client.get("/api/monitoring/alerts")
|
||||
@@ -651,7 +652,7 @@ class TestAlertmanager:
|
||||
}
|
||||
resp.raise_for_status = MagicMock()
|
||||
with (
|
||||
patch(f"{_MON}._resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.requests.get", return_value=resp),
|
||||
):
|
||||
response = test_client.get("/api/monitoring/alerts")
|
||||
@@ -669,7 +670,7 @@ class TestAlertmanager:
|
||||
resp.json.return_value = {"status": "success", "data": []}
|
||||
resp.raise_for_status = MagicMock()
|
||||
with (
|
||||
patch(f"{_MON}._resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.requests.get", return_value=resp) as mock_get,
|
||||
):
|
||||
test_client.get("/api/monitoring/alerts")
|
||||
@@ -687,7 +688,7 @@ class TestAlertmanager:
|
||||
def test_alertmanager_status_when_unreachable(self, test_client):
|
||||
service = _am_service()
|
||||
with (
|
||||
patch(f"{_MON}._resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.requests.get", side_effect=Exception("refused")),
|
||||
):
|
||||
response = test_client.get("/api/monitoring/alertmanager-status")
|
||||
@@ -707,7 +708,7 @@ class TestAlertmanager:
|
||||
}
|
||||
resp.raise_for_status = MagicMock()
|
||||
with (
|
||||
patch(f"{_MON}._resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.requests.get", return_value=resp),
|
||||
):
|
||||
response = test_client.get("/api/monitoring/alertmanager-status")
|
||||
@@ -753,7 +754,7 @@ class TestGrafanaStatus:
|
||||
id="g1", service_type="grafana", name="Grafana", config={"base_url": "http://grafana:3000"}
|
||||
)
|
||||
with (
|
||||
patch(f"{_MON}._resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.requests.get", side_effect=Exception("refused")),
|
||||
):
|
||||
response = test_client.get("/api/monitoring/grafana-status")
|
||||
@@ -771,7 +772,7 @@ class TestGrafanaStatus:
|
||||
resp.json.return_value = {"version": "11.3.1", "database": "ok"}
|
||||
resp.raise_for_status = MagicMock()
|
||||
with (
|
||||
patch(f"{_MON}._resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.requests.get", return_value=resp),
|
||||
):
|
||||
response = test_client.get("/api/monitoring/grafana-status")
|
||||
@@ -795,7 +796,7 @@ class TestPrometheusStatus:
|
||||
id="p1", service_type="prometheus", name="Prometheus", config={"base_url": "http://prometheus:9090"}
|
||||
)
|
||||
with (
|
||||
patch(f"{_MON}._resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.requests.get", side_effect=Exception("refused")),
|
||||
):
|
||||
response = test_client.get("/api/monitoring/prometheus-status")
|
||||
@@ -814,7 +815,7 @@ class TestPrometheusStatus:
|
||||
build_info.raise_for_status = MagicMock()
|
||||
build_info.json.return_value = {"status": "success", "data": {"version": "2.55.1"}}
|
||||
with (
|
||||
patch(f"{_MON}._resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.resolve_service_record", return_value=service),
|
||||
patch(f"{_MON}.requests.get", side_effect=[health, build_info]),
|
||||
):
|
||||
response = test_client.get("/api/monitoring/prometheus-status")
|
||||
|
||||
Reference in New Issue
Block a user