From 1fc04ed978ce04bcfbeff81fdcd63f3f209c110b Mon Sep 17 00:00:00 2001 From: Lukas Date: Tue, 7 Apr 2026 20:48:29 +0200 Subject: [PATCH] Extract startup resource initialization from main.py Move lifespan startup logic into app.startup and remove local imports from app.main._lifespan. Mark startup wiring issue as done. --- Docs/Tasks.md | 2 +- backend/app/main.py | 62 ++------------------------------ backend/app/startup.py | 73 ++++++++++++++++++++++++++++++++++++++ backend/tests/test_main.py | 18 +++++----- 4 files changed, 86 insertions(+), 69 deletions(-) create mode 100644 backend/app/startup.py diff --git a/Docs/Tasks.md b/Docs/Tasks.md index cddff67..e8cc377 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -29,7 +29,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. - Fix: document the single-process limitation clearly in code and project docs. - Expected outcome: authentication behavior is consistent across deployment modes, and session invalidation works correctly in multi-worker setups. -- `backend/app/main.py` uses local imports inside `_lifespan()` to avoid circular dependencies, indicating that startup logic is tightly coupled with services. +- Status: **done** — `backend/app/main.py` uses local imports inside `_lifespan()` to avoid circular dependencies, indicating that startup logic is tightly coupled with services. - Fix: evaluate whether the startup initialization can be moved into a dedicated `startup.py` or split service initialization into smaller modules; keep import order simple and explicit. - Expected outcome: cleaner startup code with lower coupling, fewer hidden circular import risks, and easier maintenance. diff --git a/backend/app/main.py b/backend/app/main.py index 5174c78..a6f701f 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -14,7 +14,6 @@ from __future__ import annotations import logging import sys from contextlib import asynccontextmanager -from pathlib import Path from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -22,9 +21,7 @@ if TYPE_CHECKING: from starlette.responses import Response as StarletteResponse -import aiohttp import structlog -from apscheduler.schedulers.asyncio import AsyncIOScheduler # type: ignore[import-untyped] from fastapi import FastAPI, Request, status from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse, RedirectResponse @@ -32,7 +29,6 @@ from starlette.middleware.base import BaseHTTPMiddleware from app import __version__ from app.config import Settings, get_settings -from app.db import init_db, open_db from app.routers import ( auth, bans, @@ -47,13 +43,9 @@ from app.routers import ( server, setup, ) -from app.tasks import blocklist_import, geo_cache_flush, geo_re_resolve, health_check, history_sync +from app.startup import startup_shared_resources from app.utils.fail2ban_client import Fail2BanConnectionError, Fail2BanProtocolError -from app.utils.jail_config import ensure_jail_configs -from app.utils.setup_state import ( - is_setup_complete_cached, - set_setup_complete_cache, -) +from app.utils.setup_state import is_setup_complete_cached, set_setup_complete_cache log: structlog.stdlib.BoundLogger = structlog.get_logger() @@ -112,58 +104,10 @@ async def _lifespan(app: FastAPI) -> AsyncGenerator[None, None]: log.info("bangui_starting_up", database_path=settings.database_path) - # --- Ensure required jail config files are present --- - ensure_jail_configs(Path(settings.fail2ban_config_dir) / "jail.d") - - # --- Application database --- - db_path: Path = Path(settings.database_path) - db_path.parent.mkdir(parents=True, exist_ok=True) - from app.services import geo_service # noqa: PLC0415 - - log.debug("database_directory_ensured", directory=str(db_path.parent)) - startup_db = await open_db(settings.database_path) - try: - await init_db(startup_db) - await geo_service.load_cache_from_db(startup_db) - unresolved_count = await geo_service.count_unresolved(startup_db) - from app.services import setup_service # noqa: PLC0415 - - setup_complete = await setup_service.is_setup_complete(startup_db) - set_setup_complete_cache(app, setup_complete) - log.debug("setup_completion_cached", completed=setup_complete) - finally: - await startup_db.close() - - if unresolved_count > 0: - log.warning("geo_cache_unresolved_ips", unresolved=unresolved_count) - - # --- Shared HTTP client session --- - http_session: aiohttp.ClientSession = aiohttp.ClientSession() + http_session, scheduler = await startup_shared_resources(app, settings) app.state.http_session = http_session - - # --- Pre-warm geo cache from the persistent store --- - geo_service.init_geoip(settings.geoip_db_path) - - # --- Background task scheduler --- - scheduler: AsyncIOScheduler = AsyncIOScheduler(timezone="UTC") - scheduler.start() app.state.scheduler = scheduler - # --- Health-check background probe --- - health_check.register(app) - - # --- Blocklist import scheduled task --- - blocklist_import.register(app) - - # --- Periodic geo cache flush to SQLite --- - geo_cache_flush.register(app) - - # --- Periodic re-resolve of NULL-country geo entries --- - geo_re_resolve.register(app) - - # --- Periodic history sync from fail2ban into BanGUI archive --- - history_sync.register(app) - log.info("bangui_started") try: diff --git a/backend/app/startup.py b/backend/app/startup.py new file mode 100644 index 0000000..fda34c5 --- /dev/null +++ b/backend/app/startup.py @@ -0,0 +1,73 @@ +"""Application startup helpers. + +This module contains shared startup logic extracted from ``app.main`` so that +initialisation is easier to reason about and unit test. The lifespan handler +in ``app.main`` delegates resource creation and task registration here. +""" + +from __future__ import annotations + +from pathlib import Path + +import aiohttp +import structlog +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.services import geo_service, setup_service +from app.tasks import blocklist_import, geo_cache_flush, geo_re_resolve, health_check, history_sync +from app.utils.jail_config import ensure_jail_configs +from app.utils.setup_state import set_setup_complete_cache + +log: structlog.stdlib.BoundLogger = structlog.get_logger() + + +async def startup_shared_resources( + app: FastAPI, + settings: Settings, +) -> tuple[aiohttp.ClientSession, AsyncIOScheduler]: + """Create shared resources needed during the application lifespan. + + Args: + app: The FastAPI application instance. + settings: Resolved application settings. + + Returns: + 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.parent.mkdir(parents=True, exist_ok=True) + + log.debug("database_directory_ensured", directory=str(db_path.parent)) + + startup_db = await open_db(settings.database_path) + try: + 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) + set_setup_complete_cache(app, setup_complete) + log.debug("setup_completion_cached", completed=setup_complete) + finally: + await startup_db.close() + + if unresolved_count > 0: + log.warning("geo_cache_unresolved_ips", unresolved=unresolved_count) + + http_session: aiohttp.ClientSession = aiohttp.ClientSession() + geo_service.init_geoip(settings.geoip_db_path) + + scheduler: AsyncIOScheduler = AsyncIOScheduler(timezone="UTC") + scheduler.start() + + health_check.register(app) + blocklist_import.register(app) + geo_cache_flush.register(app) + geo_re_resolve.register(app) + history_sync.register(app) + + return http_session, scheduler diff --git a/backend/tests/test_main.py b/backend/tests/test_main.py index e5e9c8c..366fe5f 100644 --- a/backend/tests/test_main.py +++ b/backend/tests/test_main.py @@ -97,10 +97,10 @@ async def test_lifespan_initialises_and_cleans_up_shared_resources(tmp_path: Pat mock_http_session.close = AsyncMock() with ( - patch("app.main.ensure_jail_configs"), - patch("app.main.aiohttp.ClientSession", return_value=mock_http_session), - patch("app.main.AsyncIOScheduler", return_value=mock_scheduler), - patch("app.main.init_db", new=AsyncMock()), + 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.init_db", new=AsyncMock()), 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.count_unresolved", new=AsyncMock(return_value=0)), @@ -149,12 +149,12 @@ async def test_concurrent_requests_use_request_scoped_db_connections(tmp_path: P mock_http_session.close = AsyncMock() with ( - patch("app.main.open_db", new=AsyncMock(side_effect=fake_open_db)), + patch("app.startup.open_db", new=AsyncMock(side_effect=fake_open_db)), patch("app.db.open_db", new=AsyncMock(side_effect=fake_open_db)), - patch("app.main.init_db", new=AsyncMock()), - patch("app.main.ensure_jail_configs"), - patch("app.main.aiohttp.ClientSession", return_value=mock_http_session), - patch("app.main.AsyncIOScheduler", return_value=mock_scheduler), + patch("app.startup.init_db", new=AsyncMock()), + 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.services.geo_service.init_geoip"), 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)),