T-10: Fix get_geo_batch_lookup for proper injection with GeoCache instance
Instead of returning a bound method (geo_cache.lookup_batch), now inject the GeoCache instance directly into routers and services. This provides proper runtime isolation since T-04 made GeoCache a proper object. Changes: - Remove get_geo_batch_lookup() dependency provider - Add GeoCacheDep type alias for injecting GeoCache instances - Update all routers (bans, blocklist, dashboard, jails) to use GeoCacheDep - Update ban_service, blocklist_service, jail_service to accept GeoCache - Update service protocols to match new signatures - Update docstrings to reference GeoCache methods instead of module functions All callers now call geo_cache.lookup_batch(...) directly instead of geo_batch_lookup(...), providing real dependency injection with proper testing isolation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -61,7 +61,8 @@ if TYPE_CHECKING:
|
||||
import aiohttp
|
||||
import aiosqlite
|
||||
|
||||
from app.models.geo import GeoBatchLookup, GeoCacheLookup, GeoEnricher, GeoInfo
|
||||
from app.models.geo import GeoCacheLookup, GeoEnricher, GeoInfo
|
||||
from app.services.geo_cache import GeoCache
|
||||
from app.repositories.protocols import HistoryArchiveRepository
|
||||
|
||||
log: structlog.stdlib.BoundLogger = structlog.get_logger()
|
||||
@@ -214,7 +215,7 @@ async def _enrich_bans(
|
||||
|
||||
async def get_active_bans(
|
||||
socket_path: str,
|
||||
geo_batch_lookup: GeoBatchLookup | None = None,
|
||||
geo_cache: GeoCache | None = None,
|
||||
geo_enricher: GeoEnricher | None = None,
|
||||
http_session: aiohttp.ClientSession | None = None,
|
||||
app_db: aiosqlite.Connection | None = None,
|
||||
@@ -227,7 +228,7 @@ async def get_active_bans(
|
||||
Geo enrichment strategy (highest priority first):
|
||||
|
||||
1. When *http_session* is provided the entire set of banned IPs is resolved
|
||||
in a single :func:`~app.services.geo_service.lookup_batch` call (up to
|
||||
in a single :meth:`GeoCache.lookup_batch` call (up to
|
||||
100 IPs per HTTP request). This is far more efficient than concurrent
|
||||
per-IP lookups and stays within ip-api.com rate limits.
|
||||
2. When only *geo_enricher* is provided (legacy / test path) each IP is
|
||||
@@ -239,7 +240,7 @@ async def get_active_bans(
|
||||
Used to enrich each ban entry with country and ASN data.
|
||||
Ignored when *http_session* is provided.
|
||||
http_session: Optional shared :class:`aiohttp.ClientSession`. When
|
||||
provided, :func:`~app.services.geo_service.lookup_batch` is used
|
||||
provided, :meth:`GeoCache.lookup_batch` is used
|
||||
for efficient bulk geo resolution.
|
||||
app_db: Optional BanGUI application database connection used to
|
||||
persist newly resolved geo entries across restarts. Only
|
||||
@@ -296,10 +297,10 @@ async def get_active_bans(
|
||||
if ban is not None:
|
||||
bans.append(ban)
|
||||
|
||||
if http_session is not None and bans and geo_batch_lookup is not None:
|
||||
if http_session is not None and bans and geo_cache is not None:
|
||||
all_ips: list[str] = [ban.ip for ban in bans]
|
||||
try:
|
||||
geo_map = await geo_batch_lookup(all_ips, http_session, db=app_db)
|
||||
geo_map = await geo_cache.lookup_batch(all_ips, http_session, db=app_db)
|
||||
except Exception: # noqa: BLE001
|
||||
log.warning("active_bans_batch_geo_failed")
|
||||
geo_map = {}
|
||||
@@ -331,7 +332,7 @@ async def list_bans(
|
||||
page_size: int = DEFAULT_PAGE_SIZE,
|
||||
http_session: aiohttp.ClientSession | None = None,
|
||||
app_db: aiosqlite.Connection | None = None,
|
||||
geo_batch_lookup: GeoBatchLookup | None = None,
|
||||
geo_cache: GeoCache | None = None,
|
||||
geo_enricher: GeoEnricher | None = None,
|
||||
history_archive_repo: HistoryArchiveRepository = default_history_archive_repo,
|
||||
origin: BanOrigin | None = None,
|
||||
@@ -345,7 +346,7 @@ async def list_bans(
|
||||
Geo enrichment strategy (highest priority first):
|
||||
|
||||
1. When *http_session* is provided the entire page of IPs is resolved in
|
||||
one :func:`~app.services.geo_service.lookup_batch` call (up to 100 IPs
|
||||
one :meth:`GeoCache.lookup_batch` call (up to 100 IPs
|
||||
per HTTP request). This avoids the 45 req/min rate limit of the
|
||||
single-IP endpoint and is the preferred production path.
|
||||
2. When only *geo_enricher* is provided (legacy / test path) each IP is
|
||||
@@ -359,7 +360,7 @@ async def list_bans(
|
||||
page_size: Maximum items per page, capped at ``MAX_PAGE_SIZE``
|
||||
(default: ``100``).
|
||||
http_session: Optional shared :class:`aiohttp.ClientSession`. When
|
||||
provided, :func:`~app.services.geo_service.lookup_batch` is used
|
||||
provided, :meth:`GeoCache.lookup_batch` is used
|
||||
for efficient bulk geo resolution.
|
||||
app_db: Optional BanGUI application database used to persist newly
|
||||
resolved geo entries and to read back cached results.
|
||||
@@ -414,10 +415,10 @@ async def list_bans(
|
||||
# This avoids hitting the 45 req/min single-IP rate limit when the
|
||||
# page contains many bans (e.g. after a large blocklist import).
|
||||
geo_map: dict[str, GeoInfo] = {}
|
||||
if http_session is not None and rows and geo_batch_lookup is not None:
|
||||
if http_session is not None and rows and geo_cache is not None:
|
||||
page_ips: list[str] = [r.ip for r in rows]
|
||||
try:
|
||||
geo_map = await geo_batch_lookup(page_ips, http_session, db=app_db)
|
||||
geo_map = await geo_cache.lookup_batch(page_ips, http_session, db=app_db)
|
||||
except Exception: # noqa: BLE001
|
||||
log.warning("ban_service_batch_geo_failed_list_bans")
|
||||
|
||||
@@ -499,7 +500,7 @@ async def bans_by_country(
|
||||
source: str = "fail2ban",
|
||||
http_session: aiohttp.ClientSession | None = None,
|
||||
geo_cache_lookup: GeoCacheLookup | None = None,
|
||||
geo_batch_lookup: GeoBatchLookup | None = None,
|
||||
geo_cache: GeoCache | None = None,
|
||||
geo_enricher: GeoEnricher | None = None,
|
||||
history_archive_repo: HistoryArchiveRepository = default_history_archive_repo,
|
||||
app_db: aiosqlite.Connection | None = None,
|
||||
@@ -607,11 +608,11 @@ async def bans_by_country(
|
||||
uncached=len(uncached),
|
||||
cached=len(geo_map),
|
||||
)
|
||||
if geo_batch_lookup is not None:
|
||||
if geo_cache is not None:
|
||||
# Fire-and-forget: lookup_batch handles rate-limiting / retries.
|
||||
# The dirty-set flush task persists results to the DB.
|
||||
asyncio.create_task( # noqa: RUF006
|
||||
geo_batch_lookup(uncached, http_session, db=app_db),
|
||||
geo_cache.lookup_batch(uncached, http_session, db=app_db),
|
||||
name="geo_bans_by_country",
|
||||
)
|
||||
elif geo_enricher is not None and unique_ips:
|
||||
|
||||
Reference in New Issue
Block a user