mirror of
https://github.com/ipnet-mesh/meshcore-hub.git
synced 2026-10-31 04:02:03 +01:00
fix(web): paginate /map/data node and profile fetches
The map endpoint fetched a single limit=500 page (the API's hard max, sorted last_seen desc), so networks with more than 500 nodes silently hid stale/offline nodes from the unfiltered map — including adopted infrastructure, which still appeared under the adopted_by filter because that filter narrows the query before the limit applies. Follow offset/total pagination via a new _fetch_all_pages helper (40 pages / 20k nodes safety cap, partial results on later-page failure) for both /api/v1/nodes and /api/v1/user/profiles, and report the API-reported total in debug.total_nodes.
This commit is contained in:
1 parent
c9d5e86953
commit
9b6a16d862
3 files changed
+333
-27
No files matched your search
+94
-15
@@ -83,6 +83,83 @@ def _load_asset_manifest() -> dict[str, Any]:
|
||||
return {}
|
||||
|
||||
|
||||
# Pagination bounds for web-tier aggregation fetches (e.g. /map/data).
|
||||
# List endpoints cap `limit` at 500, so fetching "everything" requires
|
||||
# following offset pages; MAP_MAX_PAGES is a safety valve against a
|
||||
# misbehaving API that keeps reporting more data.
|
||||
MAP_PAGE_SIZE = 500
|
||||
MAP_MAX_PAGES = 40
|
||||
|
||||
|
||||
async def _fetch_all_pages(
|
||||
client: httpx.AsyncClient,
|
||||
path: str,
|
||||
extra_params: dict[str, str | int] | None = None,
|
||||
page_size: int = MAP_PAGE_SIZE,
|
||||
max_pages: int = MAP_MAX_PAGES,
|
||||
) -> tuple[list[dict[str, Any]], int | None, str | None]:
|
||||
"""Fetch every item from a paginated list endpoint via offset paging.
|
||||
|
||||
List endpoints cap ``limit`` at 500, so a single request only covers the
|
||||
first page (e.g. the 500 most recently seen nodes) and silently drops the
|
||||
rest. This helper follows ``offset``/``total`` until the collection is
|
||||
exhausted.
|
||||
|
||||
Returns:
|
||||
``(items, total, error)``. ``error`` is set only when the FIRST page
|
||||
fails; a failure on a later page logs a warning and returns the items
|
||||
collected so far. ``total`` is the API-reported total, or None when
|
||||
the responses omit it.
|
||||
"""
|
||||
items: list[dict[str, Any]] = []
|
||||
total: int | None = None
|
||||
params: dict[str, str | int] = dict(extra_params or {})
|
||||
params["limit"] = page_size
|
||||
offset = 0
|
||||
|
||||
for page in range(max_pages):
|
||||
params["offset"] = offset
|
||||
response = await client.get(path, params=params)
|
||||
if response.status_code != 200:
|
||||
if page == 0:
|
||||
return [], None, f"API returned status {response.status_code}"
|
||||
logger.warning(
|
||||
"Pagination of %s aborted at offset %d: API returned status %d "
|
||||
"(returning %d items collected so far)",
|
||||
path,
|
||||
offset,
|
||||
response.status_code,
|
||||
len(items),
|
||||
)
|
||||
break
|
||||
data = response.json()
|
||||
page_items = data.get("items", [])
|
||||
if not isinstance(page_items, list):
|
||||
break
|
||||
items.extend(page_items)
|
||||
reported_total = data.get("total")
|
||||
if isinstance(reported_total, int):
|
||||
total = reported_total
|
||||
if not page_items:
|
||||
break
|
||||
offset += len(page_items)
|
||||
if total is not None:
|
||||
if offset >= total:
|
||||
break
|
||||
elif len(page_items) < page_size:
|
||||
break
|
||||
else:
|
||||
logger.warning(
|
||||
"Pagination of %s capped at %d pages (%d items); result may be "
|
||||
"truncated",
|
||||
path,
|
||||
max_pages,
|
||||
len(items),
|
||||
)
|
||||
|
||||
return items, total, None
|
||||
|
||||
|
||||
# Per-endpoint, per-method role access mapping for the API proxy.
|
||||
# Key: URL path prefix (after /api/), Value: {method -> allowed roles}.
|
||||
# _OPEN = unconditional access (OIDC on or off, anonymous OK).
|
||||
@@ -851,12 +928,10 @@ def create_app(
|
||||
nodes_with_coords = 0
|
||||
|
||||
try:
|
||||
profiles_response = await request.app.state.http_client.get(
|
||||
"/api/v1/user/profiles", params={"limit": 500}
|
||||
profile_items, _, _ = await _fetch_all_pages(
|
||||
request.app.state.http_client, "/api/v1/user/profiles"
|
||||
)
|
||||
if profiles_response.status_code == 200:
|
||||
profiles_data = profiles_response.json()
|
||||
for profile in profiles_data.get("items", []):
|
||||
for profile in profile_items:
|
||||
profiles_by_id[profile["id"]] = {
|
||||
"id": profile.get("id"),
|
||||
"name": profile.get("name"),
|
||||
@@ -864,17 +939,23 @@ def create_app(
|
||||
"roles": profile.get("roles", []),
|
||||
}
|
||||
|
||||
# Fetch all nodes from API
|
||||
nodes_params: dict[str, str | int] = {"limit": 500}
|
||||
# Fetch all nodes from the API, following pagination. The API caps
|
||||
# limit at 500 and sorts last_seen desc, so a single request only
|
||||
# covered the 500 most recently seen nodes and silently hid
|
||||
# stale/offline nodes — including adopted infrastructure — from
|
||||
# the unfiltered map.
|
||||
nodes_params: dict[str, str | int] = {}
|
||||
if adopted_by:
|
||||
nodes_params["adopted_by"] = adopted_by
|
||||
response = await request.app.state.http_client.get(
|
||||
"/api/v1/nodes", params=nodes_params
|
||||
nodes, nodes_total, fetch_error = await _fetch_all_pages(
|
||||
request.app.state.http_client, "/api/v1/nodes", nodes_params
|
||||
)
|
||||
if fetch_error is not None:
|
||||
error = fetch_error
|
||||
else:
|
||||
total_nodes = (
|
||||
nodes_total if isinstance(nodes_total, int) else len(nodes)
|
||||
)
|
||||
if response.status_code == 200:
|
||||
data = response.json()
|
||||
nodes = data.get("items", [])
|
||||
total_nodes = len(nodes)
|
||||
|
||||
for node in nodes:
|
||||
tags = node.get("tags", [])
|
||||
@@ -937,8 +1018,6 @@ def create_app(
|
||||
"owner": owner,
|
||||
}
|
||||
)
|
||||
else:
|
||||
error = f"API returned status {response.status_code}"
|
||||
|
||||
except Exception as e:
|
||||
error = str(e)
|
||||
|
||||
+31
-12
@@ -45,6 +45,10 @@ class MockHttpClient:
|
||||
def __init__(self) -> None:
|
||||
"""Initialize mock client with default responses."""
|
||||
self._responses: dict[str, dict[str, Any]] = {}
|
||||
# Param-aware handlers for paginated endpoints. Key: "METHOD:path",
|
||||
# value: callable(params) -> {"status_code": int, "json": Any}.
|
||||
# Checked before _responses so pagination tests can override defaults.
|
||||
self._paged_handlers: dict[str, Any] = {}
|
||||
# Records the params forwarded by the most recent request() call so
|
||||
# tests can assert how the proxy forwards query parameters.
|
||||
self.last_request_params: Any = None
|
||||
@@ -259,19 +263,19 @@ class MockHttpClient:
|
||||
"json": json_data,
|
||||
}
|
||||
|
||||
def _create_response(self, key: str) -> Response:
|
||||
"""Create a mock response for a given key."""
|
||||
import json as _json
|
||||
def set_paged_response(self, method: str, path: str, handler: Any) -> None:
|
||||
"""Register a param-aware handler for a paginated endpoint.
|
||||
|
||||
response_data = self._responses.get(key)
|
||||
if response_data is None:
|
||||
# Return 404 for unknown endpoints
|
||||
response = MagicMock(spec=Response)
|
||||
response.status_code = 404
|
||||
response.json.return_value = {"detail": "Not found"}
|
||||
response.content = b'{"detail": "Not found"}'
|
||||
response.headers = {"content-type": "application/json"}
|
||||
return response
|
||||
The handler receives the request params dict (including ``limit`` and
|
||||
``offset``) and returns the response payload dict
|
||||
(``{"status_code": int, "json": Any}``), enabling different pages per
|
||||
offset. Takes precedence over seeded/static responses for that path.
|
||||
"""
|
||||
self._paged_handlers[f"{method}:{path}"] = handler
|
||||
|
||||
def _response_from_data(self, response_data: dict[str, Any]) -> Response:
|
||||
"""Create a mock response from a {"status_code", "json"} payload."""
|
||||
import json as _json
|
||||
|
||||
response = MagicMock(spec=Response)
|
||||
response.status_code = response_data["status_code"]
|
||||
@@ -280,6 +284,16 @@ class MockHttpClient:
|
||||
response.headers = {"content-type": "application/json"}
|
||||
return response
|
||||
|
||||
def _create_response(self, key: str) -> Response:
|
||||
"""Create a mock response for a given key."""
|
||||
response_data = self._responses.get(key)
|
||||
if response_data is None:
|
||||
# Return 404 for unknown endpoints
|
||||
return self._response_from_data(
|
||||
{"status_code": 404, "json": {"detail": "Not found"}}
|
||||
)
|
||||
return self._response_from_data(response_data)
|
||||
|
||||
async def request(
|
||||
self,
|
||||
method: str,
|
||||
@@ -307,6 +321,11 @@ class MockHttpClient:
|
||||
) -> Response:
|
||||
"""Mock GET request."""
|
||||
self.last_get_headers = headers
|
||||
# Param-aware paged handlers take precedence over static responses
|
||||
# (including seeded defaults) so pagination tests control every page.
|
||||
handler = self._paged_handlers.get(f"GET:{path}")
|
||||
if handler is not None:
|
||||
return self._response_from_data(handler(params))
|
||||
# Try exact match first
|
||||
key = f"GET:{path}"
|
||||
if key in self._responses:
|
||||
|
||||
@@ -462,3 +462,211 @@ class TestMapDataAdoptedByFilter:
|
||||
# Default mock has 2 nodes, 1 with coordinates
|
||||
assert len(data["nodes"]) == 1
|
||||
assert data["nodes"][0]["name"] == "Node Two"
|
||||
|
||||
|
||||
class TestMapDataPagination:
|
||||
"""Tests that /map/data follows API pagination instead of one 500-item page.
|
||||
|
||||
Regression tests: the endpoint used to fetch a single ``limit=500`` page
|
||||
(the API's hard max), so networks with more than 500 nodes silently hid
|
||||
stale/offline nodes — including adopted infrastructure — from the
|
||||
unfiltered map, while the ``adopted_by`` filter (applied before the limit)
|
||||
still showed them.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _node(
|
||||
idx: int,
|
||||
name: str,
|
||||
lat: float | None = None,
|
||||
lon: float | None = None,
|
||||
last_seen: str | None = "2024-01-01T12:00:00Z",
|
||||
adopted_by: dict[str, Any] | None = None,
|
||||
) -> dict[str, Any]:
|
||||
return {
|
||||
"id": f"node-{idx}",
|
||||
"public_key": f"{idx:064x}",
|
||||
"name": name,
|
||||
"adv_type": "REPEATER",
|
||||
"lat": lat,
|
||||
"lon": lon,
|
||||
"last_seen": last_seen,
|
||||
"tags": [],
|
||||
"adopted_by": adopted_by,
|
||||
}
|
||||
|
||||
def test_map_data_includes_nodes_beyond_first_page(
|
||||
self, web_app: Any, mock_http_client: MockHttpClient
|
||||
) -> None:
|
||||
"""A stale adopted node on page 2 must appear in the unfiltered map."""
|
||||
calls: list[dict | None] = []
|
||||
|
||||
def handler(params: dict | None) -> dict[str, Any]:
|
||||
calls.append(params)
|
||||
offset = (params or {}).get("offset", 0)
|
||||
if offset == 0:
|
||||
items = [self._node(1, "Recent Node", lat=41.0, lon=-75.0)]
|
||||
else:
|
||||
items = [
|
||||
self._node(
|
||||
2,
|
||||
"Stale Repeater",
|
||||
lat=40.0,
|
||||
lon=-74.0,
|
||||
last_seen="2023-06-01T00:00:00Z",
|
||||
adopted_by={
|
||||
"user_id": "user-1",
|
||||
"name": "Operator",
|
||||
"callsign": "W1ABC",
|
||||
"profile_id": "profile-1",
|
||||
},
|
||||
)
|
||||
]
|
||||
return {"status_code": 200, "json": {"items": items, "total": 2}}
|
||||
|
||||
mock_http_client.set_paged_response("GET", "/api/v1/nodes", handler)
|
||||
|
||||
client = TestClient(web_app, raise_server_exceptions=True)
|
||||
response = client.get("/map/data")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
|
||||
# Both pages were fetched
|
||||
assert len(calls) == 2
|
||||
nodes_by_name = {n["name"]: n for n in data["nodes"]}
|
||||
assert "Recent Node" in nodes_by_name
|
||||
assert "Stale Repeater" in nodes_by_name
|
||||
assert nodes_by_name["Stale Repeater"]["is_adopted"] is True
|
||||
assert data["adopted_center"] == {"lat": 40.0, "lon": -74.0}
|
||||
# debug.total_nodes reports the API total, not the page length
|
||||
assert data["debug"]["total_nodes"] == 2
|
||||
|
||||
def test_map_data_forwards_adopted_by_on_every_page(
|
||||
self, web_app: Any, mock_http_client: MockHttpClient
|
||||
) -> None:
|
||||
"""The adopted_by filter must be forwarded to every page request."""
|
||||
calls: list[dict | None] = []
|
||||
|
||||
def handler(params: dict | None) -> dict[str, Any]:
|
||||
# Snapshot: the caller mutates the same params dict between pages
|
||||
calls.append(dict(params) if params else None)
|
||||
offset = (params or {}).get("offset", 0)
|
||||
items = [self._node(offset + 1, f"Node {offset}", lat=40.0, lon=-74.0)]
|
||||
return {"status_code": 200, "json": {"items": items, "total": 2}}
|
||||
|
||||
mock_http_client.set_paged_response("GET", "/api/v1/nodes", handler)
|
||||
|
||||
client = TestClient(web_app, raise_server_exceptions=True)
|
||||
response = client.get("/map/data?adopted_by=some-profile-uuid")
|
||||
assert response.status_code == 200
|
||||
|
||||
assert [c.get("offset") if c else None for c in calls] == [0, 1]
|
||||
for params in calls:
|
||||
assert params is not None
|
||||
assert params.get("adopted_by") == "some-profile-uuid"
|
||||
assert params.get("limit") == 500
|
||||
|
||||
def test_map_data_terminates_on_short_page_without_total(
|
||||
self, web_app: Any, mock_http_client: MockHttpClient
|
||||
) -> None:
|
||||
"""A short page with no total field means "last page" — no extra calls."""
|
||||
calls: list[dict | None] = []
|
||||
|
||||
def handler(params: dict | None) -> dict[str, Any]:
|
||||
calls.append(params)
|
||||
return {
|
||||
"status_code": 200,
|
||||
"json": {"items": [self._node(1, "Only Node", lat=40.0, lon=-74.0)]},
|
||||
}
|
||||
|
||||
mock_http_client.set_paged_response("GET", "/api/v1/nodes", handler)
|
||||
|
||||
client = TestClient(web_app, raise_server_exceptions=True)
|
||||
response = client.get("/map/data")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
|
||||
assert len(calls) == 1
|
||||
assert [n["name"] for n in data["nodes"]] == ["Only Node"]
|
||||
|
||||
def test_map_data_pagination_capped_at_max_pages(
|
||||
self, web_app: Any, mock_http_client: MockHttpClient
|
||||
) -> None:
|
||||
"""A pathological API reporting a huge total must not loop forever."""
|
||||
from meshcore_hub.web.app import MAP_MAX_PAGES
|
||||
|
||||
calls: list[dict | None] = []
|
||||
|
||||
def handler(params: dict | None) -> dict[str, Any]:
|
||||
calls.append(params)
|
||||
offset = (params or {}).get("offset", 0)
|
||||
items = [self._node(offset, f"Node {offset}", lat=40.0, lon=-74.0)]
|
||||
return {"status_code": 200, "json": {"items": items, "total": 10**9}}
|
||||
|
||||
mock_http_client.set_paged_response("GET", "/api/v1/nodes", handler)
|
||||
|
||||
client = TestClient(web_app, raise_server_exceptions=True)
|
||||
response = client.get("/map/data")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
|
||||
assert len(calls) == MAP_MAX_PAGES
|
||||
assert len(data["nodes"]) == MAP_MAX_PAGES
|
||||
# debug.total_nodes still reports the API-reported total
|
||||
assert data["debug"]["total_nodes"] == 10**9
|
||||
|
||||
def test_map_data_keeps_partial_results_when_later_page_fails(
|
||||
self, web_app: Any, mock_http_client: MockHttpClient
|
||||
) -> None:
|
||||
"""A failure on page 2 keeps the nodes collected from page 1."""
|
||||
calls: list[dict | None] = []
|
||||
|
||||
def handler(params: dict | None) -> dict[str, Any]:
|
||||
calls.append(params)
|
||||
offset = (params or {}).get("offset", 0)
|
||||
if offset == 0:
|
||||
return {
|
||||
"status_code": 200,
|
||||
"json": {
|
||||
"items": [self._node(1, "First Page Node", 40.0, -74.0)],
|
||||
"total": 3,
|
||||
},
|
||||
}
|
||||
return {"status_code": 500, "json": None}
|
||||
|
||||
mock_http_client.set_paged_response("GET", "/api/v1/nodes", handler)
|
||||
|
||||
client = TestClient(web_app, raise_server_exceptions=True)
|
||||
response = client.get("/map/data")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
|
||||
assert len(calls) == 2
|
||||
assert [n["name"] for n in data["nodes"]] == ["First Page Node"]
|
||||
|
||||
def test_map_data_paginates_profiles(
|
||||
self, web_app: Any, mock_http_client: MockHttpClient
|
||||
) -> None:
|
||||
"""The operator dropdown source (profiles) is paginated too."""
|
||||
calls: list[dict | None] = []
|
||||
|
||||
def handler(params: dict | None) -> dict[str, Any]:
|
||||
# Snapshot: the caller mutates the same params dict between pages
|
||||
calls.append(dict(params) if params else None)
|
||||
offset = (params or {}).get("offset", 0)
|
||||
items: list[dict[str, Any]]
|
||||
if offset == 0:
|
||||
items = [{"id": "p1", "name": "Op One", "callsign": None, "roles": []}]
|
||||
else:
|
||||
items = [{"id": "p2", "name": "Op Two", "callsign": None, "roles": []}]
|
||||
return {"status_code": 200, "json": {"items": items, "total": 2}}
|
||||
|
||||
mock_http_client.set_paged_response("GET", "/api/v1/user/profiles", handler)
|
||||
|
||||
client = TestClient(web_app, raise_server_exceptions=True)
|
||||
response = client.get("/map/data")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
|
||||
assert len(calls) == 2
|
||||
assert {p["id"] for p in data["profiles"]} == {"p1", "p2"}
|
||||
Reference in new issue
Block a user