From 08c3dc25c386bfb1fc774c62f5515b6d0832ee81 Mon Sep 17 00:00:00 2001 From: vansour Date: Mon, 21 Sep 2026 08:32:23 +0800 Subject: [PATCH 1/5] =?UTF-8?q?feat:=20=E5=BB=B6=E8=BF=9F=E7=9B=91?= =?UTF-8?q?=E6=8E=A7=E6=94=AF=E6=8C=81=E6=8B=96=E6=8B=BD=E6=8E=92=E5=BA=8F?= =?UTF-8?q?=EF=BC=8C=E5=85=AC=E5=BC=80=E9=A1=B5=E5=9B=BE=E8=A1=A8=E8=B7=9F?= =?UTF-8?q?=E9=9A=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 面板的「延迟」页此前按创建顺序列出监控，没有调整顺序的入口。ping_task 加 sort 列（schema v7），重排走独立的 PUT /api/ping-tasks/order，编辑监控不改动顺序。 顺序同时是 agent 运行监控的顺序，公开页延迟图表的系列、配色和图例也由采样行的 到达顺序决定，所以重排要把新列表推给已连接的 agent，ping_records 也要按面板顺序 发行，否则三处各自要等到下次重连或刷新才一致。 迁移把既有行一律置 0，读侧回落 id，升级上来的库顺序不变。 面板的拖拽逻辑原本内联在节点列表里，这次抽成两个表格共用的一份。 Closes #40 --- src/api.rs | 66 ++++++++- src/db.rs | 224 ++++++++++++++++++++++++++-- src/main.rs | 1 + web-admin/src/components/Admin.tsx | 226 +++++++++++++++++++---------- web-admin/src/lib/api.test.ts | 14 +- web-admin/src/lib/api.ts | 24 ++- 6 files changed, 466 insertions(+), 89 deletions(-) diff --git a/src/api.rs b/src/api.rs index b1380e9b..9af00ed0 100644 --- a/src/api.rs +++ b/src/api.rs @@ -849,6 +849,36 @@ pub async fn ping_tasks(_: Admin, State(app): State) -> Response { } } +#[derive(Deserialize)] +pub struct PingTaskOrder { + ids: Vec, +} + +/// The list must name every probe exactly once, checked inside the transaction +/// that renumbers rather than here, as in `reorder_nodes`. +/// +/// Pushed to every connected agent as well: the order the panel lists the probes +/// in is the order an agent runs them in, so a reorder that stopped at the +/// database would leave the two disagreeing until the next reconnect. +/// +/// Unlike `reorder_nodes` there is no snapshot to invalidate: `live_snapshot` +/// renders nodes alone, and a probe reaches a browser only through its node's +/// chart, which is queried per request. +pub async fn reorder_ping_tasks( + _: Admin, + State(app): State, + Json(order): Json, +) -> Response { + match app.db.reorder_ping_tasks(&order.ids) { + Ok(()) => { + agent_ws::push_ping_tasks(&app); + Json(json!({"ok": true})).into_response() + } + // Every failure here indicates a malformed list from the caller. + Err(e) => bad(&e.to_string()), + } +} + /// A probe target the agent can resolve: `host:port`, with an IPv6 literal /// bracketed as a URL writes one. /// @@ -1957,8 +1987,9 @@ mod tests { assert_eq!(m["net_rx"], 500); assert_eq!(m["ts"], base, "stamped with the bucket, so every series shares a grid"); - // Keyed by task rather than index: the rows share a timestamp, so - // `ORDER BY ts` leaves their order to SQLite. + // Keyed by task rather than by index: the rows of one bucket share a + // timestamp and are emitted in the panel's probe order, which this + // fixture does not arrange. let (rows, window_loss) = app.db.ping_records(id, base, 120).unwrap(); let probe = |task: i64| { rows.iter().find(|r| r["task_id"] == task).unwrap_or_else(|| panic!("no probe {task}")) @@ -2794,4 +2825,35 @@ mod tests { assert!(!body.to_string().contains(secret), "{secret}"); } } + + /// A reorder reaches two readers at once: the list the panel reads back, and + /// the list an already-connected agent is holding. Stopping at the database + /// would leave the agent running the probes in the old order until its next + /// reconnect, which on a steady link is days away. + #[tokio::test] + async fn reordering_probes_reaches_the_panel_and_the_connected_agents() { + let app = std::sync::Arc::new(app()); + let id = node(&app, "n", true); + let (a, b) = (task(&app, vec![id]), task(&app, vec![id])); + let mut frames = connect(&app, id, json!({"cpu": 1.0})); + let order = || app.db.ping_tasks().unwrap().iter().map(|t| t.id).collect::>(); + let reorder = |ids: Vec| { + reorder_ping_tasks(Admin, axum::extract::State(app.clone()), Json(PingTaskOrder { ids })) + }; + assert_eq!(order(), vec![a, b]); + + assert_eq!(reorder(vec![b, a]).await.status(), StatusCode::OK); + assert_eq!(order(), vec![b, a], "the panel's next read is the new order"); + + let frame: Value = serde_json::from_str(&frames.try_recv().expect("a push")).unwrap(); + assert_eq!(frame["method"], "ping.tasks"); + let pushed: Vec = + frame["params"].as_array().unwrap().iter().map(|t| t["id"].as_i64().unwrap()).collect(); + assert_eq!(pushed, vec![b, a], "the agent is handed the order it must run them in"); + + // A list that is not every probe is the caller's error rather than a + // failure of the hub, as in `reorder_nodes`. + assert_eq!(reorder(vec![b]).await.status(), StatusCode::BAD_REQUEST); + assert_eq!(order(), vec![b, a], "a refused list changes nothing"); + } } diff --git a/src/db.rs b/src/db.rs index 260fd7e7..13cf13a2 100644 --- a/src/db.rs +++ b/src/db.rs @@ -111,7 +111,10 @@ CREATE TABLE IF NOT EXISTS ping_task ( name TEXT NOT NULL, target TEXT NOT NULL, interval INTEGER NOT NULL DEFAULT 60, - auto_join INTEGER NOT NULL DEFAULT 0 + auto_join INTEGER NOT NULL DEFAULT 0, + -- The panel's order, as `node.sort` is. Every row an older build wrote ties at + -- 0, so a database upgrading into this column keeps its id order. + sort INTEGER NOT NULL DEFAULT 0 ); CREATE TABLE IF NOT EXISTS ping_node ( @@ -149,7 +152,7 @@ CREATE TABLE IF NOT EXISTS session ( /// cannot: `open` runs `SCHEMA` before migrating, and on an older file the /// column is not there yet. `an_upgraded_release_matches_a_fresh_database` /// holds every migration to these rules, starting from v1.0.0's schema. -const SCHEMA_VERSION: i64 = 6; +const SCHEMA_VERSION: i64 = 7; /// Adds a column older databases lack. A duplicate column indicates the /// migration has already run; every other error must propagate. @@ -266,6 +269,13 @@ fn migrate_to_6(conn: &Connection) -> Result<()> { add_column(conn, "ping_task", "auto_join INTEGER NOT NULL DEFAULT 0") } +/// Probes were listed by id until the panel gained a drag order. Every existing +/// row ties at 0, which leaves `ORDER BY sort, id` reading them in exactly the +/// order they were read in before the upgrade. +fn migrate_to_7(conn: &Connection) -> Result<()> { + add_column(conn, "ping_task", "sort INTEGER NOT NULL DEFAULT 0") +} + /// Brings a database already in service up to `SCHEMA_VERSION` and stamps it. /// `from` is its current version, so a fresh file passes `SCHEMA_VERSION` and /// receives only the stamp. @@ -297,6 +307,9 @@ fn migrate(conn: &Connection, from: i64) -> Result<()> { if from < 6 { migrate_to_6(&tx)?; } + if from < 7 { + migrate_to_7(&tx)?; + } tx.execute_batch(&format!("PRAGMA user_version = {SCHEMA_VERSION}"))?; tx.commit()?; Ok(()) @@ -479,6 +492,11 @@ pub struct PingTask { pub target: String, #[serde(default)] pub interval: i64, + /// The panel's order. Read back so the list describes its own ordering, and + /// ignored by every write: reordering goes through `reorder_ping_tasks` + /// alone, as `node.sort` goes through `reorder_nodes`. + #[serde(default)] + pub sort: i64, #[serde(default)] pub nodes: Vec, /// Nodes created later are assigned this probe as they are added. Existing @@ -1107,8 +1125,8 @@ impl Db { pub fn ping_tasks(&self) -> Result> { let conn = self.conn(); - let mut stmt = - conn.prepare("SELECT id, name, target, interval, auto_join FROM ping_task ORDER BY id")?; + let mut stmt = conn + .prepare("SELECT id, name, target, interval, auto_join, sort FROM ping_task ORDER BY sort, id")?; let tasks: Vec = stmt .query_map([], |r| { Ok(PingTask { @@ -1118,6 +1136,7 @@ impl Db { interval: r.get(3)?, nodes: Vec::new(), auto_join: r.get(4)?, + sort: r.get(5)?, base: None, }) })? @@ -1166,8 +1185,12 @@ impl Db { } t.id } else { + // A new probe belongs at the end, where the panel offers it. The + // caller sends sort 0, which would tie with whatever the last + // reorder placed first. tx.execute( - "INSERT INTO ping_task (name, target, interval, auto_join) VALUES (?1,?2,?3,?4)", + "INSERT INTO ping_task (name, target, interval, auto_join, sort) + VALUES (?1,?2,?3,?4,(SELECT COALESCE(MAX(sort),-1)+1 FROM ping_task))", params![t.name, t.target, t.interval, t.auto_join], )?; tx.last_insert_rowid() @@ -1246,6 +1269,33 @@ impl Db { Ok(()) } + /// The panel's drag order, saved whole. Every probe must be named exactly + /// once, checked inside the transaction that renumbers rather than before it: + /// re-reading the list first would only race the write it guards, the same + /// reasoning as `reorder_nodes`. + /// + /// A probe deleted from another tab is in neither the list nor the table, so + /// the count check reports that rather than renumbering around it. + pub fn reorder_ping_tasks(&self, ids: &[i64]) -> Result<()> { + let unique: HashSet<_> = ids.iter().collect(); + if unique.len() != ids.len() { + anyhow::bail!("排序里有重复的监控"); + } + let mut conn = self.conn(); + let tx = conn.transaction()?; + let count: i64 = tx.query_row("SELECT COUNT(*) FROM ping_task", [], |r| r.get(0))?; + if count as usize != ids.len() { + anyhow::bail!("排序必须包含全部监控"); + } + for (sort, id) in ids.iter().enumerate() { + if tx.execute("UPDATE ping_task SET sort=?2 WHERE id=?1", params![id, sort as i64])? != 1 { + anyhow::bail!("排序里有不存在的监控"); + } + } + tx.commit()?; + Ok(()) + } + /// The task list pushed to one agent. /// /// Ordered, because the agent keeps the first [`Self::MAX_PROBES_PER_NODE`] @@ -1254,11 +1304,15 @@ impl Db { /// the timers each time; `save_ping_task` prevents reaching that boundary, /// and this makes the backstop deterministic should a database arrive there /// by another route. + /// + /// The order is the panel's, which is what the operator rearranged: should a + /// database arrive at that boundary by another route, the truncated list + /// keeps the probes listed first rather than the ones created first. pub fn ping_tasks_for(&self, node_id: i64) -> Result> { let conn = self.conn(); let mut stmt = conn.prepare( "SELECT t.id, t.target, t.interval FROM ping_task t - JOIN ping_node n ON n.task_id = t.id WHERE n.node_id = ?1 ORDER BY t.id", + JOIN ping_node n ON n.task_id = t.id WHERE n.node_id = ?1 ORDER BY t.sort, t.id", )?; let rows = stmt.query_map([node_id], |r| { Ok(serde_json::json!({ @@ -1320,6 +1374,11 @@ impl Db { /// Probe results for one node, one sample per probe per `step` seconds: the /// bucket's median round trip, its range, and the proportion lost. /// + /// Emitted in the panel's order rather than by probe id. The chart takes its + /// series, their colours and its legend from the order these rows arrive in, + /// so this is what makes dragging a probe in the panel reorder the public + /// page's chart. + /// /// These stamps fall wherever the probe finished rather than on a minute, so /// the thinning buckets them instead of matching a multiple, as in `metrics` /// above. This is the larger half of that response, since a probe reports far @@ -1345,6 +1404,27 @@ impl Db { step: i64, ) -> Result<(Vec, serde_json::Value)> { let conn = self.conn(); + // The order the operator arranged the probes in. A statement of its own + // rather than part of [`PING_ROWS`], whose plan is asserted to be a seek + // on the window key with no sorter; this is what `close_bucket` orders by + // instead, in the fold below. + // + // Read on the connection already held, never through another `&self` + // method: `conn()` locks a plain mutex, so a nested call would deadlock. + // `stats` documents the same hazard from the other side. + let ranks: HashMap = { + let mut stmt = conn.prepare( + "SELECT id FROM ping_task + WHERE id IN (SELECT task_id FROM ping_node WHERE node_id=?1) + ORDER BY sort, id", + )?; + let ids = stmt.query_map([node_id], |r| r.get::<_, i64>(0))?; + let mut ranks = HashMap::new(); + for (position, id) in ids.enumerate() { + ranks.insert(id?, position as i64); + } + ranks + }; let mut stmt = conn.prepare_cached(PING_ROWS)?; let mut rows = stmt.query(params![node_id, since, step])?; let mut out = Vec::new(); @@ -1359,7 +1439,7 @@ impl Db { while let Some(row) = rows.next()? { let (b, task, latency) = (row.get::<_, i64>(0)?, row.get::<_, i64>(1)?, row.get::<_, i64>(2)?); if b != bucket { - close_bucket(&mut out, &mut open, bucket * step); + close_bucket(&mut out, &mut open, bucket * step, &ranks); bucket = b; } let seen = totals.entry(task).or_insert((0, 0)); @@ -1380,7 +1460,7 @@ impl Db { probe.1.push(latency); } } - close_bucket(&mut out, &mut open, bucket * step); + close_bucket(&mut out, &mut open, bucket * step, &ranks); // Unrounded: the caller decides how to render it, and rounding here would // turn 0.14% into the 0% that denotes no loss at all. let loss: serde_json::Map = totals @@ -1678,10 +1758,20 @@ impl Db { /// `"loss":0` on each would add 29 kB of nothing. Rounded up, so that the absence /// of a `loss` key means no timeouts occurred: truncating would report a bucket /// that lost 1 of 180 as clean. -fn close_bucket(out: &mut Vec, open: &mut Vec<(i64, Vec, i64)>, ts: i64) { - // Ordered by probe rather than by which answered first in this bucket, since - // the chart shades its lines by arrival order. - open.sort_unstable_by_key(|(task, ..)| *task); +fn close_bucket( + out: &mut Vec, + open: &mut Vec<(i64, Vec, i64)>, + ts: i64, + ranks: &HashMap, +) { + // Ordered by the panel's arrangement rather than by which answered first in + // this bucket, since the chart takes its series, their colours and its legend + // from the order the rows arrive in. + // + // A probe the map does not name -- an assignment removed while a result was + // still in flight -- sorts last, and by id among such probes, so the key + // stays total and the output is deterministic either way. + open.sort_unstable_by_key(|(task, ..)| (ranks.get(task).copied().unwrap_or(i64::MAX), *task)); for (task, mut answered, lost) in open.drain(..) { answered.sort_unstable(); let middle = match answered.len() { @@ -1976,6 +2066,80 @@ mod tests { .unwrap() } + fn add_probe(db: &Db, name: &str, nodes: Vec) -> i64 { + db.save_ping_task(&PingTask { + id: 0, + name: name.into(), + target: "1.1.1.1:443".into(), + interval: 60, + nodes, + ..Default::default() + }) + .unwrap() + } + + /// The mirror of `nodes_can_be_reordered_atomically`, on the table whose order + /// the chart reads as well as the panel. + #[test] + fn ping_tasks_can_be_reordered_atomically() { + let db = db(); + let (a, b, c) = + (add_probe(&db, "a", vec![]), add_probe(&db, "b", vec![]), add_probe(&db, "c", vec![])); + let order = || db.ping_tasks().unwrap().iter().map(|t| t.id).collect::>(); + assert_eq!(order(), vec![a, b, c], "a new probe starts at the end"); + + db.reorder_ping_tasks(&[c, a, b]).unwrap(); + assert_eq!(order(), vec![c, a, b]); + + // Every rejected input leaves the existing order intact. The partial list + // matters most: a stale tab would otherwise renumber around a probe it + // never saw. + assert!(db.reorder_ping_tasks(&[a, a, c]).is_err(), "duplicates"); + assert!(db.reorder_ping_tasks(&[a, b]).is_err(), "a probe left out"); + assert!(db.reorder_ping_tasks(&[a, b, 9999]).is_err(), "an id that is not a probe"); + assert_eq!(order(), vec![c, a, b]); + + // A probe added afterwards goes to the end rather than wherever sort 0 + // places it. + let d = add_probe(&db, "d", vec![]); + assert_eq!(order(), vec![c, a, b, d]); + } + + /// The public page's chart takes its series, their colours and its legend from + /// the order these rows arrive in, so the panel's arrangement has to reach + /// them. The list handed to the agent is the same order: a probe the agent + /// never runs is not one the chart can draw. + #[test] + fn a_probe_chart_follows_the_operator_order() { + let db = db(); + let id = node(&db, 1); + let (a, b, c) = + (add_probe(&db, "a", vec![id]), add_probe(&db, "b", vec![id]), add_probe(&db, "c", vec![id])); + // One bucket, so the emission order within it is the whole of what the + // chart is given. + let base = Utc::now().timestamp() / 60 * 60 - 60; + for (i, task) in [a, b, c].into_iter().enumerate() { + db.insert_ping(id, task, base + i as i64, 10 + i as i64).unwrap(); + } + let emitted = |db: &Db| { + db.ping_records(id, base, 60) + .unwrap() + .0 + .iter() + .map(|r| r["task_id"].as_i64().unwrap()) + .collect::>() + }; + let pushed = |db: &Db| { + db.ping_tasks_for(id).unwrap().iter().map(|t| t["id"].as_i64().unwrap()).collect::>() + }; + assert_eq!(emitted(&db), vec![a, b, c], "the order the probes were created in"); + assert_eq!(pushed(&db), vec![a, b, c]); + + db.reorder_ping_tasks(&[c, a, b]).unwrap(); + assert_eq!(emitted(&db), vec![c, a, b], "the chart follows the panel"); + assert_eq!(pushed(&db), vec![c, a, b], "and so does the agent's own list"); + } + /// The country is derived from the address, so it must be dropped the moment /// the address no longer matches -- and only then, or every reconnect would /// spend an outbound request repeating a settled lookup. @@ -2542,6 +2706,41 @@ mod tests { assert_eq!(version, SCHEMA_VERSION); } + /// A database predating the sort column lists its probes in the order it + /// already held them: every row the migration adds ties at 0, and the read + /// falls back to the id. One that renumbered instead would silently rearrange + /// every deployment that upgraded, with nothing to notice it by. + #[test] + fn an_upgraded_release_keeps_its_probe_order() { + let scratch = Scratch::new(); + release_file(&scratch.0); + // Two more probes, on top of the one the release file writes, so the + // order is not a single row's. + let by_id = |conn: &Connection| -> Vec { + conn.prepare("SELECT name FROM ping_task ORDER BY id") + .unwrap() + .query_map([], |r| r.get(0)) + .unwrap() + .collect::>() + .unwrap() + }; + let before = { + let conn = Connection::open(&scratch.0).unwrap(); + conn.execute_batch( + "INSERT INTO ping_task (name, target, interval) VALUES ('b', '1.1.1.2:443', 60); + INSERT INTO ping_task (name, target, interval) VALUES ('c', '1.1.1.3:443', 60); + PRAGMA user_version = 3;", + ) + .unwrap(); + by_id(&conn) + }; + assert_eq!(before, ["cm", "b", "c"], "the fixture must hold more than one probe"); + + let db = Db::open(&scratch.0).unwrap(); + let order: Vec = db.ping_tasks().unwrap().into_iter().map(|t| t.name).collect(); + assert_eq!(order, before, "the upgrade must not rearrange what was already listed"); + } + /// The trigger fails the UPDATE in `migrate_to_5` after `migrate_to_4` has /// added its columns: a failure part-way through an upgrade. #[test] @@ -2723,6 +2922,7 @@ mod tests { name: "p".into(), target: "1.1.1.1:443".into(), interval: 60, + sort: 0, nodes, auto_join: true, base: Some(base), diff --git a/src/main.rs b/src/main.rs index 363a65ef..fccb2445 100644 --- a/src/main.rs +++ b/src/main.rs @@ -426,6 +426,7 @@ async fn main() -> Result<()> { .route("/api/nodes/{id}/token", post(api::reset_token)) .route("/api/nodes/{id}/traffic", put(api::patch_traffic)) .route("/api/ping-tasks", get(api::ping_tasks).post(api::save_ping_task)) + .route("/api/ping-tasks/order", put(api::reorder_ping_tasks)) .route("/api/ping-tasks/{id}", delete(api::delete_ping_task)) .route("/api/sessions", get(api::sessions)) .route("/api/sessions/{id}", delete(api::delete_session)) diff --git a/web-admin/src/components/Admin.tsx b/web-admin/src/components/Admin.tsx index 870fc913..b3db9c17 100644 --- a/web-admin/src/components/Admin.tsx +++ b/web-admin/src/components/Admin.tsx @@ -12,7 +12,7 @@ import { Label } from "@/components/ui/label" import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select" import { Switch } from "@/components/ui/switch" import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@/components/ui/table" -import { addresses, api, badIfaceName, changes, currentIface, GIB, ifaceChoice, ifaceSpec, provisioningSite, trafficCorrection, upload, type IfaceChoice, type Node, type PingTask, type Source } from "@/lib/api" +import { addresses, api, badIfaceName, changes, currentIface, GIB, ifaceChoice, ifaceSpec, moved, provisioningSite, trafficCorrection, upload, type IfaceChoice, type Node, type PingTask, type Source } from "@/lib/api" import { bytes, CYCLES, FOREVER, money, uptime } from "@/lib/format" // Counters the panel can correct after migration or an accounting error. @@ -36,6 +36,130 @@ function animate(update: () => void) { else update() } +/** What a drag handle needs from [`useDragOrder`], so the generic hook can be passed as one type. */ +type DragController = { + begin: (id: number) => void + enter: (to: number) => void + drop: () => void + cancel: () => void + nudge: (from: number, delta: number) => void +} + +/** + * The drag order both tables share: the list as the operator arranged it, the row + * being dragged, and the drop that persists it. + * + * `items` is what the hub sent; `order` is that list with the operator's + * arrangement in front of it, so a row the hub has added since the last drag + * lands at the end and one it has deleted drops out. + */ +function useDragOrder(items: T[], endpoint: string, onSaved: () => void) { + const [manualOrder, setManualOrder] = useState([]) + const [dragging, setDragging] = useState(null) + // The order as it stood when the drag began, which a cancelled drag and a + // failed save both restore. + const settled = useRef([]) + const byId = new Map(items.map((item) => [item.id, item])) + const ordered = new Set(manualOrder) + const order = [ + ...manualOrder.map((id) => byId.get(id)).filter((item): item is T => Boolean(item)), + ...items.filter((item) => !ordered.has(item.id)), + ] + const ids = () => order.map((item) => item.id) + + // Rows are displaced while the pointer is down; the order is saved on drop. + function move(from: number, to: number) { + const next = moved(order, from, to) + if (next === order) return undefined + const nextIds = next.map((item) => item.id) + animate(() => setManualOrder(nextIds)) + return nextIds + } + + function cancel() { + setDragging(null) + const rollback = settled.current + if (rollback.length) animate(() => setManualOrder(rollback)) + } + + function save(next: number[]) { + setDragging(null) + const rollback = settled.current + if (!rollback.length || next.join() === rollback.join()) return + settled.current = next + api(endpoint, { method: "PUT", body: JSON.stringify({ ids: next }) }).then(onSaved, (e: Error) => { + setManualOrder(rollback) + toast.error(e.message) + }) + } + + return { + order, + dragging, + begin(id: number) { + settled.current = ids() + setDragging(id) + }, + // The dragged row is found by id rather than remembered as an index: the + // order changes under the pointer as rows are displaced. + enter(to: number) { + if (dragging === null) return + move(order.findIndex((item) => item.id === dragging), to) + }, + // The order sent on drop is the order of every row, which is what the hub + // renumbers. + drop() { + save(ids()) + }, + cancel, + // Keyboard reorder, saved at once: there is no drop to wait for. + nudge(from: number, delta: number) { + settled.current = ids() + const next = move(from, from + delta) + if (next) save(next) + }, + } +} + +/** + * The grip that starts a drag, and the keyboard path to the same reorder. + * `disabled` is for a filtered table: a drop sends the order of every row, and a + * filtered list offers only its own rows to drop onto. + */ +function DragHandle({ id, index, name, disabled = false, drag }: { + id: number + index: number + name: string + disabled?: boolean + drag: DragController +}) { + return ( + + ) +} + function copy(text: string) { navigator.clipboard.writeText(text).then( () => toast.success("已复制"), @@ -784,16 +908,9 @@ function Nodes({ nodes, refresh, site, canProvision }: { nodes: Node[]; refresh: const reg = useRegisterWindow() const [deleting, setDeleting] = useState(null) const [removing, setRemoving] = useState(false) - const [manualOrder, setManualOrder] = useState([]) const [query, setQuery] = useState("") - const [dragging, setDragging] = useState(null) - const orderBeforeDrag = useRef([]) - const byId = new Map(nodes.map((node) => [node.id, node])) - const orderedIds = new Set(manualOrder) - const order = [ - ...manualOrder.map((id) => byId.get(id)).filter((node): node is Node => Boolean(node)), - ...nodes.filter((node) => !orderedIds.has(node.id)), - ] + const drag = useDragOrder(nodes, "/nodes/order", refresh) + const order = drag.order // `order` itself stays whole, because the order sent on drop is the order of // every node. const visible = searchNodes(order, query) @@ -815,34 +932,6 @@ function Nodes({ nodes, refresh, site, canProvision }: { nodes: Node[]; refresh: } } - // Rows are displaced while the pointer is down; the order is saved on drop. - function move(from: number, to: number) { - if (from < 0 || to < 0 || to >= order.length || from === to) return - const next = [...order] - next.splice(to, 0, ...next.splice(from, 1)) - const ids = next.map((node) => node.id) - animate(() => setManualOrder(ids)) - return ids - } - - // Dropped outside the table or cancelled with Escape: the order is restored. - function cancel() { - setDragging(null) - const rollback = orderBeforeDrag.current - if (rollback.length) animate(() => setManualOrder(rollback)) - } - - function save(ids: number[]) { - setDragging(null) - const rollback = orderBeforeDrag.current - if (!rollback.length || ids.join() === rollback.join()) return - orderBeforeDrag.current = ids - api("/nodes/order", { method: "PUT", body: JSON.stringify({ ids }) }).then(refresh, (e: Error) => { - setManualOrder(rollback) - toast.error(e.message) - }) - } - return (
{!canProvision &&

请通过 HTTPS 域名访问面板后添加或安装节点。

} @@ -878,43 +967,18 @@ function Nodes({ nodes, refresh, site, canProvision }: { nodes: Node[]; refresh: { e.preventDefault(); e.dataTransfer.dropEffect = "move" }} - onDragEnter={() => dragging !== null && move(order.findIndex((node) => node.id === dragging), index)} - onDrop={(e) => { e.preventDefault(); save(order.map((node) => node.id)) }} + onDragEnter={() => drag.enter(index)} + onDrop={(e) => { e.preventDefault(); drag.drop() }} >
- + {/* A drop sends the order of every node, and a filtered list + offers only its own rows to drop onto, so the index below + is the full one exactly while nothing is filtered out. */} +
{n.name}
{n.country && ( { load() }, [nodes.length]) + // The table is short enough to show whole, so there is nothing to filter and + // the drag is never disabled: the order sent on drop is always every probe. + const drag = useDragOrder(tasks, "/ping-tasks/order", load) async function remove() { if (!deleting) return @@ -1189,9 +1256,22 @@ function Ping({ nodes }: { nodes: Node[] }) { - {tasks.map((t) => ( - - {t.name} + {drag.order.map((t, index) => ( + { e.preventDefault(); e.dataTransfer.dropEffect = "move" }} + onDragEnter={() => drag.enter(index)} + onDrop={(e) => { e.preventDefault(); drag.drop() }} + > + +
+ + {t.name} +
+ {t.target} {t.interval}s diff --git a/web-admin/src/lib/api.test.ts b/web-admin/src/lib/api.test.ts index 28136bca..3c0588b9 100644 --- a/web-admin/src/lib/api.test.ts +++ b/web-admin/src/lib/api.test.ts @@ -1,10 +1,22 @@ /// import assert from "node:assert/strict" -import { addresses, badIfaceName, changes, currentIface, GIB, ifaceChoice, ifaceSpec, isPublic, provisioningSite, trafficCorrection } from "./api.ts" +import { addresses, badIfaceName, changes, currentIface, GIB, ifaceChoice, ifaceSpec, isPublic, moved, provisioningSite, trafficCorrection } from "./api.ts" assert.deepEqual(changes({ public: true, price: 5 }, { price: 20 }), { price: 20 }) assert.deepEqual(changes({ total_rx: "100", month_tx: "2" }, { total_rx: "100", month_tx: "3" }), { month_tx: "3" }) assert.deepEqual(changes({ expires_at: "2030-01-01" as string | null }, { expires_at: null }), { expires_at: null }) +// A drag moves one row: both tables' reorder go through this. +assert.deepEqual(moved([1, 2, 3], 0, 2), [2, 3, 1]) +assert.deepEqual(moved([1, 2, 3], 2, 0), [3, 1, 2]) +assert.deepEqual(moved(["a"], 0, 0), ["a"]) +// An index outside the list is not a move, and the caller tells the two apart by +// identity rather than by comparing contents. +const untouched = [1, 2, 3] +assert.equal(moved(untouched, 1, 1), untouched) +assert.equal(moved(untouched, 0, 3), untouched) +assert.equal(moved(untouched, 3, 0), untouched) +assert.equal(moved(untouched, 0, -1), untouched) +assert.deepEqual(untouched, [1, 2, 3], "a refused move must not disturb the list it was given") assert.equal(provisioningSite("https://monitor.example.com:8443/"), "https://monitor.example.com:8443") for (const site of ["http://monitor.example.com", "https://127.0.0.1", "https://[::1]", "https://2130706433", "https://0x7f000001", "https://localhost", "https://user@monitor.example.com", "https://monitor.example.com/path"]) { assert.equal(provisioningSite(site), "", site) diff --git a/web-admin/src/lib/api.ts b/web-admin/src/lib/api.ts index a75b7c1e..ede73816 100644 --- a/web-admin/src/lib/api.ts +++ b/web-admin/src/lib/api.ts @@ -76,13 +76,35 @@ export type Node = { notify?: boolean } -export type PingTask = { id: number; name: string; target: string; interval: number; nodes: number[]; auto_join: boolean } +export type PingTask = { + id: number + name: string + target: string + interval: number + /** The panel's order. Written by the reorder endpoint, never by an edit. */ + sort: number + nodes: number[] + auto_join: boolean +} /** Form snapshots must never overwrite fields the user did not edit. */ export function changes(initial: T, values: Partial): Partial { return Object.fromEntries(Object.entries(values).filter(([key, value]) => value !== initial[key as keyof T])) as Partial } +/** + * The list with the item at `from` moved to `to`. Both tables' drag reorder + * through it, so the two move identically. A move onto itself, or an index + * outside the list, returns the same array rather than splicing at a position + * that would scramble the order: callers test for that by identity. + */ +export function moved(items: T[], from: number, to: number): T[] { + if (from === to || from < 0 || to < 0 || from >= items.length || to >= items.length) return items + const next = [...items] + next.splice(to, 0, ...next.splice(from, 1)) + return next +} + export const GIB = 1024 ** 3 /** From 142a5c10ce27a214d00ef8aa801cc90a9653459c Mon Sep 17 00:00:00 2001 From: vansour Date: Mon, 21 Sep 2026 09:04:29 +0800 Subject: [PATCH 2/5] =?UTF-8?q?fix:=20=E6=8B=96=E6=8B=BD=E7=9A=84=E8=A7=86?= =?UTF-8?q?=E5=9B=BE=E8=BF=87=E6=B8=A1=E6=8A=A5=E9=94=99=EF=BC=8C=E5=8E=86?= =?UTF-8?q?=E5=8F=B2=E6=9F=A5=E8=AF=A2=E6=B5=8B=E8=AF=95=E7=9A=84=E5=B9=B6?= =?UTF-8?q?=E5=8F=91=E5=B9=B2=E6=89=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 两处都是既有问题，与拖拽排序这次功能无关，是实现过程中撞见的。 视图过渡：animate() 不等 startViewTransition 返回的 promise。一次拖拽会在指针 越过的每一行上重排，松手时再重排一次，后一个过渡会跳过前一个，而被跳过的 promise 以 AbortError 拒绝——没人接，于是每次拖拽都在控制台留下一条未处理错误。 行的推移本来就是对的，只是白白报错。给 ready 和 finished 挂上 catch 即可，行为 不变。 历史查询闸门：HISTORY_GATE 是进程级 static，而 history_queries_past_the_gate_are_ refused_rather_than_queued 必须故意占满全部许可并跨 await 持有，才能观察到 503。 libtest 默认并行跑测试，于是 an_anonymous_history_window_stops_at_a_week 也被拒， 它把 503 的纯文本正文当 JSON 解析，报的是 "expected ident"，既没提闸门，也没提 真正抢走许可的那个测试。两个测试共用一把锁串行化即可；用 tokio 的 Mutex 而非标准 库的，因为标准库的 guard 跨 await 会被 clippy::await_holding_lock 拦下。 RELAY_GATE 与 PASSWORD_GATE 是同样的写法，但走它们 handler 的测试不存在，没有 这个碰撞。 验证：cargo test 默认并行连跑 8 次全绿（修复前三次里失败一到两次）；真实浏览器里 一次拖拽的 console 错误由 1 条降到 0 条，拖拽排序的 11 项端到端检查仍全部通过。 --- src/api.rs | 18 ++++++++++++++++++ web-admin/src/components/Admin.tsx | 11 +++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/api.rs b/src/api.rs index 9af00ed0..9b41fe00 100644 --- a/src/api.rs +++ b/src/api.rs @@ -1602,6 +1602,20 @@ mod tests { App::for_test(Db::open(":memory:").unwrap()) } + /// Held by every test that reaches the `metrics` handler, which is the only + /// way to the history gate. + /// + /// The gate is a process-wide static, and one of those tests deliberately + /// holds every permit to watch a request be refused. libtest runs tests in + /// parallel by default, so without this the other test is refused too and + /// fails somewhere unrelated: it parses the 503's plain-text body as JSON and + /// reports "expected ident", naming neither the gate nor the test that took + /// it. Taking this lock is what a new test reaching `metrics` has to do. + /// + /// Tokio's mutex rather than the standard one, whose guard cannot be held + /// across an await without `clippy::await_holding_lock` refusing the build. + static HISTORY_TESTS: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + #[tokio::test] async fn provisioning_requires_the_current_https_domain_entry() { let no_site = app(); @@ -2556,6 +2570,8 @@ mod tests { /// that make this process work hard. #[tokio::test] async fn history_queries_past_the_gate_are_refused_rather_than_queued() { + // This test holds every permit; see HISTORY_TESTS. + let _serial = HISTORY_TESTS.lock().await; let app = std::sync::Arc::new(app()); let id = node(&app, "n", true); let ask = || { @@ -2652,6 +2668,8 @@ mod tests { /// every row behind it holding the write connection. #[tokio::test] async fn an_anonymous_history_window_stops_at_a_week() { + // Reaches the gate through `metrics`; see HISTORY_TESTS. + let _serial = HISTORY_TESTS.lock().await; let app = std::sync::Arc::new(app()); let id = node(&app, "n", true); let now = Utc::now().timestamp(); diff --git a/web-admin/src/components/Admin.tsx b/web-admin/src/components/Admin.tsx index b3db9c17..93458380 100644 --- a/web-admin/src/components/Admin.tsx +++ b/web-admin/src/components/Admin.tsx @@ -32,8 +32,15 @@ const TRAFFIC_MODES: Record = { // Reordering uses the browser's view transitions, so displaced rows slide. // Browsers without support jump instead. function animate(update: () => void) { - if (document.startViewTransition) document.startViewTransition(() => flushSync(update)) - else update() + if (!document.startViewTransition) return update() + // A reorder landing while the previous transition is still running skips it, + // and a skipped transition's promises reject. Nothing awaits them, so one drag + // -- which rearranges on every row the pointer crosses, then again on release + // -- ended in an unhandled error. The update runs either way; only that slide + // is lost. + const transition = document.startViewTransition(() => flushSync(update)) + transition.ready.catch(() => {}) + transition.finished.catch(() => {}) } /** What a drag handle needs from [`useDragOrder`], so the generic hook can be passed as one type. */ From 65e75148be5758e7a3c623882529406b89505540 Mon Sep 17 00:00:00 2001 From: vansour Date: Mon, 21 Sep 2026 09:14:00 +0800 Subject: [PATCH 3/5] =?UTF-8?q?test:=20=E8=A1=A5=E4=B8=8A=E7=99=BB?= =?UTF-8?q?=E5=BD=95=E7=AB=AF=E7=82=B9=E7=9A=84=E7=BB=84=E5=90=88=E6=B5=8B?= =?UTF-8?q?=E8=AF=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit login 的分支各自都有测试——Throttle 的锁定生命周期、verify_password、client_ip、 会话 cookie、密码闸门——但它把这些串起来的顺序没有：先查锁定再验密码、拒绝记账、 成功清账、发出的会话能开。 计数用 Throttle::record_failure 直接堆，而不是多派几次登录：计数是 throttle 自己 的事且上面已经测过，而每次真正走到 verify_password 都要花掉 argon2 半秒（debug 构建下），十几次登录能把套件拖长一倍。 顺带把两个测试串行化：这个测试和 auth.rs 里那个持满闸门许可的 flood 测试都走 login/PASSWORD_GATE，并行跑时登录会被 429 掉、挂在与它无关的地方。flood 测试因此 从 #[test] 变成 #[tokio::test]，锁用 tokio 的而非标准库的（标准库的 guard 跨 await 会被 clippy::await_holding_lock 拦下），与 api.rs 里的 HISTORY_TESTS 同一写法。 验证：三个变异分别被抓——去掉成功后的 clear、去掉登录前的 locked 检查、去掉错误 密码的 record_failure，各自挂在对应的断言上。默认并行连跑 8 次全绿。 --- src/auth.rs | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 101 insertions(+), 2 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index e9394cdb..0acc8307 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -445,6 +445,46 @@ fn behind_local_proxy(ip: IpAddr) -> bool { mod tests { use super::*; + /// Held by every test that drives `login`, which is the only way to the argon2 + /// gate. + /// + /// That gate is a process-wide static with a single permit, and the test + /// below holds it to watch a flood be refused. libtest runs tests in parallel + /// by default, so without this a sign-in elsewhere is refused with a 429 and + /// fails for a reason it has nothing to do with. Taking this lock is what a + /// new test reaching `login` has to do. + /// + /// Tokio's mutex rather than the standard one, whose guard cannot be held + /// across an await without `clippy::await_holding_lock` refusing the build. + static PASSWORD_TESTS: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + + fn app() -> std::sync::Arc { + std::sync::Arc::new(crate::App::for_test(crate::db::Db::open(":memory:").unwrap())) + } + + /// One sign-in attempt, as the panel makes it: no proxy in front, so the peer + /// address is the caller's. + async fn sign_in(app: &std::sync::Arc, peer: &str, password: &str) -> Response { + login( + State(app.clone()), + ConnectInfo(peer.parse().unwrap()), + HeaderMap::new(), + Json(LoginBody { password: password.into() }), + ) + .await + } + + /// The token out of the `Set-Cookie` a successful sign-in answers with. + fn session_token(response: &Response) -> String { + let cookie = response + .headers() + .get(header::SET_COOKIE) + .expect("a sign-in sets a session cookie") + .to_str() + .unwrap(); + cookie.split(';').next().unwrap().split('=').nth(1).unwrap().to_owned() + } + #[test] fn a_password_round_trips_fails_closed_and_never_repeats_a_salt() { let hash = hash_password("correct horse battery staple").unwrap(); @@ -501,8 +541,10 @@ mod tests { /// The gate must refuse rather than queue: a queue admits the same flood, /// and each attempt that lands costs 19 MiB which remains in a thread's /// arena for the life of the process. - #[test] - fn the_password_gate_refuses_a_flood_rather_than_queueing_it() { + #[tokio::test] + async fn the_password_gate_refuses_a_flood_rather_than_queueing_it() { + // Takes the one permit, which no other test may want; see PASSWORD_TESTS. + let _serial = PASSWORD_TESTS.lock().await; let held: Vec<_> = (0..PASSWORD_CHECKS).map(|_| PASSWORD_GATE.try_acquire().expect("up to the limit")).collect(); assert!(PASSWORD_GATE.try_acquire().is_err(), "the attempt past the limit must be refused"); @@ -510,6 +552,63 @@ mod tests { assert!(PASSWORD_GATE.try_acquire().is_ok(), "permits come back when the checks finish"); } + /// What `login` composes out of parts each tested on their own: the lockout is + /// consulted before the password is, a refusal counts against the address, a + /// sign-in clears the count, and the session it hands out opens. + /// + /// The count is built up through `Throttle::record_failure` rather than through + /// further attempts. Counting is the throttle's own and is tested above; what + /// is under test here is how `login` drives it, and every attempt that reaches + /// `verify_password` costs argon2 a deliberate half-second in a debug build. + #[tokio::test] + async fn a_password_sign_in_counts_failures_clears_them_and_issues_a_session() { + let _serial = PASSWORD_TESTS.lock().await; + const CALLER: &str = "198.51.100.7:40000"; + let ip: IpAddr = "198.51.100.7".parse().unwrap(); + let password = "correct horse battery staple"; + let app = app(); + let fill = |n: u32| (0..n).for_each(|_| app.throttle.record_failure(ip)); + + // A hub with no password stored closes the endpoint rather than opening it: + // the hash is what first run and `--reset-password` write. + assert_eq!(sign_in(&app, CALLER, password).await.status(), StatusCode::FORBIDDEN); + app.db.set("admin_password_hash", &hash_password(password).unwrap()).unwrap(); + + // With the argon2 gate taken the attempt is refused rather than queued + // behind it, and the permit comes back when the check finishes. + let held = PASSWORD_GATE.try_acquire().expect("the only permit"); + assert_eq!(sign_in(&app, CALLER, password).await.status(), StatusCode::TOO_MANY_REQUESTS); + drop(held); + + // A refusal is counted against the address: one short of the limit, the + // attempt that follows it shuts the address out. + fill(MAX_ATTEMPTS - 1); + assert_eq!(sign_in(&app, CALLER, "wrong").await.status(), StatusCode::UNAUTHORIZED); + assert!(app.throttle.locked(ip), "the refusal was counted"); + + // The right password hands out a session that opens, and clears what the + // refusals before it had built up -- otherwise typos on either side of a + // sign-in would add up to a lockout. + app.throttle.clear(ip); + fill(MAX_ATTEMPTS - 1); + let ok = sign_in(&app, CALLER, password).await; + assert_eq!(ok.status(), StatusCode::OK); + let token = session_token(&ok); + assert!(app.db.session_valid(&sha256(&token)), "the cookie it sets opens a session"); + fill(MAX_ATTEMPTS - 1); + assert!(!app.throttle.locked(ip), "the sign-in cleared what the refusals had built up"); + + // The lockout is consulted before the password is looked at, so a shut-out + // address is refused whatever it sends. + fill(MAX_ATTEMPTS); + assert!(app.throttle.locked(ip)); + assert_eq!( + sign_in(&app, CALLER, password).await.status(), + StatusCode::TOO_MANY_REQUESTS, + "a locked-out address is refused whatever it sends" + ); + } + /// Both ends of the same redirect: every form GitHub can send must parse, /// or it never reaches the handler and can be neither logged nor explained, /// and the reason sent back must survive its query string. From b9ea4915311f1d0681416a6c35cce7e7e170128e Mon Sep 17 00:00:00 2001 From: stqfdyr <89149493+stqfdyr@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:05:15 +0800 Subject: [PATCH 4/5] =?UTF-8?q?refactor:=20=E6=94=B6=E7=AA=84=E5=88=B0?= =?UTF-8?q?=E5=BB=B6=E8=BF=9F=E6=8E=92=E5=BA=8F=E6=9C=AC=E8=BA=AB=EF=BC=8C?= =?UTF-8?q?=E4=BF=AE=E6=AD=A3=E5=9B=BE=E8=A1=A8=E8=B7=9F=E9=9A=8F=E4=B8=8E?= =?UTF-8?q?=E6=8B=96=E6=8B=BD=E5=9B=9E=E6=BB=9A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 重排不再推给 agent：agent 的列表按 id 下发，与面板顺序无关，保持不变也就不必推送 - 公开页的延迟行改为逐个探测输出：主题按探测首次出现的顺序定系列与配色，按桶输出时， 窗口开头那个不完整的桶里谁先有数据谁就排在前面，跟不上面板顺序 - 节点与延迟监控共用一份 reorder，报错改为面板能直接显示的中文 - 视图过渡只接住被跳过时 ready 的 AbortError，update 回调里的真实错误照常抛出 - 视图过渡期间 Chrome 把拖拽事件都打到根元素上，此时松手会被当成拖到表外而整次回滚， 两张表同时修正 - 两个历史查询测试共用一把锁，免得并行时互相抢闸门 - 移除与 #40 无关的登录端点测试 --- src/api.rs | 86 ++------- src/auth.rs | 103 +---------- src/db.rs | 272 ++++++++--------------------- web-admin/src/components/Admin.tsx | 207 +++++++++------------- web-admin/src/lib/api.test.ts | 14 +- web-admin/src/lib/api.ts | 24 +-- 6 files changed, 165 insertions(+), 541 deletions(-) diff --git a/src/api.rs b/src/api.rs index 5cd65162..a6ebeb20 100644 --- a/src/api.rs +++ b/src/api.rs @@ -942,14 +942,14 @@ pub async fn update_nodes( } #[derive(Deserialize)] -pub struct NodeOrder { +pub struct Order { ids: Vec, } /// The list must name every node exactly once, checked inside the transaction /// that renumbers rather than here: re-reading the node list first would only /// race the write it guards. -pub async fn reorder_nodes(_: Admin, State(app): State, Json(order): Json) -> Response { +pub async fn reorder_nodes(_: Admin, State(app): State, Json(order): Json) -> Response { match app.db.reorder_nodes(&order.ids) { Ok(()) => { invalidate_snapshot(&app); @@ -1030,32 +1030,11 @@ pub async fn ping_tasks(_: Admin, State(app): State) -> Response { } } -#[derive(Deserialize)] -pub struct PingTaskOrder { - ids: Vec, -} - -/// The list must name every probe exactly once, checked inside the transaction -/// that renumbers rather than here, as in `reorder_nodes`. -/// -/// Pushed to every connected agent as well: the order the panel lists the probes -/// in is the order an agent runs them in, so a reorder that stopped at the -/// database would leave the two disagreeing until the next reconnect. -/// -/// Unlike `reorder_nodes` there is no snapshot to invalidate: `live_snapshot` -/// renders nodes alone, and a probe reaches a browser only through its node's -/// chart, which is queried per request. -pub async fn reorder_ping_tasks( - _: Admin, - State(app): State, - Json(order): Json, -) -> Response { +/// As `reorder_nodes`. Nothing is pushed to the agents: the list they run is in +/// id order, see `ping_tasks_for`. +pub async fn reorder_ping_tasks(_: Admin, State(app): State, Json(order): Json) -> Response { match app.db.reorder_ping_tasks(&order.ids) { - Ok(()) => { - agent_ws::push_ping_tasks(&app); - Json(json!({"ok": true})).into_response() - } - // Every failure here indicates a malformed list from the caller. + Ok(()) => Json(json!({"ok": true})).into_response(), Err(e) => bad(&e.to_string()), } } @@ -1913,18 +1892,9 @@ mod tests { App::for_test(Db::open(":memory:").unwrap()) } - /// Held by every test that reaches the `metrics` handler, which is the only - /// way to the history gate. - /// - /// The gate is a process-wide static, and one of those tests deliberately - /// holds every permit to watch a request be refused. libtest runs tests in - /// parallel by default, so without this the other test is refused too and - /// fails somewhere unrelated: it parses the 503's plain-text body as JSON and - /// reports "expected ident", naming neither the gate nor the test that took - /// it. Taking this lock is what a new test reaching `metrics` has to do. - /// - /// Tokio's mutex rather than the standard one, whose guard cannot be held - /// across an await without `clippy::await_holding_lock` refusing the build. + /// Taken by every test that calls `metrics`. `HISTORY_GATE` is process-wide, + /// and a test holding all of its permits would refuse a parallel one with a + /// 503. Tokio's mutex, since the guard is held across awaits. static HISTORY_TESTS: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); /// A lookup nobody can refresh must not hide an update all day, and a panel @@ -2351,9 +2321,8 @@ mod tests { assert_eq!(m["net_rx"], 500); assert_eq!(m["ts"], base, "stamped with the bucket, so every series shares a grid"); - // Keyed by task rather than by index: the rows of one bucket share a - // timestamp and are emitted in the panel's probe order, which this - // fixture does not arrange. + // Keyed by task rather than index: the order is the panel's, which + // `a_probe_chart_follows_the_panel_order` covers. let (rows, window_loss) = app.db.ping_records(id, base, 120).unwrap(); let probe = |task: i64| { rows.iter().find(|r| r["task_id"] == task).unwrap_or_else(|| panic!("no probe {task}")) @@ -3113,7 +3082,6 @@ mod tests { /// burst of legitimate requests. #[tokio::test] async fn history_queries_past_the_gate_are_refused_rather_than_queued() { - // This test holds every permit; see HISTORY_TESTS. let _serial = HISTORY_TESTS.lock().await; let app = std::sync::Arc::new(app()); let id = node(&app, "n", true); @@ -3211,7 +3179,6 @@ mod tests { /// every row behind it holding the write connection. #[tokio::test] async fn an_anonymous_history_window_stops_at_a_week() { - // Reaches the gate through `metrics`; see HISTORY_TESTS. let _serial = HISTORY_TESTS.lock().await; let app = std::sync::Arc::new(app()); let id = node(&app, "n", true); @@ -3386,35 +3353,4 @@ mod tests { assert!(!body.to_string().contains(secret), "{secret}"); } } - - /// A reorder reaches two readers at once: the list the panel reads back, and - /// the list an already-connected agent is holding. Stopping at the database - /// would leave the agent running the probes in the old order until its next - /// reconnect, which on a steady link is days away. - #[tokio::test] - async fn reordering_probes_reaches_the_panel_and_the_connected_agents() { - let app = std::sync::Arc::new(app()); - let id = node(&app, "n", true); - let (a, b) = (task(&app, vec![id]), task(&app, vec![id])); - let mut frames = connect(&app, id, json!({"cpu": 1.0})); - let order = || app.db.ping_tasks().unwrap().iter().map(|t| t.id).collect::>(); - let reorder = |ids: Vec| { - reorder_ping_tasks(Admin, axum::extract::State(app.clone()), Json(PingTaskOrder { ids })) - }; - assert_eq!(order(), vec![a, b]); - - assert_eq!(reorder(vec![b, a]).await.status(), StatusCode::OK); - assert_eq!(order(), vec![b, a], "the panel's next read is the new order"); - - let frame: Value = serde_json::from_str(&frames.try_recv().expect("a push")).unwrap(); - assert_eq!(frame["method"], "ping.tasks"); - let pushed: Vec = - frame["params"].as_array().unwrap().iter().map(|t| t["id"].as_i64().unwrap()).collect(); - assert_eq!(pushed, vec![b, a], "the agent is handed the order it must run them in"); - - // A list that is not every probe is the caller's error rather than a - // failure of the hub, as in `reorder_nodes`. - assert_eq!(reorder(vec![b]).await.status(), StatusCode::BAD_REQUEST); - assert_eq!(order(), vec![b, a], "a refused list changes nothing"); - } } diff --git a/src/auth.rs b/src/auth.rs index 0acc8307..e9394cdb 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -445,46 +445,6 @@ fn behind_local_proxy(ip: IpAddr) -> bool { mod tests { use super::*; - /// Held by every test that drives `login`, which is the only way to the argon2 - /// gate. - /// - /// That gate is a process-wide static with a single permit, and the test - /// below holds it to watch a flood be refused. libtest runs tests in parallel - /// by default, so without this a sign-in elsewhere is refused with a 429 and - /// fails for a reason it has nothing to do with. Taking this lock is what a - /// new test reaching `login` has to do. - /// - /// Tokio's mutex rather than the standard one, whose guard cannot be held - /// across an await without `clippy::await_holding_lock` refusing the build. - static PASSWORD_TESTS: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); - - fn app() -> std::sync::Arc { - std::sync::Arc::new(crate::App::for_test(crate::db::Db::open(":memory:").unwrap())) - } - - /// One sign-in attempt, as the panel makes it: no proxy in front, so the peer - /// address is the caller's. - async fn sign_in(app: &std::sync::Arc, peer: &str, password: &str) -> Response { - login( - State(app.clone()), - ConnectInfo(peer.parse().unwrap()), - HeaderMap::new(), - Json(LoginBody { password: password.into() }), - ) - .await - } - - /// The token out of the `Set-Cookie` a successful sign-in answers with. - fn session_token(response: &Response) -> String { - let cookie = response - .headers() - .get(header::SET_COOKIE) - .expect("a sign-in sets a session cookie") - .to_str() - .unwrap(); - cookie.split(';').next().unwrap().split('=').nth(1).unwrap().to_owned() - } - #[test] fn a_password_round_trips_fails_closed_and_never_repeats_a_salt() { let hash = hash_password("correct horse battery staple").unwrap(); @@ -541,10 +501,8 @@ mod tests { /// The gate must refuse rather than queue: a queue admits the same flood, /// and each attempt that lands costs 19 MiB which remains in a thread's /// arena for the life of the process. - #[tokio::test] - async fn the_password_gate_refuses_a_flood_rather_than_queueing_it() { - // Takes the one permit, which no other test may want; see PASSWORD_TESTS. - let _serial = PASSWORD_TESTS.lock().await; + #[test] + fn the_password_gate_refuses_a_flood_rather_than_queueing_it() { let held: Vec<_> = (0..PASSWORD_CHECKS).map(|_| PASSWORD_GATE.try_acquire().expect("up to the limit")).collect(); assert!(PASSWORD_GATE.try_acquire().is_err(), "the attempt past the limit must be refused"); @@ -552,63 +510,6 @@ mod tests { assert!(PASSWORD_GATE.try_acquire().is_ok(), "permits come back when the checks finish"); } - /// What `login` composes out of parts each tested on their own: the lockout is - /// consulted before the password is, a refusal counts against the address, a - /// sign-in clears the count, and the session it hands out opens. - /// - /// The count is built up through `Throttle::record_failure` rather than through - /// further attempts. Counting is the throttle's own and is tested above; what - /// is under test here is how `login` drives it, and every attempt that reaches - /// `verify_password` costs argon2 a deliberate half-second in a debug build. - #[tokio::test] - async fn a_password_sign_in_counts_failures_clears_them_and_issues_a_session() { - let _serial = PASSWORD_TESTS.lock().await; - const CALLER: &str = "198.51.100.7:40000"; - let ip: IpAddr = "198.51.100.7".parse().unwrap(); - let password = "correct horse battery staple"; - let app = app(); - let fill = |n: u32| (0..n).for_each(|_| app.throttle.record_failure(ip)); - - // A hub with no password stored closes the endpoint rather than opening it: - // the hash is what first run and `--reset-password` write. - assert_eq!(sign_in(&app, CALLER, password).await.status(), StatusCode::FORBIDDEN); - app.db.set("admin_password_hash", &hash_password(password).unwrap()).unwrap(); - - // With the argon2 gate taken the attempt is refused rather than queued - // behind it, and the permit comes back when the check finishes. - let held = PASSWORD_GATE.try_acquire().expect("the only permit"); - assert_eq!(sign_in(&app, CALLER, password).await.status(), StatusCode::TOO_MANY_REQUESTS); - drop(held); - - // A refusal is counted against the address: one short of the limit, the - // attempt that follows it shuts the address out. - fill(MAX_ATTEMPTS - 1); - assert_eq!(sign_in(&app, CALLER, "wrong").await.status(), StatusCode::UNAUTHORIZED); - assert!(app.throttle.locked(ip), "the refusal was counted"); - - // The right password hands out a session that opens, and clears what the - // refusals before it had built up -- otherwise typos on either side of a - // sign-in would add up to a lockout. - app.throttle.clear(ip); - fill(MAX_ATTEMPTS - 1); - let ok = sign_in(&app, CALLER, password).await; - assert_eq!(ok.status(), StatusCode::OK); - let token = session_token(&ok); - assert!(app.db.session_valid(&sha256(&token)), "the cookie it sets opens a session"); - fill(MAX_ATTEMPTS - 1); - assert!(!app.throttle.locked(ip), "the sign-in cleared what the refusals had built up"); - - // The lockout is consulted before the password is looked at, so a shut-out - // address is refused whatever it sends. - fill(MAX_ATTEMPTS); - assert!(app.throttle.locked(ip)); - assert_eq!( - sign_in(&app, CALLER, password).await.status(), - StatusCode::TOO_MANY_REQUESTS, - "a locked-out address is refused whatever it sends" - ); - } - /// Both ends of the same redirect: every form GitHub can send must parse, /// or it never reaches the handler and can be neither logged nor explained, /// and the reason sent back must survive its query string. diff --git a/src/db.rs b/src/db.rs index 5ceefeec..50f379b4 100644 --- a/src/db.rs +++ b/src/db.rs @@ -124,8 +124,6 @@ CREATE TABLE IF NOT EXISTS ping_task ( target TEXT NOT NULL, interval INTEGER NOT NULL DEFAULT 60, auto_join INTEGER NOT NULL DEFAULT 0, - -- The panel's order, as `node.sort` is. Every row an older build wrote ties at - -- 0, so a database upgrading into this column keeps its id order. sort INTEGER NOT NULL DEFAULT 0 ); @@ -290,9 +288,8 @@ fn migrate_to_8(conn: &Connection) -> Result<()> { add_column(conn, "node", "country_prev TEXT NOT NULL DEFAULT ''") } -/// Probes were listed by id until the panel gained a drag order. Every existing -/// row ties at 0, which leaves `ORDER BY sort, id` reading them in exactly the -/// order they were read in before the upgrade. +/// Every existing probe ties at 0, so `ORDER BY sort, id` keeps the id order an +/// upgraded database listed them in. fn migrate_to_9(conn: &Connection) -> Result<()> { add_column(conn, "ping_task", "sort INTEGER NOT NULL DEFAULT 0") } @@ -523,11 +520,6 @@ pub struct PingTask { pub target: String, #[serde(default)] pub interval: i64, - /// The panel's order. Read back so the list describes its own ordering, and - /// ignored by every write: reordering goes through `reorder_ping_tasks` - /// alone, as `node.sort` goes through `reorder_nodes`. - #[serde(default)] - pub sort: i64, #[serde(default)] pub nodes: Vec, /// Nodes created later are assigned this probe as they are added. Existing @@ -794,19 +786,30 @@ impl Db { } pub fn reorder_nodes(&self, ids: &[i64]) -> Result<()> { + self.reorder("node", ids) + } + + pub fn reorder_ping_tasks(&self, ids: &[i64]) -> Result<()> { + self.reorder("ping_task", ids) + } + + /// Renumbers `sort` from a list that must name every row exactly once, so a + /// tab that missed an insert or a delete cannot renumber around it. + fn reorder(&self, table: &str, ids: &[i64]) -> Result<()> { let unique: HashSet<_> = ids.iter().collect(); if unique.len() != ids.len() { - anyhow::bail!("node order contains duplicates"); + anyhow::bail!("排序里有重复的条目"); } let mut conn = self.conn(); let tx = conn.transaction()?; - let count: i64 = tx.query_row("SELECT COUNT(*) FROM node", [], |r| r.get(0))?; + let count: i64 = tx.query_row(&format!("SELECT COUNT(*) FROM {table}"), [], |r| r.get(0))?; if count as usize != ids.len() { - anyhow::bail!("node order must include every node"); + anyhow::bail!("列表已在别处改动，刷新后再排序"); } + let sql = format!("UPDATE {table} SET sort=?2 WHERE id=?1"); for (sort, id) in ids.iter().enumerate() { - if tx.execute("UPDATE node SET sort=?2 WHERE id=?1", params![id, sort as i64])? != 1 { - anyhow::bail!("node order contains an unknown node"); + if tx.execute(&sql, params![id, sort as i64])? != 1 { + anyhow::bail!("列表已在别处改动，刷新后再排序"); } } tx.commit()?; @@ -1241,8 +1244,8 @@ impl Db { pub fn ping_tasks(&self) -> Result> { let conn = self.conn(); - let mut stmt = conn - .prepare("SELECT id, name, target, interval, auto_join, sort FROM ping_task ORDER BY sort, id")?; + let mut stmt = + conn.prepare("SELECT id, name, target, interval, auto_join FROM ping_task ORDER BY sort, id")?; let tasks: Vec = stmt .query_map([], |r| { Ok(PingTask { @@ -1252,7 +1255,6 @@ impl Db { interval: r.get(3)?, nodes: Vec::new(), auto_join: r.get(4)?, - sort: r.get(5)?, base: None, }) })? @@ -1306,9 +1308,7 @@ impl Db { } t.id } else { - // A new probe belongs at the end, where the panel offers it. The - // caller sends sort 0, which would tie with whatever the last - // reorder placed first. + // At the end, as in `create_node`. tx.execute( "INSERT INTO ping_task (name, target, interval, auto_join, sort) VALUES (?1,?2,?3,?4,(SELECT COALESCE(MAX(sort),-1)+1 FROM ping_task))", @@ -1390,33 +1390,6 @@ impl Db { Ok(()) } - /// The panel's drag order, saved whole. Every probe must be named exactly - /// once, checked inside the transaction that renumbers rather than before it: - /// re-reading the list first would only race the write it guards, the same - /// reasoning as `reorder_nodes`. - /// - /// A probe deleted from another tab is in neither the list nor the table, so - /// the count check reports that rather than renumbering around it. - pub fn reorder_ping_tasks(&self, ids: &[i64]) -> Result<()> { - let unique: HashSet<_> = ids.iter().collect(); - if unique.len() != ids.len() { - anyhow::bail!("排序里有重复的监控"); - } - let mut conn = self.conn(); - let tx = conn.transaction()?; - let count: i64 = tx.query_row("SELECT COUNT(*) FROM ping_task", [], |r| r.get(0))?; - if count as usize != ids.len() { - anyhow::bail!("排序必须包含全部监控"); - } - for (sort, id) in ids.iter().enumerate() { - if tx.execute("UPDATE ping_task SET sort=?2 WHERE id=?1", params![id, sort as i64])? != 1 { - anyhow::bail!("排序里有不存在的监控"); - } - } - tx.commit()?; - Ok(()) - } - /// The task list pushed to one agent. /// /// Ordered, because the agent keeps the first [`Self::MAX_PROBES_PER_NODE`] @@ -1426,14 +1399,13 @@ impl Db { /// and this makes the backstop deterministic should a database arrive there /// by another route. /// - /// The order is the panel's, which is what the operator rearranged: should a - /// database arrive at that boundary by another route, the truncated list - /// keeps the probes listed first rather than the ones created first. + /// By id rather than the panel's `sort`, so reordering in the panel changes + /// nothing an agent holds and needs no push. pub fn ping_tasks_for(&self, node_id: i64) -> Result> { let conn = self.conn(); let mut stmt = conn.prepare( "SELECT t.id, t.target, t.interval FROM ping_task t - JOIN ping_node n ON n.task_id = t.id WHERE n.node_id = ?1 ORDER BY t.sort, t.id", + JOIN ping_node n ON n.task_id = t.id WHERE n.node_id = ?1 ORDER BY t.id", )?; let rows = stmt.query_map([node_id], |r| { Ok(serde_json::json!({ @@ -1504,11 +1476,6 @@ impl Db { /// Probe results for one node, one sample per probe per `step` seconds: the /// bucket's median round trip, its range, and the proportion lost. /// - /// Emitted in the panel's order rather than by probe id. The chart takes its - /// series, their colours and its legend from the order these rows arrive in, - /// so this is what makes dragging a probe in the panel reorder the public - /// page's chart. - /// /// These stamps fall wherever the probe finished rather than on a minute, so /// the thinning buckets them instead of matching a multiple, as in `metrics` /// above. This is the larger half of that response, since a probe reports far @@ -1534,27 +1501,6 @@ impl Db { step: i64, ) -> Result<(Vec, serde_json::Value)> { let conn = self.conn(); - // The order the operator arranged the probes in. A statement of its own - // rather than part of [`PING_ROWS`], whose plan is asserted to be a seek - // on the window key with no sorter; this is what `close_bucket` orders by - // instead, in the fold below. - // - // Read on the connection already held, never through another `&self` - // method: `conn()` locks a plain mutex, so a nested call would deadlock. - // `stats` documents the same hazard from the other side. - let ranks: HashMap = { - let mut stmt = conn.prepare( - "SELECT id FROM ping_task - WHERE id IN (SELECT task_id FROM ping_node WHERE node_id=?1) - ORDER BY sort, id", - )?; - let ids = stmt.query_map([node_id], |r| r.get::<_, i64>(0))?; - let mut ranks = HashMap::new(); - for (position, id) in ids.enumerate() { - ranks.insert(id?, position as i64); - } - ranks - }; let mut stmt = conn.prepare_cached(PING_ROWS)?; let mut rows = stmt.query(params![node_id, since, step])?; let mut out = Vec::new(); @@ -1569,7 +1515,7 @@ impl Db { while let Some(row) = rows.next()? { let (b, task, latency) = (row.get::<_, i64>(0)?, row.get::<_, i64>(1)?, row.get::<_, i64>(2)?); if b != bucket { - close_bucket(&mut out, &mut open, bucket * step, &ranks); + close_bucket(&mut out, &mut open, bucket * step); bucket = b; } let seen = totals.entry(task).or_insert((0, 0)); @@ -1590,7 +1536,18 @@ impl Db { probe.1.push(latency); } } - close_bucket(&mut out, &mut open, bucket * step, &ranks); + close_bucket(&mut out, &mut open, bucket * step); + // Probe by probe in the panel's order, each probe's rows still in time + // order. Themes take their series, colours and legend from the order in + // which probes first appear; bucket by bucket, that would be whichever + // probe happened to answer inside the window's partial first bucket. + let rank: HashMap = conn + .prepare("SELECT id FROM ping_task ORDER BY sort, id")? + .query_map([], |r| r.get(0))? + .enumerate() + .map(|(i, id)| id.map(|id| (id, i))) + .collect::>()?; + out.sort_by_cached_key(|row| row["task_id"].as_i64().and_then(|id| rank.get(&id).copied())); // Unrounded: the caller decides how to render it, and rounding here would // turn 0.14% into the 0% that denotes no loss at all. let loss: serde_json::Map = totals @@ -1888,20 +1845,7 @@ impl Db { /// `"loss":0` on each would add 29 kB of nothing. Rounded up, so that the absence /// of a `loss` key means no timeouts occurred: truncating would report a bucket /// that lost 1 of 180 as clean. -fn close_bucket( - out: &mut Vec, - open: &mut Vec<(i64, Vec, i64)>, - ts: i64, - ranks: &HashMap, -) { - // Ordered by the panel's arrangement rather than by which answered first in - // this bucket, since the chart takes its series, their colours and its legend - // from the order the rows arrive in. - // - // A probe the map does not name -- an assignment removed while a result was - // still in flight -- sorts last, and by id among such probes, so the key - // stays total and the output is deterministic either way. - open.sort_unstable_by_key(|(task, ..)| (ranks.get(task).copied().unwrap_or(i64::MAX), *task)); +fn close_bucket(out: &mut Vec, open: &mut Vec<(i64, Vec, i64)>, ts: i64) { for (task, mut answered, lost) in open.drain(..) { answered.sort_unstable(); let middle = match answered.len() { @@ -2197,80 +2141,6 @@ mod tests { .unwrap() } - fn add_probe(db: &Db, name: &str, nodes: Vec) -> i64 { - db.save_ping_task(&PingTask { - id: 0, - name: name.into(), - target: "1.1.1.1:443".into(), - interval: 60, - nodes, - ..Default::default() - }) - .unwrap() - } - - /// The mirror of `nodes_can_be_reordered_atomically`, on the table whose order - /// the chart reads as well as the panel. - #[test] - fn ping_tasks_can_be_reordered_atomically() { - let db = db(); - let (a, b, c) = - (add_probe(&db, "a", vec![]), add_probe(&db, "b", vec![]), add_probe(&db, "c", vec![])); - let order = || db.ping_tasks().unwrap().iter().map(|t| t.id).collect::>(); - assert_eq!(order(), vec![a, b, c], "a new probe starts at the end"); - - db.reorder_ping_tasks(&[c, a, b]).unwrap(); - assert_eq!(order(), vec![c, a, b]); - - // Every rejected input leaves the existing order intact. The partial list - // matters most: a stale tab would otherwise renumber around a probe it - // never saw. - assert!(db.reorder_ping_tasks(&[a, a, c]).is_err(), "duplicates"); - assert!(db.reorder_ping_tasks(&[a, b]).is_err(), "a probe left out"); - assert!(db.reorder_ping_tasks(&[a, b, 9999]).is_err(), "an id that is not a probe"); - assert_eq!(order(), vec![c, a, b]); - - // A probe added afterwards goes to the end rather than wherever sort 0 - // places it. - let d = add_probe(&db, "d", vec![]); - assert_eq!(order(), vec![c, a, b, d]); - } - - /// The public page's chart takes its series, their colours and its legend from - /// the order these rows arrive in, so the panel's arrangement has to reach - /// them. The list handed to the agent is the same order: a probe the agent - /// never runs is not one the chart can draw. - #[test] - fn a_probe_chart_follows_the_operator_order() { - let db = db(); - let id = node(&db, 1); - let (a, b, c) = - (add_probe(&db, "a", vec![id]), add_probe(&db, "b", vec![id]), add_probe(&db, "c", vec![id])); - // One bucket, so the emission order within it is the whole of what the - // chart is given. - let base = Utc::now().timestamp() / 60 * 60 - 60; - for (i, task) in [a, b, c].into_iter().enumerate() { - db.insert_pings(id, &[(task, base + i as i64, 10 + i as i64)]).unwrap(); - } - let emitted = |db: &Db| { - db.ping_records(id, base, 60) - .unwrap() - .0 - .iter() - .map(|r| r["task_id"].as_i64().unwrap()) - .collect::>() - }; - let pushed = |db: &Db| { - db.ping_tasks_for(id).unwrap().iter().map(|t| t["id"].as_i64().unwrap()).collect::>() - }; - assert_eq!(emitted(&db), vec![a, b, c], "the order the probes were created in"); - assert_eq!(pushed(&db), vec![a, b, c]); - - db.reorder_ping_tasks(&[c, a, b]).unwrap(); - assert_eq!(emitted(&db), vec![c, a, b], "the chart follows the panel"); - assert_eq!(pushed(&db), vec![c, a, b], "and so does the agent's own list"); - } - /// The country is derived from the address, so it must be dropped the moment /// the address no longer matches -- and only then, or every reconnect would /// spend an outbound request repeating a settled lookup. @@ -2764,6 +2634,36 @@ mod tests { assert_eq!(db.nodes().unwrap().iter().map(|n| n.id).collect::>(), vec![c, a, b, d]); } + /// Themes draw probes in the order they first appear in the rows, so the rows + /// follow the panel's order even when a later probe alone answered in the + /// window's first bucket. + #[test] + fn a_probe_chart_follows_the_panel_order() { + let db = db(); + let id = node(&db, 1); + let probe = |name: &str| { + db.save_ping_task(&PingTask { + name: name.into(), + target: "1.1.1.1:443".into(), + interval: 60, + nodes: vec![id], + ..Default::default() + }) + .unwrap() + }; + let (a, b) = (probe("a"), probe("b")); + db.reorder_ping_tasks(&[b, a]).unwrap(); + let c = probe("c"); + let listed: Vec<_> = db.ping_tasks().unwrap().iter().map(|t| t.id).collect(); + assert_eq!(listed, vec![b, a, c], "a new probe starts at the end"); + + db.insert_pings(id, &[(a, 0, 10), (a, 60, 10), (b, 60, 20), (c, 60, 30)]).unwrap(); + let rows = db.ping_records(id, 0, 60).unwrap().0; + let drawn: Vec<_> = + rows.iter().map(|r| (r["task_id"].as_i64().unwrap(), r["ts"].as_i64().unwrap())).collect(); + assert_eq!(drawn, vec![(b, 60), (a, 0), (a, 60), (c, 60)]); + } + #[test] fn prune_drops_history_but_never_traffic_totals() { let db = db(); @@ -2902,41 +2802,6 @@ mod tests { assert_eq!(version, SCHEMA_VERSION); } - /// A database predating the sort column lists its probes in the order it - /// already held them: every row the migration adds ties at 0, and the read - /// falls back to the id. One that renumbered instead would silently rearrange - /// every deployment that upgraded, with nothing to notice it by. - #[test] - fn an_upgraded_release_keeps_its_probe_order() { - let scratch = Scratch::new(); - release_file(&scratch.0); - // Two more probes, on top of the one the release file writes, so the - // order is not a single row's. - let by_id = |conn: &Connection| -> Vec { - conn.prepare("SELECT name FROM ping_task ORDER BY id") - .unwrap() - .query_map([], |r| r.get(0)) - .unwrap() - .collect::>() - .unwrap() - }; - let before = { - let conn = Connection::open(&scratch.0).unwrap(); - conn.execute_batch( - "INSERT INTO ping_task (name, target, interval) VALUES ('b', '1.1.1.2:443', 60); - INSERT INTO ping_task (name, target, interval) VALUES ('c', '1.1.1.3:443', 60); - PRAGMA user_version = 3;", - ) - .unwrap(); - by_id(&conn) - }; - assert_eq!(before, ["cm", "b", "c"], "the fixture must hold more than one probe"); - - let db = Db::open(&scratch.0).unwrap(); - let order: Vec = db.ping_tasks().unwrap().into_iter().map(|t| t.name).collect(); - assert_eq!(order, before, "the upgrade must not rearrange what was already listed"); - } - /// The trigger fails the UPDATE in `migrate_to_5` after `migrate_to_4` has /// added its columns: a failure part-way through an upgrade. #[test] @@ -3118,7 +2983,6 @@ mod tests { name: "p".into(), target: "1.1.1.1:443".into(), interval: 60, - sort: 0, nodes, auto_join: true, base: Some(base), diff --git a/web-admin/src/components/Admin.tsx b/web-admin/src/components/Admin.tsx index d1bc30db..dbfe7dee 100644 --- a/web-admin/src/components/Admin.tsx +++ b/web-admin/src/components/Admin.tsx @@ -13,7 +13,7 @@ import { Popover, PopoverAnchor, PopoverContent } from "@/components/ui/popover" import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select" import { Switch } from "@/components/ui/switch" import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@/components/ui/table" -import { api, badIfaceName, behind, changes, configFields, configForm, configOverrides, configSections, configValues, currentIface, fits, GIB, groupsOf, ifaceChoice, ifaceSpec, inGroup, moved, outdatedAgents, provisioningSite, trafficCorrection, upload, type ConfigField, type IfaceChoice, type Node, type PingTask, type Source } from "@/lib/api" +import { api, badIfaceName, behind, changes, configFields, configForm, configOverrides, configSections, configValues, currentIface, fits, GIB, groupsOf, ifaceChoice, ifaceSpec, inGroup, outdatedAgents, provisioningSite, trafficCorrection, upload, type ConfigField, type IfaceChoice, type Node, type PingTask, type Source } from "@/lib/api" import { bytes, CYCLES, FOREVER, money, uptime } from "@/lib/format" // Counters the panel can correct after migration or an accounting error. @@ -31,71 +31,66 @@ const TRAFFIC_MODES: Record = { } // Reordering uses the browser's view transitions, so displaced rows slide. -// Browsers without support jump instead. +// Browsers without support jump instead. A drag starts a transition on every row +// it crosses, each skipping the last, and a skipped transition rejects `ready`. function animate(update: () => void) { - if (!document.startViewTransition) return update() - // A reorder landing while the previous transition is still running skips it, - // and a skipped transition's promises reject. Nothing awaits them, so one drag - // -- which rearranges on every row the pointer crosses, then again on release - // -- ended in an unhandled error. The update runs either way; only that slide - // is lost. - const transition = document.startViewTransition(() => flushSync(update)) - transition.ready.catch(() => {}) - transition.finished.catch(() => {}) + if (document.startViewTransition) document.startViewTransition(() => flushSync(update)).ready.catch(() => {}) + else update() } -/** What a drag handle needs from [`useDragOrder`], so the generic hook can be passed as one type. */ -type DragController = { - begin: (id: number) => void - enter: (to: number) => void - drop: () => void - cancel: () => void - nudge: (from: number, delta: number) => void -} - -/** - * The drag order both tables share: the list as the operator arranged it, the row - * being dragged, and the drop that persists it. - * - * `items` is what the hub sent; `order` is that list with the operator's - * arrangement in front of it, so a row the hub has added since the last drag - * lands at the end and one it has deleted drops out. - */ -function useDragOrder(items: T[], endpoint: string, onSaved: () => void) { +// Drag-to-reorder for a table whose order the hub stores at `/${path}/order`. +// Rows are displaced while the pointer is down and the whole order is saved on +// drop, so a filtered table must disable its handles: the index a row reports is +// then not its place in `order`. +function useDragOrder(items: T[], path: string, onSaved: () => void) { const [manualOrder, setManualOrder] = useState([]) const [dragging, setDragging] = useState(null) - // The order as it stood when the drag began, which a cancelled drag and a - // failed save both restore. - const settled = useRef([]) + const orderBeforeDrag = useRef([]) const byId = new Map(items.map((item) => [item.id, item])) - const ordered = new Set(manualOrder) + const orderedIds = new Set(manualOrder) const order = [ ...manualOrder.map((id) => byId.get(id)).filter((item): item is T => Boolean(item)), - ...items.filter((item) => !ordered.has(item.id)), + ...items.filter((item) => !orderedIds.has(item.id)), ] const ids = () => order.map((item) => item.id) - // Rows are displaced while the pointer is down; the order is saved on drop. + // While a view transition runs, Chrome hit-tests drag events to the root + // element rather than the row under the pointer, so releasing during a row's + // 150 ms slide would count as a drop outside the table and restore the order. + // The body fills the viewport, so the root is otherwise never the target. + useEffect(() => { + if (dragging === null) return + const accept = (e: DragEvent) => { if (e.target === document.documentElement) e.preventDefault() } + document.addEventListener("dragover", accept) + document.addEventListener("drop", accept) + return () => { + document.removeEventListener("dragover", accept) + document.removeEventListener("drop", accept) + } + }, [dragging]) + function move(from: number, to: number) { - const next = moved(order, from, to) - if (next === order) return undefined - const nextIds = next.map((item) => item.id) - animate(() => setManualOrder(nextIds)) - return nextIds + if (from < 0 || to < 0 || to >= order.length || from === to) return + const next = [...order] + next.splice(to, 0, ...next.splice(from, 1)) + const moved = next.map((item) => item.id) + animate(() => setManualOrder(moved)) + return moved } + // Dropped outside the table or cancelled with Escape: the order is restored. function cancel() { setDragging(null) - const rollback = settled.current + const rollback = orderBeforeDrag.current if (rollback.length) animate(() => setManualOrder(rollback)) } function save(next: number[]) { setDragging(null) - const rollback = settled.current + const rollback = orderBeforeDrag.current if (!rollback.length || next.join() === rollback.join()) return - settled.current = next - api(endpoint, { method: "PUT", body: JSON.stringify({ ids: next }) }).then(onSaved, (e: Error) => { + orderBeforeDrag.current = next + api(`/${path}/order`, { method: "PUT", body: JSON.stringify({ ids: next }) }).then(onSaved, (e: Error) => { setManualOrder(rollback) toast.error(e.message) }) @@ -103,65 +98,45 @@ function useDragOrder(items: T[], endpoint: string, on return { order, - dragging, - begin(id: number) { - settled.current = ids() - setDragging(id) - }, - // The dragged row is found by id rather than remembered as an index: the - // order changes under the pointer as rows are displaced. - enter(to: number) { - if (dragging === null) return - move(order.findIndex((item) => item.id === dragging), to) - }, - // The order sent on drop is the order of every row, which is what the hub - // renumbers. - drop() { - save(ids()) - }, - cancel, - // Keyboard reorder, saved at once: there is no drop to wait for. - nudge(from: number, delta: number) { - settled.current = ids() - const next = move(from, from + delta) - if (next) save(next) - }, + row: (id: number, index: number) => ({ + style: { viewTransitionName: `${path}-${id}` }, + "data-dragging": dragging === id || undefined, + className: "transition-opacity data-[dragging]:opacity-40", + onDragOver: (e: React.DragEvent) => { e.preventDefault(); e.dataTransfer.dropEffect = "move" }, + onDragEnter: () => { if (dragging !== null) move(order.findIndex((item) => item.id === dragging), index) }, + onDrop: (e: React.DragEvent) => { e.preventDefault(); save(ids()) }, + }), + handle: (id: number, index: number) => ({ + onDragStart: (e: React.DragEvent) => { + orderBeforeDrag.current = ids() + setDragging(id) + e.dataTransfer.effectAllowed = "move" + // Firefox refuses to start a drag without a payload. + e.dataTransfer.setData("text/plain", String(id)) + }, + onDragEnd: (e: React.DragEvent) => (e.dataTransfer.dropEffect === "none" ? cancel() : save(ids())), + onKeyDown: (e: React.KeyboardEvent) => { + const delta = e.key === "ArrowUp" ? -1 : e.key === "ArrowDown" ? 1 : 0 + if (!delta) return + e.preventDefault() + orderBeforeDrag.current = ids() + const next = move(index, index + delta) + if (next) save(next) + }, + }), } } -/** - * The grip that starts a drag, and the keyboard path to the same reorder. - * `disabled` is for a filtered table: a drop sends the order of every row, and a - * filtered list offers only its own rows to drop onto. - */ -function DragHandle({ id, index, name, disabled = false, drag }: { - id: number - index: number - name: string - disabled?: boolean - drag: DragController -}) { +function DragHandle({ name, disabled, title = "拖动排序", ...events }: React.ComponentProps<"button"> & { name: string }) { return ( @@ -1223,11 +1198,8 @@ function Nodes({ nodes, refresh, site, refusal }: { nodes: Node[]; refresh: () = const [query, setQuery] = useState("") const [group, setGroup] = useGroupFilter(nodes) const [grouping, setGrouping] = useState(false) - const drag = useDragOrder(nodes, "/nodes/order", refresh) - const order = drag.order - // `order` itself stays whole, because the order sent on drop is the order of - // every node. - const visible = inGroup(searchNodes(order, query), group) + const drag = useDragOrder(nodes, "nodes", refresh) + const visible = inGroup(searchNodes(drag.order, query), group) const searching = query.trim() !== "" || group !== "all" const uninstall = refusal ? "" : uninstallCommand(site) @@ -1284,21 +1256,15 @@ function Nodes({ nodes, refresh, site, refusal }: { nodes: Node[]; refresh: () = {visible.map((n, index) => ( - { e.preventDefault(); e.dataTransfer.dropEffect = "move" }} - onDragEnter={() => drag.enter(index)} - onDrop={(e) => { e.preventDefault(); drag.drop() }} - > +
- {/* A drop sends the order of every node, and a filtered list - offers only its own rows to drop onto, so the index below - is the full one exactly while nothing is filtered out. */} - +
{n.name}
{n.group &&
{n.group}
} @@ -1391,7 +1357,7 @@ function Nodes({ nodes, refresh, site, refusal }: { nodes: Node[]; refresh: () = onSaved={refresh} /> )} - {grouping && setGrouping(false)} onSaved={refresh} />} + {grouping && setGrouping(false)} onSaved={refresh} />} {billing && ( setBilling(null)} onSaved={refresh} /> )} @@ -1542,9 +1508,8 @@ function Ping({ nodes }: { nodes: Node[] }) { // A node added or removed changes assignments on the hub: auto_join adds, a // deletion cascades. useEffect(() => { load() }, [nodes.length]) - // The table is short enough to show whole, so there is nothing to filter and - // the drag is never disabled: the order sent on drop is always every probe. - const drag = useDragOrder(tasks, "/ping-tasks/order", load) + // Unfiltered, so the handles are never disabled. + const drag = useDragOrder(tasks, "ping-tasks", load) async function remove() { if (!deleting) return @@ -1582,18 +1547,10 @@ function Ping({ nodes }: { nodes: Node[] }) { {drag.order.map((t, index) => ( - { e.preventDefault(); e.dataTransfer.dropEffect = "move" }} - onDragEnter={() => drag.enter(index)} - onDrop={(e) => { e.preventDefault(); drag.drop() }} - > +
- + {t.name}
diff --git a/web-admin/src/lib/api.test.ts b/web-admin/src/lib/api.test.ts index 2ba00a56..7befd6b7 100644 --- a/web-admin/src/lib/api.test.ts +++ b/web-admin/src/lib/api.test.ts @@ -1,22 +1,10 @@ /// import assert from "node:assert/strict" -import { badIfaceName, behind, changes, configFields, configForm, configOverrides, configSections, configValues, currentIface, fits, GIB, groupsOf, ifaceChoice, ifaceSpec, inGroup, loopbackOrigin, moved, outdatedAgents, provisioningSite, provisionRefusal, trafficCorrection } from "./api.ts" +import { badIfaceName, behind, changes, configFields, configForm, configOverrides, configSections, configValues, currentIface, fits, GIB, groupsOf, ifaceChoice, ifaceSpec, inGroup, loopbackOrigin, outdatedAgents, provisioningSite, provisionRefusal, trafficCorrection } from "./api.ts" assert.deepEqual(changes({ public: true, price: 5 }, { price: 20 }), { price: 20 }) assert.deepEqual(changes({ total_rx: "100", month_tx: "2" }, { total_rx: "100", month_tx: "3" }), { month_tx: "3" }) assert.deepEqual(changes({ expires_at: "2030-01-01" as string | null }, { expires_at: null }), { expires_at: null }) -// A drag moves one row: both tables' reorder go through this. -assert.deepEqual(moved([1, 2, 3], 0, 2), [2, 3, 1]) -assert.deepEqual(moved([1, 2, 3], 2, 0), [3, 1, 2]) -assert.deepEqual(moved(["a"], 0, 0), ["a"]) -// An index outside the list is not a move, and the caller tells the two apart by -// identity rather than by comparing contents. -const untouched = [1, 2, 3] -assert.equal(moved(untouched, 1, 1), untouched) -assert.equal(moved(untouched, 0, 3), untouched) -assert.equal(moved(untouched, 3, 0), untouched) -assert.equal(moved(untouched, 0, -1), untouched) -assert.deepEqual(untouched, [1, 2, 3], "a refused move must not disturb the list it was given") assert.equal(provisioningSite("https://monitor.example.com:8443/"), "https://monitor.example.com:8443") for (const site of ["http://monitor.example.com", "https://127.0.0.1", "https://[::1]", "https://2130706433", "https://0x7f000001", "https://localhost", "https://user@monitor.example.com", "https://monitor.example.com/path"]) { assert.equal(provisioningSite(site), "", site) diff --git a/web-admin/src/lib/api.ts b/web-admin/src/lib/api.ts index 197efedf..93e52682 100644 --- a/web-admin/src/lib/api.ts +++ b/web-admin/src/lib/api.ts @@ -85,16 +85,7 @@ export type Node = { notify?: boolean } -export type PingTask = { - id: number - name: string - target: string - interval: number - /** The panel's order. Written by the reorder endpoint, never by an edit. */ - sort: number - nodes: number[] - auto_join: boolean -} +export type PingTask = { id: number; name: string; target: string; interval: number; nodes: number[]; auto_join: boolean } /** Every group in use, in the order of the first node carrying it: the node order decides the group order. */ export function groupsOf(nodes: Pick[]): string[] { @@ -113,19 +104,6 @@ export function changes(initial: T, values: Partial): Parti return Object.fromEntries(Object.entries(values).filter(([key, value]) => value !== initial[key as keyof T])) as Partial } -/** - * The list with the item at `from` moved to `to`. Both tables' drag reorder - * through it, so the two move identically. A move onto itself, or an index - * outside the list, returns the same array rather than splicing at a position - * that would scramble the order: callers test for that by identity. - */ -export function moved(items: T[], from: number, to: number): T[] { - if (from === to || from < 0 || to < 0 || from >= items.length || to >= items.length) return items - const next = [...items] - next.splice(to, 0, ...next.splice(from, 1)) - return next -} - /** One field of the settings form a theme declares under `config` in its `theme.json`. */ export type ConfigField = { key: string From 7de6e3a586c4978e0fa9cace73aae882ef52acb4 Mon Sep 17 00:00:00 2001 From: stqfdyr <89149493+stqfdyr@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:40:14 +0800 Subject: [PATCH 5/5] =?UTF-8?q?fix:=20=E6=8B=96=E6=8B=BD=E6=8C=89=E6=8C=87?= =?UTF-8?q?=E9=92=88=E4=BD=8D=E7=BD=AE=E6=8D=A2=E4=BD=8D=EF=BC=8C=E8=BF=9E?= =?UTF-8?q?=E7=BB=AD=E6=93=8D=E4=BD=9C=E4=B8=8E=E5=85=B6=E4=BB=96=E6=A0=87?= =?UTF-8?q?=E7=AD=BE=E9=A1=B5=E7=9A=84=E6=94=B9=E5=8A=A8=E4=B8=8D=E5=86=8D?= =?UTF-8?q?=E4=B8=A2=E5=A4=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 换位与接受落点都改在文档上按指针位置判断：视图过渡期间拖拽事件都命中根元素， 行上的处理会漏掉这段时间跨过的行；表格外松手仍然还原 - 移动基于最后一次请求的顺序，按住方向键或快速拖动不再丢步；过时的过渡不再覆盖新的顺序 - 保存逐个发出，两次连续排序按先后到达 hub；被拒绝时回到 hub 的列表 - hub 列出与本地相同的顺序后放弃本地覆盖，其他标签页的排序能显示出来，不会被下一次拖动覆盖 - 不再接受外部拖入的内容 - 延迟行的排序在释放数据库连接之后进行，未在排序里的探测排在最后 --- src/db.rs | 13 ++- web-admin/src/components/Admin.tsx | 123 +++++++++++++++++++---------- 2 files changed, 93 insertions(+), 43 deletions(-) diff --git a/src/db.rs b/src/db.rs index 50f379b4..cfe780d2 100644 --- a/src/db.rs +++ b/src/db.rs @@ -1542,12 +1542,21 @@ impl Db { // which probes first appear; bucket by bucket, that would be whichever // probe happened to answer inside the window's partial first bucket. let rank: HashMap = conn - .prepare("SELECT id FROM ping_task ORDER BY sort, id")? + .prepare_cached("SELECT id FROM ping_task ORDER BY sort, id")? .query_map([], |r| r.get(0))? .enumerate() .map(|(i, id)| id.map(|id| (id, i))) .collect::>()?; - out.sort_by_cached_key(|row| row["task_id"].as_i64().and_then(|id| rank.get(&id).copied())); + // Sorted after releasing the connection the agents write through. A + // probe missing from the rank, which the assignment filter in + // `PING_ROWS` rules out today, goes last rather than taking the first + // colour. + drop(rows); + drop(stmt); + drop(conn); + out.sort_by_cached_key(|row| { + row["task_id"].as_i64().and_then(|id| rank.get(&id).copied()).unwrap_or(usize::MAX) + }); // Unrounded: the caller decides how to render it, and rounding here would // turn 0.14% into the 0% that denotes no loss at all. let loss: serde_json::Map = totals diff --git a/web-admin/src/components/Admin.tsx b/web-admin/src/components/Admin.tsx index dbfe7dee..5556fc43 100644 --- a/web-admin/src/components/Admin.tsx +++ b/web-admin/src/components/Admin.tsx @@ -35,80 +35,121 @@ const TRAFFIC_MODES: Record = { // it crosses, each skipping the last, and a skipped transition rejects `ready`. function animate(update: () => void) { if (document.startViewTransition) document.startViewTransition(() => flushSync(update)).ready.catch(() => {}) - else update() + else flushSync(update) } // Drag-to-reorder for a table whose order the hub stores at `/${path}/order`. // Rows are displaced while the pointer is down and the whole order is saved on -// drop, so a filtered table must disable its handles: the index a row reports is -// then not its place in `order`. -function useDragOrder(items: T[], path: string, onSaved: () => void) { +// release, so a filtered table must disable its handles: the rows on screen are +// then not `order`. +function useDragOrder(items: T[], path: string, reload: () => void) { const [manualOrder, setManualOrder] = useState([]) const [dragging, setDragging] = useState(null) const orderBeforeDrag = useRef([]) + // The order last asked for. A view transition renders it a frame or more + // later, and a repeated key or a quick drag must build on it rather than on + // the order still on screen. + const pending = useRef(null) + const body = useRef(null) + // One save in flight at a time, so two quick reorders reach the hub in order. + const saving = useRef>(Promise.resolve()) const byId = new Map(items.map((item) => [item.id, item])) const orderedIds = new Set(manualOrder) const order = [ ...manualOrder.map((id) => byId.get(id)).filter((item): item is T => Boolean(item)), ...items.filter((item) => !orderedIds.has(item.id)), ] - const ids = () => order.map((item) => item.id) + const ids = () => pending.current ?? order.map((item) => item.id) - // While a view transition runs, Chrome hit-tests drag events to the root - // element rather than the row under the pointer, so releasing during a row's - // 150 ms slide would count as a drop outside the table and restore the order. - // The body fills the viewport, so the root is otherwise never the target. + // Once the hub lists this order, its list is followed again, so a reorder made + // in another tab appears here instead of being overwritten by the next drag. + if (dragging === null && manualOrder.length && manualOrder.join() === items.map((item) => item.id).join()) { + setManualOrder([]) + } + + // Handled on the document by where the pointer is, not by the row under it: + // while a view transition runs, Chrome hit-tests drag events to the root + // element, so row handlers would miss every row crossed during a 150 ms slide, + // and a release then would count as a drop outside the table. Re-attached on + // every render, since `order` changes as rows are displaced. useEffect(() => { - if (dragging === null) return - const accept = (e: DragEvent) => { if (e.target === document.documentElement) e.preventDefault() } - document.addEventListener("dragover", accept) - document.addEventListener("drop", accept) + const rows = body.current + if (dragging === null || !rows) return + const over = (e: DragEvent) => { + const table = rows.getBoundingClientRect() + if (e.clientX < table.left || e.clientX > table.right || e.clientY < table.top || e.clientY > table.bottom) return + e.preventDefault() + if (e.type === "drop") return + if (e.dataTransfer) e.dataTransfer.dropEffect = "move" + const at = [...rows.rows].findIndex((row) => { + const r = row.getBoundingClientRect() + return e.clientY >= r.top && e.clientY < r.bottom + }) + // By position rather than by the row there, which may be one a pending + // transition has yet to move. + if (at >= 0) move(dragging, at) + } + document.addEventListener("dragover", over) + document.addEventListener("drop", over) return () => { - document.removeEventListener("dragover", accept) - document.removeEventListener("drop", accept) + document.removeEventListener("dragover", over) + document.removeEventListener("drop", over) } - }, [dragging]) + }) + + // A transition superseded before it ran applies nothing: a later move, or a + // refused save falling back to the hub's order, has replaced it. + function show(next: number[]) { + pending.current = next + animate(() => { + if (pending.current !== next) return + pending.current = null + setManualOrder(next) + }) + } - function move(from: number, to: number) { - if (from < 0 || to < 0 || to >= order.length || from === to) return - const next = [...order] + function move(id: number, to: number) { + const next = [...ids()] + const from = next.indexOf(id) + if (from < 0 || to < 0 || to >= next.length || from === to) return next.splice(to, 0, ...next.splice(from, 1)) - const moved = next.map((item) => item.id) - animate(() => setManualOrder(moved)) - return moved + show(next) + return next } // Dropped outside the table or cancelled with Escape: the order is restored. function cancel() { setDragging(null) - const rollback = orderBeforeDrag.current - if (rollback.length) animate(() => setManualOrder(rollback)) + if (orderBeforeDrag.current.length) show(orderBeforeDrag.current) } function save(next: number[]) { setDragging(null) - const rollback = orderBeforeDrag.current - if (!rollback.length || next.join() === rollback.join()) return + const before = orderBeforeDrag.current + if (!before.length || next.join() === before.join()) return orderBeforeDrag.current = next - api(`/${path}/order`, { method: "PUT", body: JSON.stringify({ ids: next }) }).then(onSaved, (e: Error) => { - setManualOrder(rollback) + const put = () => api(`/${path}/order`, { method: "PUT", body: JSON.stringify({ ids: next }) }) + // A refusal falls back to whatever the hub holds, which a save queued + // behind it may still change. + saving.current = saving.current.then(put).then(reload, (e: Error) => { + pending.current = null + setManualOrder([]) + reload() toast.error(e.message) }) } return { order, - row: (id: number, index: number) => ({ + row: (id: number) => ({ style: { viewTransitionName: `${path}-${id}` }, "data-dragging": dragging === id || undefined, className: "transition-opacity data-[dragging]:opacity-40", - onDragOver: (e: React.DragEvent) => { e.preventDefault(); e.dataTransfer.dropEffect = "move" }, - onDragEnter: () => { if (dragging !== null) move(order.findIndex((item) => item.id === dragging), index) }, - onDrop: (e: React.DragEvent) => { e.preventDefault(); save(ids()) }, }), - handle: (id: number, index: number) => ({ - onDragStart: (e: React.DragEvent) => { + handle: (id: number) => ({ + onDragStart: (e: React.DragEvent) => { orderBeforeDrag.current = ids() + body.current = e.currentTarget.closest("tbody") setDragging(id) e.dataTransfer.effectAllowed = "move" // Firefox refuses to start a drag without a payload. @@ -120,7 +161,7 @@ function useDragOrder(items: T[], path: string, onSave if (!delta) return e.preventDefault() orderBeforeDrag.current = ids() - const next = move(index, index + delta) + const next = move(id, ids().indexOf(id) + delta) if (next) save(next) }, }), @@ -1255,12 +1296,12 @@ function Nodes({ nodes, refresh, site, refusal }: { nodes: Node[]; refresh: () = - {visible.map((n, index) => ( - + {visible.map((n) => ( +
- {drag.order.map((t, index) => ( - + {drag.order.map((t) => ( +
- + {t.name}