From ceb10869c98fee416458ce29f1435a36bf99c25d Mon Sep 17 00:00:00 2001 From: Hocuri Date: Tue, 21 Apr 2026 15:23:19 +0200 Subject: [PATCH] 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 687384604..4b375ad4c 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 137ca8cd9..781faf452 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 81f8b16b7..445ea95f7 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 3a0ca3060..b1cc23686 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 1071a8add..c6f182471 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 aebde2027..d946d7a7c 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 {