Mark startup runtime configuration task complete and update startup resource resolution

This commit is contained in:
2026-04-10 21:13:51 +02:00
parent f61d497e4e
commit 91e5792caf
3 changed files with 106 additions and 25 deletions

View File

@@ -116,5 +116,6 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
- Issue: `startup_shared_resources` creates shared resources like `aiohttp.ClientSession` and geo cache initialization from the initial environment-loaded settings, then later applies persisted runtime overrides to `app.state.settings`, producing a fragile startup ordering. - Issue: `startup_shared_resources` creates shared resources like `aiohttp.ClientSession` and geo cache initialization from the initial environment-loaded settings, then later applies persisted runtime overrides to `app.state.settings`, producing a fragile startup ordering.
- Propose: Split startup into phases that first resolve bootstrap and runtime persisted configuration, then construct shared resources and register scheduled jobs using those effective settings. - Propose: Split startup into phases that first resolve bootstrap and runtime persisted configuration, then construct shared resources and register scheduled jobs using those effective settings.
- Test: Add startup tests asserting that when persisted runtime settings differ from bootstrap settings, the final initialized resources are built from the resolved effective settings, not the original bootstrap values. - Test: Add startup tests asserting that when persisted runtime settings differ from bootstrap settings, the final initialized resources are built from the resolved effective settings, not the original bootstrap values.
- Status: completed

View File

