mirror of
https://github.com/ipnet-mesh/meshcore-hub.git
synced 2026-08-06 17:02:59 +02:00
Merge pull request #311 from ipnet-mesh/feat/cache-invalidation-on-writes
fix: route_result not refreshing after route edit (plus cache invalidation infra)
This commit is contained in:
+4
-1
@@ -418,7 +418,10 @@ PROMETHEUS_PORT=9090
|
||||
# REDIS_CACHE_TTL_ROUTE_DETAIL=300
|
||||
|
||||
# Emit HTTP Cache-Control on /api/v1/* responses + ETag/If-None-Match on
|
||||
# cached endpoints. Disable to suppress all client-side caching directives.
|
||||
# cached endpoints. The policy is `private, no-cache` on GETs (forces
|
||||
# browser revalidation so server-side cache invalidation reaches the UI
|
||||
# on the next page load) and `no-store` on mutations. Disable to suppress
|
||||
# all client-side caching directives.
|
||||
# API_CACHE_CONTROL_ENABLED=true
|
||||
|
||||
# External Alertmanager port (when using --profile metrics)
|
||||
|
||||
@@ -91,6 +91,37 @@ docker compose -f docker-compose.yml -f docker-compose.dev.yml --profile core ex
|
||||
|
||||
## Conventions
|
||||
|
||||
### Cache invalidation on writes
|
||||
|
||||
Every mutation handler (POST/PUT/DELETE) on a user/admin-mutable entity MUST call the matching `invalidate_*` helper from `meshcore_hub.api.cache_invalidation` after `session.commit()` succeeds, so the UI reflects the change on the next page load instead of waiting for the Redis TTL. The helper is a no-op when Redis is disabled and swallows backend errors, so it's always safe to call.
|
||||
|
||||
The HTTP-layer cache policy on `/api/v1/*` GETs is `private, no-cache` (i.e. must-revalidate) precisely so this works: the browser always sends `If-None-Match` on navigation, the server answers 304 when Redis is warm and unchanged (cheap — no body) or 200 after an invalidation. Do NOT change this back to `max-age>0` — server-side cache invalidation cannot reach the browser's HTTP cache, so any freshness window would let stale responses survive a mutation until expiry.
|
||||
|
||||
```python
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_channels
|
||||
|
||||
@router.put("/{channel_id}")
|
||||
def update_channel(__: RequireAdmin, session: DbSession, channel_id: str,
|
||||
body: ChannelUpdate, request: Request) -> ChannelRead:
|
||||
# ... mutate ...
|
||||
session.commit()
|
||||
session.refresh(channel)
|
||||
invalidate_channels(request) # after commit, before return
|
||||
return _channel_to_read(channel)
|
||||
```
|
||||
|
||||
Mapping (see `api/cache_invalidation.py` for the canonical prefix knowledge):
|
||||
|
||||
| Mutation | Helper(s) |
|
||||
|---|---|
|
||||
| `POST/PUT/DELETE /channels` | `invalidate_channels` |
|
||||
| `POST/PUT/DELETE /routes` | `invalidate_routes` (covers list, detail, history) |
|
||||
| `PUT /user/profile/{id}` | `invalidate_profiles` + `invalidate_dashboard` |
|
||||
| `POST/PUT/DELETE /nodes/{pk}/tags` | `invalidate_nodes` + `invalidate_messages` + `invalidate_advertisements` + `invalidate_dashboard` (tags drive names/filters across these) |
|
||||
| `POST/DELETE /adoptions` | `invalidate_nodes` + `invalidate_profiles` + `invalidate_advertisements` + `invalidate_dashboard` (`adopted_by` embedded across these) |
|
||||
|
||||
When adding a new `@cached` read endpoint, decide whether its key namespace belongs in an existing invalidate helper, and add a test in `tests/test_api/test_cache.py::TestMutationInvalidationIntegration`. Cache keys split across two formats (endpoint-name keys like `nodes:` vs URL-path keys like `/api/v1/channels:`) — the helper module encapsulates that, don't hand-roll prefixes.
|
||||
|
||||
```python
|
||||
# Imports: stdlib, third-party, local
|
||||
import os
|
||||
|
||||
@@ -77,13 +77,13 @@ Responses fall into three buckets:
|
||||
|
||||
| Bucket | Endpoints | `Cache-Control` | `ETag` |
|
||||
| --- | --- | --- | --- |
|
||||
| `@cached` GETs | All Redis-cached endpoints (`/nodes`, `/routes`, `/routes/{id}`, `/dashboard/*`, `/packets`, `/packet-groups`, `/messages`, `/advertisements`, `/channels`, `/user/profiles`) | `private, max-age=<redis_ttl>` | Strong SHA-256 hash of the body; `If-None-Match` returns `304 Not Modified` |
|
||||
| Other GETs | Per-id detail endpoints (`/nodes/{key}`, `/packets/{id}`, `/messages/{id}`, `/user/profile/{id}`, `/trace-paths`, `/telemetry`, etc.) | `private, max-age=0, must-revalidate` | _(none)_ |
|
||||
| `@cached` GETs | All Redis-cached endpoints (`/nodes`, `/routes`, `/routes/{id}`, `/dashboard/*`, `/packets`, `/packet-groups`, `/messages`, `/advertisements`, `/channels`, `/user/profiles`) | `private, no-cache` | Strong SHA-256 hash of the body; `If-None-Match` returns `304 Not Modified` |
|
||||
| Other GETs | Per-id detail endpoints (`/nodes/{key}`, `/packets/{id}`, `/messages/{id}`, `/user/profile/{id}`, `/trace-paths`, `/telemetry`, etc.) | `private, no-cache` | _(none)_ |
|
||||
| Mutating + health | `POST`/`PUT`/`DELETE` + `/health*` | `no-store` | _(none)_ |
|
||||
|
||||
All API responses use `private` because several `@cached` endpoints are role-aware — their response shape/redaction varies by trusted-proxy `X-User-Id` / `X-User-Roles` headers, so shared/CDN caches must never store them. Browser caches key by URL + request headers and so remain correct.
|
||||
|
||||
Client `max-age` matches the configured Redis TTL for the endpoint (e.g. 300 s on `/routes/{id}`, 30 s on `/dashboard/*`). The `X-Cache: HIT|MISS` observability header continues to be emitted regardless of this setting.
|
||||
The `no-cache` policy (synonymous with `max-age=0, must-revalidate`) forces the browser to revalidate via `If-None-Match` on every navigation. This is required because the server-side cache invalidation fired by mutation handlers cannot reach the browser's HTTP cache — any `max-age>0` window would let stale responses survive a mutation until expiry. The Redis cache layer (TTL-bounded per endpoint — 30 s default, 300 s on `/routes/{id}` and `/dashboard/*`) still shields the database; only the browser's local reuse window goes away. Most revalidations answer `304 Not Modified` (no body), so the cost is one cheap round-trip per navigation. The `X-Cache: HIT|MISS` observability header continues to be emitted regardless of this setting.
|
||||
|
||||
## Collector
|
||||
|
||||
|
||||
+19
-10
@@ -182,9 +182,8 @@ def create_app(
|
||||
|
||||
Buckets (only when ``app.state.api_cache_control_enabled`` is True):
|
||||
* ``@cached`` endpoints (``request.state.cache_control_ttl`` set by
|
||||
the decorator): ``private, max-age=<ttl>`` + ``ETag`` echoed back.
|
||||
* Uncached GETs under ``/api/v1``: ``private, max-age=0,
|
||||
must-revalidate`` so browsers revalidate but may store.
|
||||
the decorator): ``private, no-cache`` + ``ETag`` echoed back.
|
||||
* Other GETs under ``/api/v1``: ``private, no-cache`` as well.
|
||||
* Mutating methods (POST/PUT/DELETE/PATCH): ``no-store``.
|
||||
* ``/health*`` endpoints: ``no-store``.
|
||||
|
||||
@@ -192,6 +191,18 @@ def create_app(
|
||||
role-aware — their response shape/redaction varies by trusted-proxy
|
||||
``X-User-Id`` / ``X-User-Roles`` headers, so shared/CDN caches must
|
||||
never store them.
|
||||
|
||||
``no-cache`` (i.e. ``max-age=0, must-revalidate``) is used uniformly
|
||||
for GETs under ``/api/`` rather than ``max-age=<ttl>`` because the
|
||||
server-side cache invalidation in ``api.cache_invalidation`` cannot
|
||||
reach the browser's HTTP cache. Any ``max-age>0`` would let the
|
||||
browser reuse a stale response after a mutation until the freshness
|
||||
window expires. ``no-cache`` forces a conditional request
|
||||
(``If-None-Match``) on every navigation, so the server can answer
|
||||
304 when nothing changed (cheap — no body) or 200 after a mutation.
|
||||
The Redis cache layer (TTL-bounded by ``cache_control_ttl``) still
|
||||
shields the database; only the browser's local reuse window goes
|
||||
away.
|
||||
"""
|
||||
response = await call_next(request)
|
||||
|
||||
@@ -224,13 +235,11 @@ def create_app(
|
||||
elif path.startswith("/health"):
|
||||
response.headers["Cache-Control"] = "no-store"
|
||||
elif path.startswith("/api/"):
|
||||
ttl = getattr(request.state, "cache_control_ttl", 0)
|
||||
if isinstance(ttl, int) and ttl > 0:
|
||||
response.headers["Cache-Control"] = f"private, max-age={ttl}"
|
||||
else:
|
||||
response.headers["Cache-Control"] = (
|
||||
"private, max-age=0, must-revalidate"
|
||||
)
|
||||
# Force revalidation: server-side cache invalidation can't reach
|
||||
# the browser's HTTP cache, so any max-age>0 would serve stale
|
||||
# data after a mutation. ETag/If-None-Match still gives us cheap
|
||||
# 304s on the hot path; the Redis cache layer still protects DB.
|
||||
response.headers["Cache-Control"] = "private, no-cache"
|
||||
|
||||
return response
|
||||
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
"""Cache invalidation helpers for entity mutations.
|
||||
|
||||
After any successful write (POST/PUT/DELETE) on a user/admin-mutable entity,
|
||||
the corresponding read caches must be dropped so the UI reflects the change on
|
||||
the next page load instead of waiting for TTL expiry.
|
||||
|
||||
Cache key layout
|
||||
----------------
|
||||
There are two key formats in ``api/cache.py``, and this module must know about
|
||||
both because they coexist:
|
||||
|
||||
1. ``@cached("endpoint_name")`` with no ``key_builder`` stores keys as
|
||||
``{endpoint_name}:{sorted_query_string}`` — examples: ``nodes:``,
|
||||
``profiles:``, ``advertisements:``, ``dashboard/activity:``,
|
||||
``dashboard/packet-breakdown:``.
|
||||
2. ``@cached(..., key_builder=fn)`` ignores ``endpoint_name`` and uses whatever
|
||||
the builder returns. The shared builder pattern across role-aware endpoints
|
||||
is ``{request.url.path}:role={role}:{sorted_query_string}``, so the actual
|
||||
keys start with the literal URL path — examples: ``/api/v1/channels:``,
|
||||
``/api/v1/routes:``, ``/api/v1/routes/{id}:``,
|
||||
``/api/v1/routes/{id}/history:``, ``/api/v1/dashboard/stats:``.
|
||||
|
||||
``CacheBackend.delete(prefix)`` SCANs by ``{prefix}*``, so a single call drops
|
||||
every key under a namespace (including all role variants and all sub-paths).
|
||||
Helpers here delete every namespace an entity touches — including cross-entity
|
||||
embeddings (e.g. adoptions surface inside ``nodes``, ``profiles``, and
|
||||
``advertisements`` listings).
|
||||
|
||||
All helpers are no-ops when ``app.state.redis_cache`` is missing (cache
|
||||
disabled) and swallow any backend error so a cache outage never breaks a
|
||||
successful write — matching the resilience pattern in ``RedisCacheBackend``.
|
||||
"""
|
||||
|
||||
import logging
|
||||
from typing import Optional
|
||||
|
||||
from fastapi import Request
|
||||
|
||||
from meshcore_hub.common.redis import CacheBackend
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _cache(request: Request) -> Optional[CacheBackend]:
|
||||
"""Return the cache backend for this app, or None if caching is disabled."""
|
||||
return getattr(request.app.state, "redis_cache", None)
|
||||
|
||||
|
||||
def _drop(request: Request, prefix: str) -> None:
|
||||
"""Best-effort ``delete(prefix)``; never raises.
|
||||
|
||||
Emits structured log lines so production traces can confirm a mutation
|
||||
handler actually fired invalidation and see how many Redis keys were
|
||||
deleted. The ``backend=`` field distinguishes ``RedisCacheBackend``
|
||||
(real Redis) from ``NullCache`` (Redis disabled) in one glance — useful
|
||||
when ``REDIS_ENABLED`` is misconfigured.
|
||||
"""
|
||||
cache = _cache(request)
|
||||
if cache is None:
|
||||
logger.debug(
|
||||
"Cache invalidate skipped (no backend on app.state): prefix=%s",
|
||||
prefix,
|
||||
)
|
||||
return
|
||||
logger.info(
|
||||
"Cache invalidate start: prefix=%s backend=%s",
|
||||
prefix,
|
||||
type(cache).__name__,
|
||||
)
|
||||
try:
|
||||
cache.delete(prefix)
|
||||
logger.info("Cache invalidate ok: prefix=%s", prefix)
|
||||
except Exception as e:
|
||||
logger.warning(
|
||||
"Cache invalidate error: prefix=%s error=%s",
|
||||
prefix,
|
||||
e,
|
||||
)
|
||||
|
||||
|
||||
def invalidate_channels(request: Request) -> None:
|
||||
"""Drop cached ``GET /channels`` responses (role-aware, URL-path keys)."""
|
||||
_drop(request, "/api/v1/channels")
|
||||
|
||||
|
||||
def invalidate_routes(request: Request) -> None:
|
||||
"""Drop cached ``GET /routes``, ``/routes/{id}`` and ``/routes/{id}/history``.
|
||||
|
||||
All three endpoints share the ``/api/v1/routes`` URL-path prefix in their
|
||||
cache keys (the ``{id}`` and ``{id}/history`` sub-paths glob-match the
|
||||
same SCAN), so a single ``delete`` covers them.
|
||||
"""
|
||||
_drop(request, "/api/v1/routes")
|
||||
|
||||
|
||||
def invalidate_nodes(request: Request) -> None:
|
||||
"""Drop cached ``GET /nodes`` responses (endpoint-name keys, no key_builder)."""
|
||||
_drop(request, "nodes")
|
||||
|
||||
|
||||
def invalidate_profiles(request: Request) -> None:
|
||||
"""Drop cached ``GET /user/profiles`` responses (endpoint-name keys)."""
|
||||
_drop(request, "profiles")
|
||||
|
||||
|
||||
def invalidate_messages(request: Request) -> None:
|
||||
"""Drop cached ``GET /messages`` responses (role-aware, URL-path keys)."""
|
||||
_drop(request, "/api/v1/messages")
|
||||
|
||||
|
||||
def invalidate_advertisements(request: Request) -> None:
|
||||
"""Drop cached ``GET /advertisements`` responses (endpoint-name keys)."""
|
||||
_drop(request, "advertisements")
|
||||
|
||||
|
||||
def invalidate_dashboard(request: Request) -> None:
|
||||
"""Drop every cached ``GET /dashboard/*`` response.
|
||||
|
||||
Dashboard endpoints split across both key formats: ``stats`` and
|
||||
``message-activity`` use a ``key_builder`` (URL-path keys under
|
||||
``/api/v1/dashboard``) while ``activity``, ``packet-activity``,
|
||||
``packet-breakdown`` and ``node-count`` use endpoint-name keys under
|
||||
``dashboard``. Delete both prefixes to cover all of them.
|
||||
"""
|
||||
_drop(request, "dashboard")
|
||||
_drop(request, "/api/v1/dashboard")
|
||||
@@ -7,6 +7,12 @@ from sqlalchemy import select
|
||||
from sqlalchemy.orm import selectinload
|
||||
|
||||
from meshcore_hub.api.auth import RequireOperatorOrAdmin
|
||||
from meshcore_hub.api.cache_invalidation import (
|
||||
invalidate_advertisements,
|
||||
invalidate_dashboard,
|
||||
invalidate_nodes,
|
||||
invalidate_profiles,
|
||||
)
|
||||
from meshcore_hub.api.dependencies import DbSession
|
||||
from meshcore_hub.api.profile_utils import get_or_create_profile
|
||||
from meshcore_hub.common.models import Node, UserProfileNode
|
||||
@@ -17,6 +23,19 @@ logger = logging.getLogger(__name__)
|
||||
router = APIRouter()
|
||||
|
||||
|
||||
def _invalidate_adoption_caches(request: Request) -> None:
|
||||
"""Drop caches that embed adoption info.
|
||||
|
||||
``adopted_by`` and the ``?adopted_by=`` filter surface inside nodes,
|
||||
profiles, and advertisements listings; dashboard operator/node counts
|
||||
also depend on adoptions.
|
||||
"""
|
||||
invalidate_nodes(request)
|
||||
invalidate_profiles(request)
|
||||
invalidate_advertisements(request)
|
||||
invalidate_dashboard(request)
|
||||
|
||||
|
||||
@router.post("", response_model=AdoptedNodeRead, status_code=201)
|
||||
def adopt_node(
|
||||
adopt_request: NodeAdoptRequest,
|
||||
@@ -59,6 +78,8 @@ def adopt_node(
|
||||
session.commit()
|
||||
session.refresh(association)
|
||||
|
||||
_invalidate_adoption_caches(request)
|
||||
|
||||
logger.info(
|
||||
"User %s adopted node %s",
|
||||
caller_id,
|
||||
@@ -119,6 +140,7 @@ def release_node(
|
||||
|
||||
session.delete(association)
|
||||
session.commit()
|
||||
_invalidate_adoption_caches(request)
|
||||
|
||||
logger.info(
|
||||
"User %s released node %s",
|
||||
|
||||
@@ -5,6 +5,7 @@ from sqlalchemy import select
|
||||
|
||||
from meshcore_hub.api.auth import RequireAdmin, RequireRead
|
||||
from meshcore_hub.api.cache import cached, sorted_query_string
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_channels
|
||||
from meshcore_hub.api.channel_visibility import (
|
||||
VISIBILITY_LEVELS,
|
||||
get_max_visibility_level,
|
||||
@@ -75,6 +76,7 @@ def create_channel(
|
||||
__: RequireAdmin,
|
||||
session: DbSession,
|
||||
body: ChannelCreate,
|
||||
request: Request,
|
||||
) -> ChannelRead:
|
||||
"""Create a new channel (admin only)."""
|
||||
existing = session.execute(
|
||||
@@ -106,6 +108,7 @@ def create_channel(
|
||||
session.commit()
|
||||
session.refresh(channel)
|
||||
|
||||
invalidate_channels(request)
|
||||
return _channel_to_read(channel, include_key=True)
|
||||
|
||||
|
||||
@@ -115,6 +118,7 @@ def update_channel(
|
||||
session: DbSession,
|
||||
channel_id: str,
|
||||
body: ChannelUpdate,
|
||||
request: Request,
|
||||
) -> ChannelRead:
|
||||
"""Update a channel (admin only, name is immutable)."""
|
||||
channel = session.execute(
|
||||
@@ -145,6 +149,7 @@ def update_channel(
|
||||
session.commit()
|
||||
session.refresh(channel)
|
||||
|
||||
invalidate_channels(request)
|
||||
return _channel_to_read(channel, include_key=True)
|
||||
|
||||
|
||||
@@ -153,6 +158,7 @@ def delete_channel(
|
||||
__: RequireAdmin,
|
||||
session: DbSession,
|
||||
channel_id: str,
|
||||
request: Request,
|
||||
) -> None:
|
||||
"""Delete a channel (admin only)."""
|
||||
channel = session.execute(
|
||||
@@ -163,3 +169,4 @@ def delete_channel(
|
||||
|
||||
session.delete(channel)
|
||||
session.commit()
|
||||
invalidate_channels(request)
|
||||
|
||||
@@ -4,6 +4,12 @@ from fastapi import APIRouter, HTTPException, Request, status
|
||||
from sqlalchemy import select
|
||||
|
||||
from meshcore_hub.api.auth import RequireOperatorOrAdmin, RequireRead
|
||||
from meshcore_hub.api.cache_invalidation import (
|
||||
invalidate_advertisements,
|
||||
invalidate_dashboard,
|
||||
invalidate_messages,
|
||||
invalidate_nodes,
|
||||
)
|
||||
from meshcore_hub.api.dependencies import DbSession
|
||||
from meshcore_hub.common.models import Node, NodeTag, UserProfile, UserProfileNode
|
||||
from meshcore_hub.common.schemas.nodes import (
|
||||
@@ -16,6 +22,19 @@ from meshcore_hub.common.schemas.nodes import (
|
||||
router = APIRouter()
|
||||
|
||||
|
||||
def _invalidate_node_tag_caches(request: Request) -> None:
|
||||
"""Drop caches that embed or depend on node tags.
|
||||
|
||||
Tag values drive node display names and search/sort, message sender names,
|
||||
advertisement tag labels, and dashboard counts/friendly names — so a tag
|
||||
write invalidates all of them.
|
||||
"""
|
||||
invalidate_nodes(request)
|
||||
invalidate_messages(request)
|
||||
invalidate_advertisements(request)
|
||||
invalidate_dashboard(request)
|
||||
|
||||
|
||||
def _check_tag_access(
|
||||
session: DbSession,
|
||||
caller_info: tuple[str, list[str]],
|
||||
@@ -111,6 +130,7 @@ def create_node_tag(
|
||||
session.commit()
|
||||
session.refresh(node_tag)
|
||||
|
||||
_invalidate_node_tag_caches(request)
|
||||
return NodeTagRead.model_validate(node_tag)
|
||||
|
||||
|
||||
@@ -167,6 +187,7 @@ def update_node_tag(
|
||||
session.commit()
|
||||
session.refresh(node_tag)
|
||||
|
||||
_invalidate_node_tag_caches(request)
|
||||
return NodeTagRead.model_validate(node_tag)
|
||||
|
||||
|
||||
@@ -197,3 +218,4 @@ def delete_node_tag(
|
||||
|
||||
session.delete(node_tag)
|
||||
session.commit()
|
||||
_invalidate_node_tag_caches(request)
|
||||
|
||||
@@ -7,6 +7,7 @@ from sqlalchemy import select
|
||||
|
||||
from meshcore_hub.api.auth import RequireAdmin, RequireRead
|
||||
from meshcore_hub.api.cache import cached, sorted_query_string
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_routes
|
||||
from meshcore_hub.api.channel_visibility import (
|
||||
VISIBILITY_LEVELS,
|
||||
get_max_visibility_level,
|
||||
@@ -15,9 +16,11 @@ from meshcore_hub.api.channel_visibility import (
|
||||
from meshcore_hub.api.dependencies import DbSession
|
||||
from meshcore_hub.collector.routes import (
|
||||
derive_expected_hash,
|
||||
evaluate_route,
|
||||
evaluate_route_history,
|
||||
preview_route,
|
||||
recent_matches,
|
||||
upsert_route_result,
|
||||
)
|
||||
from meshcore_hub.common.config import get_collector_settings
|
||||
from meshcore_hub.common.models.node import Node
|
||||
@@ -142,6 +145,28 @@ def _sync_observers(
|
||||
session.add(RouteObserver(route_id=route.id, node_id=node.id))
|
||||
|
||||
|
||||
def _reevaluate_route(session: DbSession, route: Route) -> None:
|
||||
"""Synchronously evaluate *route* and persist the fresh ``RouteResult``.
|
||||
|
||||
The background evaluator (collector.route_evaluator) writes
|
||||
``RouteResult`` on a schedule (default 60s). Without this synchronous
|
||||
re-eval, the route's ``packet_count_threshold`` / ``clear_threshold``
|
||||
changes take up to that interval to surface in the UI — the list card
|
||||
displays ``route_result.threshold`` / ``effective_clear``, not the
|
||||
route's just-updated direct fields, so it shows the stale snapshot
|
||||
until the next evaluator cycle. Running the eval inline on every
|
||||
create/update keeps the post-mutation GET consistent with the new
|
||||
config at the cost of one bounded DB scan per write.
|
||||
"""
|
||||
if not route.enabled:
|
||||
return
|
||||
since = datetime.now(timezone.utc) - timedelta(hours=route.window_hours)
|
||||
state, quality, matched_count = evaluate_route(session, route, since)
|
||||
upsert_route_result(session, route, state, quality, matched_count)
|
||||
session.commit()
|
||||
session.refresh(route)
|
||||
|
||||
|
||||
@router.get("", response_model=RouteList)
|
||||
@cached("routes", key_builder=_routes_key_builder)
|
||||
def list_routes(
|
||||
@@ -167,6 +192,7 @@ def create_route(
|
||||
__: RequireAdmin,
|
||||
session: DbSession,
|
||||
body: RouteCreate,
|
||||
request: Request,
|
||||
) -> RouteRead:
|
||||
"""Create a new route (admin only)."""
|
||||
existing = session.execute(
|
||||
@@ -210,6 +236,8 @@ def create_route(
|
||||
_sync_observers(session, route, observer_nodes)
|
||||
session.commit()
|
||||
session.refresh(route)
|
||||
_reevaluate_route(session, route)
|
||||
invalidate_routes(request)
|
||||
return _route_to_read(route)
|
||||
|
||||
|
||||
@@ -320,6 +348,7 @@ def update_route(
|
||||
session: DbSession,
|
||||
route_id: str,
|
||||
body: RouteUpdate,
|
||||
request: Request,
|
||||
) -> RouteRead:
|
||||
"""Update a route (admin only)."""
|
||||
route = session.execute(
|
||||
@@ -379,6 +408,8 @@ def update_route(
|
||||
|
||||
session.commit()
|
||||
session.refresh(route)
|
||||
_reevaluate_route(session, route)
|
||||
invalidate_routes(request)
|
||||
return _route_to_read(route)
|
||||
|
||||
|
||||
@@ -387,6 +418,7 @@ def delete_route(
|
||||
__: RequireAdmin,
|
||||
session: DbSession,
|
||||
route_id: str,
|
||||
request: Request,
|
||||
) -> None:
|
||||
"""Delete a route (admin only)."""
|
||||
route = session.execute(
|
||||
@@ -396,6 +428,7 @@ def delete_route(
|
||||
raise HTTPException(status_code=404, detail="Route not found")
|
||||
session.delete(route)
|
||||
session.commit()
|
||||
invalidate_routes(request)
|
||||
|
||||
|
||||
@router.post("/preview", response_model=RoutePreviewResponse)
|
||||
|
||||
@@ -9,6 +9,10 @@ from sqlalchemy.orm import selectinload
|
||||
|
||||
from meshcore_hub.api.auth import RequireRead, RequireUserOwner, X_USER_ID_HEADER
|
||||
from meshcore_hub.api.cache import cached
|
||||
from meshcore_hub.api.cache_invalidation import (
|
||||
invalidate_dashboard,
|
||||
invalidate_profiles,
|
||||
)
|
||||
from meshcore_hub.api.dependencies import DbSession
|
||||
from meshcore_hub.api.profile_utils import get_or_create_profile
|
||||
from meshcore_hub.common.config import get_web_settings
|
||||
@@ -236,4 +240,6 @@ def update_profile(
|
||||
session.commit()
|
||||
session.refresh(profile)
|
||||
|
||||
invalidate_profiles(request)
|
||||
invalidate_dashboard(request)
|
||||
return UserProfileRead.model_validate(profile)
|
||||
|
||||
@@ -32,7 +32,9 @@ class NullCache(CacheBackend):
|
||||
pass
|
||||
|
||||
def delete(self, prefix: str) -> None:
|
||||
pass
|
||||
# Logged so production traces can distinguish "Redis disabled" from
|
||||
# "Redis enabled but matched no keys" when diagnosing invalidation.
|
||||
logger.debug("NullCache delete: prefix=%s", prefix)
|
||||
|
||||
def ping(self) -> bool:
|
||||
return False
|
||||
@@ -84,19 +86,40 @@ class RedisCacheBackend(CacheBackend):
|
||||
logger.warning("Redis SET error for %s: %s", key, e)
|
||||
|
||||
def delete(self, prefix: str) -> None:
|
||||
# Diagnostic logging: emit one INFO line per call with the prefix,
|
||||
# full prefix (incl. key_prefix), total keys deleted, and SCAN
|
||||
# iterations. ``keys_deleted=0`` after a mutation that should have
|
||||
# invalidated entries is the smoking gun for a cache-key mismatch
|
||||
# between the store path (key_builder) and the delete path (prefix).
|
||||
full_prefix = self._full_key(prefix)
|
||||
total_deleted = 0
|
||||
iterations = 0
|
||||
try:
|
||||
full_prefix = self._full_key(prefix)
|
||||
cursor = 0
|
||||
while True:
|
||||
cursor, keys = self._client.scan(
|
||||
cursor, match=f"{full_prefix}*", count=100
|
||||
)
|
||||
iterations += 1
|
||||
if keys:
|
||||
self._client.delete(*keys)
|
||||
total_deleted += len(keys)
|
||||
if cursor == 0:
|
||||
break
|
||||
logger.info(
|
||||
"Redis cache delete: prefix=%s full_prefix=%s keys_deleted=%d scan_iterations=%d",
|
||||
prefix,
|
||||
full_prefix,
|
||||
total_deleted,
|
||||
iterations,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.warning("Redis DELETE error for prefix %s: %s", prefix, e)
|
||||
logger.warning(
|
||||
"Redis DELETE error: prefix=%s full_prefix=%s error=%s",
|
||||
prefix,
|
||||
full_prefix,
|
||||
e,
|
||||
)
|
||||
|
||||
def ping(self) -> bool:
|
||||
try:
|
||||
|
||||
+638
-16
@@ -878,24 +878,32 @@ class TestCacheControlMiddleware:
|
||||
client_no_auth.app.state.redis_cache_ttl = 30
|
||||
response = client_no_auth.get("/api/v1/nodes")
|
||||
assert response.status_code == 200
|
||||
assert response.headers["cache-control"] == "private, max-age=30"
|
||||
# no-cache (must-revalidate) — see api/app.py middleware docstring.
|
||||
# The Redis TTL still flows to cache.set(...); only HTTP max-age is
|
||||
# dropped so server-side invalidation can reach the browser.
|
||||
assert response.headers["cache-control"] == "private, no-cache"
|
||||
assert "etag" in response.headers
|
||||
|
||||
def test_cached_get_cache_control_uses_overridden_ttl(self, client_no_auth):
|
||||
"""Route detail endpoint should use the route-detail TTL (300s)."""
|
||||
def test_cached_get_ttl_flows_to_redis_not_http(self, client_no_auth):
|
||||
"""Route detail endpoint's TTL must drive cache.set, not Cache-Control.
|
||||
|
||||
Regression: previously the per-endpoint TTL (e.g. 300s for
|
||||
``/routes/{id}``) was emitted as ``Cache-Control: max-age=300``,
|
||||
which let the browser serve stale data for 5 min after a mutation.
|
||||
The TTL now only bounds the Redis cache lifetime; HTTP-layer is
|
||||
always ``private, no-cache`` so the browser revalidates and the
|
||||
server-side invalidation wins.
|
||||
"""
|
||||
mock_cache = MagicMock()
|
||||
mock_cache.get.return_value = None
|
||||
client_no_auth.app.state.redis_cache = mock_cache
|
||||
client_no_auth.app.state.redis_cache_ttl = 30
|
||||
client_no_auth.app.state.redis_cache_ttl_route_detail = 300
|
||||
# Need a route to exist; this is a happy-path check of the header value
|
||||
# so we stub the cache and just hit the endpoint with a fake id. The
|
||||
# endpoint will return 404 but still flow through the @cached decorator
|
||||
# and the middleware.
|
||||
# Hit the route detail endpoint with a fake id. The endpoint returns
|
||||
# 404 but still flows through the @cached decorator; we only care
|
||||
# that the TTL is NOT surfaced as an HTTP header.
|
||||
response = client_no_auth.get("/api/v1/routes/nonexistent-id")
|
||||
# 404 is fine; we only care about the header applied by the decorator
|
||||
# via request.state.cache_control_ttl.
|
||||
assert response.headers.get("cache-control") == "private, max-age=300"
|
||||
assert response.headers.get("cache-control") == "private, no-cache"
|
||||
|
||||
def test_cached_get_304_on_matching_if_none_match(self, client_no_auth):
|
||||
mock_cache = MagicMock()
|
||||
@@ -913,22 +921,20 @@ class TestCacheControlMiddleware:
|
||||
response = client_no_auth.get("/api/v1/nodes", headers={"If-None-Match": etag})
|
||||
assert response.status_code == 304
|
||||
assert response.headers["ETag"] == etag
|
||||
assert response.headers["cache-control"] == "private, max-age=30"
|
||||
assert response.headers["cache-control"] == "private, no-cache"
|
||||
assert response.headers["x-cache"] == "HIT"
|
||||
# 304 must not carry a body.
|
||||
assert response.content in (b"", b"null")
|
||||
|
||||
def test_uncached_get_emits_must_revalidate(self, client_no_auth, sample_node):
|
||||
"""Uncached GET detail endpoints get max-age=0, must-revalidate."""
|
||||
def test_uncached_get_emits_no_cache(self, client_no_auth, sample_node):
|
||||
"""Uncached GET detail endpoints get the same no-cache policy."""
|
||||
# Force the @cached list endpoint to NOT be the target by hitting the
|
||||
# per-id endpoint, which is not cached.
|
||||
if hasattr(client_no_auth.app.state, "redis_cache"):
|
||||
del client_no_auth.app.state.redis_cache
|
||||
response = client_no_auth.get(f"/api/v1/nodes/{sample_node.public_key}")
|
||||
assert response.status_code == 200
|
||||
assert (
|
||||
response.headers["cache-control"] == "private, max-age=0, must-revalidate"
|
||||
)
|
||||
assert response.headers["cache-control"] == "private, no-cache"
|
||||
|
||||
def test_post_emits_no_store(self, client_no_auth, api_db_session):
|
||||
"""POST endpoints always get Cache-Control: no-store.
|
||||
@@ -1330,3 +1336,619 @@ class TestKeyBuilders:
|
||||
key = _messages_key_builder(request)
|
||||
assert "role=admin" in key
|
||||
assert "limit=10" in key
|
||||
|
||||
|
||||
def _make_request_with_cache(cache):
|
||||
"""Build a Request whose ``app.state.redis_cache`` is *cache* (or absent)."""
|
||||
app = FastAPI()
|
||||
if cache is not None:
|
||||
app.state.redis_cache = cache
|
||||
return Request(
|
||||
scope={"type": "http", "query_string": b"", "headers": [], "app": app}
|
||||
)
|
||||
|
||||
|
||||
class TestCacheInvalidationHelpers:
|
||||
"""Unit tests for ``meshcore_hub.api.cache_invalidation`` helpers."""
|
||||
|
||||
def test_invalidate_channels_drops_url_path_prefix(self):
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_channels
|
||||
|
||||
cache = MagicMock()
|
||||
invalidate_channels(_make_request_with_cache(cache))
|
||||
cache.delete.assert_called_once_with("/api/v1/channels")
|
||||
|
||||
def test_invalidate_routes_drops_url_path_prefix(self):
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_routes
|
||||
|
||||
cache = MagicMock()
|
||||
invalidate_routes(_make_request_with_cache(cache))
|
||||
# Single prefix covers /routes, /routes/{id}, /routes/{id}/history
|
||||
cache.delete.assert_called_once_with("/api/v1/routes")
|
||||
|
||||
def test_invalidate_nodes_drops_endpoint_name_prefix(self):
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_nodes
|
||||
|
||||
cache = MagicMock()
|
||||
invalidate_nodes(_make_request_with_cache(cache))
|
||||
cache.delete.assert_called_once_with("nodes")
|
||||
|
||||
def test_invalidate_profiles_drops_endpoint_name_prefix(self):
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_profiles
|
||||
|
||||
cache = MagicMock()
|
||||
invalidate_profiles(_make_request_with_cache(cache))
|
||||
cache.delete.assert_called_once_with("profiles")
|
||||
|
||||
def test_invalidate_messages_drops_url_path_prefix(self):
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_messages
|
||||
|
||||
cache = MagicMock()
|
||||
invalidate_messages(_make_request_with_cache(cache))
|
||||
cache.delete.assert_called_once_with("/api/v1/messages")
|
||||
|
||||
def test_invalidate_advertisements_drops_endpoint_name_prefix(self):
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_advertisements
|
||||
|
||||
cache = MagicMock()
|
||||
invalidate_advertisements(_make_request_with_cache(cache))
|
||||
cache.delete.assert_called_once_with("advertisements")
|
||||
|
||||
def test_invalidate_dashboard_drops_both_prefix_formats(self):
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_dashboard
|
||||
|
||||
cache = MagicMock()
|
||||
invalidate_dashboard(_make_request_with_cache(cache))
|
||||
# Dashboard endpoints split between endpoint-name keys and URL-path keys
|
||||
cache.delete.assert_any_call("dashboard")
|
||||
cache.delete.assert_any_call("/api/v1/dashboard")
|
||||
assert cache.delete.call_count == 2
|
||||
|
||||
def test_helpers_are_noop_when_cache_missing(self):
|
||||
# No redis_cache attribute on state — must not raise.
|
||||
from meshcore_hub.api import cache_invalidation as inv
|
||||
|
||||
request = _make_request_with_cache(cache=None)
|
||||
inv.invalidate_channels(request)
|
||||
inv.invalidate_routes(request)
|
||||
inv.invalidate_nodes(request)
|
||||
inv.invalidate_profiles(request)
|
||||
inv.invalidate_messages(request)
|
||||
inv.invalidate_advertisements(request)
|
||||
inv.invalidate_dashboard(request)
|
||||
|
||||
def test_helpers_swallow_backend_errors(self):
|
||||
from meshcore_hub.api import cache_invalidation as inv
|
||||
|
||||
cache = MagicMock()
|
||||
cache.delete.side_effect = Exception("redis down")
|
||||
request = _make_request_with_cache(cache)
|
||||
# Must not raise.
|
||||
inv.invalidate_channels(request)
|
||||
inv.invalidate_dashboard(request)
|
||||
|
||||
|
||||
class TestMutationInvalidationIntegration:
|
||||
"""End-to-end: mutation handlers must drop the expected cache prefixes."""
|
||||
|
||||
def _install_mock_cache(self, client) -> MagicMock:
|
||||
"""Attach a mock cache that always misses; return it for assertions."""
|
||||
mock_cache = MagicMock()
|
||||
mock_cache.get.return_value = None
|
||||
client.app.state.redis_cache = mock_cache
|
||||
client.app.state.redis_cache_ttl = 30
|
||||
client.app.state.redis_cache_ttl_dashboard = 300
|
||||
client.app.state.redis_cache_ttl_route_detail = 300
|
||||
return mock_cache
|
||||
|
||||
# --- Channels ---------------------------------------------------------
|
||||
|
||||
def test_create_channel_invalidates_channels(self, client_no_auth, api_db_session):
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.post(
|
||||
"/api/v1/channels",
|
||||
json={
|
||||
"name": "InvalidationTest",
|
||||
"key_hex": "AABBCCDDEEFF00112233445566778899",
|
||||
"visibility": "community",
|
||||
"enabled": True,
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
mock_cache.delete.assert_any_call("/api/v1/channels")
|
||||
|
||||
def test_update_channel_invalidates_channels(self, client_no_auth, sample_channel):
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.put(
|
||||
f"/api/v1/channels/{sample_channel.id}",
|
||||
json={"enabled": False},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
mock_cache.delete.assert_any_call("/api/v1/channels")
|
||||
|
||||
def test_delete_channel_invalidates_channels(self, client_no_auth, sample_channel):
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.delete(f"/api/v1/channels/{sample_channel.id}")
|
||||
assert resp.status_code == 204
|
||||
mock_cache.delete.assert_any_call("/api/v1/channels")
|
||||
|
||||
# --- Routes -----------------------------------------------------------
|
||||
|
||||
def test_create_route_invalidates_routes(self, client_no_auth, api_db_session):
|
||||
from meshcore_hub.common.models import Node
|
||||
|
||||
n1 = Node(public_key="aa" * 16, name="A")
|
||||
n2 = Node(public_key="bb" * 16, name="B")
|
||||
api_db_session.add_all([n1, n2])
|
||||
api_db_session.commit()
|
||||
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.post(
|
||||
"/api/v1/routes",
|
||||
json={
|
||||
"from_label": "A",
|
||||
"to_label": "B",
|
||||
"node_public_keys": [n1.public_key, n2.public_key],
|
||||
"match_width": 2,
|
||||
},
|
||||
headers={"X-User-Roles": "admin"},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
mock_cache.delete.assert_any_call("/api/v1/routes")
|
||||
|
||||
def test_update_route_invalidates_routes(self, client_no_auth, api_db_session):
|
||||
from meshcore_hub.common.models import Node, Route, RouteNode
|
||||
|
||||
nodes = [Node(public_key=f"{c:02x}" * 16, name=str(c)) for c in (1, 2)]
|
||||
api_db_session.add_all(nodes)
|
||||
api_db_session.flush()
|
||||
route = Route(from_label="X", to_label="Y")
|
||||
api_db_session.add(route)
|
||||
api_db_session.flush()
|
||||
for pos, n in enumerate(nodes):
|
||||
api_db_session.add(
|
||||
RouteNode(
|
||||
route_id=route.id,
|
||||
node_id=n.id,
|
||||
position=pos,
|
||||
expected_hash=n.public_key[:2].upper(),
|
||||
)
|
||||
)
|
||||
api_db_session.commit()
|
||||
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.put(
|
||||
f"/api/v1/routes/{route.id}",
|
||||
json={"from_label": "NewFrom", "to_label": "NewTo"},
|
||||
headers={"X-User-Roles": "admin"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
mock_cache.delete.assert_any_call("/api/v1/routes")
|
||||
|
||||
def test_delete_route_invalidates_routes(self, client_no_auth, api_db_session):
|
||||
from meshcore_hub.common.models import Node, Route, RouteNode
|
||||
|
||||
nodes = [Node(public_key=f"{c:02x}" * 16, name=str(c)) for c in (3, 4)]
|
||||
api_db_session.add_all(nodes)
|
||||
api_db_session.flush()
|
||||
route = Route(from_label="P", to_label="Q")
|
||||
api_db_session.add(route)
|
||||
api_db_session.flush()
|
||||
for pos, n in enumerate(nodes):
|
||||
api_db_session.add(
|
||||
RouteNode(
|
||||
route_id=route.id,
|
||||
node_id=n.id,
|
||||
position=pos,
|
||||
expected_hash=n.public_key[:2].upper(),
|
||||
)
|
||||
)
|
||||
api_db_session.commit()
|
||||
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.delete(
|
||||
f"/api/v1/routes/{route.id}",
|
||||
headers={"X-User-Roles": "admin"},
|
||||
)
|
||||
assert resp.status_code == 204
|
||||
mock_cache.delete.assert_any_call("/api/v1/routes")
|
||||
|
||||
# --- User profiles ----------------------------------------------------
|
||||
|
||||
def test_update_profile_invalidates_profiles_and_dashboard(
|
||||
self, client_no_auth, sample_user_profile
|
||||
):
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.put(
|
||||
f"/api/v1/user/profile/{sample_user_profile.id}",
|
||||
json={"name": "Renamed"},
|
||||
headers={
|
||||
"X-User-Id": sample_user_profile.user_id,
|
||||
"X-User-Roles": "operator",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
mock_cache.delete.assert_any_call("profiles")
|
||||
mock_cache.delete.assert_any_call("dashboard")
|
||||
mock_cache.delete.assert_any_call("/api/v1/dashboard")
|
||||
|
||||
# --- Node tags --------------------------------------------------------
|
||||
|
||||
def test_create_node_tag_invalidates_cross_entity_caches(
|
||||
self, client_no_auth, sample_node, sample_operator_adoption
|
||||
):
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.post(
|
||||
f"/api/v1/nodes/{sample_node.public_key}/tags",
|
||||
json={"key": "name", "value": "Friendly"},
|
||||
headers={
|
||||
"X-User-Id": "operator-123",
|
||||
"X-User-Roles": "operator",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
for prefix in ("nodes", "/api/v1/messages", "advertisements"):
|
||||
mock_cache.delete.assert_any_call(prefix)
|
||||
# Dashboard covers both key formats
|
||||
mock_cache.delete.assert_any_call("dashboard")
|
||||
mock_cache.delete.assert_any_call("/api/v1/dashboard")
|
||||
|
||||
def test_delete_node_tag_invalidates_cross_entity_caches(
|
||||
self, client_no_auth, sample_node, sample_node_tag, sample_operator_adoption
|
||||
):
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.delete(
|
||||
f"/api/v1/nodes/{sample_node.public_key}/tags/{sample_node_tag.key}",
|
||||
headers={
|
||||
"X-User-Id": "operator-123",
|
||||
"X-User-Roles": "operator",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 204
|
||||
for prefix in ("nodes", "/api/v1/messages", "advertisements", "dashboard"):
|
||||
mock_cache.delete.assert_any_call(prefix)
|
||||
|
||||
# --- Adoptions --------------------------------------------------------
|
||||
|
||||
def test_adopt_node_invalidates_cross_entity_caches(
|
||||
self, client_no_auth, sample_node
|
||||
):
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.post(
|
||||
"/api/v1/adoptions",
|
||||
json={"public_key": sample_node.public_key},
|
||||
headers={"X-User-Id": "adopter-1", "X-User-Roles": "operator"},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
for prefix in ("nodes", "profiles", "advertisements", "dashboard"):
|
||||
mock_cache.delete.assert_any_call(prefix)
|
||||
mock_cache.delete.assert_any_call("/api/v1/dashboard")
|
||||
|
||||
def test_release_node_invalidates_cross_entity_caches(
|
||||
self, client_no_auth, sample_node, sample_adopted_node
|
||||
):
|
||||
mock_cache = self._install_mock_cache(client_no_auth)
|
||||
resp = client_no_auth.delete(
|
||||
f"/api/v1/adoptions/{sample_node.public_key}",
|
||||
headers={"X-User-Id": "oidc-user-123", "X-User-Roles": "operator"},
|
||||
)
|
||||
assert resp.status_code == 204
|
||||
for prefix in ("nodes", "profiles", "advertisements", "dashboard"):
|
||||
mock_cache.delete.assert_any_call(prefix)
|
||||
|
||||
# --- Resilience -------------------------------------------------------
|
||||
|
||||
def test_cache_delete_error_does_not_break_mutation(
|
||||
self, client_no_auth, sample_channel
|
||||
):
|
||||
"""If Redis is down, the mutation must still succeed."""
|
||||
mock_cache = MagicMock()
|
||||
mock_cache.get.return_value = None
|
||||
mock_cache.delete.side_effect = Exception("redis down")
|
||||
client_no_auth.app.state.redis_cache = mock_cache
|
||||
client_no_auth.app.state.redis_cache_ttl = 30
|
||||
|
||||
resp = client_no_auth.put(
|
||||
f"/api/v1/channels/{sample_channel.id}",
|
||||
json={"enabled": False},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
|
||||
|
||||
class TestMutationVisibilityThroughHttpCache:
|
||||
"""Regression: stale browser HTTP cache after a mutation.
|
||||
|
||||
Scenario reported in the wild: user edits a Route, the routes list page
|
||||
keeps showing old values for ~30s. Root cause was the API emitting
|
||||
``Cache-Control: private, max-age=30`` on ``@cached`` GETs, which lets
|
||||
the browser serve its local copy without revalidating — so the
|
||||
server-side invalidation never had a chance to fire.
|
||||
|
||||
The policy is now ``private, no-cache`` for all ``@cached`` GETs, which
|
||||
forces the browser to send ``If-None-Match`` on every navigation. This
|
||||
test models the full round trip: cache-fill, conditional 304, mutation
|
||||
(invalidating Redis), then conditional 200 with the fresh body.
|
||||
"""
|
||||
|
||||
def test_routes_list_refresh_after_mutation(self, client_no_auth, api_db_session):
|
||||
from meshcore_hub.common.models import Node, Route, RouteNode
|
||||
|
||||
# Seed a route the user will later edit.
|
||||
nodes = [Node(public_key=f"{c:02x}" * 16, name=str(c)) for c in (1, 2)]
|
||||
api_db_session.add_all(nodes)
|
||||
api_db_session.flush()
|
||||
route = Route(from_label="Origin", to_label="Dest")
|
||||
api_db_session.add(route)
|
||||
api_db_session.flush()
|
||||
for pos, n in enumerate(nodes):
|
||||
api_db_session.add(
|
||||
RouteNode(
|
||||
route_id=route.id,
|
||||
node_id=n.id,
|
||||
position=pos,
|
||||
expected_hash=n.public_key[:2].upper(),
|
||||
)
|
||||
)
|
||||
api_db_session.commit()
|
||||
|
||||
# Real in-memory cache so set/get/delete behave end-to-end. Keys
|
||||
# store the envelope the @cached decorator writes.
|
||||
store: dict[str, str] = {}
|
||||
|
||||
class _FakeCache:
|
||||
def get(self, key):
|
||||
return store.get(key)
|
||||
|
||||
def set(self, key, value, ttl):
|
||||
store[key] = value
|
||||
|
||||
def delete(self, prefix):
|
||||
# SCAN-style prefix glob, matching RedisCacheBackend.delete.
|
||||
for k in list(store.keys()):
|
||||
if k.startswith(prefix):
|
||||
del store[k]
|
||||
|
||||
def ping(self):
|
||||
return True
|
||||
|
||||
client_no_auth.app.state.redis_cache = _FakeCache()
|
||||
client_no_auth.app.state.redis_cache_ttl = 30
|
||||
|
||||
# 1) Initial GET — populates cache and returns an ETag.
|
||||
first = client_no_auth.get("/api/v1/routes", headers={"X-User-Roles": "admin"})
|
||||
assert first.status_code == 200
|
||||
assert first.headers["x-cache"] == "MISS"
|
||||
assert first.headers["cache-control"] == "private, no-cache"
|
||||
first_etag = first.headers["etag"]
|
||||
first_body = first.json()
|
||||
assert first_body["items"][0]["from_label"] == "Origin"
|
||||
|
||||
# 2) Immediate re-fetch with If-None-Match must 304 (cache HIT,
|
||||
# ETag matches). This is the cheap fast path the policy
|
||||
# preserves: browser revalidates, server answers 304, no body.
|
||||
cond = client_no_auth.get(
|
||||
"/api/v1/routes",
|
||||
headers={"X-User-Roles": "admin", "If-None-Match": first_etag},
|
||||
)
|
||||
assert cond.status_code == 304
|
||||
assert cond.headers["x-cache"] == "HIT"
|
||||
assert cond.headers["cache-control"] == "private, no-cache"
|
||||
assert cond.content in (b"", b"null")
|
||||
|
||||
# 3) Mutate the route. The handler calls invalidate_routes(request),
|
||||
# which must drop the cached entry so the next GET is a MISS.
|
||||
mut = client_no_auth.put(
|
||||
f"/api/v1/routes/{route.id}",
|
||||
json={"from_label": "NewOrigin", "to_label": "NewDest"},
|
||||
headers={"X-User-Roles": "admin"},
|
||||
)
|
||||
assert mut.status_code == 200
|
||||
# Mutations are always no-store.
|
||||
assert mut.headers["cache-control"] == "no-store"
|
||||
|
||||
# 4) Browser navigates again, sending the stale If-None-Match from
|
||||
# step 1. The server MUST NOT 304 here: Redis was invalidated,
|
||||
# so the handler re-runs, produces a new ETag, and returns 200
|
||||
# with the fresh body. This is exactly the bug the user hit —
|
||||
# under the old max-age=30 policy the browser never sent this
|
||||
# request at all.
|
||||
after = client_no_auth.get(
|
||||
"/api/v1/routes",
|
||||
headers={"X-User-Roles": "admin", "If-None-Match": first_etag},
|
||||
)
|
||||
assert after.status_code == 200
|
||||
assert after.headers["x-cache"] == "MISS"
|
||||
assert after.headers["etag"] != first_etag
|
||||
assert after.json()["items"][0]["from_label"] == "NewOrigin"
|
||||
|
||||
def test_cached_gets_always_emit_no_cache_regardless_of_ttl(self, client_no_auth):
|
||||
"""Even a 300s dashboard TTL must not surface as max-age=300.
|
||||
|
||||
The dashboard endpoints have ``redis_cache_ttl_dashboard=300``, but
|
||||
that bound applies only to the Redis cache. The HTTP policy is
|
||||
always ``private, no-cache`` so server-side invalidation can reach
|
||||
the browser after any mutation.
|
||||
"""
|
||||
mock_cache = MagicMock()
|
||||
mock_cache.get.return_value = None
|
||||
client_no_auth.app.state.redis_cache = mock_cache
|
||||
client_no_auth.app.state.redis_cache_ttl = 30
|
||||
client_no_auth.app.state.redis_cache_ttl_dashboard = 300
|
||||
|
||||
resp = client_no_auth.get("/api/v1/dashboard/activity")
|
||||
assert resp.status_code == 200
|
||||
assert resp.headers["cache-control"] == "private, no-cache"
|
||||
|
||||
|
||||
class TestInvalidationLogging:
|
||||
"""Diagnostic logging for cache invalidation.
|
||||
|
||||
These tests pin down the log lines that operators grep for when
|
||||
diagnosing whether mutation handlers actually fire invalidation and
|
||||
whether Redis SCAN matches stored keys. The shape of the log output
|
||||
is part of the contract — changing it breaks log dashboards and the
|
||||
diagnostic runbook.
|
||||
"""
|
||||
|
||||
def test_drop_logs_start_and_ok_on_success(self, caplog):
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_routes
|
||||
|
||||
cache = MagicMock()
|
||||
cache.__class__.__name__ = "RedisCacheBackend"
|
||||
request = _make_request_with_cache(cache)
|
||||
|
||||
with caplog.at_level("INFO", logger="meshcore_hub.api.cache_invalidation"):
|
||||
invalidate_routes(request)
|
||||
|
||||
messages = [r.message for r in caplog.records]
|
||||
assert any(
|
||||
"Cache invalidate start" in m
|
||||
and "prefix=/api/v1/routes" in m
|
||||
and "backend=RedisCacheBackend" in m
|
||||
for m in messages
|
||||
), f"start line missing or malformed: {messages}"
|
||||
assert any(
|
||||
"Cache invalidate ok" in m and "prefix=/api/v1/routes" in m
|
||||
for m in messages
|
||||
), f"ok line missing or malformed: {messages}"
|
||||
|
||||
def test_drop_logs_warning_on_backend_error(self, caplog):
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_channels
|
||||
|
||||
cache = MagicMock()
|
||||
cache.delete.side_effect = Exception("redis down")
|
||||
request = _make_request_with_cache(cache)
|
||||
|
||||
with caplog.at_level("WARNING", logger="meshcore_hub.api.cache_invalidation"):
|
||||
invalidate_channels(request)
|
||||
|
||||
# Must not raise; warning must carry prefix + error text.
|
||||
assert any(
|
||||
"Cache invalidate error" in r.message
|
||||
and "prefix=/api/v1/channels" in r.message
|
||||
and "redis down" in r.message
|
||||
for r in caplog.records
|
||||
), [r.message for r in caplog.records]
|
||||
|
||||
def test_drop_logs_skipped_when_no_backend(self, caplog):
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_nodes
|
||||
|
||||
# No redis_cache attribute on app.state.
|
||||
request = _make_request_with_cache(cache=None)
|
||||
|
||||
with caplog.at_level("DEBUG", logger="meshcore_hub.api.cache_invalidation"):
|
||||
invalidate_nodes(request)
|
||||
|
||||
assert any(
|
||||
"Cache invalidate skipped" in r.message and "prefix=nodes" in r.message
|
||||
for r in caplog.records
|
||||
), [r.message for r in caplog.records]
|
||||
|
||||
def test_drop_logs_backend_name_distinguishes_nullcache(self, caplog):
|
||||
"""If NullCache is wired in, the start log must say so.
|
||||
|
||||
Catches the 'REDIS_ENABLED is actually false in production' case
|
||||
in one log line.
|
||||
"""
|
||||
from meshcore_hub.api.cache_invalidation import invalidate_routes
|
||||
|
||||
null_cache = NullCache()
|
||||
request = _make_request_with_cache(null_cache)
|
||||
|
||||
with caplog.at_level("INFO", logger="meshcore_hub.api.cache_invalidation"):
|
||||
invalidate_routes(request)
|
||||
|
||||
messages = [r.message for r in caplog.records]
|
||||
assert any(
|
||||
"backend=NullCache" in m and "prefix=/api/v1/routes" in m for m in messages
|
||||
), f"expected backend=NullCache in start log, got: {messages}"
|
||||
|
||||
def test_redis_delete_logs_keys_deleted_count(self, caplog):
|
||||
with patch("redis.Redis") as mock_redis_cls:
|
||||
mock_client = MagicMock()
|
||||
mock_redis_cls.return_value = mock_client
|
||||
mock_client.scan.return_value = (0, [b"hub:nodes:1", b"hub:nodes:2"])
|
||||
|
||||
backend = RedisCacheBackend(key_prefix="hub")
|
||||
with caplog.at_level("INFO", logger="meshcore_hub.common.redis"):
|
||||
backend.delete("nodes")
|
||||
|
||||
# One INFO line with prefix, full_prefix, and keys_deleted=2.
|
||||
info_records = [r for r in caplog.records if r.levelname == "INFO"]
|
||||
assert len(info_records) == 1, [r.message for r in caplog.records]
|
||||
msg = info_records[0].message
|
||||
assert "Redis cache delete" in msg
|
||||
assert "prefix=nodes" in msg
|
||||
assert "full_prefix=hub:nodes" in msg
|
||||
assert "keys_deleted=2" in msg
|
||||
assert "scan_iterations=1" in msg
|
||||
|
||||
def test_redis_delete_logs_zero_keys_on_empty_scan(self, caplog):
|
||||
"""The smoking-gun signal for the production bug.
|
||||
|
||||
If invalidation fires but SCAN matches nothing, ``keys_deleted=0``
|
||||
appears in the log. That points directly at a cache-key mismatch
|
||||
between the store path (key_builder) and the delete path (prefix).
|
||||
"""
|
||||
with patch("redis.Redis") as mock_redis_cls:
|
||||
mock_client = MagicMock()
|
||||
mock_redis_cls.return_value = mock_client
|
||||
mock_client.scan.return_value = (0, [])
|
||||
|
||||
backend = RedisCacheBackend(key_prefix="hub")
|
||||
with caplog.at_level("INFO", logger="meshcore_hub.common.redis"):
|
||||
backend.delete("/api/v1/routes")
|
||||
|
||||
info_records = [r for r in caplog.records if r.levelname == "INFO"]
|
||||
assert len(info_records) == 1
|
||||
msg = info_records[0].message
|
||||
assert "keys_deleted=0" in msg
|
||||
assert "prefix=/api/v1/routes" in msg
|
||||
assert "full_prefix=hub:/api/v1/routes" in msg
|
||||
|
||||
def test_redis_delete_warning_includes_full_prefix(self, caplog):
|
||||
with patch("redis.Redis") as mock_redis_cls:
|
||||
mock_client = MagicMock()
|
||||
mock_redis_cls.return_value = mock_client
|
||||
mock_client.scan.side_effect = Exception("scan timeout")
|
||||
|
||||
backend = RedisCacheBackend(key_prefix="hub")
|
||||
with caplog.at_level("WARNING", logger="meshcore_hub.common.redis"):
|
||||
backend.delete("nodes")
|
||||
|
||||
warning_records = [r for r in caplog.records if r.levelname == "WARNING"]
|
||||
assert len(warning_records) == 1
|
||||
msg = warning_records[0].message
|
||||
assert "Redis DELETE error" in msg
|
||||
assert "prefix=nodes" in msg
|
||||
assert "full_prefix=hub:nodes" in msg
|
||||
assert "scan timeout" in msg
|
||||
|
||||
def test_redis_delete_multi_page_scan_logs_total_keys(self, caplog):
|
||||
"""Multi-page SCAN must accumulate keys_deleted across iterations."""
|
||||
with patch("redis.Redis") as mock_redis_cls:
|
||||
mock_client = MagicMock()
|
||||
mock_redis_cls.return_value = mock_client
|
||||
mock_client.scan.side_effect = [
|
||||
(42, [b"hub:nodes:1", b"hub:nodes:2"]),
|
||||
(0, [b"hub:nodes:3"]),
|
||||
]
|
||||
|
||||
backend = RedisCacheBackend(key_prefix="hub")
|
||||
with caplog.at_level("INFO", logger="meshcore_hub.common.redis"):
|
||||
backend.delete("nodes")
|
||||
|
||||
info_records = [r for r in caplog.records if r.levelname == "INFO"]
|
||||
assert len(info_records) == 1
|
||||
msg = info_records[0].message
|
||||
assert "keys_deleted=3" in msg
|
||||
assert "scan_iterations=2" in msg
|
||||
|
||||
def test_nullcache_delete_emits_debug_log(self, caplog):
|
||||
cache = NullCache()
|
||||
with caplog.at_level("DEBUG", logger="meshcore_hub.common.redis"):
|
||||
cache.delete("nodes")
|
||||
assert any(
|
||||
"NullCache delete" in r.message and "prefix=nodes" in r.message
|
||||
for r in caplog.records
|
||||
), [r.message for r in caplog.records]
|
||||
|
||||
@@ -384,6 +384,124 @@ class TestUpdateRoute:
|
||||
public_keys = [rn["public_key"] for rn in data["route_nodes"]]
|
||||
assert new_node.public_key in public_keys
|
||||
|
||||
def test_update_threshold_immediately_reflects_in_route_result(
|
||||
self, client_no_auth, api_db_session
|
||||
):
|
||||
"""Regression: PUT-changed threshold must surface in route_result now.
|
||||
|
||||
Before this fix, ``route_result`` (written by a background
|
||||
evaluator on a 30-60s schedule) kept the OLD threshold until the
|
||||
next evaluator cycle. The routes list card displays
|
||||
``route_result.threshold`` / ``effective_clear``, so the UI showed
|
||||
stale values for ~30s after a PUT even though the server returned
|
||||
``x-cache: MISS`` with the route's direct fields updated. The
|
||||
PUT handler now runs ``_reevaluate_route`` synchronously after
|
||||
commit so the very next GET sees a fresh ``route_result``.
|
||||
"""
|
||||
from meshcore_hub.common.models.route_result import RouteResult
|
||||
|
||||
nodes = _sample_nodes(api_db_session, 2)
|
||||
route = Route(
|
||||
from_label="Sync",
|
||||
to_label="Eval",
|
||||
packet_count_threshold=6,
|
||||
clear_threshold=12,
|
||||
enabled=True,
|
||||
)
|
||||
api_db_session.add(route)
|
||||
api_db_session.flush()
|
||||
for pos, n in enumerate(nodes):
|
||||
api_db_session.add(
|
||||
RouteNode(
|
||||
route_id=route.id,
|
||||
node_id=n.id,
|
||||
position=pos,
|
||||
expected_hash=n.public_key[:2].upper(),
|
||||
)
|
||||
)
|
||||
# Seed a stale RouteResult snapshot from a hypothetical prior
|
||||
# evaluator run using the OLD config (threshold=6, clear=12).
|
||||
# Without synchronous re-eval, this is what the PUT response
|
||||
# would continue to return until the next background sweep.
|
||||
api_db_session.add(
|
||||
RouteResult(
|
||||
route_id=route.id,
|
||||
state="healthy",
|
||||
quality="clear",
|
||||
matched_count=24,
|
||||
threshold=6,
|
||||
effective_clear=12,
|
||||
evaluated_at=datetime.now(timezone.utc),
|
||||
)
|
||||
)
|
||||
api_db_session.commit()
|
||||
|
||||
resp = client_no_auth.put(
|
||||
f"/api/v1/routes/{route.id}",
|
||||
json={"packet_count_threshold": 3, "clear_threshold": 6},
|
||||
headers={"X-User-Roles": "admin"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
# The route's direct fields reflect the new config...
|
||||
assert data["packet_count_threshold"] == 3
|
||||
assert data["clear_threshold"] == 6
|
||||
# ...AND route_result must reflect them too, not the stale
|
||||
# snapshot from the seeded prior evaluation.
|
||||
assert data["route_result"] is not None
|
||||
assert (
|
||||
data["route_result"]["threshold"] == 3
|
||||
), "route_result.threshold should reflect the new packet_count_threshold"
|
||||
assert (
|
||||
data["route_result"]["effective_clear"] == 6
|
||||
), "route_result.effective_clear should reflect the new clear_threshold"
|
||||
|
||||
def test_disabled_route_does_not_trigger_evaluation(
|
||||
self, client_no_auth, api_db_session, monkeypatch
|
||||
):
|
||||
"""Disabled routes short-circuit ``_reevaluate_route`` (no point
|
||||
evaluating a route that won't be displayed as active). Guards
|
||||
against unnecessary DB scans on bulk config changes."""
|
||||
from meshcore_hub.api.routes import routes as routes_module
|
||||
|
||||
called = {"count": 0}
|
||||
|
||||
def _spy_evaluate(*args, **kwargs):
|
||||
called["count"] += 1
|
||||
return ("healthy", "clear", 0)
|
||||
|
||||
monkeypatch.setattr(routes_module, "evaluate_route", _spy_evaluate)
|
||||
|
||||
nodes = _sample_nodes(api_db_session, 2)
|
||||
route = Route(
|
||||
from_label="Off",
|
||||
to_label="Line",
|
||||
packet_count_threshold=3,
|
||||
enabled=False,
|
||||
)
|
||||
api_db_session.add(route)
|
||||
api_db_session.flush()
|
||||
for pos, n in enumerate(nodes):
|
||||
api_db_session.add(
|
||||
RouteNode(
|
||||
route_id=route.id,
|
||||
node_id=n.id,
|
||||
position=pos,
|
||||
expected_hash=n.public_key[:2].upper(),
|
||||
)
|
||||
)
|
||||
api_db_session.commit()
|
||||
|
||||
resp = client_no_auth.put(
|
||||
f"/api/v1/routes/{route.id}",
|
||||
json={"description": "still off"},
|
||||
headers={"X-User-Roles": "admin"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert (
|
||||
called["count"] == 0
|
||||
), "evaluate_route must not be called for disabled routes"
|
||||
|
||||
|
||||
class TestDeleteRoute:
|
||||
def test_delete_success(self, client_no_auth, api_db_session):
|
||||
|
||||
Reference in New Issue
Block a user