From b10d54d6efa2e864f11e9dcc5f00fbf4985e03c1 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Thu, 12 Feb 2026 04:03:13 -0300 Subject: [PATCH 01/11] feat: Resend the last 10 messages to new broadcast member (#7678) Messages are sent and encrypted only to the new member. This way we at least postpone spreading the information that the new member joined: even if the server operator is a broadcast member, they can't know that immediately. --- deltachat-ffi/src/lib.rs | 7 +- deltachat-jsonrpc/src/api.rs | 12 +- deltachat-repl/src/cmdline.rs | 8 +- src/chat.rs | 200 ++++++++++++++++++++------- src/chat/chat_tests.rs | 52 ++++++- src/constants.rs | 3 + src/mimefactory.rs | 35 +++-- src/mimefactory/mimefactory_tests.rs | 8 +- src/test_utils.rs | 62 +++++---- 9 files changed, 275 insertions(+), 112 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 4b375ad4c5..6873846045 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -22,7 +22,7 @@ use std::sync::{Arc, LazyLock, Mutex}; use std::time::{Duration, SystemTime}; use anyhow::Context as _; -use deltachat::chat::{ChatId, ChatVisibility, MessageListOptions, MuteDuration}; +use deltachat::chat::{ChatId, ChatMsgsFilter, ChatVisibility, GetChatMsgsOptions, MuteDuration}; use deltachat::constants::DC_MSG_ID_LAST_SPECIAL; use deltachat::contact::{Contact, ContactId, Origin}; use deltachat::context::{Context, ContextBuilder}; @@ -1345,9 +1345,10 @@ pub unsafe extern "C" fn dc_get_chat_msgs( chat::get_chat_msgs_ex( ctx, ChatId::new(chat_id), - MessageListOptions { - info_only, + GetChatMsgsOptions { + filter: ChatMsgsFilter::info_only(info_only), add_daymarker, + ..Default::default() }, ) .await diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index 781faf452d..137ca8cd90 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -12,7 +12,7 @@ use deltachat::calls::ice_servers; use deltachat::chat::{ self, add_contact_to_chat, forward_msgs, forward_msgs_2ctx, get_chat_media, get_chat_msgs, get_chat_msgs_ex, markfresh_chat, marknoticed_all_chats, marknoticed_chat, - remove_contact_from_chat, Chat, ChatId, ChatItem, MessageListOptions, + remove_contact_from_chat, Chat, ChatId, ChatItem, ChatMsgsFilter, GetChatMsgsOptions, }; use deltachat::chatlist::Chatlist; use deltachat::config::{get_all_ui_config_keys, Config}; @@ -1382,9 +1382,10 @@ impl CommandApi { let msg = get_chat_msgs_ex( &ctx, ChatId::new(chat_id), - MessageListOptions { - info_only, + GetChatMsgsOptions { + filter: ChatMsgsFilter::info_only(info_only), add_daymarker, + ..Default::default() }, ) .await?; @@ -1428,9 +1429,10 @@ impl CommandApi { let msg = get_chat_msgs_ex( &ctx, ChatId::new(chat_id), - MessageListOptions { - info_only, + GetChatMsgsOptions { + filter: ChatMsgsFilter::info_only(info_only), add_daymarker, + ..Default::default() }, ) .await?; diff --git a/deltachat-repl/src/cmdline.rs b/deltachat-repl/src/cmdline.rs index 19ae991984..81f8b16b72 100644 --- a/deltachat-repl/src/cmdline.rs +++ b/deltachat-repl/src/cmdline.rs @@ -6,7 +6,9 @@ use std::str::FromStr; use std::time::Duration; use anyhow::{bail, ensure, Result}; -use deltachat::chat::{self, Chat, ChatId, ChatItem, ChatVisibility, MuteDuration}; +use deltachat::chat::{ + self, Chat, ChatId, ChatItem, ChatVisibility, GetChatMsgsOptions, MuteDuration, +}; use deltachat::chatlist::*; use deltachat::constants::*; use deltachat::contact::*; @@ -622,9 +624,9 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu let msglist = chat::get_chat_msgs_ex( &context, sel_chat.get_id(), - chat::MessageListOptions { - info_only: false, + GetChatMsgsOptions { add_daymarker: true, + ..Default::default() }, ) .await?; diff --git a/src/chat.rs b/src/chat.rs index 541e25dd5b..3a0ca30609 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -23,8 +23,9 @@ use crate::chatlist_events; use crate::color::str_to_color; use crate::config::Config; use crate::constants::{ - Blocked, Chattype, DC_CHAT_ID_ALLDONE_HINT, DC_CHAT_ID_ARCHIVED_LINK, DC_CHAT_ID_LAST_SPECIAL, - DC_CHAT_ID_TRASH, DC_RESEND_USER_AVATAR_DAYS, EDITED_PREFIX, TIMESTAMP_SENT_TOLERANCE, + self, Blocked, Chattype, DC_CHAT_ID_ALLDONE_HINT, DC_CHAT_ID_ARCHIVED_LINK, + DC_CHAT_ID_LAST_SPECIAL, DC_CHAT_ID_TRASH, DC_RESEND_USER_AVATAR_DAYS, EDITED_PREFIX, + TIMESTAMP_SENT_TOLERANCE, }; use crate::contact::{self, Contact, ContactId, Origin}; use crate::context::Context; @@ -34,7 +35,7 @@ use crate::download::{ }; use crate::ephemeral::{Timer as EphemeralTimer, start_chat_ephemeral_timers}; use crate::events::EventType; -use crate::key::self_fingerprint; +use crate::key::{Fingerprint, self_fingerprint}; use crate::location; use crate::log::{LogExt, warn}; use crate::logged_debug_assert; @@ -2753,7 +2754,7 @@ async fn prepare_send_msg( } chat.prepare_msg_raw(context, msg, update_msg_id).await?; - let row_ids = create_send_msg_jobs(context, msg) + let row_ids = create_send_msg_jobs(context, msg, None) .await .context("Failed to create send jobs")?; if !row_ids.is_empty() { @@ -2823,7 +2824,14 @@ async fn render_mime_message_and_pre_message( /// Returns row ids if `smtp` table jobs were created or an empty `Vec` otherwise. /// /// The caller has to interrupt SMTP loop or otherwise process new rows. -pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -> Result> { +/// +/// * `row_id` - Actual Message ID, if `Some`. This is to avoid updating the `msgs` row, in which +/// case `msg.id` is fake (`u32::MAX`); +pub(crate) async fn create_send_msg_jobs( + context: &Context, + msg: &mut Message, + row_id: Option, +) -> Result> { let cmd = msg.param.get_cmd(); if cmd == SystemMessage::GroupNameChanged || cmd == SystemMessage::GroupDescriptionChanged { msg.chat_id @@ -2840,7 +2848,7 @@ pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) - } let needs_encryption = msg.param.get_bool(Param::GuaranteeE2ee).unwrap_or_default(); - let mimefactory = match MimeFactory::from_msg(context, msg.clone()).await { + let mimefactory = match MimeFactory::from_msg(context, msg.clone(), row_id).await { Ok(mf) => mf, Err(err) => { // Mark message as failed @@ -3102,42 +3110,61 @@ async fn donation_request_maybe(context: &Context) -> Result<()> { .await } -/// Chat message list request options. -#[derive(Debug)] -pub struct MessageListOptions { - /// Return only info messages. - pub info_only: bool, +/// Controls which messages [`get_chat_msgs_ex`] returns. +#[derive(Debug, Default, PartialEq)] +pub enum ChatMsgsFilter { + /// All messages. + #[default] + All, + /// Info messages. + Info, + /// Non-info non-system messages. + NonInfoNonSystem, +} + +impl ChatMsgsFilter { + /// Returns filter capturing only info messages or all messages. + pub fn info_only(arg: bool) -> Self { + match arg { + true => Self::Info, + false => Self::All, + } + } +} + +/// [`get_chat_msgs_ex`] options. +#[derive(Debug, Default)] +pub struct GetChatMsgsOptions { + /// Which messages to return. + pub filter: ChatMsgsFilter, /// Add day markers before each date regarding the local timezone. pub add_daymarker: bool, + + /// If `Some(n)`, return up to `n` last (by timestamp) messages. + pub n_last: Option, } /// Returns all messages belonging to the chat. pub async fn get_chat_msgs(context: &Context, chat_id: ChatId) -> Result> { - get_chat_msgs_ex( - context, - chat_id, - MessageListOptions { - info_only: false, - add_daymarker: false, - }, - ) - .await + get_chat_msgs_ex(context, chat_id, Default::default()).await } /// Returns messages belonging to the chat according to the given options. +/// Older messages go first. #[expect(clippy::arithmetic_side_effects)] pub async fn get_chat_msgs_ex( context: &Context, chat_id: ChatId, - options: MessageListOptions, + options: GetChatMsgsOptions, ) -> Result> { - let MessageListOptions { - info_only, + let GetChatMsgsOptions { + filter, add_daymarker, + n_last, } = options; - let process_row = if info_only { - |row: &rusqlite::Row| { + let process_row = |row: &rusqlite::Row| { + if filter != ChatMsgsFilter::All { // is_info logic taken from Message.is_info() let params = row.get::<_, String>("param")?; let (from_id, to_id) = ( @@ -3157,15 +3184,13 @@ pub async fn get_chat_msgs_ex( Ok(( row.get::<_, i64>("timestamp")?, row.get::<_, MsgId>("id")?, - !is_info_msg, + is_info_msg == (filter == ChatMsgsFilter::Info), )) - } - } else { - |row: &rusqlite::Row| { + } else { Ok(( row.get::<_, i64>("timestamp")?, row.get::<_, MsgId>("id")?, - false, + true, )) } }; @@ -3174,8 +3199,8 @@ pub async fn get_chat_msgs_ex( // let sqlite execute an ORDER BY clause. let mut sorted_rows = Vec::new(); for row in rows { - let (ts, curr_id, exclude_message): (i64, MsgId, bool) = row?; - if !exclude_message { + let (ts, curr_id, include): (i64, MsgId, bool) = row?; + if include { sorted_rows.push((ts, curr_id)); } } @@ -3202,21 +3227,27 @@ pub async fn get_chat_msgs_ex( Ok(ret) }; - let items = if info_only { + let n_last_subst = match n_last { + Some(n) => format!("ORDER BY timestamp DESC, id DESC LIMIT {n}"), + None => "".to_string(), + }; + let items = if filter != ChatMsgsFilter::All { context .sql .query_map( - // GLOB is used here instead of LIKE because it is case-sensitive - "SELECT m.id AS id, m.timestamp AS timestamp, m.param AS param, m.from_id AS from_id, m.to_id AS to_id - FROM msgs m - WHERE m.chat_id=? - AND m.hidden=0 - AND ( - m.param GLOB '*\nS=*' OR param GLOB 'S=*' - OR m.from_id == ? - OR m.to_id == ? - );", - (chat_id, ContactId::INFO, ContactId::INFO), + &format!(" +SELECT m.id AS id, m.timestamp AS timestamp, m.param AS param, m.from_id AS from_id, m.to_id AS to_id +FROM msgs m +WHERE m.chat_id=? + AND m.hidden=0 + AND ?=( + m.param GLOB '*\nS=*' OR param GLOB 'S=*' + OR m.from_id == ? + OR m.to_id == ? + ) +{n_last_subst}" + ), + (chat_id, filter == ChatMsgsFilter::Info, ContactId::INFO, ContactId::INFO), process_row, process_rows, ) @@ -3225,10 +3256,14 @@ pub async fn get_chat_msgs_ex( context .sql .query_map( - "SELECT m.id AS id, m.timestamp AS timestamp - FROM msgs m - WHERE m.chat_id=? - AND m.hidden=0;", + &format!( + " +SELECT m.id AS id, m.timestamp AS timestamp +FROM msgs m +WHERE m.chat_id=? + AND m.hidden=0 +{n_last_subst}" + ), (chat_id,), process_row, process_rows, @@ -4009,6 +4044,29 @@ pub(crate) async fn add_contact_to_chat_ex( if sync.into() { chat.sync_contacts(context).await.log_err(context).ok(); } + let resend_last_msgs = || async { + let items = get_chat_msgs_ex( + context, + chat.id, + GetChatMsgsOptions { + filter: ChatMsgsFilter::NonInfoNonSystem, + n_last: Some(constants::N_MSGS_TO_NEW_BROADCAST_MEMBER), + ..Default::default() + }, + ) + .await?; + let msgs: Vec<_> = items + .into_iter() + .filter_map(|i| match i { + ChatItem::Message { msg_id } => Some(msg_id), + _ => None, + }) + .collect(); + resend_msgs_ex(context, &msgs, contact.fingerprint()).await + }; + if chat.typ == Chattype::OutBroadcast { + resend_last_msgs().await.log_err(context).ok(); + } Ok(true) } @@ -4566,7 +4624,10 @@ pub async fn forward_msgs_2ctx( chat.prepare_msg_raw(ctx_dst, &mut msg, None).await?; curr_timestamp += 1; - if !create_send_msg_jobs(ctx_dst, &mut msg).await?.is_empty() { + if !create_send_msg_jobs(ctx_dst, &mut msg, None) + .await? + .is_empty() + { ctx_dst.scheduler.interrupt_smtp().await; } created_msgs.push(msg.id); @@ -4675,10 +4736,28 @@ pub(crate) async fn save_copy_in_self_talk( Ok(msg.rfc724_mid) } -/// Resends given messages with the same Message-ID. +/// Resends given messages to members of the corresponding chats. /// /// This is primarily intended to make existing webxdcs available to new chat members. pub async fn resend_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> { + resend_msgs_ex(context, msg_ids, None).await +} + +/// Resends given messages to a contact with fingerprint `to_fingerprint` or, if it's `None`, to +/// members of the corresponding chats. +/// +/// NB: Actually `to_fingerprint` is only passed for `OutBroadcast` chats when a new member is +/// added. Currently webxdc status updates are re-sent to all broadcast members instead of the +/// requested contact, but this will be changed soon: webxdc status updates won't be re-sent at all +/// as they may contain confidential data sent by subscribers to the owner. Of course this may also +/// happen without resending subscribers' status updates if a webxdc app is "unsafe" for use in +/// broadcasts on its own, but this is a separate problem. +pub(crate) async fn resend_msgs_ex( + context: &Context, + msg_ids: &[MsgId], + to_fingerprint: Option, +) -> Result<()> { + let to_fingerprint = to_fingerprint.map(|f| f.hex()); let mut msgs: Vec = Vec::new(); for msg_id in msg_ids { let msg = Message::load_from_db(context, *msg_id).await?; @@ -4697,11 +4776,25 @@ pub async fn resend_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> { | MessageState::OutFailed | MessageState::OutDelivered | MessageState::OutMdnRcvd => { - message::update_msg_state(context, msg.id, MessageState::OutPending).await? + // Broadcast owners shouldn't see spinners on messages being auto-re-sent to new + // subscribers (otherwise big channel owners will see spinners most of the time). + if to_fingerprint.is_none() { + message::update_msg_state(context, msg.id, MessageState::OutPending).await?; + } } msg_state => bail!("Unexpected message state {msg_state}"), } - if create_send_msg_jobs(context, &mut msg).await?.is_empty() { + let mut row_id = None; + if let Some(to_fingerprint) = &to_fingerprint { + msg.param.set(Param::Arg4, to_fingerprint.clone()); + // Avoid updating the `msgs` row. + row_id = Some(msg.id); + msg.id = MsgId::new(u32::MAX); + } + if create_send_msg_jobs(context, &mut msg, row_id) + .await? + .is_empty() + { continue; } @@ -4712,7 +4805,8 @@ pub async fn resend_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> { chat_id: msg.chat_id, msg_id: msg.id, }); - // note(treefit): only matters if it is the last message in chat (but probably to expensive to check, debounce also solves it) + // The event only matters if the message is last in the chat. + // But it's probably too expensive check, and UIs anyways need to debounce. chatlist_events::emit_chatlist_item_changed(context, msg.chat_id); if msg.viewtype == Viewtype::Webxdc { diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 75f8b55327..e3875283c1 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use super::*; use crate::Event; use crate::chatlist::get_archived_cnt; -use crate::constants::{DC_GCL_ARCHIVED_ONLY, DC_GCL_NO_SPECIALS}; +use crate::constants::{DC_GCL_ARCHIVED_ONLY, DC_GCL_NO_SPECIALS, N_MSGS_TO_NEW_BROADCAST_MEMBER}; use crate::ephemeral::Timer; use crate::headerdef::HeaderDef; use crate::imex::{ImexMode, has_backup, imex}; @@ -2947,6 +2947,56 @@ async fn test_broadcast_change_name() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_broadcast_resend_to_new_member() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let fiona = &tcm.fiona().await; + + let alice_bc_id = create_broadcast(alice, "Channel".to_string()).await?; + let qr = get_securejoin_qr(alice, Some(alice_bc_id)).await.unwrap(); + + tcm.exec_securejoin_qr(bob, alice, &qr).await; + let mut alice_msg_ids = Vec::new(); + for i in 0..(N_MSGS_TO_NEW_BROADCAST_MEMBER + 1) { + alice_msg_ids.push( + alice + .send_text(alice_bc_id, &i.to_string()) + .await + .sender_msg_id, + ); + } + let fiona_bc_id = tcm.exec_securejoin_qr(fiona, alice, &qr).await; + for msg_id in alice_msg_ids { + assert_eq!(msg_id.get_state(alice).await?, MessageState::OutDelivered); + } + for i in 0..N_MSGS_TO_NEW_BROADCAST_MEMBER { + let rev_order = false; + let resent_msg = alice + .pop_sent_msg_ex(rev_order, Duration::ZERO) + .await + .unwrap(); + let fiona_msg = fiona.recv_msg(&resent_msg).await; + assert_eq!(fiona_msg.chat_id, fiona_bc_id); + assert_eq!(fiona_msg.text, (i + 1).to_string()); + assert!(resent_msg.recipients.contains("fiona@example.net")); + assert!(!resent_msg.recipients.contains("bob@")); + // The message is undecryptable for Bob, he mustn't be able to know yet that somebody joined + // the broadcast even if he is a postman in this land. E.g. Fiona may leave after fetching + // the news, Bob won't know about that. + assert!( + MimeMessage::from_bytes(bob, resent_msg.payload().as_bytes()) + .await? + .decryption_error + .is_some() + ); + bob.recv_msg_trash(&resent_msg).await; + } + assert!(alice.pop_sent_msg_opt(Duration::ZERO).await.is_none()); + Ok(()) +} + /// - Alice has multiple devices /// - Alice creates a broadcast and sends a message into it /// - Alice's second device sees the broadcast diff --git a/src/constants.rs b/src/constants.rs index fe6d11c56c..fe3a50028f 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -244,6 +244,9 @@ Here is what to do: If you have any questions, please send an email to delta@merlinux.eu or ask at https://support.delta.chat/."#; +/// How many recent messages should be re-sent to a new broadcast member. +pub(crate) const N_MSGS_TO_NEW_BROADCAST_MEMBER: usize = 10; + #[cfg(test)] mod tests { use num_traits::FromPrimitive; diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 1d643e8422..1071a8add3 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -194,8 +194,16 @@ fn new_address_with_name(name: &str, address: String) -> Address<'static> { } impl MimeFactory { + /// Returns `MimeFactory` for rendering `msg`. + /// + /// * `row_id` - Actual Message ID, if `Some`. This is to avoid updating the `msgs` row, in + /// which case `msg.id` is fake (`u32::MAX`); #[expect(clippy::arithmetic_side_effects)] - pub async fn from_msg(context: &Context, msg: Message) -> Result { + pub async fn from_msg( + context: &Context, + msg: Message, + row_id: Option, + ) -> Result { let now = time(); let chat = Chat::load_from_db(context, msg.chat_id).await?; let attach_profile_data = Self::should_attach_profile_data(&msg); @@ -505,12 +513,13 @@ impl MimeFactory { }; } + let msg_id = row_id.unwrap_or(msg.id); let (in_reply_to, references) = context .sql .query_row( "SELECT mime_in_reply_to, IFNULL(mime_references, '') FROM msgs WHERE id=?", - (msg.id,), + (msg_id,), |row| { let in_reply_to: String = row.get(0)?; let references: String = row.get(1)?; @@ -2225,18 +2234,18 @@ fn should_encrypt_symmetrically(msg: &Message, chat: &Chat) -> bool { /// rather than all recipients. /// This function returns the fingerprint of the recipient the message should be sent to. fn must_have_only_one_recipient<'a>(msg: &'a Message, chat: &Chat) -> Option> { - if chat.typ == Chattype::OutBroadcast - && matches!( - msg.param.get_cmd(), - SystemMessage::MemberRemovedFromGroup | SystemMessage::MemberAddedToGroup - ) - { - let Some(fp) = msg.param.get(Param::Arg4) else { - return Some(Err(format_err!("Missing removed/added member"))); - }; - return Some(Ok(fp)); + if chat.typ != Chattype::OutBroadcast { + None + } else if let Some(fp) = msg.param.get(Param::Arg4) { + Some(Ok(fp)) + } else if matches!( + msg.param.get_cmd(), + SystemMessage::MemberRemovedFromGroup | SystemMessage::MemberAddedToGroup + ) { + Some(Err(format_err!("Missing removed/added member"))) + } else { + None } - None } async fn build_body_file(context: &Context, msg: &Message) -> Result> { diff --git a/src/mimefactory/mimefactory_tests.rs b/src/mimefactory/mimefactory_tests.rs index fe03953a91..9d981e0a1f 100644 --- a/src/mimefactory/mimefactory_tests.rs +++ b/src/mimefactory/mimefactory_tests.rs @@ -275,7 +275,7 @@ async fn test_subject_mdn() { chat::send_msg(&t, new_msg.chat_id, &mut new_msg) .await .unwrap(); - let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap(); + let mf = MimeFactory::from_msg(&t, new_msg, None).await.unwrap(); // The subject string should not be "Re: message opened" assert_eq!("Re: Hello, Bob", mf.subject_str(&t).await.unwrap()); } @@ -412,7 +412,7 @@ async fn first_subject_str(t: TestContext) -> String { new_msg.chat_id = chat_id; chat::send_msg(&t, chat_id, &mut new_msg).await.unwrap(); - let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap(); + let mf = MimeFactory::from_msg(&t, new_msg, None).await.unwrap(); mf.subject_str(&t).await.unwrap() } @@ -500,7 +500,7 @@ async fn msg_to_subject_str_inner( chat::send_msg(&t, new_msg.chat_id, &mut new_msg) .await .unwrap(); - let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap(); + let mf = MimeFactory::from_msg(&t, new_msg, None).await.unwrap(); mf.subject_str(&t).await.unwrap() } @@ -545,7 +545,7 @@ async fn test_render_reply() { .await; chat::send_msg(&t, msg.chat_id, &mut msg).await.unwrap(); - let mimefactory = MimeFactory::from_msg(&t, msg).await.unwrap(); + let mimefactory = MimeFactory::from_msg(&t, msg, None).await.unwrap(); let recipients = mimefactory.recipients(); assert_eq!(recipients, vec!["charlie@example.com"]); diff --git a/src/test_utils.rs b/src/test_utils.rs index 1329dcc510..aebde20277 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -21,9 +21,7 @@ use tempfile::{TempDir, tempdir}; use tokio::runtime::Handle; use tokio::{fs, task}; -use crate::chat::{ - self, Chat, ChatId, ChatIdBlocked, MessageListOptions, add_to_chat_contacts_table, create_group, -}; +use crate::chat::{self, Chat, ChatId, ChatIdBlocked, add_to_chat_contacts_table, create_group}; use crate::chatlist::Chatlist; use crate::config::Config; use crate::constants::{Blocked, Chattype}; @@ -275,16 +273,17 @@ impl TestContextManager { let chat_id = join_securejoin(&joiner.ctx, qr).await.unwrap(); - loop { + for _ in 0..2 { let mut something_sent = false; - if let Some(sent) = joiner.pop_sent_msg_opt(Duration::ZERO).await { + let rev_order = false; + if let Some(sent) = joiner.pop_sent_msg_ex(rev_order, Duration::ZERO).await { for inviter in inviters { inviter.recv_msg_opt(&sent).await; } something_sent = true; } for inviter in inviters { - if let Some(sent) = inviter.pop_sent_msg_opt(Duration::ZERO).await { + if let Some(sent) = inviter.pop_sent_msg_ex(rev_order, Duration::ZERO).await { joiner.recv_msg_opt(&sent).await; something_sent = true; } @@ -623,25 +622,35 @@ impl TestContext { } pub async fn pop_sent_msg_opt(&self, timeout: Duration) -> Option> { + let rev_order = true; + self.pop_sent_msg_ex(rev_order, timeout).await + } + + pub async fn pop_sent_msg_ex( + &self, + rev_order: bool, + timeout: Duration, + ) -> Option> { let start = Instant::now(); + let mut query = " +SELECT id, msg_id, mime, recipients +FROM smtp +ORDER BY id" + .to_string(); + if rev_order { + query += " DESC"; + } let (rowid, msg_id, payload, recipients) = loop { let row = self .ctx .sql - .query_row_optional( - r#" - SELECT id, msg_id, mime, recipients - FROM smtp - ORDER BY id DESC"#, - (), - |row| { - let rowid: i64 = row.get(0)?; - let msg_id: MsgId = row.get(1)?; - let mime: String = row.get(2)?; - let recipients: String = row.get(3)?; - Ok((rowid, msg_id, mime, recipients)) - }, - ) + .query_row_optional(&query, (), |row| { + let rowid: i64 = row.get(0)?; + let msg_id: MsgId = row.get(1)?; + let mime: String = row.get(2)?; + let recipients: String = row.get(3)?; + Ok((rowid, msg_id, mime, recipients)) + }) .await .expect("query_row_optional failed"); if let Some(row) = row { @@ -1083,16 +1092,9 @@ impl TestContext { async fn display_chat(&self, chat_id: ChatId) -> String { let mut res = String::new(); - let msglist = chat::get_chat_msgs_ex( - self, - chat_id, - MessageListOptions { - info_only: false, - add_daymarker: false, - }, - ) - .await - .unwrap(); + let msglist = chat::get_chat_msgs_ex(self, chat_id, Default::default()) + .await + .unwrap(); let msglist: Vec = msglist .into_iter() .filter_map(|x| match x { From cc818d9099bc747247250b614f1d61f83b236f3f Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 14:51:30 +0200 Subject: [PATCH 02/11] Document that Param::Arg4 is also used for resent messages --- src/param.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/param.rs b/src/param.rs index dfd48d31fc..087bba93ea 100644 --- a/src/param.rs +++ b/src/param.rs @@ -136,6 +136,10 @@ pub enum Param { /// For "MemberAddedToGroup" and "MemberRemovedFromGroup", /// this is the fingerprint added to / removed from the group. /// + /// For messages resent when adding a new member to a broadcast channel, + /// this is the fingerprint of the added member; + /// the message must only be sent to this one member then. + /// /// For call messages, this is the end timsetamp. Arg4 = b'H', From ceb10869c98fee416458ce29f1435a36bf99c25d Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 15:23:19 +0200 Subject: [PATCH 03/11] Start extracting the logic for getting the last n messages into its own function --- deltachat-ffi/src/lib.rs | 7 +- deltachat-jsonrpc/src/api.rs | 12 +- deltachat-repl/src/cmdline.rs | 6 +- src/chat.rs | 206 ++++++++++++++++++++-------------- src/mimefactory.rs | 1 + src/test_utils.rs | 17 ++- 6 files changed, 146 insertions(+), 103 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 6873846045..4b375ad4c5 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -22,7 +22,7 @@ use std::sync::{Arc, LazyLock, Mutex}; use std::time::{Duration, SystemTime}; use anyhow::Context as _; -use deltachat::chat::{ChatId, ChatMsgsFilter, ChatVisibility, GetChatMsgsOptions, MuteDuration}; +use deltachat::chat::{ChatId, ChatVisibility, MessageListOptions, MuteDuration}; use deltachat::constants::DC_MSG_ID_LAST_SPECIAL; use deltachat::contact::{Contact, ContactId, Origin}; use deltachat::context::{Context, ContextBuilder}; @@ -1345,10 +1345,9 @@ pub unsafe extern "C" fn dc_get_chat_msgs( chat::get_chat_msgs_ex( ctx, ChatId::new(chat_id), - GetChatMsgsOptions { - filter: ChatMsgsFilter::info_only(info_only), + MessageListOptions { + info_only, add_daymarker, - ..Default::default() }, ) .await diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index 137ca8cd90..781faf452d 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -12,7 +12,7 @@ use deltachat::calls::ice_servers; use deltachat::chat::{ self, add_contact_to_chat, forward_msgs, forward_msgs_2ctx, get_chat_media, get_chat_msgs, get_chat_msgs_ex, markfresh_chat, marknoticed_all_chats, marknoticed_chat, - remove_contact_from_chat, Chat, ChatId, ChatItem, ChatMsgsFilter, GetChatMsgsOptions, + remove_contact_from_chat, Chat, ChatId, ChatItem, MessageListOptions, }; use deltachat::chatlist::Chatlist; use deltachat::config::{get_all_ui_config_keys, Config}; @@ -1382,10 +1382,9 @@ impl CommandApi { let msg = get_chat_msgs_ex( &ctx, ChatId::new(chat_id), - GetChatMsgsOptions { - filter: ChatMsgsFilter::info_only(info_only), + MessageListOptions { + info_only, add_daymarker, - ..Default::default() }, ) .await?; @@ -1429,10 +1428,9 @@ impl CommandApi { let msg = get_chat_msgs_ex( &ctx, ChatId::new(chat_id), - GetChatMsgsOptions { - filter: ChatMsgsFilter::info_only(info_only), + MessageListOptions { + info_only, add_daymarker, - ..Default::default() }, ) .await?; diff --git a/deltachat-repl/src/cmdline.rs b/deltachat-repl/src/cmdline.rs index 81f8b16b72..445ea95f76 100644 --- a/deltachat-repl/src/cmdline.rs +++ b/deltachat-repl/src/cmdline.rs @@ -7,7 +7,7 @@ use std::time::Duration; use anyhow::{bail, ensure, Result}; use deltachat::chat::{ - self, Chat, ChatId, ChatItem, ChatVisibility, GetChatMsgsOptions, MuteDuration, + self, Chat, ChatId, ChatItem, ChatVisibility, MessageListOptions, MuteDuration, }; use deltachat::chatlist::*; use deltachat::constants::*; @@ -624,9 +624,9 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu let msglist = chat::get_chat_msgs_ex( &context, sel_chat.get_id(), - GetChatMsgsOptions { + MessageListOptions { + info_only: false, add_daymarker: true, - ..Default::default() }, ) .await?; diff --git a/src/chat.rs b/src/chat.rs index 3a0ca30609..b1cc23686f 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3110,44 +3110,27 @@ async fn donation_request_maybe(context: &Context) -> Result<()> { .await } -/// Controls which messages [`get_chat_msgs_ex`] returns. -#[derive(Debug, Default, PartialEq)] -pub enum ChatMsgsFilter { - /// All messages. - #[default] - All, - /// Info messages. - Info, - /// Non-info non-system messages. - NonInfoNonSystem, -} - -impl ChatMsgsFilter { - /// Returns filter capturing only info messages or all messages. - pub fn info_only(arg: bool) -> Self { - match arg { - true => Self::Info, - false => Self::All, - } - } -} - -/// [`get_chat_msgs_ex`] options. -#[derive(Debug, Default)] -pub struct GetChatMsgsOptions { - /// Which messages to return. - pub filter: ChatMsgsFilter, +/// Chat message list request options. +#[derive(Debug)] +pub struct MessageListOptions { + /// Return only info messages. + pub info_only: bool, /// Add day markers before each date regarding the local timezone. pub add_daymarker: bool, - - /// If `Some(n)`, return up to `n` last (by timestamp) messages. - pub n_last: Option, } /// Returns all messages belonging to the chat. pub async fn get_chat_msgs(context: &Context, chat_id: ChatId) -> Result> { - get_chat_msgs_ex(context, chat_id, Default::default()).await + get_chat_msgs_ex( + context, + chat_id, + MessageListOptions { + info_only: false, + add_daymarker: false, + }, + ) + .await } /// Returns messages belonging to the chat according to the given options. @@ -3156,15 +3139,15 @@ pub async fn get_chat_msgs(context: &Context, chat_id: ChatId) -> Result Result> { - let GetChatMsgsOptions { - filter, + let MessageListOptions { + info_only, add_daymarker, - n_last, } = options; - let process_row = |row: &rusqlite::Row| { - if filter != ChatMsgsFilter::All { + // TODO: Remove `info_only` parameter; it's not used by anything + let process_row = if info_only { + |row: &rusqlite::Row| { // is_info logic taken from Message.is_info() let params = row.get::<_, String>("param")?; let (from_id, to_id) = ( @@ -3184,13 +3167,15 @@ pub async fn get_chat_msgs_ex( Ok(( row.get::<_, i64>("timestamp")?, row.get::<_, MsgId>("id")?, - is_info_msg == (filter == ChatMsgsFilter::Info), + !is_info_msg, )) - } else { + } + } else { + |row: &rusqlite::Row| { Ok(( row.get::<_, i64>("timestamp")?, row.get::<_, MsgId>("id")?, - true, + false, )) } }; @@ -3199,8 +3184,8 @@ pub async fn get_chat_msgs_ex( // let sqlite execute an ORDER BY clause. let mut sorted_rows = Vec::new(); for row in rows { - let (ts, curr_id, include): (i64, MsgId, bool) = row?; - if include { + let (ts, curr_id, exclude_message): (i64, MsgId, bool) = row?; + if !exclude_message { sorted_rows.push((ts, curr_id)); } } @@ -3227,27 +3212,21 @@ pub async fn get_chat_msgs_ex( Ok(ret) }; - let n_last_subst = match n_last { - Some(n) => format!("ORDER BY timestamp DESC, id DESC LIMIT {n}"), - None => "".to_string(), - }; - let items = if filter != ChatMsgsFilter::All { + let items = if info_only { context .sql .query_map( - &format!(" -SELECT m.id AS id, m.timestamp AS timestamp, m.param AS param, m.from_id AS from_id, m.to_id AS to_id -FROM msgs m -WHERE m.chat_id=? - AND m.hidden=0 - AND ?=( - m.param GLOB '*\nS=*' OR param GLOB 'S=*' - OR m.from_id == ? - OR m.to_id == ? - ) -{n_last_subst}" - ), - (chat_id, filter == ChatMsgsFilter::Info, ContactId::INFO, ContactId::INFO), + // GLOB is used here instead of LIKE because it is case-sensitive + "SELECT m.id AS id, m.timestamp AS timestamp, m.param AS param, m.from_id AS from_id, m.to_id AS to_id + FROM msgs m + WHERE m.chat_id=? + AND m.hidden=0 + AND ( + m.param GLOB '*\nS=*' OR param GLOB 'S=*' + OR m.from_id == ? + OR m.to_id == ? + );", + (chat_id, ContactId::INFO, ContactId::INFO), process_row, process_rows, ) @@ -3256,14 +3235,10 @@ WHERE m.chat_id=? context .sql .query_map( - &format!( - " -SELECT m.id AS id, m.timestamp AS timestamp -FROM msgs m -WHERE m.chat_id=? - AND m.hidden=0 -{n_last_subst}" - ), + "SELECT m.id AS id, m.timestamp AS timestamp + FROM msgs m + WHERE m.chat_id=? + AND m.hidden=0;", (chat_id,), process_row, process_rows, @@ -3303,6 +3278,81 @@ pub async fn marknoticed_all_chats(context: &Context) -> Result<()> { Ok(()) } +/// TODO +pub(crate) async fn get_msgs_for_resending( + context: &Context, + chat_id: ChatId, + n_last: usize, +) -> Result> { + let process_row = |row: &rusqlite::Row| { + // is_info logic taken from Message::is_info() + let params = row.get::<_, String>("param")?; + let (from_id, to_id) = ( + row.get::<_, ContactId>("from_id")?, + row.get::<_, ContactId>("to_id")?, + ); + // TODO probably we don't actually need to check `is_info_msg` here, + // because the SQL statement does it for us? + let is_info_msg: bool = from_id == ContactId::INFO + || to_id == ContactId::INFO + || match Params::from_str(¶ms) { + Ok(p) => { + let cmd = p.get_cmd(); + cmd != SystemMessage::Unknown && cmd != SystemMessage::AutocryptSetupMessage + } + _ => false, + }; + + Ok(( + row.get::<_, i64>("timestamp")?, + row.get::<_, MsgId>("id")?, + !is_info_msg, + )) + }; + let process_rows = |rows: rusqlite::AndThenRows<_>| { + let mut filtered_rows = Vec::new(); + for row in rows { + let (ts, curr_id, include): (i64, MsgId, bool) = row?; + if include { + filtered_rows.push((ts, curr_id)); + } + } + + let mut ret = Vec::new(); + let mut last_day = 0; + let cnv_to_local = gm2local_offset(); + + for (ts, curr_id) in filtered_rows.into_iter().rev() { + ret.push(curr_id); + } + Ok(ret) + }; + + let items = + context + .sql + .query_map( + &format!(" +SELECT m.id AS id, m.timestamp AS timestamp, m.param AS param, m.from_id AS from_id, m.to_id AS to_id +FROM msgs m +WHERE m.chat_id=? + AND m.hidden=0 + AND NOT ( -- Exclude info and system messages + m.param GLOB '*\nS=*' OR param GLOB 'S=*' + OR m.from_id=? + OR m.to_id=? + ) +ORDER BY timestamp DESC, id DESC LIMIT ?" + ), + (chat_id, ContactId::INFO, ContactId::INFO, n_last), + process_row, + process_rows, + ) + .await? + ; + Ok(items) +} + /// Marks all messages in the chat as noticed. /// If the given chat-id is the archive-link, marks all messages in all archived chats as noticed. pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()> { @@ -4045,23 +4095,9 @@ pub(crate) async fn add_contact_to_chat_ex( chat.sync_contacts(context).await.log_err(context).ok(); } let resend_last_msgs = || async { - let items = get_chat_msgs_ex( - context, - chat.id, - GetChatMsgsOptions { - filter: ChatMsgsFilter::NonInfoNonSystem, - n_last: Some(constants::N_MSGS_TO_NEW_BROADCAST_MEMBER), - ..Default::default() - }, - ) - .await?; - let msgs: Vec<_> = items - .into_iter() - .filter_map(|i| match i { - ChatItem::Message { msg_id } => Some(msg_id), - _ => None, - }) - .collect(); + let msgs = + get_msgs_for_resending(context, chat.id, constants::N_MSGS_TO_NEW_BROADCAST_MEMBER) + .await?; resend_msgs_ex(context, &msgs, contact.fingerprint()).await }; if chat.typ == Chattype::OutBroadcast { diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 1071a8add3..c6f1824717 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -2236,6 +2236,7 @@ fn should_encrypt_symmetrically(msg: &Message, chat: &Chat) -> bool { fn must_have_only_one_recipient<'a>(msg: &'a Message, chat: &Chat) -> Option> { if chat.typ != Chattype::OutBroadcast { None + // TODO problem here is that Param::Arg4 may be the end timestamp of a call } else if let Some(fp) = msg.param.get(Param::Arg4) { Some(Ok(fp)) } else if matches!( diff --git a/src/test_utils.rs b/src/test_utils.rs index aebde20277..d946d7a7ca 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -21,7 +21,9 @@ use tempfile::{TempDir, tempdir}; use tokio::runtime::Handle; use tokio::{fs, task}; -use crate::chat::{self, Chat, ChatId, ChatIdBlocked, add_to_chat_contacts_table, create_group}; +use crate::chat::{ + self, Chat, ChatId, ChatIdBlocked, MessageListOptions, add_to_chat_contacts_table, create_group, +}; use crate::chatlist::Chatlist; use crate::config::Config; use crate::constants::{Blocked, Chattype}; @@ -1092,9 +1094,16 @@ ORDER BY id" async fn display_chat(&self, chat_id: ChatId) -> String { let mut res = String::new(); - let msglist = chat::get_chat_msgs_ex(self, chat_id, Default::default()) - .await - .unwrap(); + let msglist = chat::get_chat_msgs_ex( + self, + chat_id, + MessageListOptions { + info_only: false, + add_daymarker: false, + }, + ) + .await + .unwrap(); let msglist: Vec = msglist .into_iter() .filter_map(|x| match x { From 6601495d392c9b73c3db95b03f36698b328017c6 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 15:35:59 +0200 Subject: [PATCH 04/11] Refactor and simplify code --- src/chat.rs | 86 ++++++++++++++--------------------------------------- 1 file changed, 22 insertions(+), 64 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index b1cc23686f..daf02214e1 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3282,74 +3282,32 @@ pub async fn marknoticed_all_chats(context: &Context) -> Result<()> { pub(crate) async fn get_msgs_for_resending( context: &Context, chat_id: ChatId, - n_last: usize, + n_msgs: usize, ) -> Result> { - let process_row = |row: &rusqlite::Row| { - // is_info logic taken from Message::is_info() - let params = row.get::<_, String>("param")?; - let (from_id, to_id) = ( - row.get::<_, ContactId>("from_id")?, - row.get::<_, ContactId>("to_id")?, - ); - // TODO probably we don't actually need to check `is_info_msg` here, - // because the SQL statement does it for us? - let is_info_msg: bool = from_id == ContactId::INFO - || to_id == ContactId::INFO - || match Params::from_str(¶ms) { - Ok(p) => { - let cmd = p.get_cmd(); - cmd != SystemMessage::Unknown && cmd != SystemMessage::AutocryptSetupMessage - } - _ => false, - }; - - Ok(( - row.get::<_, i64>("timestamp")?, - row.get::<_, MsgId>("id")?, - !is_info_msg, - )) - }; - let process_rows = |rows: rusqlite::AndThenRows<_>| { - let mut filtered_rows = Vec::new(); - for row in rows { - let (ts, curr_id, include): (i64, MsgId, bool) = row?; - if include { - filtered_rows.push((ts, curr_id)); - } - } - - let mut ret = Vec::new(); - let mut last_day = 0; - let cnv_to_local = gm2local_offset(); - - for (ts, curr_id) in filtered_rows.into_iter().rev() { - ret.push(curr_id); - } - Ok(ret) - }; - - let items = - context - .sql - .query_map( - &format!(" -SELECT m.id AS id, m.timestamp AS timestamp, m.param AS param, m.from_id AS from_id, m.to_id AS to_id -FROM msgs m -WHERE m.chat_id=? - AND m.hidden=0 + let items = context + .sql + .query_map_vec( + &format!( + " +SELECT id +FROM msgs +WHERE chat_id=? + AND hidden=0 AND NOT ( -- Exclude info and system messages - m.param GLOB '*\nS=*' OR param GLOB 'S=*' - OR m.from_id=? - OR m.to_id=? + param GLOB '*\nS=*' OR param GLOB 'S=*' + OR from_id=? + OR to_id=? ) ORDER BY timestamp DESC, id DESC LIMIT ?" - ), - (chat_id, ContactId::INFO, ContactId::INFO, n_last), - process_row, - process_rows, - ) - .await? - ; + ), + (chat_id, ContactId::INFO, ContactId::INFO, n_msgs), + |row: &rusqlite::Row| Ok(row.get::<_, MsgId>(0)?), + ) + .await? + .into_iter() + .rev() + .collect(); + Ok(items) } From 38d57ebb30be92b8b422f685a66f8e727bd8951d Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 15:55:45 +0200 Subject: [PATCH 05/11] For now, don't resend webxdc's at all --- src/chat.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index daf02214e1..aadf5456c9 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3298,9 +3298,16 @@ WHERE chat_id=? OR from_id=? OR to_id=? ) + AND viewtype!=? ORDER BY timestamp DESC, id DESC LIMIT ?" ), - (chat_id, ContactId::INFO, ContactId::INFO, n_msgs), + ( + chat_id, + ContactId::INFO, + ContactId::INFO, + Viewtype::Webxdc, + n_msgs, + ), |row: &rusqlite::Row| Ok(row.get::<_, MsgId>(0)?), ) .await? @@ -4741,11 +4748,9 @@ pub async fn resend_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> { /// members of the corresponding chats. /// /// NB: Actually `to_fingerprint` is only passed for `OutBroadcast` chats when a new member is -/// added. Currently webxdc status updates are re-sent to all broadcast members instead of the -/// requested contact, but this will be changed soon: webxdc status updates won't be re-sent at all -/// as they may contain confidential data sent by subscribers to the owner. Of course this may also -/// happen without resending subscribers' status updates if a webxdc app is "unsafe" for use in -/// broadcasts on its own, but this is a separate problem. +/// added. Regarding webxdc's: It is not trivial to resend only the own status updates, +/// and it is not trivial to resend them only to the newly-joined member, +/// so that for now, webxdc's are not resent at all. pub(crate) async fn resend_msgs_ex( context: &Context, msg_ids: &[MsgId], From d75c3231b9f0f41e77f2c79d28e2badc466d205b Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 16:02:38 +0200 Subject: [PATCH 06/11] Move function --- src/chat.rs | 80 ++++++++++++++++++++++++++--------------------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index aadf5456c9..857f7c1206 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3278,46 +3278,6 @@ pub async fn marknoticed_all_chats(context: &Context) -> Result<()> { Ok(()) } -/// TODO -pub(crate) async fn get_msgs_for_resending( - context: &Context, - chat_id: ChatId, - n_msgs: usize, -) -> Result> { - let items = context - .sql - .query_map_vec( - &format!( - " -SELECT id -FROM msgs -WHERE chat_id=? - AND hidden=0 - AND NOT ( -- Exclude info and system messages - param GLOB '*\nS=*' OR param GLOB 'S=*' - OR from_id=? - OR to_id=? - ) - AND viewtype!=? -ORDER BY timestamp DESC, id DESC LIMIT ?" - ), - ( - chat_id, - ContactId::INFO, - ContactId::INFO, - Viewtype::Webxdc, - n_msgs, - ), - |row: &rusqlite::Row| Ok(row.get::<_, MsgId>(0)?), - ) - .await? - .into_iter() - .rev() - .collect(); - - Ok(items) -} - /// Marks all messages in the chat as noticed. /// If the given chat-id is the archive-link, marks all messages in all archived chats as noticed. pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()> { @@ -4071,6 +4031,46 @@ pub(crate) async fn add_contact_to_chat_ex( Ok(true) } +/// TODO +async fn get_msgs_for_resending( + context: &Context, + chat_id: ChatId, + n_msgs: usize, +) -> Result> { + let items = context + .sql + .query_map_vec( + &format!( + " +SELECT id +FROM msgs +WHERE chat_id=? + AND hidden=0 + AND NOT ( -- Exclude info and system messages + param GLOB '*\nS=*' OR param GLOB 'S=*' + OR from_id=? + OR to_id=? + ) + AND viewtype!=? +ORDER BY timestamp DESC, id DESC LIMIT ?" + ), + ( + chat_id, + ContactId::INFO, + ContactId::INFO, + Viewtype::Webxdc, + n_msgs, + ), + |row: &rusqlite::Row| Ok(row.get::<_, MsgId>(0)?), + ) + .await? + .into_iter() + .rev() + .collect(); + + Ok(items) +} + /// Returns true if an avatar should be attached in the given chat. /// /// This function does not check if the avatar is set. From 524db830f1bf4b285895164da67d6339dbbc5f0d Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 16:08:39 +0200 Subject: [PATCH 07/11] refactor: Extract function resend_last_msgs() --- src/chat.rs | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 857f7c1206..384cea4239 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4019,25 +4019,22 @@ pub(crate) async fn add_contact_to_chat_ex( if sync.into() { chat.sync_contacts(context).await.log_err(context).ok(); } - let resend_last_msgs = || async { - let msgs = - get_msgs_for_resending(context, chat.id, constants::N_MSGS_TO_NEW_BROADCAST_MEMBER) - .await?; - resend_msgs_ex(context, &msgs, contact.fingerprint()).await - }; if chat.typ == Chattype::OutBroadcast { - resend_last_msgs().await.log_err(context).ok(); + resend_last_msgs(context, &chat, &contact) + .await + .log_err(context) + .ok(); } Ok(true) } -/// TODO -async fn get_msgs_for_resending( +async fn resend_last_msgs( context: &Context, - chat_id: ChatId, - n_msgs: usize, -) -> Result> { - let items = context + chat: &Chat, + to_contact: &Contact, +) -> std::result::Result<(), anyhow::Error> { + let chat_id = chat.id; + let msgs: Vec = context .sql .query_map_vec( &format!( @@ -4059,7 +4056,7 @@ ORDER BY timestamp DESC, id DESC LIMIT ?" ContactId::INFO, ContactId::INFO, Viewtype::Webxdc, - n_msgs, + constants::N_MSGS_TO_NEW_BROADCAST_MEMBER, ), |row: &rusqlite::Row| Ok(row.get::<_, MsgId>(0)?), ) @@ -4067,8 +4064,7 @@ ORDER BY timestamp DESC, id DESC LIMIT ?" .into_iter() .rev() .collect(); - - Ok(items) + resend_msgs_ex(context, &msgs, to_contact.fingerprint()).await } /// Returns true if an avatar should be attached in the given chat. @@ -4750,7 +4746,7 @@ pub async fn resend_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> { /// NB: Actually `to_fingerprint` is only passed for `OutBroadcast` chats when a new member is /// added. Regarding webxdc's: It is not trivial to resend only the own status updates, /// and it is not trivial to resend them only to the newly-joined member, -/// so that for now, webxdc's are not resent at all. +/// so that for now, [`resend_last_msgs`] does not automatically resend webxdc's at all. pub(crate) async fn resend_msgs_ex( context: &Context, msg_ids: &[MsgId], From e7d0687d907536bb9a46278cb4fd121c86faa2e4 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 16:15:21 +0200 Subject: [PATCH 08/11] Don't set fake `msg_id` in resent messages Setting the msg_id to `u32::MAX` is hacky, and may just as well break things as it may fix things, because some code may use the msg.id to load information from the database, like `get_iroh_topic_for_msg()`. From reading the code, I couldn't find any problem with leaving the correct `msg_id`, and if there is one, then we should add a function parameter `is_resending` that is checked in the corresponding places. --- deltachat-repl/src/cmdline.rs | 6 ++--- src/chat.rs | 33 +++++----------------------- src/mimefactory.rs | 12 ++-------- src/mimefactory/mimefactory_tests.rs | 8 +++---- 4 files changed, 14 insertions(+), 45 deletions(-) diff --git a/deltachat-repl/src/cmdline.rs b/deltachat-repl/src/cmdline.rs index 445ea95f76..19ae991984 100644 --- a/deltachat-repl/src/cmdline.rs +++ b/deltachat-repl/src/cmdline.rs @@ -6,9 +6,7 @@ use std::str::FromStr; use std::time::Duration; use anyhow::{bail, ensure, Result}; -use deltachat::chat::{ - self, Chat, ChatId, ChatItem, ChatVisibility, MessageListOptions, MuteDuration, -}; +use deltachat::chat::{self, Chat, ChatId, ChatItem, ChatVisibility, MuteDuration}; use deltachat::chatlist::*; use deltachat::constants::*; use deltachat::contact::*; @@ -624,7 +622,7 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu let msglist = chat::get_chat_msgs_ex( &context, sel_chat.get_id(), - MessageListOptions { + chat::MessageListOptions { info_only: false, add_daymarker: true, }, diff --git a/src/chat.rs b/src/chat.rs index 384cea4239..78c4db6a0a 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2754,7 +2754,7 @@ async fn prepare_send_msg( } chat.prepare_msg_raw(context, msg, update_msg_id).await?; - let row_ids = create_send_msg_jobs(context, msg, None) + let row_ids = create_send_msg_jobs(context, msg) .await .context("Failed to create send jobs")?; if !row_ids.is_empty() { @@ -2824,14 +2824,7 @@ async fn render_mime_message_and_pre_message( /// Returns row ids if `smtp` table jobs were created or an empty `Vec` otherwise. /// /// The caller has to interrupt SMTP loop or otherwise process new rows. -/// -/// * `row_id` - Actual Message ID, if `Some`. This is to avoid updating the `msgs` row, in which -/// case `msg.id` is fake (`u32::MAX`); -pub(crate) async fn create_send_msg_jobs( - context: &Context, - msg: &mut Message, - row_id: Option, -) -> Result> { +pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -> Result> { let cmd = msg.param.get_cmd(); if cmd == SystemMessage::GroupNameChanged || cmd == SystemMessage::GroupDescriptionChanged { msg.chat_id @@ -2848,7 +2841,7 @@ pub(crate) async fn create_send_msg_jobs( } let needs_encryption = msg.param.get_bool(Param::GuaranteeE2ee).unwrap_or_default(); - let mimefactory = match MimeFactory::from_msg(context, msg.clone(), row_id).await { + let mimefactory = match MimeFactory::from_msg(context, msg.clone()).await { Ok(mf) => mf, Err(err) => { // Mark message as failed @@ -4028,11 +4021,7 @@ pub(crate) async fn add_contact_to_chat_ex( Ok(true) } -async fn resend_last_msgs( - context: &Context, - chat: &Chat, - to_contact: &Contact, -) -> std::result::Result<(), anyhow::Error> { +async fn resend_last_msgs(context: &Context, chat: &Chat, to_contact: &Contact) -> Result<()> { let chat_id = chat.id; let msgs: Vec = context .sql @@ -4621,10 +4610,7 @@ pub async fn forward_msgs_2ctx( chat.prepare_msg_raw(ctx_dst, &mut msg, None).await?; curr_timestamp += 1; - if !create_send_msg_jobs(ctx_dst, &mut msg, None) - .await? - .is_empty() - { + if !create_send_msg_jobs(ctx_dst, &mut msg).await?.is_empty() { ctx_dst.scheduler.interrupt_smtp().await; } created_msgs.push(msg.id); @@ -4779,17 +4765,10 @@ pub(crate) async fn resend_msgs_ex( } msg_state => bail!("Unexpected message state {msg_state}"), } - let mut row_id = None; if let Some(to_fingerprint) = &to_fingerprint { msg.param.set(Param::Arg4, to_fingerprint.clone()); - // Avoid updating the `msgs` row. - row_id = Some(msg.id); - msg.id = MsgId::new(u32::MAX); } - if create_send_msg_jobs(context, &mut msg, row_id) - .await? - .is_empty() - { + if create_send_msg_jobs(context, &mut msg).await?.is_empty() { continue; } diff --git a/src/mimefactory.rs b/src/mimefactory.rs index c6f1824717..b863e7109f 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -195,15 +195,8 @@ fn new_address_with_name(name: &str, address: String) -> Address<'static> { impl MimeFactory { /// Returns `MimeFactory` for rendering `msg`. - /// - /// * `row_id` - Actual Message ID, if `Some`. This is to avoid updating the `msgs` row, in - /// which case `msg.id` is fake (`u32::MAX`); #[expect(clippy::arithmetic_side_effects)] - pub async fn from_msg( - context: &Context, - msg: Message, - row_id: Option, - ) -> Result { + pub async fn from_msg(context: &Context, msg: Message) -> Result { let now = time(); let chat = Chat::load_from_db(context, msg.chat_id).await?; let attach_profile_data = Self::should_attach_profile_data(&msg); @@ -513,13 +506,12 @@ impl MimeFactory { }; } - let msg_id = row_id.unwrap_or(msg.id); let (in_reply_to, references) = context .sql .query_row( "SELECT mime_in_reply_to, IFNULL(mime_references, '') FROM msgs WHERE id=?", - (msg_id,), + (msg.id,), |row| { let in_reply_to: String = row.get(0)?; let references: String = row.get(1)?; diff --git a/src/mimefactory/mimefactory_tests.rs b/src/mimefactory/mimefactory_tests.rs index 9d981e0a1f..fe03953a91 100644 --- a/src/mimefactory/mimefactory_tests.rs +++ b/src/mimefactory/mimefactory_tests.rs @@ -275,7 +275,7 @@ async fn test_subject_mdn() { chat::send_msg(&t, new_msg.chat_id, &mut new_msg) .await .unwrap(); - let mf = MimeFactory::from_msg(&t, new_msg, None).await.unwrap(); + let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap(); // The subject string should not be "Re: message opened" assert_eq!("Re: Hello, Bob", mf.subject_str(&t).await.unwrap()); } @@ -412,7 +412,7 @@ async fn first_subject_str(t: TestContext) -> String { new_msg.chat_id = chat_id; chat::send_msg(&t, chat_id, &mut new_msg).await.unwrap(); - let mf = MimeFactory::from_msg(&t, new_msg, None).await.unwrap(); + let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap(); mf.subject_str(&t).await.unwrap() } @@ -500,7 +500,7 @@ async fn msg_to_subject_str_inner( chat::send_msg(&t, new_msg.chat_id, &mut new_msg) .await .unwrap(); - let mf = MimeFactory::from_msg(&t, new_msg, None).await.unwrap(); + let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap(); mf.subject_str(&t).await.unwrap() } @@ -545,7 +545,7 @@ async fn test_render_reply() { .await; chat::send_msg(&t, msg.chat_id, &mut msg).await.unwrap(); - let mimefactory = MimeFactory::from_msg(&t, msg, None).await.unwrap(); + let mimefactory = MimeFactory::from_msg(&t, msg).await.unwrap(); let recipients = mimefactory.recipients(); assert_eq!(recipients, vec!["charlie@example.com"]); From 96018dc02fd2a74492c9a78a696e1f6247aa42b7 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 16:24:56 +0200 Subject: [PATCH 09/11] clippy --- src/chat.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 78c4db6a0a..d6057e5f44 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4026,8 +4026,7 @@ async fn resend_last_msgs(context: &Context, chat: &Chat, to_contact: &Contact) let msgs: Vec = context .sql .query_map_vec( - &format!( - " + " SELECT id FROM msgs WHERE chat_id=? @@ -4038,8 +4037,7 @@ WHERE chat_id=? OR to_id=? ) AND viewtype!=? -ORDER BY timestamp DESC, id DESC LIMIT ?" - ), +ORDER BY timestamp DESC, id DESC LIMIT ?", ( chat_id, ContactId::INFO, From 5e840eebf9e10312803f60dafa23b724247ad266 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 16:29:43 +0200 Subject: [PATCH 10/11] Small tweaks --- src/chat.rs | 13 ++++++------- src/mimefactory.rs | 1 - 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index d6057e5f44..66f68e2def 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3126,8 +3126,8 @@ pub async fn get_chat_msgs(context: &Context, chat_id: ChatId) -> Result Result<()> { - let chat_id = chat.id; +async fn resend_last_msgs(context: &Context, chat_id: ChatId, to_contact: &Contact) -> Result<()> { let msgs: Vec = context .sql .query_map_vec( @@ -4728,9 +4727,9 @@ pub async fn resend_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> { /// members of the corresponding chats. /// /// NB: Actually `to_fingerprint` is only passed for `OutBroadcast` chats when a new member is -/// added. Regarding webxdc's: It is not trivial to resend only the own status updates, +/// added. Regarding webxdcs: It is not trivial to resend only the own status updates, /// and it is not trivial to resend them only to the newly-joined member, -/// so that for now, [`resend_last_msgs`] does not automatically resend webxdc's at all. +/// so that for now, [`resend_last_msgs`] does not automatically resend webxdcs at all. pub(crate) async fn resend_msgs_ex( context: &Context, msg_ids: &[MsgId], diff --git a/src/mimefactory.rs b/src/mimefactory.rs index b863e7109f..f598333000 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -2228,7 +2228,6 @@ fn should_encrypt_symmetrically(msg: &Message, chat: &Chat) -> bool { fn must_have_only_one_recipient<'a>(msg: &'a Message, chat: &Chat) -> Option> { if chat.typ != Chattype::OutBroadcast { None - // TODO problem here is that Param::Arg4 may be the end timestamp of a call } else if let Some(fp) = msg.param.get(Param::Arg4) { Some(Ok(fp)) } else if matches!( From 730afedccb1ab7dce5d99f3e7cbe9b92488347db Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 16:37:04 +0200 Subject: [PATCH 11/11] Fix sql statement --- src/chat.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chat.rs b/src/chat.rs index 66f68e2def..966a041ca1 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4035,7 +4035,7 @@ WHERE chat_id=? OR from_id=? OR to_id=? ) - AND viewtype!=? + AND type!=? ORDER BY timestamp DESC, id DESC LIMIT ?", ( chat_id,