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.
 

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:

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

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)),