Refactor geo re-resolve endpoint into geo_service and add typed response
This commit is contained in:
@@ -189,7 +189,7 @@ The business logic layer. Services orchestrate operations, enforce rules, and co
|
|||||||
| `log_service.py` | Log preview and regex test operations (extracted from config_service) |
|
| `log_service.py` | Log preview and regex test operations (extracted from config_service) |
|
||||||
| `history_service.py` | Queries the fail2ban database for historical ban records, builds per-IP timelines, computes ban counts and repeat-offender flags, and syncs new records into BanGUI's archive table |
|
| `history_service.py` | Queries the fail2ban database for historical ban records, builds per-IP timelines, computes ban counts and repeat-offender flags, and syncs new records into BanGUI's archive table |
|
||||||
| `blocklist_service.py` | Downloads blocklists via aiohttp, validates IPs/CIDRs, applies bans through fail2ban or iptables, logs import results |
|
| `blocklist_service.py` | Downloads blocklists via aiohttp, validates IPs/CIDRs, applies bans through fail2ban or iptables, logs import results |
|
||||||
| `geo_service.py` | Resolves IP addresses to country, ASN, and RIR using external APIs or a local database, caches results |
|
| `geo_service.py` | Resolves IP addresses to country, ASN, and RIR using external APIs or a local database, caches results, and re-resolves unresolved geo cache entries |
|
||||||
| `server_service.py` | Reads and writes fail2ban server-level settings (log level, log target, syslog socket, DB location, purge age) |
|
| `server_service.py` | Reads and writes fail2ban server-level settings (log level, log target, syslog socket, DB location, purge age) |
|
||||||
| `health_service.py` | Probes fail2ban socket connectivity, retrieves server version and global stats, reports online/offline status |
|
| `health_service.py` | Probes fail2ban socket connectivity, retrieves server version and global stats, reports online/offline status |
|
||||||
|
|
||||||
|
|||||||
@@ -246,7 +246,9 @@ The router knows the internal job ID string and directly manipulates the schedul
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### TASK-08 — Remove inverted layer dependency: `utils/fail2ban_db_utils.py` importing a service 🟠
|
### TASK-08 — Remove inverted layer dependency: `utils/fail2ban_db_utils.py` importing a service ✅
|
||||||
|
|
||||||
|
**Status:** Completed ✅
|
||||||
|
|
||||||
**Where:**
|
**Where:**
|
||||||
`backend/app/utils/fail2ban_db_utils.py` — line 8:
|
`backend/app/utils/fail2ban_db_utils.py` — line 8:
|
||||||
@@ -275,7 +277,9 @@ A util importing a service creates an implicit upward dependency that the archit
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### TASK-09 — Move `re_resolve_geo` orchestration into `geo_service` 🟠
|
### TASK-09 — Move `re_resolve_geo` orchestration into `geo_service` ✅
|
||||||
|
|
||||||
|
**Status:** Completed ✅
|
||||||
|
|
||||||
**Where:**
|
**Where:**
|
||||||
`backend/app/routers/geo.py` — `async def re_resolve_geo()` (lines ~142–174).
|
`backend/app/routers/geo.py` — `async def re_resolve_geo()` (lines ~142–174).
|
||||||
|
|||||||
@@ -57,6 +57,19 @@ class GeoCacheStatsResponse(BaseModel):
|
|||||||
dirty_size: int = Field(..., description="Number of newly resolved entries not yet flushed to disk.")
|
dirty_size: int = Field(..., description="Number of newly resolved entries not yet flushed to disk.")
|
||||||
|
|
||||||
|
|
||||||
|
class GeoReResolveResponse(BaseModel):
|
||||||
|
"""Response for ``POST /api/geo/re-resolve``.
|
||||||
|
|
||||||
|
Reports how many previously unresolved IPs were retried and how many
|
||||||
|
gained a resolved country code after the re-resolve operation.
|
||||||
|
"""
|
||||||
|
|
||||||
|
model_config = ConfigDict(strict=True)
|
||||||
|
|
||||||
|
resolved: int = Field(..., description="Number of IPs successfully resolved.")
|
||||||
|
total: int = Field(..., description="Number of IPs retried.")
|
||||||
|
|
||||||
|
|
||||||
class IpLookupResponse(BaseModel):
|
class IpLookupResponse(BaseModel):
|
||||||
"""Response for ``GET /api/geo/lookup/{ip}``.
|
"""Response for ``GET /api/geo/lookup/{ip}``.
|
||||||
|
|
||||||
|
|||||||
@@ -11,8 +11,6 @@ from __future__ import annotations
|
|||||||
from typing import TYPE_CHECKING, Annotated
|
from typing import TYPE_CHECKING, Annotated
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
import aiohttp
|
|
||||||
|
|
||||||
from app.services.jail_service import IpLookupResult
|
from app.services.jail_service import IpLookupResult
|
||||||
|
|
||||||
import aiosqlite
|
import aiosqlite
|
||||||
@@ -20,12 +18,11 @@ from fastapi import APIRouter, Depends, HTTPException, Path, status
|
|||||||
|
|
||||||
from app.dependencies import (
|
from app.dependencies import (
|
||||||
AuthDep,
|
AuthDep,
|
||||||
DbDep,
|
|
||||||
Fail2BanSocketDep,
|
Fail2BanSocketDep,
|
||||||
HttpSessionDep,
|
HttpSessionDep,
|
||||||
get_db,
|
get_db,
|
||||||
)
|
)
|
||||||
from app.models.geo import GeoCacheStatsResponse, GeoDetail, GeoInfo, IpLookupResponse
|
from app.models.geo import GeoCacheStatsResponse, GeoDetail, GeoInfo, GeoReResolveResponse, IpLookupResponse
|
||||||
from app.services import geo_service, jail_service
|
from app.services import geo_service, jail_service
|
||||||
from app.utils.fail2ban_client import Fail2BanConnectionError
|
from app.utils.fail2ban_client import Fail2BanConnectionError
|
||||||
|
|
||||||
@@ -136,12 +133,13 @@ async def geo_stats(
|
|||||||
@router.post(
|
@router.post(
|
||||||
"/re-resolve",
|
"/re-resolve",
|
||||||
summary="Re-resolve all IPs whose country could not be determined",
|
summary="Re-resolve all IPs whose country could not be determined",
|
||||||
|
response_model=GeoReResolveResponse,
|
||||||
)
|
)
|
||||||
async def re_resolve_geo(
|
async def re_resolve_geo(
|
||||||
_auth: AuthDep,
|
_auth: AuthDep,
|
||||||
db: Annotated[aiosqlite.Connection, Depends(get_db)],
|
db: Annotated[aiosqlite.Connection, Depends(get_db)],
|
||||||
http_session: HttpSessionDep,
|
http_session: HttpSessionDep,
|
||||||
) -> dict[str, int]:
|
) -> GeoReResolveResponse:
|
||||||
"""Retry geo resolution for every IP in ``geo_cache`` with a null country.
|
"""Retry geo resolution for every IP in ``geo_cache`` with a null country.
|
||||||
|
|
||||||
Clears the in-memory negative cache first so that previously failing IPs
|
Clears the in-memory negative cache first so that previously failing IPs
|
||||||
@@ -153,22 +151,6 @@ async def re_resolve_geo(
|
|||||||
http_session: Shared HTTP session for geo lookups.
|
http_session: Shared HTTP session for geo lookups.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
JSON object ``{"resolved": N, "total": M}`` where *N* is the number
|
A :class:`~app.models.geo.GeoReResolveResponse` with retry counts.
|
||||||
of IPs that gained a country code and *M* is the total number of IPs
|
|
||||||
that were retried.
|
|
||||||
"""
|
"""
|
||||||
# Collect all IPs in geo_cache that still lack a country code.
|
return await geo_service.re_resolve_all(db, http_session)
|
||||||
unresolved = await geo_service.get_unresolved_ips(db)
|
|
||||||
|
|
||||||
if not unresolved:
|
|
||||||
return {"resolved": 0, "total": 0}
|
|
||||||
|
|
||||||
# Clear negative cache so these IPs bypass the TTL check.
|
|
||||||
geo_service.clear_neg_cache()
|
|
||||||
|
|
||||||
geo_map = await geo_service.lookup_batch(unresolved, http_session, db=db)
|
|
||||||
|
|
||||||
resolved_count = sum(
|
|
||||||
1 for info in geo_map.values() if info.country_code is not None
|
|
||||||
)
|
|
||||||
return {"resolved": resolved_count, "total": len(unresolved)}
|
|
||||||
|
|||||||
@@ -187,6 +187,41 @@ async def get_unresolved_ips(db: aiosqlite.Connection) -> list[str]:
|
|||||||
return await geo_cache_repo.get_unresolved_ips(db)
|
return await geo_cache_repo.get_unresolved_ips(db)
|
||||||
|
|
||||||
|
|
||||||
|
async def re_resolve_all(
|
||||||
|
db: aiosqlite.Connection,
|
||||||
|
http_session: aiohttp.ClientSession,
|
||||||
|
) -> dict[str, int]:
|
||||||
|
"""Retry geo resolution for all unresolved cache entries.
|
||||||
|
|
||||||
|
This helper clears the in-memory negative cache before attempting a
|
||||||
|
fresh batch lookup, then returns counters for how many IPs were retried
|
||||||
|
and how many gained a resolved country code.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
db: BanGUI application database connection.
|
||||||
|
http_session: Shared aiohttp client session.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
A dict with ``resolved`` and ``total`` counts.
|
||||||
|
"""
|
||||||
|
unresolved = await get_unresolved_ips(db)
|
||||||
|
if not unresolved:
|
||||||
|
return {"resolved": 0, "total": 0}
|
||||||
|
|
||||||
|
clear_neg_cache()
|
||||||
|
geo_map = await lookup_batch(unresolved, http_session, db=db)
|
||||||
|
resolved_count = sum(
|
||||||
|
1 for info in geo_map.values() if info.country_code is not None
|
||||||
|
)
|
||||||
|
|
||||||
|
log.info(
|
||||||
|
"geo_re_resolve_complete",
|
||||||
|
total=len(unresolved),
|
||||||
|
resolved=resolved_count,
|
||||||
|
)
|
||||||
|
return {"resolved": resolved_count, "total": len(unresolved)}
|
||||||
|
|
||||||
|
|
||||||
def init_geoip(mmdb_path: str | None) -> None:
|
def init_geoip(mmdb_path: str | None) -> None:
|
||||||
"""Initialise the MaxMind GeoLite2-Country database reader.
|
"""Initialise the MaxMind GeoLite2-Country database reader.
|
||||||
|
|
||||||
|
|||||||
@@ -48,7 +48,9 @@ async def geo_client(tmp_path: Path) -> AsyncClient: # type: ignore[misc]
|
|||||||
|
|
||||||
transport = ASGITransport(app=app)
|
transport = ASGITransport(app=app)
|
||||||
async with AsyncClient(transport=transport, base_url="http://test") as ac:
|
async with AsyncClient(transport=transport, base_url="http://test") as ac:
|
||||||
await ac.post("/api/setup", json=_SETUP_PAYLOAD)
|
setup_payload = _SETUP_PAYLOAD.copy()
|
||||||
|
setup_payload["database_path"] = settings.database_path
|
||||||
|
await ac.post("/api/setup", json=setup_payload)
|
||||||
login = await ac.post(
|
login = await ac.post(
|
||||||
"/api/auth/login",
|
"/api/auth/login",
|
||||||
json={"password": _SETUP_PAYLOAD["master_password"]},
|
json={"password": _SETUP_PAYLOAD["master_password"]},
|
||||||
@@ -170,15 +172,15 @@ class TestReResolve:
|
|||||||
async def test_returns_200_with_counts(self, geo_client: AsyncClient) -> None:
|
async def test_returns_200_with_counts(self, geo_client: AsyncClient) -> None:
|
||||||
"""POST /api/geo/re-resolve returns 200 with resolved/total counts."""
|
"""POST /api/geo/re-resolve returns 200 with resolved/total counts."""
|
||||||
with patch(
|
with patch(
|
||||||
"app.routers.geo.geo_service.lookup_batch",
|
"app.routers.geo.geo_service.re_resolve_all",
|
||||||
AsyncMock(return_value={}),
|
AsyncMock(return_value={"resolved": 0, "total": 0}),
|
||||||
):
|
):
|
||||||
resp = await geo_client.post("/api/geo/re-resolve")
|
resp = await geo_client.post("/api/geo/re-resolve")
|
||||||
|
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
data = resp.json()
|
data = resp.json()
|
||||||
assert "resolved" in data
|
assert data["resolved"] == 0
|
||||||
assert "total" in data
|
assert data["total"] == 0
|
||||||
|
|
||||||
async def test_empty_when_no_unresolved_ips(self, geo_client: AsyncClient) -> None:
|
async def test_empty_when_no_unresolved_ips(self, geo_client: AsyncClient) -> None:
|
||||||
"""Returns resolved=0, total=0 when geo_cache has no NULL country_code rows."""
|
"""Returns resolved=0, total=0 when geo_cache has no NULL country_code rows."""
|
||||||
|
|||||||
@@ -842,6 +842,54 @@ class TestLookupCachedOnly:
|
|||||||
assert uncached.count("9.9.9.9") == 1
|
assert uncached.count("9.9.9.9") == 1
|
||||||
|
|
||||||
|
|
||||||
|
class TestReResolveAll:
|
||||||
|
"""Tests for :func:`~app.services.geo_service.re_resolve_all`."""
|
||||||
|
|
||||||
|
async def test_returns_zero_when_no_unresolved_ips(self) -> None:
|
||||||
|
"""The service returns zero counts when there are no unresolved IPs."""
|
||||||
|
db = MagicMock()
|
||||||
|
session = MagicMock()
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"app.services.geo_service.get_unresolved_ips",
|
||||||
|
AsyncMock(return_value=[]),
|
||||||
|
), patch("app.services.geo_service.lookup_batch", AsyncMock()) as mock_lookup, patch(
|
||||||
|
"app.services.geo_service.clear_neg_cache",
|
||||||
|
MagicMock(),
|
||||||
|
) as mock_clear:
|
||||||
|
result = await geo_service.re_resolve_all(db, session)
|
||||||
|
|
||||||
|
assert result == {"resolved": 0, "total": 0}
|
||||||
|
mock_clear.assert_not_called()
|
||||||
|
mock_lookup.assert_not_called()
|
||||||
|
|
||||||
|
async def test_clears_neg_cache_and_returns_counts(self) -> None:
|
||||||
|
"""The service clears negative cache and returns resolved and total counts."""
|
||||||
|
db = MagicMock()
|
||||||
|
session = MagicMock()
|
||||||
|
ips = ["1.1.1.1", "2.2.2.2"]
|
||||||
|
geo_map = {
|
||||||
|
"1.1.1.1": GeoInfo(country_code="DE", country_name="Germany", asn=None, org=None),
|
||||||
|
"2.2.2.2": GeoInfo(country_code=None, country_name=None, asn=None, org=None),
|
||||||
|
}
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"app.services.geo_service.get_unresolved_ips",
|
||||||
|
AsyncMock(return_value=ips),
|
||||||
|
), patch(
|
||||||
|
"app.services.geo_service.lookup_batch",
|
||||||
|
AsyncMock(return_value=geo_map),
|
||||||
|
) as mock_lookup, patch(
|
||||||
|
"app.services.geo_service.clear_neg_cache",
|
||||||
|
MagicMock(),
|
||||||
|
) as mock_clear:
|
||||||
|
result = await geo_service.re_resolve_all(db, session)
|
||||||
|
|
||||||
|
assert result == {"resolved": 1, "total": 2}
|
||||||
|
mock_clear.assert_called_once()
|
||||||
|
mock_lookup.assert_awaited_once_with(ips, session, db=db)
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Bulk DB writes via executemany (Task 3)
|
# Bulk DB writes via executemany (Task 3)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user