diff --git a/.env.example b/.env.example index 4972949..0148eed 100644 --- a/.env.example +++ b/.env.example @@ -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) diff --git a/AGENTS.md b/AGENTS.md index d7f24a5..acb911b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 diff --git a/docs/configuration.md b/docs/configuration.md index 57fb494..fec99c6 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -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=` | 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 diff --git a/src/meshcore_hub/api/app.py b/src/meshcore_hub/api/app.py index c4c2fba..4b358ff 100644 --- a/src/meshcore_hub/api/app.py +++ b/src/meshcore_hub/api/app.py @@ -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=`` + ``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=`` 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 diff --git a/src/meshcore_hub/api/cache_invalidation.py b/src/meshcore_hub/api/cache_invalidation.py new file mode 100644 index 0000000..d198587 --- /dev/null +++ b/src/meshcore_hub/api/cache_invalidation.py @@ -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") diff --git a/src/meshcore_hub/api/routes/adoptions.py b/src/meshcore_hub/api/routes/adoptions.py index aa7393f..000f939 100644 --- a/src/meshcore_hub/api/routes/adoptions.py +++ b/src/meshcore_hub/api/routes/adoptions.py @@ -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", diff --git a/src/meshcore_hub/api/routes/channels.py b/src/meshcore_hub/api/routes/channels.py index e10265d..a9d40d6 100644 --- a/src/meshcore_hub/api/routes/channels.py +++ b/src/meshcore_hub/api/routes/channels.py @@ -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) diff --git a/src/meshcore_hub/api/routes/node_tags.py b/src/meshcore_hub/api/routes/node_tags.py index d80f406..f3dfcd1 100644 --- a/src/meshcore_hub/api/routes/node_tags.py +++ b/src/meshcore_hub/api/routes/node_tags.py @@ -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) diff --git a/src/meshcore_hub/api/routes/routes.py b/src/meshcore_hub/api/routes/routes.py index 923a4c4..0df70c0 100644 --- a/src/meshcore_hub/api/routes/routes.py +++ b/src/meshcore_hub/api/routes/routes.py @@ -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) diff --git a/src/meshcore_hub/api/routes/user_profiles.py b/src/meshcore_hub/api/routes/user_profiles.py index 4464de4..7be0c31 100644 --- a/src/meshcore_hub/api/routes/user_profiles.py +++ b/src/meshcore_hub/api/routes/user_profiles.py @@ -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) diff --git a/src/meshcore_hub/common/redis.py b/src/meshcore_hub/common/redis.py index 8f4dbc5..8024442 100644 --- a/src/meshcore_hub/common/redis.py +++ b/src/meshcore_hub/common/redis.py @@ -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: diff --git a/tests/test_api/test_cache.py b/tests/test_api/test_cache.py index 80baaf1..99a94d9 100644 --- a/tests/test_api/test_cache.py +++ b/tests/test_api/test_cache.py @@ -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] diff --git a/tests/test_api/test_routes.py b/tests/test_api/test_routes.py index 9d191b4..209e4bf 100644 --- a/tests/test_api/test_routes.py +++ b/tests/test_api/test_routes.py @@ -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):