diff --git a/crates/xmedia-bot/src/ctx.rs b/crates/xmedia-bot/src/ctx.rs index f5922b5..712902a 100644 --- a/crates/xmedia-bot/src/ctx.rs +++ b/crates/xmedia-bot/src/ctx.rs @@ -131,7 +131,8 @@ pub(crate) mod test_support { }, ); }) - .await; + .await + .expect("seed prompt state"); } pub(crate) struct TestStores { diff --git a/crates/xmedia-bot/src/handlers/callback.rs b/crates/xmedia-bot/src/handlers/callback.rs index b6f913e..15aa678 100644 --- a/crates/xmedia-bot/src/handlers/callback.rs +++ b/crates/xmedia-bot/src/handlers/callback.rs @@ -59,7 +59,8 @@ async fn handle_callback( }; // Lazy expiry: a stale record (past the TTL, not yet swept) is dropped. if edit.created_at + ttl_secs <= unix_now() { - ctx.chat_store + let _ = ctx + .chat_store .update(chat_id, |data| { data.edit_message.remove(&prompt_message_id); }) @@ -80,7 +81,8 @@ async fn handle_callback( // "do not forward this" answer, and it drops the record so the forward // can never happen later. log::info!("edit-before-forward prompt {prompt_message_id} skipped"); - ctx.chat_store + let _ = ctx + .chat_store .update(chat_id, |data| { data.edit_message.remove(&prompt_message_id); }) @@ -150,7 +152,8 @@ async fn handle_callback( .sender .delete_message(ChatId(chat_id), MessageId(prompt_message_id as i32)) .await; - ctx.chat_store + let _ = ctx + .chat_store .update(chat_id, |data| { data.edit_message.remove(&prompt_message_id); }) @@ -190,7 +193,8 @@ async fn handle_callback( .await { super::EditOutcome::Applied => { - ctx.chat_store + let _ = ctx + .chat_store .update(chat_id, |data| { if let Some(entry) = data.edit_message.get_mut(&prompt_message_id) { entry.template = name.to_string(); diff --git a/crates/xmedia-bot/src/handlers/commands.rs b/crates/xmedia-bot/src/handlers/commands.rs index 49846e0..d4ef65d 100644 --- a/crates/xmedia-bot/src/handlers/commands.rs +++ b/crates/xmedia-bot/src/handlers/commands.rs @@ -74,6 +74,8 @@ fn parse_arg_remainder(s: String) -> Result<(String,), ParseError> { /// `x_media::site::caption_from_fields` substitutes. const FORMAT_PLACEHOLDERS: [&str; 6] = ["url", "author", "author_url", "title", "content", "tags"]; +const CHAT_STATE_READ_ERROR: &str = "Couldn't read chat settings; try again."; + /// `/start`'s welcome: what the bot is for, where links work, where to look /// next. The old "Hello!" left a first-time user with nothing. const START_TEXT: &str = "\ @@ -294,14 +296,19 @@ pub(crate) async fn execute_command( } Command::SetForwardChannel(channel) => { let result = match set_forward_channel_handler(bot, message, channel).await { - Ok(channel_id) => { - ctx.chat_store - .update(message.chat.id.0, |data| { - data.forward_channel_id = Some(channel_id); - }) - .await; - "Add successfully.".to_string() - } + Ok(channel_id) => match ctx + .chat_store + .update(message.chat.id.0, |data| { + data.forward_channel_id = Some(channel_id); + }) + .await + { + Ok((_, true)) => "Add successfully.".to_string(), + Ok((_, false)) => { + "Forward channel set only in memory; retry later.".to_string() + } + Err(()) => CHAT_STATE_READ_ERROR.to_string(), + }, Err(SetForwardChannelError::EmptyParameter) => { "Receive empty parameter.\nYou should enter a channel id or username" .to_string() @@ -323,7 +330,7 @@ pub(crate) async fn execute_command( } Command::RemoveForwardChannel => { let chat_id = message.chat.id.0; - let (text, _) = ctx + let text = match ctx .chat_store .update(chat_id, |data| { if data.forward_channel_id.is_some() { @@ -333,12 +340,16 @@ pub(crate) async fn execute_command( "No channel to remove.".to_string() } }) - .await; + .await + { + Ok((text, _)) => text, + Err(()) => CHAT_STATE_READ_ERROR.to_string(), + }; reply(ctx.sender, message.chat.id.0, message.id, text).await?; } Command::EditBeforeForward => { let chat_id = message.chat.id.0; - let (text, _) = ctx + let text = match ctx .chat_store .update(chat_id, |data| { if data.forward_channel_id.is_none() { @@ -352,7 +363,11 @@ pub(crate) async fn execute_command( "Enable edit before forward.".to_string() } }) - .await; + .await + { + Ok((text, _)) => text, + Err(()) => CHAT_STATE_READ_ERROR.to_string(), + }; reply(ctx.sender, message.chat.id.0, message.id, text).await?; } Command::SetTemplate(name) => { @@ -366,15 +381,22 @@ pub(crate) async fn execute_command( } else if name.is_empty() { "Please provide a name for the template.".to_string() } else { - ctx.chat_store + match ctx + .chat_store .update(chat_id, |data| { data.template.insert( name, html_escape::encode_text(reply_text).into_owned(), ); }) - .await; - "Template set.".to_string() + .await + { + Ok((_, true)) => "Template set.".to_string(), + Ok((_, false)) => { + "Template set only in memory; retry later.".to_string() + } + Err(()) => CHAT_STATE_READ_ERROR.to_string(), + } } } }; @@ -393,21 +415,21 @@ pub(crate) async fn execute_command( .await?; return Ok(()); } - let (removed, _) = ctx + let text = match ctx .chat_store .update(chat_id, |data| data.template.remove(&name).is_some()) - .await; - let text = if removed { - format!("Template '{name}' removed.") - } else { - // Name the live templates: a typo would otherwise look like a - // successful delete. - let names = sorted_template_names(&ctx.chat_store.get(chat_id).await); - if names.is_empty() { - format!("No template named '{name}'. None are saved yet.") - } else { - format!("No template named '{name}'. Saved: {}", names.join(", ")) + .await + { + Ok((true, _)) => format!("Template '{name}' removed."), + Ok((false, _)) => { + let names = sorted_template_names(&ctx.chat_store.get(chat_id).await); + if names.is_empty() { + format!("No template named '{name}'. None are saved yet.") + } else { + format!("No template named '{name}'. Saved: {}", names.join(", ")) + } } + Err(()) => CHAT_STATE_READ_ERROR.to_string(), }; reply(ctx.sender, chat_id, message.id, text).await?; } @@ -461,23 +483,18 @@ pub(crate) async fn execute_command( // set a format once could never get back to the default (the // built-in format string is not something a user can retype). if format == "-" { - let (_, saved) = ctx + let text = match ctx .chat_store .update(chat_id, |data| { data.message_format.remove(site); }) - .await; - reply( - ctx.sender, - message.chat.id.0, - message.id, - if saved { - "Format reset to the built-in one.".to_string() - } else { - "Reset in memory only: the database write failed, so it will be lost on restart.".to_string() - }, - ) - .await?; + .await + { + Ok((_, true)) => "Format reset to the built-in one.".to_string(), + Ok((_, false)) => "Reset in memory only; retry later.".to_string(), + Err(()) => CHAT_STATE_READ_ERROR.to_string(), + }; + reply(ctx.sender, message.chat.id.0, message.id, text).await?; return Ok(()); } // A typo like {titel} would otherwise be rendered literally into @@ -500,23 +517,20 @@ pub(crate) async fn execute_command( .await?; return Ok(()); } - let (_, saved) = ctx + let text = match ctx .chat_store .update(chat_id, |data| { data.message_format.insert(site.to_string(), format); }) - .await; - reply( - ctx.sender, - message.chat.id.0, - message.id, - if saved { + .await + { + Ok((_, true)) => { "Format set. Use /debug to preview the caption.".to_string() - } else { - "Set in memory only: the database write failed, so it will be lost on restart. Use /debug to preview the caption.".to_string() - }, - ) - .await?; + } + Ok((_, false)) => "Format set only in memory; retry later.".to_string(), + Err(()) => CHAT_STATE_READ_ERROR.to_string(), + }; + reply(ctx.sender, message.chat.id.0, message.id, text).await?; } Command::ClearCache(arg) => { let Some(sender_id) = require_admin(ctx, message).await? else { diff --git a/crates/xmedia-bot/src/handlers/mod.rs b/crates/xmedia-bot/src/handlers/mod.rs index d39ba11..eed85a9 100644 --- a/crates/xmedia-bot/src/handlers/mod.rs +++ b/crates/xmedia-bot/src/handlers/mod.rs @@ -145,7 +145,8 @@ async fn edit_message_handler( // (not yet swept) is dropped and the reply falls through to the normal // message flow instead of rewriting a caption from a dead prompt. if edit.created_at + ctx.config.edit_message_ttl.as_secs() as i64 <= crate::db::unix_now() { - ctx.chat_store + let _ = ctx + .chat_store .update(chat_id, |data| { data.edit_message.remove(&reply_to_message_id); }) diff --git a/crates/xmedia-bot/src/handlers/urls.rs b/crates/xmedia-bot/src/handlers/urls.rs index 049105b..93e776b 100644 --- a/crates/xmedia-bot/src/handlers/urls.rs +++ b/crates/xmedia-bot/src/handlers/urls.rs @@ -1009,7 +1009,8 @@ mod tests { data.forward_channel_id = Some(2); data.edit_before_forward = true; }) - .await; + .await + .unwrap(); } #[tokio::test] diff --git a/crates/xmedia-bot/src/main.rs b/crates/xmedia-bot/src/main.rs index 0919328..25d9e44 100644 --- a/crates/xmedia-bot/src/main.rs +++ b/crates/xmedia-bot/src/main.rs @@ -448,7 +448,8 @@ mod tests { }, ); }) - .await; + .await + .unwrap(); let (stop_tx, stop_rx) = watch::channel(false); let sweep = periodic_sweep(stores.ctx(&sender), stop_rx); diff --git a/crates/xmedia-bot/src/send/post_send.rs b/crates/xmedia-bot/src/send/post_send.rs index 0f78d48..1927448 100644 --- a/crates/xmedia-bot/src/send/post_send.rs +++ b/crates/xmedia-bot/src/send/post_send.rs @@ -301,7 +301,8 @@ pub(crate) async fn post_send_actions(ctx: &AppContext<'_>, task: &Task, message log_key(&source_url) ); let source_url = source_url.clone(); - ctx.chat_store + let _ = ctx + .chat_store .update(chat_id, move |data| { data.edit_message.insert( prompt_id, diff --git a/crates/xmedia-bot/src/state.rs b/crates/xmedia-bot/src/state.rs index 3628aef..8db9c8c 100644 --- a/crates/xmedia-bot/src/state.rs +++ b/crates/xmedia-bot/src/state.rs @@ -151,25 +151,17 @@ impl ChatStore { /// e.g. a second `edit_message` record. The per-chat lock makes the /// cycle atomic. Returns the closure's result plus whether the DB write /// landed (see [`Self::set`]); callers that do not care ignore the flag. - pub async fn update( + pub async fn update( &self, chat_id: i64, f: impl FnOnce(&mut ChatData) -> R, - ) -> (R, bool) { + ) -> Result<(R, bool), ()> { let lock = self.lock_for(chat_id); let _guard = lock.lock().await; - let mut data = match self.load(chat_id).await { - Ok(data) => data, - Err(()) => { - // Do not run the mutation closure on an empty fallback: a - // command could otherwise report success after the failed - // read and the next update could write those defaults back. - return (R::default(), false); - } - }; + let mut data = self.load(chat_id).await?; let r = f(&mut data); let saved = self.set(chat_id, &data).await; - (r, saved) + Ok((r, saved)) } /// Removes edit-before-forward records whose `created_at + ttl` is in the @@ -276,7 +268,8 @@ mod tests { }, ); }) - .await; + .await + .unwrap(); })); } for h in handles { @@ -312,7 +305,8 @@ mod tests { data.edit_message.insert(1, edit_entry(7, now - 3600)); data.edit_message.insert(2, edit_entry(7, now)); }) - .await; + .await + .unwrap(); let removed = store.prune_expired(Duration::from_secs(60)).await; @@ -339,7 +333,8 @@ mod tests { data.forward_channel_id = Some(-100); data.message_format.insert("twitter".into(), "{url}".into()); }) - .await; + .await + .unwrap(); assert!(store.cache.lock().contains_key(&9)); let removed = store.prune_expired(Duration::from_secs(60)).await; @@ -368,7 +363,8 @@ mod tests { .update(10, |data| { data.edit_message.insert(1, edit_entry(10, unix_now())); }) - .await; + .await + .unwrap(); store.prune_expired(Duration::from_secs(3600)).await; @@ -390,7 +386,8 @@ mod tests { data.template.insert("keep".into(), "[]".into()); data.edit_message.insert(1, edit_entry(8, 0)); }) - .await; + .await + .unwrap(); let removed = store.prune_expired(Duration::from_secs(60)).await; @@ -446,30 +443,32 @@ mod tests { let path = dir.path().join("update.db"); let pool = crate::db::open_store(path.to_str().unwrap()).unwrap(); let raw = rusqlite::Connection::open(&path).unwrap(); - let real = ChatData { - forward_channel_id: Some(42), - ..ChatData::default() - }; + let stored = "{\"forward_channel_id\":"; raw.execute( "INSERT INTO chat_state (chat_id, payload) VALUES ('7', ?1)", - rusqlite::params![serde_json::to_string(&real).unwrap()], + rusqlite::params![stored], ) .unwrap(); - raw.execute_batch("DROP TABLE chat_state").unwrap(); let store = ChatStore::new(pool); - let mut called = false; - let (result, saved) = store + let result = store .update(7, |data| { called = true; data.message_format.insert("twitter".into(), "{url}".into()); }) .await; - assert_eq!(result, ()); - assert!(!saved); + assert!(result.is_err()); assert!(!called, "a failed load must not run a destructive mutation"); assert!(!store.cache.lock().contains_key(&7)); + let payload: String = raw + .query_row( + "SELECT payload FROM chat_state WHERE chat_id='7'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(payload, stored, "the original row must remain unchanged"); } #[tokio::test]