Finish M2: run lock, systemd units, discover; drop the web UI
Designing the configuration UI in full made the case against building it. Its audience would be people who find TOML hard, but with bring-your-own OAuth client settled, every user must first create a Google Cloud project, configure a consent screen and put a secret in a keyring — a far higher bar than editing thirty lines of config. Anyone who clears it can edit the file; anyone who cannot never reaches the file. Against that stood three dependencies, five modules, an auth.rs refactor and a security surface guarding something that reads the config and touches the keyring. `doctor` and `status` had already absorbed most of what it was for. The reasoning is recorded in TODO.md and SPECS.md rather than left as an apparent oversight. The run lock is not a UI feature and closes a gap that already existed: nothing stopped a timer firing into a hand-run cycle, and two cycles interleaving writes over the same vdirs is what the design otherwise avoids. flock is used rather than a pid file because the kernel releases it however the process ends, so a crash cannot leave a lock to clear by hand — which also means a lock we failed to take is held by a live process, so the pid in it is worth reporting. The one idea worth keeping from the UI design was collection discovery, which needed no web layer. `calcalist discover` prints a ready-to-paste endpoint block per calendar a server offers, removing the most error-prone field in the config. pimsync's discovery output is undocumented, so the format was established against a real server first. Two things it teaches: everything arrives on stdout including failures, and a pair has two storages, so pimsync reports the scratch vdir's contents too — parsing anchors on the heading naming the server, or a probe directory's leftovers would be offered as the user's calendars. Verified against Posteo as well as Radicale: all four calendars found, the first matching the URL already configured. Also fixes a real defect in the test harness rather than its symptom. Ports were chosen by binding one and letting go, so two tests could pick the same number — and the loser's readiness check then succeeded against the winner's server, silently sharing it. Startup now confirms the child we spawned is the one alive, retries on another port if not, and waits for a real HTTP response rather than an open socket. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
47ad8b4c47
commit
ef6482ed62
13 changed files with 808 additions and 92 deletions
|
|
@ -14,6 +14,9 @@ use std::time::{Duration, Instant};
|
|||
/// How long to wait for a server to start answering.
|
||||
const STARTUP_TIMEOUT: Duration = Duration::from_secs(20);
|
||||
|
||||
/// How many ports to try before giving up on starting a server.
|
||||
const ATTEMPTS: usize = 8;
|
||||
|
||||
/// The account Radicale files everything under. With authentication disabled it
|
||||
/// accepts whatever name arrives, and `owner_only` rights then grant that name
|
||||
/// its own tree — so the URLs below all live beneath it.
|
||||
|
|
@ -46,29 +49,40 @@ impl Radicale {
|
|||
pub fn start(root: &Path) -> Radicale {
|
||||
let storage = root.join("radicale");
|
||||
std::fs::create_dir_all(&storage).expect("create storage");
|
||||
let port = free_port();
|
||||
|
||||
let process = Command::new("radicale")
|
||||
.arg("--config")
|
||||
.arg("")
|
||||
.arg("--server-hosts")
|
||||
.arg(format!("127.0.0.1:{port}"))
|
||||
.arg("--auth-type")
|
||||
.arg("none")
|
||||
.arg("--storage-filesystem-folder")
|
||||
.arg(&storage)
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()
|
||||
.expect("radicale should start");
|
||||
// A free port is chosen by binding one and letting go, so two tests
|
||||
// running at once can pick the same number. The loser's radicale then
|
||||
// fails to bind and exits — and, worse, waiting for the port to answer
|
||||
// succeeds anyway, because the winner is listening on it. Tests would
|
||||
// quietly share a server. So: confirm the child we spawned is the one
|
||||
// still alive, and take a different port if it is not.
|
||||
for _ in 0..ATTEMPTS {
|
||||
let port = free_port();
|
||||
let mut process = Command::new("radicale")
|
||||
.arg("--config")
|
||||
.arg("")
|
||||
.arg("--server-hosts")
|
||||
.arg(format!("127.0.0.1:{port}"))
|
||||
.arg("--auth-type")
|
||||
.arg("none")
|
||||
.arg("--storage-filesystem-folder")
|
||||
.arg(&storage)
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()
|
||||
.expect("radicale should start");
|
||||
|
||||
let server = Radicale {
|
||||
process,
|
||||
port,
|
||||
storage,
|
||||
};
|
||||
wait_until_listening(port, "radicale");
|
||||
server
|
||||
if listening(&mut process, port) {
|
||||
return Radicale {
|
||||
process,
|
||||
port,
|
||||
storage,
|
||||
};
|
||||
}
|
||||
let _ = process.kill();
|
||||
let _ = process.wait();
|
||||
}
|
||||
panic!("radicale did not come up on a port of its own after {ATTEMPTS} attempts");
|
||||
}
|
||||
|
||||
pub fn url(&self, calendar: &str) -> String {
|
||||
|
|
@ -138,8 +152,34 @@ impl Radicale {
|
|||
}
|
||||
|
||||
fn request(&self, method: &str, path: &str, content_type: &str, body: &str) -> String {
|
||||
// Retried because a server under parallel test load occasionally resets
|
||||
// a connection it has accepted; that says nothing about the request.
|
||||
let mut last = String::new();
|
||||
for attempt in 0..ATTEMPTS {
|
||||
match self.try_request(method, path, content_type, body) {
|
||||
Ok(response) => return response,
|
||||
Err(error) => {
|
||||
last = error;
|
||||
std::thread::sleep(Duration::from_millis(50 * (attempt as u64 + 1)));
|
||||
}
|
||||
}
|
||||
}
|
||||
panic!("{method} {path} never got a response: {last}");
|
||||
}
|
||||
|
||||
fn try_request(
|
||||
&self,
|
||||
method: &str,
|
||||
path: &str,
|
||||
content_type: &str,
|
||||
body: &str,
|
||||
) -> Result<String, String> {
|
||||
let address = SocketAddr::from((Ipv4Addr::LOCALHOST, self.port));
|
||||
let mut stream = TcpStream::connect(address).expect("connect to radicale");
|
||||
let mut stream = TcpStream::connect_timeout(&address, Duration::from_secs(2))
|
||||
.map_err(|error| error.to_string())?;
|
||||
stream
|
||||
.set_read_timeout(Some(Duration::from_secs(10)))
|
||||
.map_err(|error| error.to_string())?;
|
||||
// With authentication disabled the credentials only pick the account.
|
||||
let authorization = base64(format!("{USER}:password").as_bytes());
|
||||
let request = format!(
|
||||
|
|
@ -149,12 +189,14 @@ impl Radicale {
|
|||
self.port,
|
||||
body.len()
|
||||
);
|
||||
stream.write_all(request.as_bytes()).expect("send request");
|
||||
stream
|
||||
.write_all(request.as_bytes())
|
||||
.map_err(|error| error.to_string())?;
|
||||
let mut response = String::new();
|
||||
stream
|
||||
.read_to_string(&mut response)
|
||||
.expect("read the response");
|
||||
response
|
||||
.map_err(|error| error.to_string())?;
|
||||
Ok(response)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -279,16 +321,42 @@ fn free_port() -> u16 {
|
|||
listener.local_addr().expect("port").port()
|
||||
}
|
||||
|
||||
fn wait_until_listening(port: u16, what: &str) {
|
||||
/// Whether a real HTTP request gets a real HTTP response.
|
||||
fn answers(address: SocketAddr) -> bool {
|
||||
let Ok(mut stream) = TcpStream::connect_timeout(&address, Duration::from_millis(200)) else {
|
||||
return false;
|
||||
};
|
||||
let _ = stream.set_read_timeout(Some(Duration::from_secs(2)));
|
||||
let request = format!("OPTIONS / HTTP/1.1\r\nHost: {address}\r\nConnection: close\r\n\r\n");
|
||||
if stream.write_all(request.as_bytes()).is_err() {
|
||||
return false;
|
||||
}
|
||||
let mut response = String::new();
|
||||
stream.read_to_string(&mut response).is_ok() && response.starts_with("HTTP/")
|
||||
}
|
||||
|
||||
/// Waits for our own radicale to answer, giving up as soon as it has exited.
|
||||
///
|
||||
/// The liveness check is the point: without it a process that lost a port race
|
||||
/// looks healthy, because something else is answering on that port.
|
||||
fn listening(process: &mut Child, port: u16) -> bool {
|
||||
let address = SocketAddr::from((Ipv4Addr::LOCALHOST, port));
|
||||
let deadline = Instant::now() + STARTUP_TIMEOUT;
|
||||
while Instant::now() < deadline {
|
||||
if TcpStream::connect_timeout(&address, Duration::from_millis(200)).is_ok() {
|
||||
return;
|
||||
match process.try_wait() {
|
||||
Ok(Some(_)) => return false,
|
||||
Ok(None) => {}
|
||||
Err(_) => return false,
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(100));
|
||||
// An open port is not readiness: the socket listens before the
|
||||
// application behind it can answer, and under parallel tests that gap
|
||||
// is wide enough to get a connection reset instead of a response.
|
||||
if answers(address) {
|
||||
return true;
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(50));
|
||||
}
|
||||
panic!("{what} did not start listening on {port} within {STARTUP_TIMEOUT:?}");
|
||||
false
|
||||
}
|
||||
|
||||
fn base64(input: &[u8]) -> String {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue