diff --git a/crates/collab-net/src/barred_tests.rs b/crates/collab-net/src/barred_tests.rs new file mode 100644 index 00000000..c9355a5f --- /dev/null +++ b/crates/collab-net/src/barred_tests.rs @@ -0,0 +1,53 @@ +//! Addresses removed from a file are kept only while its session lasts, so a long-running server +//! doesn't collect them for every file it ever had open. + +use std::collections::HashMap; +use std::net::{IpAddr, Ipv4Addr}; + +use figcraft_collab::{Limits, Server}; +use figcraft_doc::Document; + +use super::{Admission, Backend, Closing, Hub, InFlight, Room}; + +/// A backend that closes a file as soon as everyone left it. +struct Closes; + +impl Backend for Closes { + fn admit(&mut self, _file: &str, _token: &str) -> Result { + Err("no".into()) + } + + fn open(&mut self, _file: &str) -> Result { + Err("no".into()) + } + + fn idle(&mut self, _file: &str, _server: &mut Server) -> bool { + true + } + + fn closing(&self) -> Closing { + Closing::Restarting + } +} + +#[test] +fn a_closed_file_forgets_whom_it_barred() { + let mut doc = Document::new("File"); + doc.repair(); + let mut hub = Hub { + in_flight: InFlight(Default::default()), + backend: Closes, + rooms: HashMap::from([("f".to_string(), Room::new(Server::new(doc, "", Limits::default())))]), + conns: HashMap::new(), + local: None, + outbox: Vec::new(), + barred: HashMap::new(), + }; + let ip = IpAddr::V4(Ipv4Addr::new(10, 0, 0, 7)); + hub.barred.entry("f".into()).or_default().insert(ip); + hub.barred.entry("other".into()).or_default().insert(ip); + hub.close_if_empty("f"); + assert!(!hub.rooms.contains_key("f"), "everyone left: the file closed"); + assert!(!hub.barred.contains_key("f"), "and its barred addresses went with it"); + assert!(hub.barred.contains_key("other"), "another file's stay"); +} diff --git a/crates/collab-net/src/hub.rs b/crates/collab-net/src/hub.rs index cdd06915..5cf3e519 100644 --- a/crates/collab-net/src/hub.rs +++ b/crates/collab-net/src/hub.rs @@ -1147,7 +1147,8 @@ struct Hub { local: Option, /// What the servers said, sent once the backend made what changed durable. outbox: Vec<(String, Outgoing)>, - /// Addresses ([`ip_key`]) removed from each file: they can't join it again while the hub runs. + /// Addresses ([`ip_key`]) removed from each open file: they can't join it again until its + /// session closes (the file is closed or removed; the entry goes with it). barred: HashMap>, } @@ -1414,8 +1415,8 @@ impl Hub { } /// Remove client `id` from `file`: they are told the session is over for them and - /// disconnected, and their address can't join the file again while the hub runs. The person - /// hosting can't be removed. + /// disconnected, and their address can't join the file again while its session lasts. The + /// person hosting can't be removed. fn remove_peer(&mut self, file: &str, id: u32) -> Result<(), String> { let room = self.rooms.get(file).ok_or("nobody is in this file")?; if room.local.as_ref().is_some_and(|(local, _)| *local == id) { @@ -1558,6 +1559,7 @@ impl Hub { .collect(); // Closed without saving: the file is gone. self.rooms.remove(&file); + self.barred.remove(&file); self.kick(&users, "this file was removed from the server"); } if let Some(ip) = handled.blocked { @@ -1635,6 +1637,7 @@ impl Hub { let Some(room) = self.rooms.get_mut(file).filter(|r| r.empty()) else { return }; if self.backend.idle(file, &mut room.server) { self.rooms.remove(file); + self.barred.remove(file); } } @@ -1740,3 +1743,7 @@ pub(crate) fn to_local(inbox: &Inbox, local: u32, msg: &ServerMsg) { #[cfg(test)] #[path = "queue_tests.rs"] mod queue_tests; + +#[cfg(test)] +#[path = "barred_tests.rs"] +mod barred_tests; diff --git a/crates/collab/src/message.rs b/crates/collab/src/message.rs index 0463ccd7..d035042b 100644 --- a/crates/collab/src/message.rs +++ b/crates/collab/src/message.rs @@ -275,7 +275,7 @@ pub enum Admin { /// [`AdminReply::Owned`]. Servers older than this request can't read it. Owned { files: Vec<(String, String)> }, /// Remove client `client` from `file`'s session: they are disconnected and their address - /// can't join the file again while the server runs. With the server key, or on an open + /// can't join the file again while its session lasts. With the server key, or on an open /// hosting connection with `file`'s owner token. Servers older than this request can't read it. RemovePeer { file: String, client: u32 }, } diff --git a/docs/mcp.md b/docs/mcp.md index 0e147bb0..8459987d 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1092,7 +1092,7 @@ only: a host stops hosting it, a guest leaves it. pan or zoom of your own, or Escape). - `collab.removePerson {client}` removes that person from the session: they are told the session is over for them (they keep their copy and don't reconnect), and their address can't join the - file again while the session (or the server) runs. Only the person hosting, or the owner of a + file again while the session runs. Only the person hosting, or the owner of a file on a server (with its owner token), can; a server older than this request says it can't. Returns `{removed, name}`. - `collab.cursorChat {text}` shows `text` next to your cursor for everyone in the session (put the