fix: federation remoteId must be integer index, not URL string
- fetch_remote now accepts index i from enumerate() and sets remote_id: int = i
instead of remote.get("id", url), which fell back to the URL string
- asyncio.gather uses enumerate(remote_instances) to pass the integer index
- federation_connect route: remote_id type changed from str to int; FastAPI
now validates path param at framework level, simplifying the handler body
- Update test_federation_sessions_includes_remote_failure_status to expect
integer 0 (not the legacy 'remote-1' id field string)
- Update test_federation_connect_returns_404_for_non_integer_remote_id: now
expects 422 (FastAPI schema validation) instead of 404
- Add test_federation_sessions_remote_id_is_integer_index verifying remoteId
is 0 for first remote and 1 for second
This commit is contained in:
+6
-10
@@ -892,12 +892,12 @@ async def federation_sessions(request: Request) -> list[dict]:
|
||||
# Fetch remote sessions concurrently
|
||||
http_client: httpx.AsyncClient = request.app.state.federation_client
|
||||
|
||||
async def fetch_remote(remote: dict) -> list[dict]:
|
||||
async def fetch_remote(i: int, remote: dict) -> list[dict]:
|
||||
"""Fetch /api/sessions from a remote instance, returning session dicts or a status entry."""
|
||||
url: str = remote.get("url", "")
|
||||
key: str = remote.get("key", "")
|
||||
remote_name: str = remote.get("name", url)
|
||||
remote_id: str = remote.get("id", url)
|
||||
remote_id: int = i
|
||||
try:
|
||||
resp = await http_client.get(
|
||||
f"{url.rstrip('/')}/api/sessions",
|
||||
@@ -937,7 +937,7 @@ async def federation_sessions(request: Request) -> list[dict]:
|
||||
]
|
||||
|
||||
remote_results: list[list[dict]] = await asyncio.gather(
|
||||
*(fetch_remote(remote) for remote in remote_instances)
|
||||
*(fetch_remote(i, remote) for i, remote in enumerate(remote_instances))
|
||||
)
|
||||
|
||||
all_sessions: list[dict] = list(local_sessions)
|
||||
@@ -969,7 +969,7 @@ async def federation_generate_key() -> dict:
|
||||
|
||||
@app.post("/api/federation/{remote_id}/connect/{session_name}")
|
||||
async def federation_connect(
|
||||
remote_id: str, session_name: str, request: Request
|
||||
remote_id: int, session_name: str, request: Request
|
||||
) -> dict:
|
||||
"""Proxy a connect POST to a remote instance to spawn its ttyd.
|
||||
|
||||
@@ -981,16 +981,12 @@ async def federation_connect(
|
||||
"""
|
||||
settings = load_settings()
|
||||
remotes = settings.get("remote_instances", [])
|
||||
try:
|
||||
idx = int(remote_id)
|
||||
if idx < 0 or idx >= len(remotes):
|
||||
raise IndexError
|
||||
remote = remotes[idx]
|
||||
except (ValueError, IndexError):
|
||||
if remote_id < 0 or remote_id >= len(remotes):
|
||||
raise HTTPException(
|
||||
status_code=404,
|
||||
detail=f"Remote instance '{remote_id}' not found",
|
||||
)
|
||||
remote = remotes[remote_id]
|
||||
|
||||
remote_url: str = remote.get("url", "").rstrip("/")
|
||||
remote_key: str = remote.get("key", "")
|
||||
|
||||
@@ -1571,6 +1571,98 @@ def test_federation_sessions_returns_local_sessions(client, monkeypatch, tmp_pat
|
||||
assert local["remoteId"] is None
|
||||
|
||||
|
||||
def test_federation_sessions_remote_id_is_integer_index(
|
||||
client, monkeypatch, tmp_path
|
||||
):
|
||||
"""GET /api/federation/sessions returns integer remoteId (index) for remote sessions.
|
||||
|
||||
remoteId must be the enumerate index (0, 1, 2...) of the remote in
|
||||
remote_instances -- NOT the URL string and NOT any 'id' field from the
|
||||
remote config dict.
|
||||
"""
|
||||
import json
|
||||
|
||||
import httpx
|
||||
|
||||
import muxplex.settings as settings_mod
|
||||
|
||||
settings_path = tmp_path / "settings.json"
|
||||
monkeypatch.setattr(settings_mod, "SETTINGS_PATH", settings_path)
|
||||
|
||||
# Two remote instances -- first will succeed, second will fail
|
||||
settings_path.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"device_name": "local-host",
|
||||
"remote_instances": [
|
||||
{
|
||||
"url": "http://spark-2:8088",
|
||||
"key": "abc123",
|
||||
"name": "spark-2",
|
||||
},
|
||||
{
|
||||
"url": "http://spark-3:8088",
|
||||
"key": "def456",
|
||||
"name": "spark-3",
|
||||
},
|
||||
],
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
monkeypatch.setattr("muxplex.main.get_session_list", lambda: [])
|
||||
monkeypatch.setattr("muxplex.main.get_snapshots", lambda: {})
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
# First remote returns one session; second is unreachable
|
||||
async def mock_get(url, **kwargs):
|
||||
if "spark-2" in url:
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.status_code = 200
|
||||
mock_resp.raise_for_status = lambda: None
|
||||
mock_resp.json = lambda: [{"name": "work", "snapshot": "", "bell": {}}]
|
||||
return mock_resp
|
||||
raise httpx.ConnectError("refused")
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.get = mock_get
|
||||
monkeypatch.setattr(client.app.state, "federation_client", mock_client)
|
||||
|
||||
response = client.get("/api/federation/sessions")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
|
||||
# The successful remote session (spark-2, index 0) must have remoteId == 0
|
||||
remote_entries = [s for s in data if s.get("remoteId") is not None]
|
||||
assert len(remote_entries) == 2, (
|
||||
f"Expected 2 remote entries (1 session + 1 unreachable), got: {remote_entries}"
|
||||
)
|
||||
|
||||
spark2_session = next(
|
||||
(s for s in remote_entries if s.get("deviceName") == "spark-2"), None
|
||||
)
|
||||
assert spark2_session is not None, "Expected a session entry from spark-2"
|
||||
assert spark2_session["remoteId"] == 0, (
|
||||
f"remoteId for first remote (index 0) must be integer 0, "
|
||||
f"got: {spark2_session['remoteId']!r}"
|
||||
)
|
||||
assert isinstance(spark2_session["remoteId"], int), (
|
||||
f"remoteId must be an int, got {type(spark2_session['remoteId'])}"
|
||||
)
|
||||
|
||||
# The unreachable remote (spark-3, index 1) must have remoteId == 1
|
||||
spark3_entry = next(
|
||||
(s for s in remote_entries if s.get("deviceName") == "spark-3"), None
|
||||
)
|
||||
assert spark3_entry is not None, "Expected a status entry from spark-3"
|
||||
assert spark3_entry["remoteId"] == 1, (
|
||||
f"remoteId for second remote (index 1) must be integer 1, "
|
||||
f"got: {spark3_entry['remoteId']!r}"
|
||||
)
|
||||
|
||||
|
||||
|
||||
def test_federation_sessions_includes_remote_failure_status(
|
||||
client, monkeypatch, tmp_path
|
||||
):
|
||||
@@ -1635,7 +1727,7 @@ def test_federation_sessions_includes_remote_failure_status(
|
||||
)
|
||||
entry = failure_entries[0]
|
||||
assert entry["status"] == "unreachable"
|
||||
assert entry.get("remoteId") == "remote-1"
|
||||
assert entry.get("remoteId") == 0 # integer index, not the "id" field string
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -1732,7 +1824,11 @@ def test_federation_connect_returns_404_for_invalid_remote_id(
|
||||
def test_federation_connect_returns_404_for_non_integer_remote_id(
|
||||
client, monkeypatch, tmp_path
|
||||
):
|
||||
"""POST /api/federation/{remote_id}/connect/{session_name} returns 404 when remote_id is not an integer."""
|
||||
"""POST /api/federation/{remote_id}/connect/{session_name} returns 422 when remote_id is not an integer.
|
||||
|
||||
With remote_id typed as int, FastAPI validates the path parameter at the
|
||||
framework level and returns 422 Unprocessable Entity for non-integer values.
|
||||
"""
|
||||
import json
|
||||
|
||||
import muxplex.settings as settings_mod
|
||||
@@ -1742,7 +1838,7 @@ def test_federation_connect_returns_404_for_non_integer_remote_id(
|
||||
settings_path.write_text(json.dumps({"remote_instances": []}))
|
||||
|
||||
response = client.post("/api/federation/not-an-int/connect/my-session")
|
||||
assert response.status_code == 404
|
||||
assert response.status_code == 422 # FastAPI rejects non-integer path param
|
||||
|
||||
|
||||
def test_federation_connect_returns_503_when_remote_unreachable(
|
||||
|
||||
Reference in New Issue
Block a user