Discord: log node-reported stream state; remove a radio channel or a server
- handle_actual logs every node-reported state TRANSITION (not heartbeats) with stream/node/guild/channel/token ids and the node's error text: info for connected/connecting/stopped, warning for error, and a dedicated "radio bot X failed to log in on node N: <reason>" line for login_failed. The node's reason code is now stored in actual.code and shown next to the error on Settings > Discord and the node page. - Remove a radio channel permanently: streams.delete_stream stops it through desired state first (node republished without it, token -> releasing per #216, freed on the node's ack), then deletes the doc in a transaction that refuses if the channel was restarted in between. DELETE /discord/streams/{id} (platform admin) and DELETE /discord/nodes/{node}/streams/{id} (admin or org owner, same org + node only; operators can still stop, not remove). Audited. - Remove a server: DELETE /discord/guilds/{id} (admin) deletes every radio channel in it the same way, then tombstones discord_guilds (org_id None + unlinked_at, #218 semantics, plus removed_at) so it never falls back to DISCORD_UNLINKED_GUILD_ORG_ID; the admin page hides it, and /drb link or the admin link brings it back. Optional leave_radio_bots makes pool bots and the server's org's own BYO bots leave via Discord REST; the DRB server bot stays until kicked, and the UI says so. Audited. - UI: Remove (with confirm) on Radio channels, Servers and the node page list. Refs #213, #221 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Logan Cusano
co-authored by
Claude Opus 5.5
Claude Opus 5.5
parent
fabdf5839e
commit
c92bdfe841
@@ -75,6 +75,9 @@ class _Txn:
|
||||
def update(self, ref: _DocRef, data: dict):
|
||||
self.writes.append(("update", ref.coll, ref.id, copy.deepcopy(data), True))
|
||||
|
||||
def delete(self, ref: _DocRef):
|
||||
self.writes.append(("delete", ref.coll, ref.id, {}, False))
|
||||
|
||||
|
||||
class FakeFirestore:
|
||||
def __init__(self):
|
||||
@@ -91,6 +94,10 @@ class FakeFirestore:
|
||||
|
||||
def _write(self, kind: str, coll: str, doc_id: str, data: dict, merge: bool):
|
||||
docs = self.data.setdefault(coll, {})
|
||||
if kind == "delete":
|
||||
docs.pop(doc_id, None)
|
||||
self.versions[(coll, doc_id)] = self.versions.get((coll, doc_id), 0) + 1
|
||||
return
|
||||
if kind == "update" and doc_id not in docs:
|
||||
raise KeyError(f"No document to update: {coll}/{doc_id}")
|
||||
if merge and doc_id in docs:
|
||||
|
||||
@@ -0,0 +1,362 @@
|
||||
"""
|
||||
Discord radio channels: server-side state logging, permanent removal of a
|
||||
radio channel, and removal of a whole Discord server (server-26#213 / #221
|
||||
follow-up).
|
||||
|
||||
Removal must go through desired state first — the node is told to drop the
|
||||
channel (and logs the bot out), the token is only ``releasing`` until the node
|
||||
acks — and only then is the doc deleted. Deleting first would leave the
|
||||
token's lease naming a stream nobody can stop.
|
||||
"""
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.config import settings
|
||||
from app.internal import discord_guild_link as glink
|
||||
from app.internal import discord_leases as leases
|
||||
from app.internal import discord_streams as streams
|
||||
from app.internal.auth import require_admin_token, require_firebase_token
|
||||
from app.main import app
|
||||
from app.routers import discord_web as web
|
||||
from tests.fake_firestore import FakeFirestore
|
||||
|
||||
ORG, OTHER, SYS = "org-a", "org-b", "sys-1"
|
||||
ADMIN = {"uid": "u-admin", "role": "admin", "email": "admin@example.test"}
|
||||
OWNER = {"uid": "u-own", "org_id": ORG, "org_role": "owner"}
|
||||
OPERATOR = {"uid": "u-op", "org_id": ORG, "org_role": "member", "role": "operator", "owned_node_ids": ["n1"]}
|
||||
STRANGER = {"uid": "u-x", "org_id": OTHER, "org_role": "owner"}
|
||||
|
||||
client = TestClient(app)
|
||||
|
||||
|
||||
class _Log:
|
||||
def __init__(self):
|
||||
self.lines: list[tuple[str, str]] = []
|
||||
|
||||
def __getattr__(self, level):
|
||||
return lambda msg, *a, **k: self.lines.append((level, str(msg)))
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def fs(monkeypatch):
|
||||
fake = FakeFirestore.install(monkeypatch)
|
||||
fake.published = []
|
||||
fake.on_publish = None
|
||||
|
||||
def _publish(node_id, payload):
|
||||
if fake.on_publish:
|
||||
fake.on_publish(node_id, payload)
|
||||
fake.published.append((node_id, payload))
|
||||
return True
|
||||
|
||||
from app.internal.mqtt_handler import mqtt_handler
|
||||
monkeypatch.setattr(mqtt_handler, "publish_discord_desired", _publish)
|
||||
streams._publish_locks.clear()
|
||||
|
||||
async def _no_discord(token_ids):
|
||||
return 0
|
||||
monkeypatch.setattr(leases, "refresh_membership", _no_discord)
|
||||
monkeypatch.setattr(settings, "discord_unlinked_guild_org_id", "")
|
||||
|
||||
for nid, org in (("n1", ORG), ("n2", ORG), ("nx", OTHER)):
|
||||
fake.put("nodes", nid, {"node_id": nid, "name": f"Node {nid}", "org_id": org, "status": "online",
|
||||
"approval_status": "approved", "assigned_system_id": SYS})
|
||||
fake.put("systems", SYS, {"system_id": SYS, "name": "County P25", "org_id": ORG})
|
||||
fake.put("organizations", ORG, {"org_id": ORG, "name": "Alpha"})
|
||||
fake.put("discord_guilds", "100", {"guild_id": "100", "org_id": ORG, "guild_name": "Fire Co. 7",
|
||||
"bot_present": True,
|
||||
"voice_channels": [{"channel_id": "1001", "name": "Dispatch"},
|
||||
{"channel_id": "1002", "name": "Fireground"}]})
|
||||
for tid, org in (("t1", None), ("t2", None)):
|
||||
fake.put("bot_tokens", tid, {"token_id": tid, "name": f"Radio {tid}", "token": f"secret-{tid}",
|
||||
"org_id": org, "guild_ids": ["100", "200"], "valid": True,
|
||||
"application_id": f"app-{tid}", "lease": None})
|
||||
yield fake
|
||||
app.dependency_overrides.clear()
|
||||
|
||||
|
||||
async def _start(channel="1001", node_id="n1", guild="100", org=ORG):
|
||||
return await streams.start_stream(org_id=org, guild_id=guild, channel_id=channel, system_id=SYS,
|
||||
node_id=node_id)
|
||||
|
||||
|
||||
def _lease(fs, tid):
|
||||
return (fs.get("bot_tokens", tid) or {}).get("lease")
|
||||
|
||||
|
||||
def _audit(fs, action):
|
||||
return [a for a in (fs.data.get("audit_log") or {}).values() if a["action"] == action]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 1. Logging node-reported state transitions
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
async def test_actual_logs_transitions_not_heartbeats_and_keeps_the_code(fs, monkeypatch):
|
||||
log = _Log()
|
||||
monkeypatch.setattr(streams, "logger", log)
|
||||
doc = await _start()
|
||||
sid, tok = doc["stream_id"], doc["token_id"]
|
||||
|
||||
report = {"streams": {sid: {"state": "connected", "error": None, "code": None}}}
|
||||
await streams.handle_actual("n1", report)
|
||||
await streams.handle_actual("n1", report) # heartbeat: same state
|
||||
infos = [m for lvl, m in log.lines if lvl == "info" and "pending -> connected" in m]
|
||||
assert len(infos) == 1
|
||||
assert f"stream={sid}" in infos[0] and "node=n1" in infos[0]
|
||||
assert "guild=100" in infos[0] and "channel=1001" in infos[0] and f"token={tok}" in infos[0]
|
||||
|
||||
err = {"streams": {sid: {"state": "error", "code": "connect_failed",
|
||||
"error": "Could not connect to the voice channel (TimeoutError: )."}}}
|
||||
await streams.handle_actual("n1", err)
|
||||
await streams.handle_actual("n1", err)
|
||||
warns = [m for lvl, m in log.lines if lvl == "warning"]
|
||||
assert len(warns) == 1
|
||||
assert "connected -> error [connect_failed]" in warns[0] and "TimeoutError" in warns[0]
|
||||
actual = fs.get("discord_streams", sid)["actual"]
|
||||
assert actual["code"] == "connect_failed" and actual["error"].startswith("Could not connect")
|
||||
|
||||
|
||||
async def test_login_failure_reason_is_logged_against_the_token(fs, monkeypatch):
|
||||
log = _Log()
|
||||
monkeypatch.setattr(streams, "logger", log)
|
||||
doc = await _start()
|
||||
await streams.handle_actual("n1", {"streams": {doc["stream_id"]: {
|
||||
"state": "error", "code": "login_failed",
|
||||
"error": "The radio bot could not log in to Discord (LoginFailure: Improper token has been passed.)."}}})
|
||||
warns = [m for lvl, m in log.lines if lvl == "warning"]
|
||||
assert len(warns) == 1
|
||||
assert warns[0].startswith(f"Radio bot {doc['token_id']} failed to log in on node n1:")
|
||||
assert "Improper token" in warns[0]
|
||||
|
||||
|
||||
async def test_actual_from_a_node_not_carrying_the_stream_is_neither_stored_nor_logged(fs, monkeypatch):
|
||||
log = _Log()
|
||||
monkeypatch.setattr(streams, "logger", log)
|
||||
doc = await _start(node_id="n1")
|
||||
await streams.handle_actual("n2", {"streams": {doc["stream_id"]: {"state": "error", "error": "spoof"}}})
|
||||
assert fs.get("discord_streams", doc["stream_id"])["actual"]["state"] == "pending"
|
||||
assert not [m for _, m in log.lines if "spoof" in m]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 2. Remove a radio channel
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
async def test_delete_stops_through_desired_state_before_deleting(fs):
|
||||
doc = await _start()
|
||||
sid, tok = doc["stream_id"], doc["token_id"]
|
||||
seen = []
|
||||
|
||||
def _on_publish(node_id, payload):
|
||||
# The node must be told to drop the channel while the doc still exists.
|
||||
cur = fs.get("discord_streams", sid)
|
||||
seen.append((node_id, [s["stream_id"] for s in payload["streams"]], cur and cur["desired"]))
|
||||
fs.on_publish = _on_publish
|
||||
|
||||
before = await streams.delete_stream(sid, "removed")
|
||||
assert before["stream_id"] == sid
|
||||
assert seen == [("n1", [], "off")] # stop published first, doc still there
|
||||
assert fs.get("discord_streams", sid) is None # then deleted
|
||||
|
||||
# Lease follows #216: releasing until the node acks a list without it.
|
||||
lease = _lease(fs, tok)
|
||||
assert lease["releasing"] is True and lease["streams"] == []
|
||||
await streams.handle_actual("n1", {"applied_seq": lease["release_seq"], "sessions": [tok], "streams": {}})
|
||||
assert _lease(fs, tok) is not None # still logged in on the node
|
||||
await streams.handle_actual("n1", {"applied_seq": lease["release_seq"], "sessions": [], "streams": {}})
|
||||
assert _lease(fs, tok) is None # node logged out -> back in the pool
|
||||
|
||||
|
||||
async def test_delete_of_an_already_stopped_stream_does_not_republish(fs):
|
||||
doc = await _start()
|
||||
await streams.stop_stream(doc["stream_id"])
|
||||
n = len(fs.published)
|
||||
assert await streams.delete_stream(doc["stream_id"])
|
||||
assert len(fs.published) == n
|
||||
assert fs.get("discord_streams", doc["stream_id"]) is None
|
||||
assert await streams.delete_stream(doc["stream_id"]) is None
|
||||
|
||||
|
||||
async def test_delete_refuses_when_the_stream_is_restarted_in_between(fs, monkeypatch):
|
||||
doc = await _start()
|
||||
real_stop = streams.stop_stream
|
||||
|
||||
async def _stop_then_restart(stream_id, reason="stopped"):
|
||||
await real_stop(stream_id, reason)
|
||||
await _start() # /join lands between stop and delete
|
||||
monkeypatch.setattr(streams, "stop_stream", _stop_then_restart)
|
||||
|
||||
with pytest.raises(streams.StreamError) as e:
|
||||
await streams.delete_stream(doc["stream_id"])
|
||||
assert e.value.code == "restarted" and e.value.status == 409
|
||||
cur = fs.get("discord_streams", doc["stream_id"])
|
||||
assert cur["desired"] == "on" and doc["stream_id"] in _lease(fs, cur["token_id"])["streams"]
|
||||
|
||||
|
||||
def test_admin_route_deletes_and_audits(fs):
|
||||
import asyncio
|
||||
doc = asyncio.run(_start())
|
||||
app.dependency_overrides[require_admin_token] = lambda: ADMIN
|
||||
assert client.delete("/discord/streams/not-a-stream").status_code == 422
|
||||
assert client.delete("/discord/streams/100:9999").status_code == 404
|
||||
r = client.delete(f"/discord/streams/{doc['stream_id']}")
|
||||
assert r.status_code == 200 and r.json()["ok"] is True
|
||||
assert fs.get("discord_streams", doc["stream_id"]) is None
|
||||
(entry,) = _audit(fs, "discord.stream.delete")
|
||||
assert entry["actor_uid"] == "u-admin"
|
||||
assert entry["details"]["stream_id"] == doc["stream_id"] and entry["details"]["was_desired"] == "on"
|
||||
assert entry["details"]["org_id"] == ORG and entry["details"]["node_id"] == "n1"
|
||||
|
||||
|
||||
def test_admin_route_requires_platform_admin(fs):
|
||||
def deny():
|
||||
raise HTTPException(403, "Admin access required")
|
||||
app.dependency_overrides[require_admin_token] = deny
|
||||
assert client.delete("/discord/streams/100:1001").status_code == 403
|
||||
assert client.delete("/discord/guilds/100").status_code == 403
|
||||
|
||||
|
||||
async def test_node_page_remove_is_owner_or_admin_and_org_scoped(fs):
|
||||
a = await _start("1001", node_id="n1")
|
||||
b = await _start("1002", node_id="n2")
|
||||
|
||||
# An operator can stop but not permanently remove.
|
||||
with pytest.raises(HTTPException) as e:
|
||||
await web.remove_node_stream("n1", a["stream_id"], OPERATOR)
|
||||
assert e.value.status_code == 403
|
||||
# Another org's owner cannot even see the node.
|
||||
with pytest.raises(HTTPException) as e:
|
||||
await web.remove_node_stream("n1", a["stream_id"], STRANGER)
|
||||
assert e.value.status_code == 404
|
||||
# A stream on another node of the same org is not reachable through n1's URL.
|
||||
with pytest.raises(HTTPException) as e:
|
||||
await web.remove_node_stream("n1", b["stream_id"], OWNER)
|
||||
assert e.value.status_code == 404
|
||||
|
||||
assert (await web.remove_node_stream("n1", a["stream_id"], OWNER))["ok"] is True
|
||||
assert fs.get("discord_streams", a["stream_id"]) is None
|
||||
assert fs.get("discord_streams", b["stream_id"])["desired"] == "on" # untouched
|
||||
(entry,) = _audit(fs, "discord.stream.delete")
|
||||
assert entry["actor_uid"] == "u-own" and entry["details"]["via"] == "node_page"
|
||||
|
||||
assert (await web.remove_node_stream("n2", b["stream_id"], ADMIN))["ok"] is True
|
||||
|
||||
|
||||
async def test_join_options_says_who_may_remove(fs):
|
||||
assert (await web.join_options("n1", OWNER))["can_remove"] is True
|
||||
assert (await web.join_options("n1", ADMIN))["can_remove"] is True
|
||||
opts = await web.join_options("n1", OPERATOR)
|
||||
assert opts["can_control"] is True and opts["can_remove"] is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 3. Remove a Discord server
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
async def test_remove_guild_deletes_streams_and_unlinks_without_fallback(fs, monkeypatch):
|
||||
monkeypatch.setattr(settings, "discord_unlinked_guild_org_id", ORG)
|
||||
a = await _start("1001", node_id="n1")
|
||||
b = await _start("1002", node_id="n1")
|
||||
await streams.stop_stream(b["stream_id"]) # one already stopped
|
||||
out = await streams.remove_guild("100", "u-admin")
|
||||
|
||||
assert sorted(out["streams_removed"]) == sorted([a["stream_id"], b["stream_id"]])
|
||||
assert fs.data["discord_streams"] == {}
|
||||
g = fs.get("discord_guilds", "100")
|
||||
assert g["org_id"] is None and g["unlinked_at"] and g["removed_at"] and g["removed_by"] == "u-admin"
|
||||
# No fallback even though DISCORD_UNLINKED_GUILD_ORG_ID is set.
|
||||
assert await streams.resolve_guild_org("100") is None
|
||||
# Leases follow #216: releasing, freed on the node's ack.
|
||||
toks = {a["token_id"], b["token_id"]}
|
||||
assert len(toks) == 2 # one bot per guild voice connection
|
||||
leased = [_lease(fs, t) for t in toks]
|
||||
assert all(lease["releasing"] for lease in leased)
|
||||
seq = max(lease["release_seq"] for lease in leased)
|
||||
await streams.handle_actual("n1", {"applied_seq": seq, "sessions": [], "streams": {}})
|
||||
assert [_lease(fs, t) for t in toks] == [None, None]
|
||||
|
||||
|
||||
def test_remove_guild_route_hides_server_and_relink_restores_it(fs):
|
||||
import asyncio
|
||||
asyncio.run(_start("1001"))
|
||||
app.dependency_overrides[require_admin_token] = lambda: ADMIN
|
||||
assert client.delete("/discord/guilds/abc").status_code == 400
|
||||
assert client.delete("/discord/guilds/999").status_code == 404
|
||||
|
||||
r = client.delete("/discord/guilds/100")
|
||||
assert r.status_code == 200 and r.json()["streams_removed"] == ["100:1001"]
|
||||
assert r.json()["radio_bots"] == [] # not asked to leave
|
||||
body = client.get("/discord/admin/overview").json()
|
||||
assert body["guilds"] == [] and body["streams"] == []
|
||||
(entry,) = _audit(fs, "discord.guild.remove")
|
||||
assert entry["details"]["previous_org_id"] == ORG and entry["details"]["streams_removed"] == ["100:1001"]
|
||||
|
||||
# Admin link brings it back.
|
||||
assert client.put("/discord/guilds/100/link", json={"org_id": ORG}).status_code == 200
|
||||
g = fs.get("discord_guilds", "100")
|
||||
assert g["removed_at"] is None and g["unlinked_at"] is None
|
||||
assert [x["guild_id"] for x in client.get("/discord/admin/overview").json()["guilds"]] == ["100"]
|
||||
|
||||
|
||||
async def test_redeeming_a_link_code_restores_a_removed_server(fs):
|
||||
await streams.remove_guild("100", "u-admin")
|
||||
minted = await glink.mint_code(ORG, "u-own")
|
||||
await glink.redeem_code(minted["code"], "100", "Fire Co. 7", "4242")
|
||||
g = fs.get("discord_guilds", "100")
|
||||
assert g["org_id"] == ORG and g["removed_at"] is None
|
||||
|
||||
|
||||
def test_remove_guild_can_make_pool_and_own_radio_bots_leave(fs, monkeypatch):
|
||||
import httpx
|
||||
fs.put("bot_tokens", "byo-mine", {"token_id": "byo-mine", "name": "Ours", "token": "secret-mine",
|
||||
"org_id": ORG, "guild_ids": ["100"], "valid": True})
|
||||
fs.put("bot_tokens", "byo-theirs", {"token_id": "byo-theirs", "name": "Theirs", "token": "secret-theirs",
|
||||
"org_id": OTHER, "guild_ids": ["100"], "valid": True})
|
||||
fs.put("bot_tokens", "elsewhere", {"token_id": "elsewhere", "name": "Elsewhere", "token": "secret-else",
|
||||
"org_id": None, "guild_ids": ["300"], "valid": True})
|
||||
calls = []
|
||||
|
||||
class _Resp:
|
||||
def __init__(self, code):
|
||||
self.status_code = code
|
||||
|
||||
class _Client:
|
||||
def __init__(self, *, headers, **kw):
|
||||
self.auth = headers["Authorization"]
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, *exc):
|
||||
return False
|
||||
|
||||
async def delete(self, url):
|
||||
calls.append((self.auth, url))
|
||||
return _Resp(500 if self.auth == "Bot secret-t2" else 204)
|
||||
|
||||
monkeypatch.setattr(httpx, "AsyncClient", _Client)
|
||||
app.dependency_overrides[require_admin_token] = lambda: ADMIN
|
||||
r = client.delete("/discord/guilds/100?leave_radio_bots=true")
|
||||
assert r.status_code == 200
|
||||
tried = sorted(a for a, _ in calls)
|
||||
# pool bots in the server + this org's BYO; never another org's bot, never a bot not in it
|
||||
assert tried == ["Bot secret-mine", "Bot secret-t1", "Bot secret-t2"]
|
||||
assert all(u == "/users/@me/guilds/100" for _, u in calls)
|
||||
rows = {x["name"]: x for x in r.json()["radio_bots"]}
|
||||
assert rows["Radio t1"]["left"] is True and rows["Radio t2"]["left"] is False
|
||||
assert "secret" not in r.text
|
||||
assert fs.get("bot_tokens", "t1")["guild_ids"] == ["200"]
|
||||
assert fs.get("bot_tokens", "t2")["guild_ids"] == ["100", "200"] # failed: membership unchanged
|
||||
assert fs.get("bot_tokens", "byo-theirs")["guild_ids"] == ["100"]
|
||||
(entry,) = _audit(fs, "discord.guild.remove")
|
||||
assert sorted(entry["details"]["radio_bots_left"]) == ["byo-mine", "t1"]
|
||||
assert entry["details"]["radio_bots_failed"] == ["t2"]
|
||||
|
||||
|
||||
def test_node_page_routes_need_a_signed_in_user(fs):
|
||||
def deny():
|
||||
raise HTTPException(401, "Missing authorization")
|
||||
app.dependency_overrides[require_firebase_token] = deny
|
||||
assert client.delete("/discord/nodes/n1/streams/100:1001").status_code == 401
|
||||
Reference in New Issue
Block a user