Start extracting the logic for getting the last n messages into its own function

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 {
+                MessageListOptions {
-                    filter: ChatMsgsFilter::info_only(info_only),
+                    info_only,
                     add_daymarker,
-                    ..Default::default()
                 },
             )
             .await

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 {
+            MessageListOptions {
-                filter: ChatMsgsFilter::info_only(info_only),
+                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 {
+            MessageListOptions {
-                filter: ChatMsgsFilter::info_only(info_only),
+                info_only,
                 add_daymarker,
-                ..Default::default()
             },
         )
         .await?;

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?;

src/chat.rs

@@ -3110,44 +3110,27 @@ async fn donation_request_maybe(context: &Context) -> Result<()> {
         .await
 }
 
-/// Controls which messages [`get_chat_msgs_ex`] returns.
+/// Chat message list request options.
-#[derive(Debug, Default, PartialEq)]
+#[derive(Debug)]
-pub enum ChatMsgsFilter {
+pub struct MessageListOptions {
-    /// All messages.
+    /// Return only info messages.
-    #[default]
+    pub info_only: bool,
-    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<usize>,
 }
 
 /// Returns all messages belonging to the chat.
 pub async fn get_chat_msgs(context: &Context, chat_id: ChatId) -> Result<Vec<ChatItem>> {
-    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<Vec<Cha
 pub async fn get_chat_msgs_ex(
     context: &Context,
     chat_id: ChatId,
-    options: GetChatMsgsOptions,
+    options: MessageListOptions,
 ) -> Result<Vec<ChatItem>> {
-    let GetChatMsgsOptions {
+    let MessageListOptions {
-        filter,
+        info_only,
         add_daymarker,
-        n_last,
     } = options;
-    let process_row = |row: &rusqlite::Row| {
+    // TODO: Remove `info_only` parameter; it's not used by anything
-        if filter != ChatMsgsFilter::All {
+    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 {
+        |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?;
+            let (ts, curr_id, exclude_message): (i64, MsgId, bool) = row?;
-            if include {
+            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 {
+    let items = if info_only {
-        Some(n) => format!("ORDER BY timestamp DESC, id DESC LIMIT {n}"),
-        None => "".to_string(),
-    };
-    let items = if filter != ChatMsgsFilter::All {
         context
             .sql
             .query_map(
-                &format!("
+        // 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
+                "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 ?=(
+                AND (
                     m.param GLOB '*\nS=*' OR param GLOB 'S=*'
                     OR m.from_id == ?
                     OR m.to_id == ?
-    )
+                );",
-{n_last_subst}"
+                (chat_id, ContactId::INFO, ContactId::INFO),
-                ),
-                (chat_id, filter == ChatMsgsFilter::Info, 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
-                    "
-SELECT m.id AS id, m.timestamp AS timestamp
                FROM msgs m
               WHERE m.chat_id=?
-    AND m.hidden=0
+                AND m.hidden=0;",
-{n_last_subst}"
-                ),
                 (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<Vec<MsgId>> {
+    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(&params) {
+                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(
+        let msgs =
-            context,
+            get_msgs_for_resending(context, chat.id, constants::N_MSGS_TO_NEW_BROADCAST_MEMBER)
-            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 {

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<Result<&'a str>> {
     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!(

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,7 +1094,14 @@ 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())
+        let msglist = chat::get_chat_msgs_ex(
+            self,
+            chat_id,
+            MessageListOptions {
+                info_only: false,
+                add_daymarker: false,
+            },
+        )
         .await
         .unwrap();
         let msglist: Vec<MsgId> = msglist