Discord: several radio streams per voice channel; adding never overwrites (#213)
Owner-reported on live: adding a radio channel for a server + voice channel that already had one (different label, system, even bot) overwrote the existing stream and made its bot leave, because stream_id was guild:channel. - New streams get guild:channel:<12 hex> ids (leases.new_stream_id); old guild:channel docs keep working for read/stop/remove/node reports. Route validators accept both (leases.STREAM_ID_PATTERN). - start_stream: no stream_id = ADD. Same system already on in that channel is idempotent (already_on, label/mode untouched); a stopped one of that system is restarted; anything else is a new doc. stream_id = explicit restart/edit (admin Start button, reconciler), incl. moving to another bot. - preferred_token_id is strict: bot already in this server -> 409 preferred_busy naming the stream; on another node -> preferred_other_node; unusable -> preferred_unavailable. No silent swap. pool_exhausted now says one-bot-per-server and to add + invite another bot. - One-token-per-guild was already enforced inside the lease transaction; now covered by race tests. Per-guild asyncio lock serialises the already-on / server-cap check with the create (guild half of #223). - Stop by stream_id (POST /discord/guilds/{id}/stop {stream_id}); admin UI Stop/Start/Remove act on the row's stream; node picker lists every stream in a channel and adds beside other systems. - Bot: /leave stream: (autocomplete) stops one; /leave channel: with several streams asks which unless all:True; bare /leave still stops the server. /status and /join name each stream's bot. 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
16a609e35b
commit
8af410aede
@@ -34,6 +34,7 @@ def fs(monkeypatch):
|
||||
from app.internal.mqtt_handler import mqtt_handler
|
||||
monkeypatch.setattr(mqtt_handler, "publish_discord_desired", _publish)
|
||||
streams._publish_locks.clear()
|
||||
streams._guild_locks.clear()
|
||||
|
||||
async def _no_discord(token_ids):
|
||||
return 0
|
||||
@@ -70,6 +71,15 @@ def _lease(fake, token_id):
|
||||
return (fake.get("bot_tokens", token_id) or {}).get("lease")
|
||||
|
||||
|
||||
def _sid(fake, guild="g1", channel="c1"):
|
||||
"""The id of the one stream start_stream created in this channel (new
|
||||
streams get a unique ``guild:channel:suffix`` id — server-26#213)."""
|
||||
ids = [k for k, d in (fake.data.get("discord_streams") or {}).items()
|
||||
if d.get("guild_id") == guild and d.get("channel_id") == channel]
|
||||
assert len(ids) == 1, ids
|
||||
return ids[0]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# pick_token — pure choice
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -118,7 +128,8 @@ async def test_rejoin_reuses_the_lease_instead_of_taking_a_second_token(fs):
|
||||
assert again["token_id"] == first["token_id"] and again["reused"] is True
|
||||
leased = [t for t in ("t1", "t2") if _lease(fs, t)]
|
||||
assert leased == [first["token_id"]] # server-26#204 item 2
|
||||
assert _lease(fs, first["token_id"])["streams"] == ["g1:c1"]
|
||||
assert _lease(fs, first["token_id"])["streams"] == [first["stream_id"]]
|
||||
assert again["stream_id"] == first["stream_id"] and again["already_on"] is True
|
||||
|
||||
|
||||
async def test_preferred_token_is_honoured(fs):
|
||||
@@ -144,9 +155,9 @@ async def test_join_never_moves_another_guilds_stream(fs):
|
||||
_system(fs); _node(fs, "n1"); _token(fs, "t1")
|
||||
await _start("gA", "c1")
|
||||
await _start("gB", "c1")
|
||||
a = fs.get("discord_streams", "gA:c1")
|
||||
a = fs.get("discord_streams", _sid(fs, "gA", "c1"))
|
||||
assert a["desired"] == "on" and a["token_id"] == "t1"
|
||||
assert sorted(_lease(fs, "t1")["streams"]) == ["gA:c1", "gB:c1"]
|
||||
assert sorted(_lease(fs, "t1")["streams"]) == sorted([_sid(fs, "gA", "c1"), _sid(fs, "gB", "c1")])
|
||||
|
||||
|
||||
async def test_desired_state_never_carries_the_token_string(fs):
|
||||
@@ -236,7 +247,7 @@ async def test_two_guilds_same_node_racing_share_one_session(fs):
|
||||
async def test_stop_marks_releasing_and_holds_until_the_node_acks(fs):
|
||||
_system(fs); _node(fs, "n1"); _node(fs, "n2"); _token(fs, "t1")
|
||||
await _start()
|
||||
await streams.stop_stream("g1:c1")
|
||||
await streams.stop_stream(_sid(fs, "g1", "c1"))
|
||||
lease = _lease(fs, "t1")
|
||||
assert lease["releasing"] is True and lease["streams"] == []
|
||||
drop_seq = lease["release_seq"]
|
||||
@@ -260,25 +271,26 @@ async def test_stop_marks_releasing_and_holds_until_the_node_acks(fs):
|
||||
async def test_restart_before_release_reuses_the_logged_in_session(fs):
|
||||
_system(fs); _node(fs, "n1"); _token(fs, "t1"); _token(fs, "t2")
|
||||
first = await _start()
|
||||
await streams.stop_stream("g1:c1")
|
||||
await streams.stop_stream(_sid(fs, "g1", "c1"))
|
||||
again = await _start()
|
||||
assert again["token_id"] == first["token_id"]
|
||||
lease = _lease(fs, first["token_id"])
|
||||
assert lease["releasing"] is False and lease["streams"] == ["g1:c1"]
|
||||
assert lease["releasing"] is False and lease["streams"] == [first["stream_id"]]
|
||||
assert again["stream_id"] == first["stream_id"] # restarted, not a second doc
|
||||
|
||||
|
||||
async def test_handle_actual_settles_and_ignores_other_nodes_streams(fs):
|
||||
_system(fs); _node(fs, "n1"); _node(fs, "n2"); _token(fs, "t1"); _token(fs, "t2")
|
||||
await _start("g1", "c1", node_id="n1")
|
||||
await _start("g2", "c1", node_id="n2")
|
||||
a = await _start("g1", "c1", node_id="n1")
|
||||
b = await _start("g2", "c1", node_id="n2")
|
||||
await streams.handle_actual("n1", {
|
||||
"applied_seq": 1, "sessions": ["t1"],
|
||||
"streams": {"g1:c1": {"state": "connected"}, "g2:c1": {"state": "error", "error": "spoof"}},
|
||||
"streams": {a["stream_id"]: {"state": "connected"}, b["stream_id"]: {"state": "error", "error": "spoof"}},
|
||||
})
|
||||
assert fs.get("discord_streams", "g1:c1")["actual"]["state"] == "connected"
|
||||
assert fs.get("discord_streams", "g2:c1")["actual"]["state"] == "pending" # not n1's to report
|
||||
assert fs.get("discord_streams", _sid(fs, "g1", "c1"))["actual"]["state"] == "connected"
|
||||
assert fs.get("discord_streams", _sid(fs, "g2", "c1"))["actual"]["state"] == "pending" # not n1's to report
|
||||
|
||||
await streams.stop_stream("g1:c1")
|
||||
await streams.stop_stream(_sid(fs, "g1", "c1"))
|
||||
seq = _lease(fs, "t1")["release_seq"]
|
||||
await streams.handle_actual("n1", {"applied_seq": seq, "sessions": [], "streams": {}})
|
||||
assert _lease(fs, "t1") is None
|
||||
@@ -295,7 +307,7 @@ async def test_failed_publish_is_retried_by_the_reconciler(fs):
|
||||
n = len(fs.published)
|
||||
await streams.reconcile_once()
|
||||
assert len(fs.published) > n and fs.get("discord_nodes", "n1")["published"] is True
|
||||
assert fs.published[-1][1]["streams"][0]["stream_id"] == "g1:c1"
|
||||
assert fs.published[-1][1]["streams"][0]["stream_id"] == doc["stream_id"]
|
||||
|
||||
|
||||
async def test_concurrent_publishes_keep_the_retained_list_newest(fs):
|
||||
@@ -315,9 +327,9 @@ async def test_leave_in_one_guild_never_touches_another(fs):
|
||||
await _start("gA", "c1")
|
||||
await _start("gB", "c1")
|
||||
stopped = await streams.stop_guild("gA", "leave")
|
||||
assert stopped == ["gA:c1"]
|
||||
assert fs.get("discord_streams", "gB:c1")["desired"] == "on"
|
||||
assert fs.get("discord_streams", "gA:c1")["desired"] == "off"
|
||||
assert stopped == [_sid(fs, "gA", "c1")]
|
||||
assert fs.get("discord_streams", _sid(fs, "gB", "c1"))["desired"] == "on"
|
||||
assert fs.get("discord_streams", _sid(fs, "gA", "c1"))["desired"] == "off"
|
||||
|
||||
|
||||
async def test_guild_org_resolution(fs, monkeypatch):
|
||||
@@ -340,7 +352,7 @@ async def test_offline_node_keeps_its_lease_inside_the_grace(fs, monkeypatch):
|
||||
_node(fs, "n1", status="offline", age=120)
|
||||
await streams.reconcile_once()
|
||||
assert _lease(fs, "t1")["node_id"] == "n1"
|
||||
assert fs.get("discord_streams", "g1:c1")["actual"]["state"] == "node_offline"
|
||||
assert fs.get("discord_streams", _sid(fs, "g1", "c1"))["actual"]["state"] == "node_offline"
|
||||
|
||||
|
||||
async def test_persistent_stream_moves_after_grace_adhoc_stops(fs, monkeypatch):
|
||||
@@ -351,10 +363,10 @@ async def test_persistent_stream_moves_after_grace_adhoc_stops(fs, monkeypatch):
|
||||
_node(fs, "n1", status="offline", age=900)
|
||||
_node(fs, "n2")
|
||||
await streams.reconcile_once()
|
||||
moved = fs.get("discord_streams", "g1:c1")
|
||||
moved = fs.get("discord_streams", _sid(fs, "g1", "c1"))
|
||||
assert moved["desired"] == "on" and moved["node_id"] == "n2" and moved["token_id"]
|
||||
assert _lease(fs, moved["token_id"])["node_id"] == "n2"
|
||||
assert fs.get("discord_streams", "g2:c1")["desired"] == "off"
|
||||
assert fs.get("discord_streams", _sid(fs, "g2", "c1"))["desired"] == "off"
|
||||
assert all((_lease(fs, t) or {}).get("node_id") != "n1" for t in ("t1", "t2"))
|
||||
|
||||
|
||||
@@ -365,7 +377,7 @@ async def test_leave_while_node_offline_frees_token_after_grace(fs, monkeypatch)
|
||||
_system(fs); _node(fs, "n1"); _token(fs, "t1")
|
||||
await _start()
|
||||
_node(fs, "n1", status="offline", age=120)
|
||||
await streams.stop_stream("g1:c1")
|
||||
await streams.stop_stream(_sid(fs, "g1", "c1"))
|
||||
await streams.reconcile_once()
|
||||
assert _lease(fs, "t1")["releasing"] is True # inside the grace: held
|
||||
_node(fs, "n1", status="offline", age=900)
|
||||
@@ -377,7 +389,7 @@ async def test_deleted_node_lease_freed_after_grace_from_last_report(fs, monkeyp
|
||||
monkeypatch.setattr(streams.settings, "discord_lease_offline_release_seconds", 600)
|
||||
_system(fs); _node(fs, "n1"); _token(fs, "t1")
|
||||
await _start()
|
||||
await streams.stop_stream("g1:c1")
|
||||
await streams.stop_stream(_sid(fs, "g1", "c1"))
|
||||
fs.put("discord_nodes", "n1", {**(fs.get("discord_nodes", "n1") or {}),
|
||||
"actual_at": datetime.now(timezone.utc).isoformat()})
|
||||
await fs.doc_delete("nodes", "n1")
|
||||
@@ -401,12 +413,12 @@ async def test_release_seq_never_names_a_list_that_still_holds_the_stream(fs, mo
|
||||
out = await real(node_id)
|
||||
if not fired:
|
||||
fired.append(1)
|
||||
await leases.detach_stream("g1:c1", {"desired": "off"})
|
||||
await leases.detach_stream(_sid(fs, "g1", "c1"), {"desired": "off"})
|
||||
return out
|
||||
monkeypatch.setattr(streams, "desired_for_node", _racy)
|
||||
await streams.publish_desired("n1")
|
||||
racy_seq = fs.published[-1][1]["seq"]
|
||||
assert any(d["stream_id"] == "g1:c1" for d in fs.published[-1][1]["streams"])
|
||||
assert any(d["stream_id"] == _sid(fs, "g1", "c1") for d in fs.published[-1][1]["streams"])
|
||||
rseq = _lease(fs, "t1")["release_seq"]
|
||||
assert rseq is None or rseq > racy_seq
|
||||
assert await leases.settle_releases("n1", set(), racy_seq) == []
|
||||
@@ -417,7 +429,7 @@ async def test_retuned_node_gives_up_a_persistent_stream(fs):
|
||||
await _start(mode="persistent")
|
||||
_node(fs, "n1", system="sys-2")
|
||||
await streams.reconcile_once()
|
||||
s = fs.get("discord_streams", "g1:c1")
|
||||
s = fs.get("discord_streams", _sid(fs, "g1", "c1"))
|
||||
assert s["desired"] == "on" and s["token_id"] is None and s["actual"]["state"] == "error"
|
||||
assert _lease(fs, "t1")["releasing"] is True
|
||||
|
||||
@@ -432,7 +444,7 @@ async def test_force_release_detaches_streams(fs):
|
||||
old = await leases.force_release_token("t1")
|
||||
assert old["node_id"] == "n1"
|
||||
assert _lease(fs, "t1") is None
|
||||
assert fs.get("discord_streams", "g1:c1")["token_id"] is None
|
||||
assert fs.get("discord_streams", _sid(fs, "g1", "c1"))["token_id"] is None
|
||||
|
||||
|
||||
def test_node_token_pickup_only_for_the_lease_holder(fs):
|
||||
|
||||
@@ -0,0 +1,440 @@
|
||||
"""
|
||||
Several radio streams in ONE Discord voice channel (server-26#213 follow-up,
|
||||
owner decision 2026-10-05).
|
||||
|
||||
Owner-reported bug on live: adding a radio channel for a server + voice
|
||||
channel that already had one — different label, system, even a different
|
||||
preferred bot — overwrote the existing stream and made its bot leave, because
|
||||
the stream id was ``guild:channel``. Now every new stream gets its own id;
|
||||
each stream has its own bot (Discord: one voice connection per bot per
|
||||
server), and a chosen bot that is already in the server is refused by name.
|
||||
"""
|
||||
import asyncio
|
||||
import threading
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.config import settings
|
||||
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_service_key_or_admin
|
||||
from app.main import app
|
||||
from app.routers import discord_web as web
|
||||
from tests.fake_firestore import FakeFirestore
|
||||
|
||||
ORG = "org-a"
|
||||
P25, FIRE = "sys-p25", "sys-fire"
|
||||
ADMIN = {"uid": "u-admin", "role": "admin", "email": "admin@example.test"}
|
||||
OWNER = {"uid": "u-own", "org_id": ORG, "org_role": "owner"}
|
||||
client = TestClient(app)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def fs(monkeypatch):
|
||||
fake = FakeFirestore.install(monkeypatch)
|
||||
fake.published = []
|
||||
from app.internal.mqtt_handler import mqtt_handler
|
||||
monkeypatch.setattr(mqtt_handler, "publish_discord_desired",
|
||||
lambda n, p: fake.published.append((n, p)) or True)
|
||||
streams._publish_locks.clear()
|
||||
streams._guild_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", "")
|
||||
monkeypatch.setattr(settings, "discord_guild_max_streams", 4)
|
||||
monkeypatch.setattr(settings, "discord_node_max_streams", 4)
|
||||
|
||||
# One node per system: a node plays the one system it is tuned to.
|
||||
for nid, system in (("n-p25", P25), ("n-fire", FIRE)):
|
||||
fake.put("nodes", nid, {"node_id": nid, "name": f"Node {nid}", "org_id": ORG, "status": "online",
|
||||
"approval_status": "approved", "assigned_system_id": system})
|
||||
fake.put("systems", P25, {"system_id": P25, "name": "County P25", "org_id": ORG})
|
||||
fake.put("systems", FIRE, {"system_id": FIRE, "name": "City Fire", "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 in ("t1", "t2", "t3"):
|
||||
fake.put("bot_tokens", tid, {"token_id": tid, "name": f"Radio {tid}", "token": f"secret-{tid}",
|
||||
"org_id": None, "guild_ids": ["100", "200"], "valid": True,
|
||||
"application_id": f"app-{tid}", "lease": None})
|
||||
yield fake
|
||||
app.dependency_overrides.clear()
|
||||
|
||||
|
||||
def _lease(fs, tid):
|
||||
return (fs.get("bot_tokens", tid) or {}).get("lease")
|
||||
|
||||
|
||||
def _live(fs, channel="1001"):
|
||||
return sorted((d for d in (fs.data.get("discord_streams") or {}).values()
|
||||
if d.get("channel_id") == channel and d.get("desired") == "on"),
|
||||
key=lambda d: d.get("created_at") or "")
|
||||
|
||||
|
||||
def _post(body):
|
||||
app.dependency_overrides[require_service_key_or_admin] = lambda: ADMIN
|
||||
return client.post("/discord/streams", json={"guild_id": "100", "channel_id": "1001", **body})
|
||||
|
||||
|
||||
def _legacy(fs, token="t1", node="n-p25", system=P25, channel="1001"):
|
||||
"""A stream doc as it was created before this change: id guild:channel."""
|
||||
sid = f"100:{channel}"
|
||||
fs.put("discord_streams", sid, {
|
||||
"stream_id": sid, "org_id": ORG, "guild_id": "100", "channel_id": channel, "channel_name": "Old label",
|
||||
"system_id": system, "system_name": "County P25", "mode": "persistent", "desired": "on",
|
||||
"node_id": node, "token_id": token, "lease_id": "L-old", "actual": {"state": "connected"},
|
||||
"created_at": "2026-10-01T00:00:00+00:00",
|
||||
})
|
||||
fs.put("bot_tokens", token, {**fs.get("bot_tokens", token),
|
||||
"lease": {"node_id": node, "lease_id": "L-old", "since": "x",
|
||||
"streams": [sid], "releasing": False, "release_seq": None}})
|
||||
return sid
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The reported bug
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_adding_a_second_stream_to_a_channel_never_touches_the_first(fs):
|
||||
"""Same server + voice channel, different label, system and preferred bot:
|
||||
a second row, each with its own bot — the first keeps playing."""
|
||||
a = _post({"system_id": P25, "channel_name": "County", "preferred_token_id": "t1"})
|
||||
assert a.status_code == 200, a.text
|
||||
first = fs.get("discord_streams", a.json()["stream_id"])
|
||||
|
||||
b = _post({"system_id": FIRE, "channel_name": "Fire", "preferred_token_id": "t2"})
|
||||
assert b.status_code == 200, b.text
|
||||
assert b.json()["stream_id"] != a.json()["stream_id"]
|
||||
assert a.json()["stream_id"].startswith("100:1001:") and b.json()["stream_id"].startswith("100:1001:")
|
||||
|
||||
assert fs.get("discord_streams", a.json()["stream_id"]) == first # byte-for-byte untouched
|
||||
live = _live(fs)
|
||||
assert [(s["system_id"], s["channel_name"], s["token_id"], s["node_id"]) for s in live] == [
|
||||
(P25, "County", "t1", "n-p25"), (FIRE, "Fire", "t2", "n-fire")]
|
||||
assert _lease(fs, "t1")["streams"] == [a.json()["stream_id"]] # t1 never left
|
||||
assert _lease(fs, "t2")["streams"] == [b.json()["stream_id"]]
|
||||
# Each node is told only about its own stream.
|
||||
last = {nid: p for nid, p in fs.published}
|
||||
assert [s["stream_id"] for s in last["n-p25"]["streams"]] == [a.json()["stream_id"]]
|
||||
assert [s["stream_id"] for s in last["n-fire"]["streams"]] == [b.json()["stream_id"]]
|
||||
|
||||
|
||||
def test_any_free_bot_never_reuses_a_bot_already_in_the_server(fs):
|
||||
"""Two streams in one channel on ONE node (same token pool): different bots."""
|
||||
fs.put("nodes", "n-fire", {**fs.get("nodes", "n-fire"), "assigned_system_id": P25})
|
||||
fs.put("systems", "sys-3", {"system_id": "sys-3", "name": "Third", "org_id": ORG})
|
||||
a = asyncio.run(streams.start_stream(org_id=ORG, guild_id="100", channel_id="1001", system_id=P25,
|
||||
node_id="n-p25"))
|
||||
b = asyncio.run(streams.start_stream(org_id=ORG, guild_id="100", channel_id="1001", system_id="sys-3",
|
||||
node_id="n-p25"))
|
||||
assert a["stream_id"] != b["stream_id"] and a["token_id"] != b["token_id"]
|
||||
assert sorted(_lease(fs, a["token_id"])["streams"] + _lease(fs, b["token_id"])["streams"]) == \
|
||||
sorted([a["stream_id"], b["stream_id"]])
|
||||
|
||||
|
||||
def test_preferred_bot_already_in_the_server_is_refused_by_name(fs):
|
||||
a = _post({"system_id": P25, "channel_name": "County", "preferred_token_id": "t1"})
|
||||
first = fs.get("discord_streams", a.json()["stream_id"])
|
||||
|
||||
r = _post({"system_id": FIRE, "channel_name": "Fire", "preferred_token_id": "t1"})
|
||||
assert r.status_code == 409
|
||||
body = r.json()
|
||||
assert body["code"] == "preferred_busy" and body["busy_stream_id"] == a.json()["stream_id"]
|
||||
assert "Radio t1" in body["detail"] and "County P25" in body["detail"] and "#County" in body["detail"]
|
||||
# Not a silent swap: nothing moved, no second stream left behind half-made.
|
||||
assert fs.get("discord_streams", a.json()["stream_id"]) == first
|
||||
assert len(_live(fs)) == 1 and _lease(fs, "t1")["streams"] == [a.json()["stream_id"]]
|
||||
assert all(_lease(fs, t) is None for t in ("t2", "t3"))
|
||||
|
||||
# Same bot in ANOTHER channel of the same server: still one voice connection per server.
|
||||
app.dependency_overrides[require_service_key_or_admin] = lambda: ADMIN
|
||||
r2 = client.post("/discord/streams", json={"guild_id": "100", "channel_id": "1002", "system_id": P25,
|
||||
"preferred_token_id": "t1"})
|
||||
assert r2.status_code == 409 and r2.json()["code"] == "preferred_busy"
|
||||
|
||||
|
||||
def test_preferred_bot_on_another_node_is_refused_not_swapped(fs):
|
||||
# t1 is logged in on n-p25 (serving another server); a FIRE stream needs n-fire.
|
||||
fs.put("bot_tokens", "t1", {**fs.get("bot_tokens", "t1"),
|
||||
"lease": {"node_id": "n-p25", "lease_id": "L", "streams": ["200:2001:aaaaaaaaaaaa"],
|
||||
"releasing": False, "release_seq": None}})
|
||||
r = _post({"system_id": FIRE, "preferred_token_id": "t1"})
|
||||
assert r.status_code == 409 and r.json()["code"] == "preferred_other_node"
|
||||
assert "Node n-p25" in r.json()["detail"]
|
||||
assert _live(fs) == []
|
||||
|
||||
|
||||
def test_pool_exhausted_for_the_server_says_to_add_and_invite_a_bot(fs):
|
||||
for tid in ("t2", "t3"):
|
||||
fs.put("bot_tokens", tid, {**fs.get("bot_tokens", tid), "valid": False})
|
||||
assert _post({"system_id": P25}).status_code == 200
|
||||
r = _post({"system_id": FIRE})
|
||||
assert r.status_code == 409 and r.json()["code"] == "pool_exhausted"
|
||||
assert "one voice channel per server" in r.json()["detail"] and "invite it" in r.json()["detail"]
|
||||
|
||||
|
||||
def test_same_system_twice_in_a_channel_is_the_same_stream(fs):
|
||||
a = _post({"system_id": P25, "channel_name": "County", "mode": "persistent"}).json()
|
||||
again = _post({"system_id": P25, "channel_name": "Renamed", "mode": "adhoc"})
|
||||
assert again.status_code == 200
|
||||
assert again.json()["stream_id"] == a["stream_id"] and again.json()["already_on"] is True
|
||||
doc = fs.get("discord_streams", a["stream_id"])
|
||||
assert doc["channel_name"] == "County" and doc["mode"] == "persistent" # a re-add is not an edit
|
||||
assert len(_live(fs)) == 1
|
||||
# ...and naming a DIFFERENT bot for it is a clear conflict, not a swap.
|
||||
r = _post({"system_id": P25, "preferred_token_id": "t3" if a["token_id"] != "t3" else "t2"})
|
||||
assert r.status_code == 409 and r.json()["code"] == "duplicate"
|
||||
|
||||
|
||||
def test_stop_by_stream_id_leaves_the_other_stream_in_the_channel_on(fs):
|
||||
a = _post({"system_id": P25}).json()
|
||||
b = _post({"system_id": FIRE}).json()
|
||||
r = client.post("/discord/guilds/100/stop", json={"stream_id": b["stream_id"], "reason": "test"})
|
||||
assert r.status_code == 200 and r.json()["stopped"] == [b["stream_id"]]
|
||||
assert [s["stream_id"] for s in _live(fs)] == [a["stream_id"]]
|
||||
# A stream id from another server matches nothing in this one.
|
||||
assert client.post("/discord/guilds/999/stop", json={"stream_id": a["stream_id"]}).json()["stopped"] == []
|
||||
assert client.post("/discord/guilds/100/stop", json={"stream_id": "100/1001"}).status_code == 422
|
||||
# Channel-wide stop still stops every stream in the channel.
|
||||
b2 = _post({"system_id": FIRE}).json()
|
||||
assert b2["stream_id"] == b["stream_id"] # restarted, not a new doc
|
||||
r = client.post("/discord/guilds/100/stop", json={"channel_id": "1001"})
|
||||
assert sorted(r.json()["stopped"]) == sorted([a["stream_id"], b["stream_id"]])
|
||||
|
||||
|
||||
def test_restart_and_edit_by_stream_id(fs):
|
||||
a = _post({"system_id": P25, "channel_name": "County"}).json()
|
||||
b = _post({"system_id": FIRE, "channel_name": "Fire"}).json()
|
||||
client.post("/discord/guilds/100/stop", json={"stream_id": a["stream_id"]})
|
||||
# The admin page's Start button names the stream: it restarts THAT one.
|
||||
r = _post({"system_id": P25, "stream_id": a["stream_id"]})
|
||||
assert r.status_code == 200 and r.json()["stream_id"] == a["stream_id"]
|
||||
assert fs.get("discord_streams", b["stream_id"])["token_id"] == b["token_id"]
|
||||
# Explicit edit to a free bot moves the stream to it (and only it).
|
||||
free = next(t for t in ("t1", "t2", "t3") if t not in (r.json()["token_id"], b["token_id"]))
|
||||
e = _post({"system_id": P25, "stream_id": a["stream_id"], "preferred_token_id": free})
|
||||
assert e.status_code == 200 and e.json()["token_id"] == free
|
||||
assert a["stream_id"] not in (_lease(fs, r.json()["token_id"]) or {}).get("streams", [])
|
||||
# Editing to the bot the OTHER stream in this server uses is refused.
|
||||
e2 = _post({"system_id": P25, "stream_id": a["stream_id"], "preferred_token_id": b["token_id"]})
|
||||
assert e2.status_code == 409 and e2.json()["code"] == "preferred_busy"
|
||||
# A stream id must belong to the server/channel it is posted with.
|
||||
bad = client.post("/discord/streams", json={"guild_id": "100", "channel_id": "1002", "system_id": P25,
|
||||
"stream_id": a["stream_id"]})
|
||||
assert bad.status_code == 400
|
||||
missing = _post({"system_id": P25, "stream_id": "100:1001:0123456789ab"})
|
||||
assert missing.status_code == 404
|
||||
|
||||
|
||||
async def test_reconciler_replaces_each_stream_in_a_shared_channel_by_id(fs, monkeypatch):
|
||||
monkeypatch.setattr(settings, "discord_lease_offline_release_seconds", 600)
|
||||
fs.put("nodes", "n-p25b", {"node_id": "n-p25b", "name": "Node n-p25b", "org_id": ORG, "status": "online",
|
||||
"approval_status": "approved", "assigned_system_id": P25})
|
||||
a = await streams.start_stream(org_id=ORG, guild_id="100", channel_id="1001", system_id=P25,
|
||||
mode="persistent", node_id="n-p25")
|
||||
b = await streams.start_stream(org_id=ORG, guild_id="100", channel_id="1001", system_id=FIRE,
|
||||
mode="persistent")
|
||||
from datetime import datetime, timezone, timedelta
|
||||
old = (datetime.now(timezone.utc) - timedelta(seconds=900)).isoformat()
|
||||
fs.put("nodes", "n-p25", {**fs.get("nodes", "n-p25"), "status": "offline", "last_seen": old})
|
||||
await streams.reconcile_once()
|
||||
moved = fs.get("discord_streams", a["stream_id"])
|
||||
assert moved["desired"] == "on" and moved["node_id"] == "n-p25b" and moved["token_id"]
|
||||
assert moved["token_id"] != fs.get("discord_streams", b["stream_id"])["token_id"]
|
||||
assert fs.get("discord_streams", b["stream_id"])["node_id"] == "n-fire"
|
||||
assert len(_live(fs)) == 2 # no third doc
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Races — the per-server bot rule holds under concurrency
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _hold_both_at_commit(fake):
|
||||
barrier = threading.Barrier(2, timeout=5)
|
||||
|
||||
def _hook(attempt):
|
||||
if attempt == 0:
|
||||
barrier.wait()
|
||||
fake.before_commit = _hook
|
||||
|
||||
|
||||
async def test_two_streams_one_channel_racing_never_share_a_bot(fs):
|
||||
"""Two NEW streams in one server on one node, leased concurrently: the
|
||||
transaction re-reads the pool, so they end up on two different bots."""
|
||||
s1, s2 = leases.new_stream_id("100", "1001"), leases.new_stream_id("100", "1001")
|
||||
_hold_both_at_commit(fs)
|
||||
r1, r2 = await asyncio.gather(
|
||||
leases.lease_for_stream(s1, node_id="n-p25", org_id=ORG),
|
||||
leases.lease_for_stream(s2, node_id="n-p25", org_id=ORG),
|
||||
)
|
||||
assert fs.retries >= 1 # the race really happened
|
||||
assert r1.ok and r2.ok and r1.token_id != r2.token_id
|
||||
for tid in (r1.token_id, r2.token_id):
|
||||
assert len([s for s in _lease(fs, tid)["streams"] if leases.guild_of(s) == "100"]) == 1
|
||||
|
||||
|
||||
async def test_racing_for_the_last_free_bot_in_a_server_one_loses_cleanly(fs):
|
||||
fs.put("bot_tokens", "t3", {**fs.get("bot_tokens", "t3"), "valid": False})
|
||||
fs.put("bot_tokens", "t2", {**fs.get("bot_tokens", "t2"),
|
||||
"lease": {"node_id": "n-p25", "lease_id": "L2", "streams": ["100:1002:bbbbbbbbbbbb"],
|
||||
"releasing": False, "release_seq": None}})
|
||||
s1, s2 = leases.new_stream_id("100", "1001"), leases.new_stream_id("100", "1001")
|
||||
_hold_both_at_commit(fs)
|
||||
r1, r2 = await asyncio.gather(
|
||||
leases.lease_for_stream(s1, node_id="n-p25", org_id=ORG),
|
||||
leases.lease_for_stream(s2, node_id="n-p25", org_id=ORG),
|
||||
)
|
||||
assert sorted([r1.ok, r2.ok]) == [False, True]
|
||||
loser = r2 if r1.ok else r1
|
||||
assert loser.error == "pool_exhausted"
|
||||
assert len(_lease(fs, "t1")["streams"]) == 1
|
||||
|
||||
|
||||
async def test_racing_same_preferred_bot_for_two_streams_in_one_server(fs):
|
||||
s1, s2 = leases.new_stream_id("100", "1001"), leases.new_stream_id("100", "1002")
|
||||
_hold_both_at_commit(fs)
|
||||
r1, r2 = await asyncio.gather(
|
||||
leases.lease_for_stream(s1, node_id="n-p25", org_id=ORG, preferred_token_id="t1"),
|
||||
leases.lease_for_stream(s2, node_id="n-p25", org_id=ORG, preferred_token_id="t1"),
|
||||
)
|
||||
assert sorted([r1.ok, r2.ok]) == [False, True]
|
||||
loser = r2 if r1.ok else r1
|
||||
assert loser.error == "preferred_busy" and loser.detail["busy_stream_id"] in (s1, s2)
|
||||
assert len(_lease(fs, "t1")["streams"]) == 1
|
||||
|
||||
|
||||
def _slow_find(monkeypatch):
|
||||
"""Yield to the other requests right after every stream-list read (the
|
||||
"already on?" and "server at its cap?" checks), so unserialised starts
|
||||
would all read before any of them writes — as real Firestore reads allow."""
|
||||
from app.internal import firestore as fstore
|
||||
real = fstore.collection_list
|
||||
|
||||
async def _list(coll, **filters):
|
||||
out = await real(coll, **filters)
|
||||
if coll == streams.STREAMS:
|
||||
await asyncio.sleep(0.05)
|
||||
return out
|
||||
monkeypatch.setattr(fstore, "collection_list", _list)
|
||||
|
||||
|
||||
async def test_concurrent_adds_of_one_system_to_one_channel_make_one_stream(fs, monkeypatch):
|
||||
_slow_find(monkeypatch)
|
||||
r1, r2 = await asyncio.gather(
|
||||
streams.start_stream(org_id=ORG, guild_id="100", channel_id="1001", system_id=P25),
|
||||
streams.start_stream(org_id=ORG, guild_id="100", channel_id="1001", system_id=P25),
|
||||
)
|
||||
assert r1["stream_id"] == r2["stream_id"]
|
||||
assert len(_live(fs)) == 1
|
||||
assert len([t for t in ("t1", "t2", "t3") if _lease(fs, t)]) == 1
|
||||
|
||||
|
||||
async def test_concurrent_adds_respect_the_server_cap(fs, monkeypatch):
|
||||
_slow_find(monkeypatch)
|
||||
fs.put("systems", "sys-3", {"system_id": "sys-3", "name": "Third", "org_id": ORG})
|
||||
fs.put("nodes", "n-3", {"node_id": "n-3", "name": "Node n-3", "org_id": ORG, "status": "online",
|
||||
"approval_status": "approved", "assigned_system_id": "sys-3"})
|
||||
results = await asyncio.gather(*(
|
||||
streams.start_stream(org_id=ORG, guild_id="100", channel_id="1001", system_id=sid, max_streams=2)
|
||||
for sid in (P25, FIRE, "sys-3")), return_exceptions=True)
|
||||
errors = [r for r in results if isinstance(r, streams.StreamError)]
|
||||
assert len(errors) == 1 and errors[0].code == "limit"
|
||||
assert len(_live(fs)) == 2
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Old guild:channel ids keep working
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_old_id_stop_and_admin_remove(fs):
|
||||
sid = _legacy(fs)
|
||||
app.dependency_overrides[require_service_key_or_admin] = lambda: ADMIN
|
||||
app.dependency_overrides[require_admin_token] = lambda: ADMIN
|
||||
assert client.get(f"/discord/streams/{sid}").json()["stream_id"] == sid
|
||||
r = client.post("/discord/guilds/100/stop", json={"stream_id": sid})
|
||||
assert r.json()["stopped"] == [sid]
|
||||
assert _lease(fs, "t1")["releasing"] is True and _lease(fs, "t1")["streams"] == []
|
||||
r = client.delete(f"/discord/streams/{sid}")
|
||||
assert r.status_code == 200 and fs.get("discord_streams", sid) is None
|
||||
|
||||
|
||||
def test_old_id_doc_is_not_overwritten_by_an_add_and_is_restarted_by_its_own_system(fs):
|
||||
sid = _legacy(fs)
|
||||
b = _post({"system_id": FIRE, "channel_name": "Fire"}).json()
|
||||
assert b["stream_id"] != sid and b["token_id"] != "t1"
|
||||
old = fs.get("discord_streams", sid)
|
||||
assert old["desired"] == "on" and old["token_id"] == "t1" and old["channel_name"] == "Old label"
|
||||
# Re-adding the old stream's own system lands on the old doc (idempotent).
|
||||
again = _post({"system_id": P25}).json()
|
||||
assert again["stream_id"] == sid and again["already_on"] is True
|
||||
|
||||
|
||||
async def test_old_id_node_page_stop_and_remove(fs):
|
||||
sid = _legacy(fs)
|
||||
r = await web.stop_node_streams("n-p25", web.NodeStreamStop(stream_id=sid), OWNER)
|
||||
assert r["stopped"] == [sid]
|
||||
r = await web.remove_node_stream("n-p25", sid, OWNER)
|
||||
assert r["ok"] and fs.get("discord_streams", sid) is None
|
||||
|
||||
|
||||
async def test_old_id_still_reported_on_by_the_node(fs):
|
||||
sid = _legacy(fs)
|
||||
await streams.handle_actual("n-p25", {"streams": {sid: {"state": "error", "error": "x", "code": "no_audio"},
|
||||
"100/evil": {"state": "connected"}}})
|
||||
assert fs.get("discord_streams", sid)["actual"]["code"] == "no_audio"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Node page + admin page flows
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
async def test_node_page_adds_beside_another_systems_stream(fs):
|
||||
p25 = await web.start_node_stream("n-p25", web.NodeStreamStart(guild_id="100", channel_id="1001"), OWNER)
|
||||
fire = await web.start_node_stream("n-fire", web.NodeStreamStart(guild_id="100", channel_id="1001"), OWNER)
|
||||
assert p25["stream_id"] != fire["stream_id"] and p25["token_id"] != fire["token_id"]
|
||||
assert fs.get("discord_streams", p25["stream_id"])["node_id"] == "n-p25" # not moved
|
||||
|
||||
opts = await web.join_options("n-fire", OWNER)
|
||||
ch = next(c for c in opts["guilds"][0]["voice_channels"] if c["channel_id"] == "1001")
|
||||
assert [s["system_name"] for s in ch["streams"]] == ["County P25", "City Fire"]
|
||||
assert {s["token_name"] for s in ch["streams"]} == {f"Radio {p25['token_id']}", f"Radio {fire['token_id']}"}
|
||||
assert ch["stream"]["stream_id"] == fire["stream_id"] # this node's system's one
|
||||
assert [s["stream_id"] for s in opts["streams"]] == [fire["stream_id"]]
|
||||
assert opts["guilds"][0]["streams_on"] == 2
|
||||
|
||||
# Re-picking on the same node is idempotent (and free under the node cap).
|
||||
again = await web.start_node_stream("n-fire", web.NodeStreamStart(guild_id="100", channel_id="1001"), OWNER)
|
||||
assert again["stream_id"] == fire["stream_id"]
|
||||
# Removing one leaves the other.
|
||||
await web.remove_node_stream("n-fire", fire["stream_id"], OWNER)
|
||||
assert [s["stream_id"] for s in _live(fs)] == [p25["stream_id"]]
|
||||
|
||||
|
||||
def test_admin_overview_lists_one_row_per_stream_with_its_bot(fs):
|
||||
app.dependency_overrides[require_admin_token] = lambda: ADMIN
|
||||
a = _post({"system_id": P25, "channel_name": "County"}).json()
|
||||
b = _post({"system_id": FIRE, "channel_name": "Fire"}).json()
|
||||
body = client.get("/discord/admin/overview").json()
|
||||
rows = [(r["stream_id"], r["channel_id"], r["system_name"], r["token_name"]) for r in body["streams"]]
|
||||
assert rows == [(a["stream_id"], "1001", "County P25", f"Radio {a['token_id']}"),
|
||||
(b["stream_id"], "1001", "City Fire", f"Radio {b['token_id']}")]
|
||||
assert body["guilds"][0]["live_streams"] == 2
|
||||
# Remove one by id: the other row stays and stays on.
|
||||
assert client.delete(f"/discord/streams/{a['stream_id']}").status_code == 200
|
||||
body = client.get("/discord/admin/overview").json()
|
||||
assert [(r["stream_id"], r["desired"]) for r in body["streams"]] == [(b["stream_id"], "on")]
|
||||
|
||||
|
||||
def test_bot_overview_names_each_streams_bot(fs):
|
||||
a = _post({"system_id": P25}).json()
|
||||
b = _post({"system_id": FIRE}).json()
|
||||
body = client.get("/discord/guilds/100/overview").json()
|
||||
assert {(s["stream_id"], s["token_name"]) for s in body["streams"]} == {
|
||||
(a["stream_id"], f"Radio {a['token_id']}"), (b["stream_id"], f"Radio {b['token_id']}")}
|
||||
busy = {x["token_id"]: x["busy_here"] for x in body["bots"]}
|
||||
assert busy[a["token_id"]] and busy[b["token_id"]]
|
||||
@@ -53,6 +53,7 @@ def fs(monkeypatch):
|
||||
from app.internal.mqtt_handler import mqtt_handler
|
||||
monkeypatch.setattr(mqtt_handler, "publish_discord_desired", _publish)
|
||||
streams._publish_locks.clear()
|
||||
streams._guild_locks.clear()
|
||||
|
||||
async def _no_discord(token_ids):
|
||||
return 0
|
||||
@@ -301,18 +302,18 @@ def test_remove_guild_interrupted_by_a_restart_is_still_audited(fs, monkeypatch)
|
||||
|
||||
def test_remove_guild_route_hides_server_and_relink_restores_it(fs):
|
||||
import asyncio
|
||||
asyncio.run(_start("1001"))
|
||||
sid = asyncio.run(_start("1001"))["stream_id"]
|
||||
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.status_code == 200 and r.json()["streams_removed"] == [sid]
|
||||
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"]
|
||||
assert entry["details"]["previous_org_id"] == ORG and entry["details"]["streams_removed"] == [sid]
|
||||
|
||||
# Admin link brings it back.
|
||||
assert client.put("/discord/guilds/100/link", json={"org_id": ORG}).status_code == 200
|
||||
|
||||
@@ -39,6 +39,7 @@ def fs(monkeypatch):
|
||||
from app.internal.mqtt_handler import mqtt_handler
|
||||
monkeypatch.setattr(mqtt_handler, "publish_discord_desired", lambda n, p: published.append((n, p)) or True)
|
||||
streams._publish_locks.clear()
|
||||
streams._guild_locks.clear()
|
||||
|
||||
async def _no_discord(token_ids):
|
||||
return 0
|
||||
@@ -149,14 +150,14 @@ def test_options_reports_missing_bot_with_invite(fs):
|
||||
|
||||
def test_start_pins_stream_to_node_with_names(fs):
|
||||
r = run(web.start_node_stream("n1", web.NodeStreamStart(guild_id="100", channel_id="1001"), OWNER))
|
||||
assert r["ok"] and r["stream_id"] == "100:1001" and r["token_id"] == "t1"
|
||||
doc = fs.get("discord_streams", "100:1001")
|
||||
assert r["ok"] and r["stream_id"].startswith("100:1001:") and r["token_id"] == "t1"
|
||||
doc = fs.get("discord_streams", r["stream_id"])
|
||||
assert doc["node_id"] == "n1" and doc["org_id"] == ORG and doc["system_id"] == SYS
|
||||
assert doc["channel_name"] == "Dispatch" and doc["guild_name"] == "Fire Co. 7"
|
||||
assert doc["created_by_discord_id"] == "web:u-own"
|
||||
assert fs.published and fs.published[-1][0] == "n1"
|
||||
opts = run(web.join_options("n1", OWNER))
|
||||
assert [s["stream_id"] for s in opts["streams"]] == ["100:1001"]
|
||||
assert [s["stream_id"] for s in opts["streams"]] == [r["stream_id"]]
|
||||
assert opts["guilds"][0]["voice_channels"][0]["stream"]["node_name"] == "Node n1"
|
||||
|
||||
|
||||
@@ -191,13 +192,13 @@ def test_start_lease_refusal_carries_invite(fs):
|
||||
|
||||
|
||||
def test_stop_one_and_scoped_to_node(fs):
|
||||
run(web.start_node_stream("n1", web.NodeStreamStart(guild_id="100", channel_id="1001"), OWNER))
|
||||
sid = run(web.start_node_stream("n1", web.NodeStreamStart(guild_id="100", channel_id="1001"), OWNER))["stream_id"]
|
||||
with pytest.raises(HTTPException) as e:
|
||||
run(web.stop_node_streams("n2", web.NodeStreamStop(stream_id="100:1001"), ADMIN))
|
||||
run(web.stop_node_streams("n2", web.NodeStreamStop(stream_id=sid), ADMIN))
|
||||
assert e.value.status_code == 404
|
||||
r = run(web.stop_node_streams("n1", web.NodeStreamStop(stream_id="100:1001"), OWNER))
|
||||
assert r["stopped"] == ["100:1001"]
|
||||
assert fs.get("discord_streams", "100:1001")["desired"] == "off"
|
||||
r = run(web.stop_node_streams("n1", web.NodeStreamStop(stream_id=sid), OWNER))
|
||||
assert r["stopped"] == [sid]
|
||||
assert fs.get("discord_streams", sid)["desired"] == "off"
|
||||
|
||||
|
||||
def test_legacy_raw_id_join_refuses_other_orgs_server(fs):
|
||||
@@ -210,8 +211,10 @@ def test_legacy_raw_id_join_refuses_other_orgs_server(fs):
|
||||
def test_stop_rejects_malformed_stream_id():
|
||||
# "a/b" would otherwise reach Firestore as a document path and 500.
|
||||
from pydantic import ValidationError
|
||||
for bad in ("100/1001", "../x", "100:1001/extra", ""):
|
||||
for bad in ("100/1001", "../x", "100:1001/extra", "", "100:1001:ab/cd", "100:1001:XYZ", "100:1001:"):
|
||||
with pytest.raises(ValidationError):
|
||||
web.NodeStreamStop(stream_id=bad)
|
||||
assert web.NodeStreamStop(stream_id="100:1001").stream_id == "100:1001"
|
||||
assert web.NodeStreamStop(stream_id="100:1001").stream_id == "100:1001" # legacy id
|
||||
new = leases.new_stream_id("100", "1001")
|
||||
assert web.NodeStreamStop(stream_id=new).stream_id == new # per-stream id
|
||||
assert web.NodeStreamStop().stream_id is None
|
||||
|
||||
Reference in New Issue
Block a user