mirror of
https://github.com/ipnet-mesh/meshcore-hub.git
synced 2026-08-06 17:02:59 +02:00
fix: re-evaluate route_result synchronously after route create/update
Reported symptom: user edits a Route (e.g. lowers packet_count_threshold from 6 to 3), and the routes list card still shows the old threshold for ~30s. The cache stack was exonerated — x-cache: MISS on the stale GET, response body had the new packet_count_threshold=3, but route_result. threshold remained at 6. Root cause was neither HTTP cache nor Redis cache. The list card's stats row (renderStatsRow in routes.js:82) displays route_result.threshold / effective_clear / matched_count, which are persisted by the background route_evaluator on a 30-60s schedule — separate DB row from the route's direct fields. The PUT handler updated the route row immediately but never triggered a re-evaluation, so route_result carried the stale snapshot from the prior evaluator cycle until the next sweep. Fix: add _reevaluate_route(session, route) helper that runs evaluate_route + upsert_route_result synchronously after the route commit. Called from create_route (initial evaluation) and update_route (refresh on every config change). Disabled routes short-circuit (no point evaluating a route that's turned off). One bounded DB scan per write — same cost as a single-route evaluator tick. After this fix, the PUT response itself carries a fresh route_result reflecting the new packet_count_threshold / clear_threshold, and the next GET /api/v1/routes returns it. The list card updates on the very next render cycle. Tests: - test_update_threshold_immediately_reflects_in_route_result: seeds a stale RouteResult with the OLD threshold, sends a PUT, asserts the response's route_result.threshold/effective_clear now match the new route config. - test_disabled_route_does_not_trigger_evaluation: monkeypatches evaluate_route to a spy, asserts it's never called for disabled routes.
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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