diff --git a/src/meshcore_hub/api/routes/routes.py b/src/meshcore_hub/api/routes/routes.py index d82aafe..0df70c0 100644 --- a/src/meshcore_hub/api/routes/routes.py +++ b/src/meshcore_hub/api/routes/routes.py @@ -16,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 @@ -143,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( @@ -212,6 +236,7 @@ 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) @@ -383,6 +408,7 @@ def update_route( session.commit() session.refresh(route) + _reevaluate_route(session, route) invalidate_routes(request) return _route_to_read(route) 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):