lukas.pupkalipinski/BanGUI
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix: Consolidate divergent _since_unix implementations (T-09)

Browse Source 
Consolidate the two divergent implementations of _since_unix from ban_service.py
and history_service.py into a single shared utility function in time_utils.py.

Changes:
- Move _since_unix to app/utils/time_utils.py with consistent time.time() approach
- Move TIME_RANGE_SLACK_SECONDS constant to app/utils/constants.py
- Update ban_service.py to import since_unix from time_utils
- Update history_service.py to import since_unix from time_utils
- Both services now use the same window boundary calculation with 60-second slack
- Add comprehensive tests for the shared since_unix function
- Document timestamp handling rationale in Backend-Development.md

This ensures dashboard and history queries return consistent row counts for the
same time range by using the same timestamp calculation and slack window across
all services.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Lukas
2026-04-25 18:44:59 +02:00
parent 420ea18fd9
commit ac2028e1c2
6 changed files with 132 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
backend/app/services/ban_service.py
View File

@@ -13,7 +13,6 @@ from __future__ import annotations
import asyncio
import contextlib
import ipaddress
import time
from typing import TYPE_CHECKING, cast
import structlog
@@ -23,7 +22,6 @@ from app.models.ban import (
BLOCKLIST_JAIL,
BUCKET_SECONDS,
BUCKET_SIZE_LABEL,
TIME_RANGE_SECONDS,
ActiveBan,
ActiveBanListResponse,
BanOrigin,
@@ -57,6 +55,7 @@ from app.utils.fail2ban_response import (
ok,
to_dict,
)
from app.utils.time_utils import since_unix
if TYPE_CHECKING:
import aiohttp
@@ -318,33 +317,6 @@ async def get_active_bans(
log.info("active_bans_fetched", total=len(bans))
return ActiveBanListResponse(bans=bans, total=len(bans))
_TIME_RANGE_SLACK_SECONDS: int = 60
def _since_unix(range_: TimeRange) -> int:
"""Return the Unix timestamp representing the start of the time window.
Uses :func:`time.time` (always UTC epoch seconds on all platforms) to be
consistent with how fail2ban stores ``timeofban`` values in its SQLite
database. fail2ban records ``time.time()`` values directly, so
comparing against a timezone-aware ``datetime.now(UTC).timestamp()`` would
theoretically produce the same number but using :func:`time.time` avoids
any tz-aware datetime pitfalls on misconfigured systems.
Args:
range_: One of the supported time-range presets.
Returns:
Unix timestamp (seconds since epoch) equal to *now − range_* with a
small slack window for clock drift and test seeding delays.
"""
seconds: int = TIME_RANGE_SECONDS[range_]
return int(time.time()) - seconds - _TIME_RANGE_SLACK_SECONDS
# ---------------------------------------------------------------------------
# Public API
# ---------------------------------------------------------------------------
@@ -401,7 +373,7 @@ async def list_bans(
paginated items and total count.
"""
since: int = _since_unix(range_)
since: int = since_unix(range_)
effective_page_size: int = min(page_size, MAX_PAGE_SIZE)
offset: int = (page - 1) * effective_page_size
@@ -570,7 +542,7 @@ async def bans_by_country(
aggregation and the companion ban list.
"""
since: int = _since_unix(range_)
since: int = since_unix(range_)
if source not in ("fail2ban", "archive"):
raise ValueError(f"Unsupported source: {source!r}")
@@ -820,7 +792,7 @@ async def ban_trend(
:class:`~app.models.ban.BanTrendResponse` with a full bucket list
and the human-readable bucket-size label.
"""
since: int = _since_unix(range_)
since: int = since_unix(range_)
bucket_secs: int = BUCKET_SECONDS[range_]
num_buckets: int = bucket_count(range_)
@@ -920,7 +892,7 @@ async def bans_by_jail(
:class:`~app.models.ban.BansByJailResponse` with per-jail counts
sorted descending and the total ban count.
"""
since: int = _since_unix(range_)
since: int = since_unix(range_)
if source not in ("fail2ban", "archive"):
raise ValueError(f"Unsupported source: {source!r}")

21
backend/app/services/history_service.py
View File

@@ -15,7 +15,7 @@ from typing import TYPE_CHECKING
import structlog
from app.models.ban import TIME_RANGE_SECONDS, BanOrigin, TimeRange
from app.models.ban import BanOrigin, TimeRange
from app.services import geo_service
if TYPE_CHECKING:
@@ -35,6 +35,7 @@ from app.repositories import history_archive_repo as default_history_archive_rep
from app.services.fail2ban_metadata_service import default_fail2ban_metadata_service
from app.utils.constants import DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE
from app.utils.fail2ban_db_utils import parse_data_json, ts_to_iso
from app.utils.time_utils import since_unix
log: structlog.stdlib.BoundLogger = structlog.get_logger()
@@ -43,25 +44,11 @@ async def get_fail2ban_db_path(socket_path: str) -> str:
"""Return the fail2ban database path using the shared metadata cache."""
return await default_fail2ban_metadata_service.get_db_path(socket_path)
# ---------------------------------------------------------------------------
# Constants
# Internal Helpers
# ---------------------------------------------------------------------------
def _since_unix(range_: TimeRange) -> int:
"""Return the Unix timestamp for the start of the given time window.
Args:
range_: One of the supported time-range presets.
Returns:
Unix timestamp (seconds since epoch) equal to *now − range_*.
"""
seconds: int = TIME_RANGE_SECONDS[range_]
return int(datetime.now(tz=UTC).timestamp()) - seconds
async def _resolve_geo_info(
ip: str,
*,
@@ -199,7 +186,7 @@ async def list_history(
# Build WHERE clauses dynamically.
since: int | None = None
if range_ is not None:
since = _since_unix(range_)
since = since_unix(range_)
db_path: str = await get_fail2ban_db_path(socket_path)
log.info(

3
backend/app/utils/constants.py
View File

@@ -65,6 +65,9 @@ TIME_RANGE_HOURS: Final[dict[str, int]] = {
TIME_RANGE_365D: 365 * 24,
}
TIME_RANGE_SLACK_SECONDS: Final[int] = 60
"""Clock drift and test seeding tolerance for timestamp comparisons."""
# ---------------------------------------------------------------------------
# Pagination
# ---------------------------------------------------------------------------

29
backend/app/utils/time_utils.py
View File

@@ -7,6 +7,7 @@ for working with time throughout the backend.
"""
import datetime
import time
def utc_now() -> datetime.datetime:
@@ -65,3 +66,31 @@ def hours_ago(hours: int) -> datetime.datetime:
Timezone-aware UTC :class:`datetime.datetime`.
"""
return utc_now() - datetime.timedelta(hours=hours)
def since_unix(range_: str) -> int:
"""Return the Unix timestamp for the start of a time-range window.
Uses :func:`time.time` (always UTC epoch seconds on all platforms) to be
consistent with how fail2ban stores ``timeofban`` values in its SQLite
database. fail2ban records :func:`time.time()` values directly, so using
a timezone-aware :func:`datetime.datetime.now`\\ ``(UTC).timestamp()``
would theoretically produce the same result but using :func:`time.time`
avoids any timezone-aware datetime pitfalls on misconfigured systems.
A 60-second slack window is applied to accommodate clock drift and
test seeding delays. This ensures consistent query windows across services
(e.g., dashboard vs. history).
Args:
range_: One of the supported time-range presets (e.g., ``"24h"``).
Returns:
Unix timestamp (seconds since epoch) representing the start of the
time window: *now − range_ − slack*.
"""
from app.models.ban import TIME_RANGE_SECONDS # noqa: F401
from app.utils.constants import TIME_RANGE_SLACK_SECONDS
seconds: int = TIME_RANGE_SECONDS[range_]
return int(time.time()) - seconds - TIME_RANGE_SLACK_SECONDS

59
backend/tests/test_services/test_time_utils.py
View File

@@ -1,8 +1,17 @@
"""Tests for app.utils.time_utils."""
import datetime
import time
from app.utils.time_utils import add_minutes, hours_ago, is_expired, utc_from_timestamp, utc_now
from app.utils.time_utils import (
add_minutes,
hours_ago,
is_expired,
since_unix,
utc_from_timestamp,
utc_now,
)
from app.utils.constants import TIME_RANGE_SLACK_SECONDS
class TestUtcNow:
@@ -77,3 +86,51 @@ class TestHoursAgo:
expected_min = before - datetime.timedelta(hours=1, seconds=1)
expected_max = after - datetime.timedelta(hours=1) + datetime.timedelta(seconds=1)
assert expected_min <= result <= expected_max
class TestSinceUnix:
"""Tests for :func:`since_unix`."""
def test_since_unix_24h_returns_unix_timestamp(self) -> None:
"""Verify since_unix returns an integer timestamp."""
result = since_unix("24h")
assert isinstance(result, int)
def test_since_unix_24h_is_roughly_24_hours_ago(self) -> None:
"""Verify 24h preset returns a timestamp ~24 hours in the past."""
before = int(time.time())
result = since_unix("24h")
after = int(time.time())
# Allow 1 second tolerance for execution time
expected_min = before - (24 * 3600) - TIME_RANGE_SLACK_SECONDS - 1
expected_max = after - (24 * 3600) - TIME_RANGE_SLACK_SECONDS + 1
assert expected_min <= result <= expected_max
def test_since_unix_7d_is_roughly_7_days_ago(self) -> None:
"""Verify 7d preset returns a timestamp ~7 days in the past."""
before = int(time.time())
result = since_unix("7d")
after = int(time.time())
# Allow 1 second tolerance for execution time
expected_min = before - (7 * 24 * 3600) - TIME_RANGE_SLACK_SECONDS - 1
expected_max = after - (7 * 24 * 3600) - TIME_RANGE_SLACK_SECONDS + 1
assert expected_min <= result <= expected_max
def test_since_unix_includes_slack_window(self) -> None:
"""Verify 60-second slack is included in all presets."""
now = int(time.time())
result = since_unix("24h")
# Verify slack is included: result should be (now - 24h - 60s)
# within tolerance
diff_without_slack = now - result
expected_without_slack = 24 * 3600
actual_slack = diff_without_slack - expected_without_slack
# The slack should be ~60 seconds
assert actual_slack >= TIME_RANGE_SLACK_SECONDS - 1
assert actual_slack <= TIME_RANGE_SLACK_SECONDS + 1