diff --git a/crates/rustmail-api/src/handlers.rs b/crates/rustmail-api/src/handlers.rs index 5906382..6773f48 100644 --- a/crates/rustmail-api/src/handlers.rs +++ b/crates/rustmail-api/src/handlers.rs @@ -672,16 +672,7 @@ pub async fn release_message( let raw = state.repo.get_raw(&id).await?; let msg = state.repo.get(&id).await?; - let envelope = lettre::address::Envelope::new( - msg.sender.parse().ok(), - serde_json::from_str::>(&msg.recipients) - .unwrap_or_default() - .iter() - .filter_map(|r| r.parse().ok()) - .collect(), - ); - - match envelope { + match release_envelope(&msg.sender, &msg.recipients) { Ok(envelope) => { use lettre::AsyncTransport; @@ -721,16 +712,41 @@ pub async fn release_message( } } } - Err(e) => Ok( + Err(reason) => Ok( ( StatusCode::BAD_REQUEST, - Json(serde_json::json!({ "error": format!("Invalid envelope: {}", e) })), + Json(serde_json::json!({ "error": format!("Invalid envelope: {reason}") })), ) .into_response(), ), } } +/// The captured envelope a release sends again, exactly as captured. +/// +/// Every address has to carry over: dropping one the relay cannot take would +/// deliver to fewer recipients, or from another sender, than the message was +/// sent with, and still report success. Only an empty sender, the null +/// reverse-path `MAIL FROM:<>`, becomes no sender. +fn release_envelope(sender: &str, recipients: &str) -> Result { + let from = if sender.is_empty() { + None + } else { + Some( + sender + .parse() + .map_err(|_| "the captured sender is not an address a relay accepts".to_string())?, + ) + }; + let to = serde_json::from_str::>(recipients) + .map_err(|_| "the captured recipients could not be read".to_string())? + .iter() + .map(|recipient| recipient.parse()) + .collect::, _>>() + .map_err(|_| "a captured recipient is not an address a relay accepts".to_string())?; + lettre::address::Envelope::new(from, to).map_err(|e| e.to_string()) +} + #[derive(Debug, Serialize)] pub struct AuthResults { pub dkim: Vec, @@ -1001,6 +1017,56 @@ impl IntoResponse for AppError { } } +#[cfg(test)] +mod release_envelope_tests { + use super::release_envelope; + + #[test] + fn a_captured_envelope_carries_over_whole() { + let envelope = release_envelope( + "from@example.test", + r#"["a@example.test","b@example.test"]"#, + ) + .unwrap(); + + assert_eq!( + envelope.from().map(ToString::to_string).as_deref(), + Some("from@example.test") + ); + assert_eq!(envelope.to().len(), 2); + } + + #[test] + fn the_null_sender_releases_without_one() { + let envelope = release_envelope("", r#"["a@example.test"]"#).unwrap(); + + assert!(envelope.from().is_none()); + } + + #[test] + fn a_recipient_a_relay_cannot_take_refuses_the_release() { + let refused = release_envelope( + "from@example.test", + r#"["a@example.test","not an address"]"#, + ); + + assert_eq!( + refused.unwrap_err(), + "a captured recipient is not an address a relay accepts" + ); + } + + #[test] + fn a_sender_a_relay_cannot_take_refuses_the_release() { + let refused = release_envelope("not an address", r#"["a@example.test"]"#); + + assert_eq!( + refused.unwrap_err(), + "the captured sender is not an address a relay accepts" + ); + } +} + #[cfg(test)] mod auth_parser_tests { use super::*; diff --git a/crates/rustmail-api/src/state.rs b/crates/rustmail-api/src/state.rs index e977621..dc4757b 100644 --- a/crates/rustmail-api/src/state.rs +++ b/crates/rustmail-api/src/state.rs @@ -209,6 +209,25 @@ mod tests { assert_eq!(wire(&decoded), wire(&event)); } + #[test] + fn a_new_message_frame_decodes_back_to_its_event() { + let event = WsEvent::MessageNew(rustmail_storage::MessageSummary { + id: "a".into(), + sender: "s@example.test".into(), + recipients: r#"["r@example.test"]"#.into(), + subject: None, + size: 1, + has_attachments: false, + is_read: false, + is_starred: false, + tags: "[]".into(), + created_at: "2026-09-23T00:00:00Z".into(), + }); + let decoded = WsFrame::encode(&event).unwrap().decode().unwrap(); + + assert_eq!(wire(&decoded), wire(&event)); + } + #[test] fn a_malformed_frame_decodes_to_a_ws_frame_error() { let frame = WsFrame(r#"{"type":"not-an-event"}"#.into()); diff --git a/crates/rustmail-api/tests/api.rs b/crates/rustmail-api/tests/api.rs index 2466e7a..97eb644 100644 --- a/crates/rustmail-api/tests/api.rs +++ b/crates/rustmail-api/tests/api.rs @@ -605,6 +605,47 @@ async fn release_rejects_wrong_host() { assert_eq!(response.status(), StatusCode::FORBIDDEN); } +#[tokio::test] +async fn release_refuses_an_envelope_it_cannot_carry_whole() { + let pool = sqlx::sqlite::SqlitePoolOptions::new() + .connect("sqlite::memory:") + .await + .unwrap(); + initialize_database(&pool).await.unwrap(); + let repo = MessageRepository::new(pool); + let (ws_tx, _) = broadcast::channel::(256); + let state = AppState::new(repo.clone(), ws_tx, Some("relay.invalid".into()), Some(587)); + let app = router(state); + + let summary = repo + .insert( + "a@t.com", + &["b@t.com".into(), "not an address".into()], + &raw_email("Release", "a@t.com", "b@t.com"), + ) + .await + .unwrap(); + + let response = app + .oneshot( + Request::builder() + .method("POST") + .uri(format!("/api/v1/messages/{}/release", summary.id)) + .header("content-type", "application/json") + .body(Body::from(r#"{"host": "relay.invalid"}"#)) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + let body = json_body(response).await; + assert_eq!( + body["error"], + "Invalid envelope: a captured recipient is not an address a relay accepts" + ); +} + #[tokio::test] async fn security_headers_present() { let (app, _, _) = setup().await; diff --git a/crates/rustmail-server/src/main.rs b/crates/rustmail-server/src/main.rs index 724fb71..68699c5 100644 --- a/crates/rustmail-server/src/main.rs +++ b/crates/rustmail-server/src/main.rs @@ -8,7 +8,7 @@ use rustls::pki_types::pem::PemObject; use rustls::pki_types::{CertificateDer, PrivateKeyDer}; use serde::Deserialize; use time::OffsetDateTime; -use tokio::sync::{broadcast, mpsc, oneshot}; +use tokio::sync::{broadcast, mpsc, oneshot, watch}; use tracing::{info, warn}; use rustmail_api::{AppState, Hostname, Origin, WsEvent, WsFrame}; @@ -185,6 +185,7 @@ struct TomlConfig { release_host: Option, allowed_origins: Option>, allowed_hosts: Option>, + ws_buffer: Option, } fn apply_toml_to_env(config: &TomlConfig) { @@ -239,6 +240,9 @@ fn apply_toml_to_env(config: &TomlConfig) { if let Some(v) = &config.allowed_hosts { set_if_absent("RUSTMAIL_ALLOWED_HOSTS", &v.join(",")); } + if let Some(v) = config.ws_buffer { + set_if_absent("RUSTMAIL_WS_BUFFER", &v.to_string()); + } } fn main() -> Result<()> { @@ -405,22 +409,65 @@ const BLOCKING_PARSE_THRESHOLD_BYTES: usize = 256 * 1024; /// How long closing the database, and with it the final WAL checkpoint, may take. const DB_CLOSE_DEADLINE: std::time::Duration = std::time::Duration::from_secs(2); -/// Resolves when the process is asked to stop, by `SIGTERM` or Ctrl-C. +/// Whether the process has been asked to stop, by `SIGTERM` or Ctrl-C. /// -/// `SIGTERM` needs a handler of its own: as PID 1 in a container the kernel -/// ignores it by default, so `docker stop` would otherwise wait out its -/// timeout and then `SIGKILL` the server. -async fn shutdown_signal() -> std::io::Result<()> { - #[cfg(unix)] - { - let mut terminate = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate())?; - tokio::select! { - result = tokio::signal::ctrl_c() => result, - _ = terminate.recv() => Ok(()), - } +/// One listener, started before the database is prepared, serves the whole +/// run. Once Tokio has installed its handler for a signal, a signal that +/// arrives while nothing is waiting on it is consumed and lost, so startup and +/// serving must not each wait on their own. `SIGTERM` needs the handler at +/// all because as PID 1 in a container the kernel ignores it by default, and +/// `docker stop` would otherwise wait out its timeout and `SIGKILL` the server. +#[derive(Clone)] +struct StopRequest(watch::Receiver); + +/// The task behind a [`StopRequest`], stopped when this is dropped. +struct StopListener(tokio::task::JoinHandle<()>); + +impl Drop for StopListener { + fn drop(&mut self) { + self.0.abort(); + } +} + +impl StopRequest { + /// Installs the signal handlers and starts listening. + fn listen() -> std::io::Result<(Self, StopListener)> { + let (requested, stop) = watch::channel(false); + #[cfg(unix)] + let task = { + use tokio::signal::unix::{SignalKind, signal}; + let mut terminate = signal(SignalKind::terminate())?; + let mut interrupt = signal(SignalKind::interrupt())?; + tokio::spawn(async move { + tokio::select! { + _ = terminate.recv() => {} + _ = interrupt.recv() => {} + } + let _ = requested.send(true); + }) + }; + #[cfg(not(unix))] + let task = tokio::spawn(async move { + match tokio::signal::ctrl_c().await { + Ok(()) => { + let _ = requested.send(true); + } + Err(e) => { + tracing::error!(error = %e, "failed to listen for Ctrl-C; stop the process another way") + } + } + }); + Ok((Self(stop), StopListener(task))) + } + + fn is_requested(&self) -> bool { + *self.0.borrow() + } + + /// Resolves once a stop has been requested, at once if it already was. + async fn requested(&mut self) { + let _ = self.0.wait_for(|requested| *requested).await; } - #[cfg(not(unix))] - tokio::signal::ctrl_c().await } /// Receives the next run of queued deliveries into `batch`, closing the queue @@ -703,21 +750,8 @@ async fn connect_writer(db_url: &str) -> Result { /// A stop requested while a migration runs pauses it after the batch in /// flight; the next start resumes it. Returns whether startup should go on: /// `false` once a stop was requested, even if the migration had finished. -async fn prepare_database_file(db_path: &Path) -> Result { - let stop_requested = Arc::new(std::sync::atomic::AtomicBool::new(false)); - let listener = { - let stop_requested = Arc::clone(&stop_requested); - tokio::spawn(async move { - if shutdown_signal().await.is_ok() { - stop_requested.store(true, std::sync::atomic::Ordering::SeqCst); - } - }) - }; - let preparation = rustmail_storage::prepare_database_file(db_path, || { - stop_requested.load(std::sync::atomic::Ordering::SeqCst) - }) - .await; - listener.abort(); +async fn prepare_database_file(db_path: &Path, stop: &StopRequest) -> Result { + let preparation = rustmail_storage::prepare_database_file(db_path, || stop.is_requested()).await; let preparation = preparation.with_context(|| format!("failed to prepare database {}", db_path.display()))?; if let rustmail_storage::Preparation::Paused { migrated, total } = preparation { @@ -727,7 +761,7 @@ async fn prepare_database_file(db_path: &Path) -> Result { ); return Ok(false); } - if stop_requested.load(std::sync::atomic::Ordering::SeqCst) { + if stop.is_requested() { info!("Stop requested during startup; exiting before serving"); return Ok(false); } @@ -1018,6 +1052,9 @@ async fn run_serve(args: ServeArgs) -> Result<()> { ); } + let (mut stop, _stop_listener) = + StopRequest::listen().context("failed to listen for shutdown signals")?; + let db_url = if args.ephemeral { info!("Running in ephemeral mode (in-memory database)"); IN_MEMORY_DB_URL.to_string() @@ -1027,7 +1064,7 @@ async fn run_serve(args: ServeArgs) -> Result<()> { std::fs::create_dir_all(parent)?; } info!(path = %db_path.display(), "Using persistent database"); - if !prepare_database_file(&db_path).await? { + if !prepare_database_file(&db_path, &stop).await? { return Ok(()); } format!("sqlite:{}?mode=rwc", db_path.display()) @@ -1183,9 +1220,7 @@ async fn run_serve(args: ServeArgs) -> Result<()> { anyhow::bail!("Message processor stopped unexpectedly"); } _ = &mut retention_task => {} - result = shutdown_signal() => { - result.context("failed to listen for shutdown signals")?; - } + _ = stop.requested() => {} } info!("Shutting down: SMTP closed, draining queued messages"); @@ -1224,6 +1259,26 @@ async fn run_serve(args: ServeArgs) -> Result<()> { Ok(()) } +#[cfg(test)] +mod stop_request_tests { + use super::*; + + const WAIT_DEADLINE: std::time::Duration = std::time::Duration::from_secs(1); + + #[tokio::test] + async fn a_stop_requested_while_nothing_waits_is_still_seen_later() { + let (requested, receiver) = watch::channel(false); + let mut stop = StopRequest(receiver); + + requested.send(true).unwrap(); + + assert!(stop.is_requested()); + tokio::time::timeout(WAIT_DEADLINE, stop.requested()) + .await + .expect("a stop requested before serving must end the serve loop at once"); + } +} + #[cfg(test)] mod version_tests { use super::*; diff --git a/crates/rustmail-server/tests/integration.rs b/crates/rustmail-server/tests/integration.rs index 7aa75ad..c20eaa0 100644 --- a/crates/rustmail-server/tests/integration.rs +++ b/crates/rustmail-server/tests/integration.rs @@ -1466,6 +1466,36 @@ async fn config_env_overrides_toml() { assert!(banner.starts_with("220"), "got: {banner}"); } +#[tokio::test] +async fn config_ws_buffer_reaches_the_command_line_check() { + use std::io::Write; + + let mut toml_file = tempfile::Builder::new().suffix(".toml").tempfile().unwrap(); + write!( + toml_file, + "smtp_port = {ANY_FREE_PORT}\nhttp_port = {ANY_FREE_PORT}\nephemeral = true\nws_buffer = 0\n" + ) + .unwrap(); + + let output = tokio::time::timeout( + std::time::Duration::from_secs(PROMPT_EXIT_SECS), + rustmail_command() + .args(["serve", "--config", toml_file.path().to_str().unwrap()]) + .env_remove("RUSTMAIL_WS_BUFFER") + .output(), + ) + .await + .expect("a refused config must exit promptly") + .unwrap(); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !output.status.success(), + "ws_buffer = 0 from the config file must be refused" + ); + assert!(stderr.contains("--ws-buffer"), "got: {stderr}"); +} + #[tokio::test] async fn config_toml_used_when_no_env() { use std::io::Write; diff --git a/crates/rustmail-storage/src/models.rs b/crates/rustmail-storage/src/models.rs index ecdb3af..26db01e 100644 --- a/crates/rustmail-storage/src/models.rs +++ b/crates/rustmail-storage/src/models.rs @@ -1,4 +1,4 @@ -use serde::{Deserialize, Serialize, Serializer}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// An email message with its parsed bodies. /// @@ -14,7 +14,10 @@ pub struct Message { /// MAIL FROM address. pub sender: String, /// JSON-encoded array of RCPT TO addresses. - #[serde(serialize_with = "serialize_json_string_as_array")] + #[serde( + serialize_with = "serialize_json_string_as_array", + deserialize_with = "deserialize_array_as_json_string" + )] pub recipients: String, /// Parsed Subject header, if present. pub subject: Option, @@ -31,7 +34,10 @@ pub struct Message { /// Whether the message has been starred. pub is_starred: bool, /// JSON-encoded array of user-assigned tags. - #[serde(serialize_with = "serialize_json_string_as_array")] + #[serde( + serialize_with = "serialize_json_string_as_array", + deserialize_with = "deserialize_array_as_json_string" + )] pub tags: String, /// ISO 8601 timestamp of when the message was received. pub created_at: String, @@ -44,14 +50,20 @@ pub struct Message { pub struct MessageSummary { pub id: String, pub sender: String, - #[serde(serialize_with = "serialize_json_string_as_array")] + #[serde( + serialize_with = "serialize_json_string_as_array", + deserialize_with = "deserialize_array_as_json_string" + )] pub recipients: String, pub subject: Option, pub size: i64, pub has_attachments: bool, pub is_read: bool, pub is_starred: bool, - #[serde(serialize_with = "serialize_json_string_as_array")] + #[serde( + serialize_with = "serialize_json_string_as_array", + deserialize_with = "deserialize_array_as_json_string" + )] pub tags: String, pub created_at: String, } @@ -97,3 +109,39 @@ fn serialize_json_string_as_array( }); tags.serialize(serializer) } + +/// Reads back what [`serialize_json_string_as_array`] writes: an array of +/// strings, kept as the JSON text the database stores. +fn deserialize_array_as_json_string<'de, D: Deserializer<'de>>( + deserializer: D, +) -> Result { + let items = Vec::::deserialize(deserializer)?; + serde_json::to_string(&items).map_err(serde::de::Error::custom) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_summary_reads_back_the_arrays_it_serializes() { + let summary = MessageSummary { + id: "01J0000000000000000000000".into(), + sender: "a@example.test".into(), + recipients: r#"["b@example.test","c@example.test"]"#.into(), + subject: Some("s".into()), + size: 10, + has_attachments: false, + is_read: false, + is_starred: false, + tags: r#"["ci"]"#.into(), + created_at: "2026-09-23T00:00:00Z".into(), + }; + + let json = serde_json::to_string(&summary).unwrap(); + let back: MessageSummary = serde_json::from_str(&json).unwrap(); + + assert_eq!(back.recipients, summary.recipients); + assert_eq!(back.tags, summary.tags); + } +} diff --git a/crates/rustmail-tui/src/app.rs b/crates/rustmail-tui/src/app.rs index c30ef0d..4ccc8a6 100644 --- a/crates/rustmail-tui/src/app.rs +++ b/crates/rustmail-tui/src/app.rs @@ -630,11 +630,14 @@ impl App { } } + /// Refetches a stale view once the throttle allows, but never over a fetch + /// still in flight: a newer request would supersede it, and a server slower + /// than the throttle would then never land one. async fn refetch_if_stale(&mut self) { let throttle_elapsed = self .last_fetch_at .is_none_or(|at| at.elapsed() >= STALE_VIEW_REFETCH_INTERVAL); - if self.view_stale && throttle_elapsed { + if self.view_stale && !self.loading && throttle_elapsed { self.fetch_messages().await; } } @@ -1211,30 +1214,63 @@ async fn connect_ws(url: &str, tx: &mpsc::Sender) -> Result<()> { let (ws_stream, _) = connect_async(url).await?; let _ = tx.send(Event::WsStatus(true)).await; - let (_, mut read) = ws_stream.split(); - - while let Some(msg) = read.next().await { - match msg { - Ok(tokio_tungstenite::tungstenite::Message::Text(text)) => { - forward_ws_frame(tx, text.to_string()); - } - Ok(tokio_tungstenite::tungstenite::Message::Close(_)) => break, - Err(_) => break, - _ => {} - } - } + let (_, read) = ws_stream.split(); + use tokio_tungstenite::tungstenite::Message; + let frames = read + .take_while(|msg| std::future::ready(!matches!(msg, Ok(Message::Close(_)) | Err(_)))) + .filter_map(|msg| { + std::future::ready(match msg { + Ok(Message::Text(text)) => Some(text.to_string()), + _ => None, + }) + }); + pump_ws_frames(frames, tx).await; Ok(()) } -/// Forwards a WebSocket frame without blocking the socket reader. If the -/// bounded event queue is full, the frame is dropped and a single -/// [`Event::WsOverflow`] marker is attempted in its place, so the app marks -/// its view stale and resyncs once it catches up, instead of the reader -/// stalling and the server treating this client as lagging. -fn forward_ws_frame(tx: &mpsc::Sender, text: String) { - if tx.try_send(Event::WsMessage(text)).is_err() { - let _ = tx.try_send(Event::WsOverflow); +/// Forwards WebSocket text frames to the event queue without blocking the +/// socket reader on it. +/// +/// A frame that finds the bounded queue full is dropped and the view owes a +/// resync. That debt is kept here until an [`Event::WsOverflow`] fits, rather +/// than tried once on a queue that is still full, so a drop is never lost +/// together with the signal that would repair it. Frames arriving meanwhile +/// are dropped too, since the resync covers them. The reader never stalls, so +/// the server never sees this client as lagging. A debt still owed when the +/// socket closes is paid before returning, waiting for room if it must. +async fn pump_ws_frames( + frames: impl futures_util::Stream, + tx: &mpsc::Sender, +) { + use futures_util::StreamExt; + + let mut frames = std::pin::pin!(frames); + let mut overflow_owed = false; + loop { + tokio::select! { + permit = tx.reserve(), if overflow_owed => match permit { + Ok(permit) => { + permit.send(Event::WsOverflow); + overflow_owed = false; + } + Err(_) => return, + }, + frame = frames.next() => match frame { + Some(text) => { + if overflow_owed { + continue; + } + if tx.try_send(Event::WsMessage(text)).is_err() { + overflow_owed = tx.try_send(Event::WsOverflow).is_err(); + } + } + None => break, + }, + } + } + if overflow_owed { + let _ = tx.send(Event::WsOverflow).await; } } @@ -1645,31 +1681,83 @@ mod tests { assert!(app.view_stale); } + fn frames_then_silence(frames: &[&str]) -> impl futures_util::Stream { + use futures_util::StreamExt; + let frames: Vec = frames.iter().map(|f| f.to_string()).collect(); + futures_util::stream::iter(frames).chain(futures_util::stream::pending()) + } + #[tokio::test] async fn ws_frame_forwarding_delivers_when_the_queue_has_room() { let (tx, mut rx) = mpsc::channel::(1); - forward_ws_frame(&tx, "first".into()); + let pump = + tokio::spawn(async move { pump_ws_frames(frames_then_silence(&["first"]), &tx).await }); - match rx.recv().await.unwrap() { + match tokio::time::timeout(SHUTDOWN_TEST_DEADLINE, rx.recv()) + .await + .unwrap() + .unwrap() + { Event::WsMessage(text) => assert_eq!(text, "first"), other => panic!("unexpected event: {other:?}"), } + pump.abort(); } #[tokio::test] - async fn ws_frame_forwarding_drops_the_frame_when_the_queue_stays_full() { - let (tx, rx) = mpsc::channel::(1); + async fn a_frame_dropped_on_a_full_queue_is_followed_by_a_resync_once_it_drains() { + let (tx, mut rx) = mpsc::channel::(1); tx.try_send(Event::Tick).unwrap(); + let pump = tokio::spawn(async move { + pump_ws_frames(frames_then_silence(&["dropped", "also dropped"]), &tx).await + }); + tokio::task::yield_now().await; - forward_ws_frame(&tx, "dropped".into()); + assert!(matches!(rx.recv().await, Some(Event::Tick))); + let next = tokio::time::timeout(SHUTDOWN_TEST_DEADLINE, rx.recv()) + .await + .expect("the owed overflow marker must be delivered once there is room"); - let mut rx = rx; - let mut remaining = Vec::new(); - while let Ok(event) = rx.try_recv() { - remaining.push(event); - } - assert_eq!(remaining.len(), 1); - assert!(matches!(remaining[0], Event::Tick)); + assert!(matches!(next, Some(Event::WsOverflow))); + assert!( + rx.try_recv().is_err(), + "frames dropped while the resync was owed stay dropped" + ); + pump.abort(); + } + + #[tokio::test] + async fn a_resync_owed_when_the_socket_closes_is_still_delivered() { + let (tx, mut rx) = mpsc::channel::(1); + tx.try_send(Event::Tick).unwrap(); + let pump = tokio::spawn(async move { + pump_ws_frames(futures_util::stream::iter(vec!["dropped".to_string()]), &tx).await + }); + tokio::task::yield_now().await; + + assert!(matches!(rx.recv().await, Some(Event::Tick))); + let next = tokio::time::timeout(SHUTDOWN_TEST_DEADLINE, rx.recv()) + .await + .expect("the owed overflow marker must outlive the socket"); + + assert!(matches!(next, Some(Event::WsOverflow))); + tokio::time::timeout(SHUTDOWN_TEST_DEADLINE, pump) + .await + .unwrap() + .unwrap(); + } + + #[tokio::test] + async fn a_stale_view_is_not_refetched_over_a_fetch_in_flight() { + let mut app = app_with_messages(1); + app.view_stale = true; + app.loading = true; + app.last_fetch_at = None; + let generation = app.fetch_generation; + + app.refetch_if_stale().await; + + assert_eq!(app.fetch_generation, generation); } #[tokio::test] diff --git a/docs/api.yaml b/docs/api.yaml index 8fa2158..577a79e 100644 --- a/docs/api.yaml +++ b/docs/api.yaml @@ -539,7 +539,9 @@ paths: '400': description: > `port` is not one of 25, 465, 587 or 2525, or the captured envelope - cannot be sent (for example no recipient address parses). + cannot be sent as captured: it has no recipient, or its sender + (other than the empty `MAIL FROM:<>`) or any one recipient is not + an address a relay accepts. Nothing is sent to the relay then. content: application/json: schema: @@ -553,6 +555,10 @@ paths: summary: captured envelope cannot be sent value: error: 'Invalid envelope: missing destination address, invalid envelope' + invalidRecipient: + summary: a captured recipient is not a relayable address + value: + error: 'Invalid envelope: a captured recipient is not an address a relay accepts' '403': description: > Release is disabled (no `--release-host`), `host` is not the diff --git a/ui/src/stores/messages.test.ts b/ui/src/stores/messages.test.ts index aca02a6..ae74124 100644 --- a/ui/src/stores/messages.test.ts +++ b/ui/src/stores/messages.test.ts @@ -2466,3 +2466,72 @@ describe("clearInboxPrompt", () => { expect(clearInboxPrompt().message).toBe(everything); }); }); + +describe("flag changes under a filter", () => { + beforeEach(async () => { + useFakeClock(); + FakeSocket.last = null; + vi.stubGlobal("WebSocket", FakeSocket); + vi.stubGlobal("location", { protocol: "http:", host: "inbox.test" }); + listMessages.mockResolvedValue(page([message(0, { is_starred: true })])); + toggleFilter("starred"); + await vi.advanceTimersByTimeAsync(0); + connectWebSocket(); + listMessages.mockClear(); + }); + + afterEach(() => { + disconnectWebSocket(); + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + it("asks the server for the count when a message it never loaded is deleted", async () => { + listMessages.mockResolvedValue(page([], 1)); + + deliver(JSON.stringify({ type: "message:delete", data: { id: "id-7" } })); + await vi.advanceTimersByTimeAsync(0); + + expect(filteredMessages().map((m) => m.id)).toEqual(["id-0"]); + expect(total()).toBe(1); + expect(listMessages).toHaveBeenCalledWith( + expect.objectContaining({ limit: 1 }), + ); + }); +}); + +describe("deleting a row a flag change took out of the filter", () => { + beforeEach(async () => { + FakeSocket.last = null; + vi.stubGlobal("WebSocket", FakeSocket); + vi.stubGlobal("location", { protocol: "http:", host: "inbox.test" }); + listMessages.mockResolvedValue( + page([ + message(0, { is_starred: true }), + message(1, { is_starred: true }), + ]), + ); + toggleFilter("starred"); + await vi.waitFor(() => expect(loading()).toBe(false)); + connectWebSocket(); + }); + + afterEach(() => { + disconnectWebSocket(); + vi.unstubAllGlobals(); + }); + + it("does not count it out a second time", () => { + deliver( + JSON.stringify({ + type: "message:starred", + data: { id: "id-0", is_starred: false }, + }), + ); + expect(total()).toBe(1); + + deliver(JSON.stringify({ type: "message:delete", data: { id: "id-0" } })); + + expect(total()).toBe(1); + }); +}); diff --git a/ui/src/stores/messages.ts b/ui/src/stores/messages.ts index 9bfcedb..549f8ba 100644 --- a/ui/src/stores/messages.ts +++ b/ui/src/stores/messages.ts @@ -635,7 +635,26 @@ async function refreshTotal(): Promise { } } -/** Applies either confirmation once, removing the row regardless of the snapshot. */ +/** + * Whether the total counts `id`, or `undefined` when only the server knows. + * + * A row the list holds counts while it matches the filters: one a flag change + * took out of them was already subtracted, though it stays on screen. A row + * the list never loaded counts in an unnarrowed view, which counts everything, + * but a search or a filter cannot tell whether it was among its matches. + */ +function countedInTotal(id: string): boolean | undefined { + const row = findMessage(id) ?? heldRows.get(id); + if (row !== undefined) return matchesFilters(row, filters()); + return search() !== "" || hasActiveFilters() ? undefined : true; +} + +/** + * Applies either confirmation once, removing the row regardless of the snapshot. + * + * The total drops only by a message it counts, per {@link countedInTotal}, and + * is read from the server when that cannot be told locally. + */ function reconcileDeletion(id: string): void { const issued = issuedDeletes.get(id); if (issued?.confirmed === true) { @@ -645,7 +664,9 @@ function reconcileDeletion(id: string): void { const cleared = issued !== undefined && issued.clearRevision !== clearRevision; + const counted = countedInTotal(id); const countIsCurrent = + counted !== undefined && !countNeedsRefresh && (issued === undefined || issued.snapshot === snapshot); if (issued) issued.confirmed = true; @@ -655,7 +676,7 @@ function reconcileDeletion(id: string): void { } batch(() => { forgetMessage(id); - if (!cleared && countIsCurrent) { + if (!cleared && countIsCurrent && counted) { setStoredTotal((current) => Math.max(0, current - 1)); } });