@@ -9,13 +9,12 @@ from __future__ import annotations
from contextlib import suppress from contextlib import suppress
from pathlib import Path from pathlib import Path
from typing import TYPE_CHECKING
import aiohttp import aiohttp
import structlog import structlog
from apscheduler.schedulers.asyncio import AsyncIOScheduler # type: ignore[import-untyped] from apscheduler.schedulers.asyncio import AsyncIOScheduler # type: ignore[import-untyped]
from fastapi import FastAPI
from app.config import Settings
from app.db import init_db, open_db from app.db import init_db, open_db
from app.services import geo_service, setup_service from app.services import geo_service, setup_service
from app.tasks import blocklist_import, geo_cache_flush, geo_re_resolve, health_check, history_sync from app.tasks import blocklist_import, geo_cache_flush, geo_re_resolve, health_check, history_sync
@@ -23,6 +22,11 @@ from app.utils.jail_config import ensure_jail_configs
from app.utils.runtime_state import set_runtime_settings from app.utils.runtime_state import set_runtime_settings
from app.utils.setup_state import set_setup_complete_cache from app.utils.setup_state import set_setup_complete_cache
if TYPE_CHECKING:
from fastapi import FastAPI
from app.config import Settings
log: structlog.stdlib.BoundLogger = structlog.get_logger() log: structlog.stdlib.BoundLogger = structlog.get_logger()
@@ -64,8 +68,6 @@ async def startup_shared_resources(
Returns: Returns:
A tuple of ``(http_session, scheduler)``. A tuple of ``(http_session, scheduler)``.
""" """
ensure_jail_configs(Path(settings.fail2ban_config_dir) / "jail.d")
db_path: Path = Path(settings.database_path) db_path: Path = Path(settings.database_path)
db_path.parent.mkdir(parents=True, exist_ok=True) db_path.parent.mkdir(parents=True, exist_ok=True)
@@ -75,8 +77,6 @@ async def startup_shared_resources(
startup_db = await open_db(settings.database_path) startup_db = await open_db(settings.database_path)
try: try:
await init_db(startup_db) await init_db(startup_db)
await geo_service.load_cache_from_db(startup_db)
unresolved_count = await geo_service.count_unresolved(startup_db)
setup_complete = await setup_service.is_setup_complete(startup_db) setup_complete = await setup_service.is_setup_complete(startup_db)
set_setup_complete_cache(app, setup_complete) set_setup_complete_cache(app, setup_complete)
log.debug("setup_completion_cached", completed=setup_complete) log.debug("setup_completion_cached", completed=setup_complete)
@@ -95,9 +95,22 @@ async def startup_shared_resources(
"runtime_settings_overridden_from_setup", "runtime_settings_overridden_from_setup",
overrides=persisted_runtime_settings, overrides=persisted_runtime_settings,
) )
if Path(settings.database_path).resolve() != original_db_path:
runtime_db = await open_db(settings.database_path)
try:
await geo_service.load_cache_from_db(runtime_db)
unresolved_count = await geo_service.count_unresolved(runtime_db)
finally:
await runtime_db.close()
else:
await geo_service.load_cache_from_db(startup_db)
unresolved_count = await geo_service.count_unresolved(startup_db)
finally: finally:
await startup_db.close() await startup_db.close()
ensure_jail_configs(Path(settings.fail2ban_config_dir) / "jail.d")
if unresolved_count > 0: if unresolved_count > 0:
log.warning("geo_cache_unresolved_ips", unresolved=unresolved_count) log.warning("geo_cache_unresolved_ips", unresolved=unresolved_count)

View File

@@ -4,8 +4,8 @@ import asyncio
from pathlib import Path from pathlib import Path
from unittest.mock import AsyncMock, MagicMock, patch from unittest.mock import AsyncMock, MagicMock, patch
import pytest
import aiosqlite import aiosqlite
import pytest
from httpx import ASGITransport, AsyncClient from httpx import ASGITransport, AsyncClient
from app.config import Settings from app.config import Settings
@@ -166,24 +166,22 @@ async def test_lifespan_cleans_up_resources_when_startup_fails(tmp_path: Path) -
mock_http_session = MagicMock() mock_http_session = MagicMock()
mock_http_session.close = AsyncMock() mock_http_session.close = AsyncMock()
with ( with pytest.raises(RuntimeError, match="startup failed"), \
patch("app.startup.ensure_jail_configs"), patch("app.startup.ensure_jail_configs"), \
patch("app.startup.aiohttp.ClientSession", return_value=mock_http_session), patch("app.startup.aiohttp.ClientSession", return_value=mock_http_session), \
patch("app.startup.AsyncIOScheduler", return_value=mock_scheduler), patch("app.startup.AsyncIOScheduler", return_value=mock_scheduler), \
patch("app.startup.init_db", new=AsyncMock()), patch("app.startup.init_db", new=AsyncMock()), \
patch("app.services.geo_service.init_geoip"), patch("app.services.geo_service.init_geoip"), \
patch("app.services.geo_service.load_cache_from_db", new=AsyncMock(return_value=None)), patch("app.services.geo_service.load_cache_from_db", new=AsyncMock(return_value=None)), \
patch("app.services.geo_service.count_unresolved", new=AsyncMock(return_value=0)), patch("app.services.geo_service.count_unresolved", new=AsyncMock(return_value=0)), \
patch("app.services.setup_service.is_setup_complete", new=AsyncMock(return_value=False)), patch("app.services.setup_service.is_setup_complete", new=AsyncMock(return_value=False)), \
patch("app.tasks.health_check.register", side_effect=RuntimeError("startup failed")), patch("app.tasks.health_check.register", side_effect=RuntimeError("startup failed")), \
patch("app.tasks.blocklist_import.register"), patch("app.tasks.blocklist_import.register"), \
patch("app.tasks.geo_cache_flush.register"), patch("app.tasks.geo_cache_flush.register"), \
patch("app.tasks.geo_re_resolve.register"), patch("app.tasks.geo_re_resolve.register"), \
patch("app.tasks.history_sync.register"), patch("app.tasks.history_sync.register"):
): async with _lifespan(app):
with pytest.raises(RuntimeError, match="startup failed"): pass
async with _lifespan(app):
pass
mock_http_session.close.assert_awaited_once() mock_http_session.close.assert_awaited_once()
mock_scheduler.shutdown.assert_called_once_with(wait=False) mock_scheduler.shutdown.assert_called_once_with(wait=False)
@@ -297,6 +295,75 @@ async def test_startup_overrides_settings_from_persisted_setup(tmp_path: Path) -
assert Path(runtime_db_path).exists() assert Path(runtime_db_path).exists()
async def test_startup_loads_geo_cache_from_persisted_runtime_database(tmp_path: Path) -> None:
"""Startup must load geo cache from the resolved runtime database."""
env_settings = Settings(
database_path=str(tmp_path / "pointer.db"),
fail2ban_socket="/tmp/fake_fail2ban.sock",
fail2ban_config_dir=str(tmp_path / "fail2ban"),
session_secret="test-startup-secret",
session_duration_minutes=60,
timezone="UTC",
log_level="debug",
)
app = create_app(settings=env_settings)
runtime_db_path = str(tmp_path / "runtime.db")
opened_connections: list[tuple[str, aiosqlite.Connection]] = []
async def fake_open_db(path: str) -> aiosqlite.Connection:
connection = await aiosqlite.connect(path)
opened_connections.append((path, connection))
return connection
mock_scheduler = MagicMock()
mock_scheduler.start = MagicMock()
mock_scheduler.shutdown = MagicMock()
mock_http_session = MagicMock()
mock_http_session.close = AsyncMock()
load_cache = AsyncMock()
with (
patch("app.startup.ensure_jail_configs"),
patch("app.startup.aiohttp.ClientSession", return_value=mock_http_session),
patch("app.startup.AsyncIOScheduler", return_value=mock_scheduler),
patch("app.startup.open_db", new=AsyncMock(side_effect=fake_open_db)),
patch("app.startup.init_db", new=AsyncMock()),
patch("app.services.geo_service.init_geoip"),
patch("app.services.geo_service.load_cache_from_db", new=load_cache),
patch("app.services.geo_service.count_unresolved", new=AsyncMock(return_value=0)),
patch("app.services.setup_service.is_setup_complete", new=AsyncMock(return_value=True)),
patch(
"app.services.setup_service.get_persisted_runtime_settings",
new=AsyncMock(
return_value={
"database_path": runtime_db_path,
"fail2ban_socket": "/tmp/persisted.sock",
"timezone": "Europe/Berlin",
"session_duration_minutes": 123,
}
),
),
patch("app.tasks.health_check.register"),
patch("app.tasks.blocklist_import.register"),
patch("app.tasks.geo_cache_flush.register"),
patch("app.tasks.geo_re_resolve.register"),
patch("app.tasks.history_sync.register"),
):
async with _lifespan(app):
loaded_db = load_cache.call_args.args[0]
runtime_connections = [conn for path, conn in opened_connections if path == runtime_db_path]
assert runtime_connections, "Expected runtime database to be opened"
assert loaded_db in runtime_connections
assert app.state.runtime_settings is not None
assert app.state.runtime_settings.database_path == runtime_db_path
for _, connection in opened_connections:
await connection.close()
async def test_concurrent_requests_use_request_scoped_db_connections(tmp_path: Path) -> None: async def test_concurrent_requests_use_request_scoped_db_connections(tmp_path: Path) -> None:
"""Concurrent requests each open and close their own database connection.""" """Concurrent requests each open and close their own database connection."""
settings = Settings( settings = Settings(