diff --git a/deltachat-jsonrpc/src/api/types/chat_list.rs b/deltachat-jsonrpc/src/api/types/chat_list.rs index b47f7a7283..6ac726826c 100644 --- a/deltachat-jsonrpc/src/api/types/chat_list.rs +++ b/deltachat-jsonrpc/src/api/types/chat_list.rs @@ -1,12 +1,9 @@ use anyhow::{Context, Result}; +use deltachat::chat::{get_chat_contacts, ChatVisibility}; use deltachat::chat::{Chat, ChatId}; -use deltachat::chatlist::get_last_message_for_chat; +use deltachat::chatlist::Chatlist; use deltachat::constants::*; use deltachat::contact::{Contact, ContactId}; -use deltachat::{ - chat::{get_chat_contacts, ChatVisibility}, - chatlist::Chatlist, -}; use num_traits::cast::ToPrimitive; use serde::Serialize; use typescript_type_def::TypeDef; @@ -67,10 +64,8 @@ pub(crate) async fn get_chat_list_item_by_id( }); } - let last_msgid = get_last_message_for_chat(ctx, chat_id).await?; - let chat = Chat::load_from_db(ctx, chat_id).await.context("chat")?; - let summary = Chatlist::get_summary2(ctx, chat_id, last_msgid, Some(&chat)) + let summary = Chatlist::get_summary2(ctx, chat_id, Some(&chat)) .await .context("summary")?; @@ -86,15 +81,12 @@ pub(crate) async fn get_chat_list_item_by_id( .await? .map(|path| path.to_str().unwrap_or("invalid/path").to_owned()); - let (last_updated, message_type) = match last_msgid { - Some(id) => { - let last_message = deltachat::message::Message::load_from_db(ctx, id).await?; - ( - Some(last_message.get_timestamp() * 1000), - Some(last_message.get_viewtype().into()), - ) - } + let (last_updated, message_type) = match summary.id { None => (None, None), + Some(_) => ( + Some(summary.timestamp * 1000), + Some(summary.viewtype.into()), + ), }; let chat_contacts = get_chat_contacts(ctx, chat_id).await?; @@ -145,6 +137,6 @@ pub(crate) async fn get_chat_list_item_by_id( dm_chat_contact, was_seen_recently, last_message_type: message_type, - last_message_id: last_msgid.map(|id| id.to_u32()), + last_message_id: summary.id.map(|id| id.to_u32()), }) } diff --git a/src/chat.rs b/src/chat.rs index 1903e8431c..429db6c07f 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -40,6 +40,7 @@ use crate::receive_imf::ReceivedMsg; use crate::smtp::send_msg_to_smtp; use crate::sql; use crate::stock_str; +use crate::summary::Summary; use crate::sync::{self, Sync::*, SyncData}; use crate::tools::{ buf_compress, create_id, create_outgoing_rfc724_mid, create_smeared_timestamp, @@ -1309,6 +1310,52 @@ impl ChatId { Ok(()) } + /// Returns a summary for a given chat. + pub async fn get_summary( + self, + context: &Context, + chat: Option<&Chat>, + msg: Option<&Message>, + ) -> Result { + let chat_loaded: Chat; + let chat = if let Some(chat) = chat { + chat + } else { + let chat = Chat::load_from_db(context, self).await?; + chat_loaded = chat; + &chat_loaded + }; + + let lastcontact = if let Some(msg) = msg { + if msg.from_id == ContactId::SELF { + None + } else { + match chat.typ { + Chattype::Group | Chattype::Broadcast | Chattype::Mailinglist => { + let lastcontact = Contact::get_by_id(context, msg.from_id) + .await + .context("Loading contact failed")?; + Some(lastcontact) + } + Chattype::Single => None, + } + } + } else { + None + }; + + if chat.id.is_archived_link() { + Ok(Default::default()) + } else if let Some(msg) = msg.filter(|msg| msg.from_id != ContactId::UNDEFINED) { + Ok(Summary::new(context, msg, chat, lastcontact.as_ref()).await) + } else { + Ok(Summary { + text: stock_str::no_messages(context).await, + ..Default::default() + }) + } + } + /// Returns true if the chat is protected. pub async fn is_protected(self, context: &Context) -> Result { let protection_status = context diff --git a/src/chatlist.rs b/src/chatlist.rs index 99a50a33bc..276e2cd605 100644 --- a/src/chatlist.rs +++ b/src/chatlist.rs @@ -8,11 +8,10 @@ use crate::constants::{ Blocked, Chattype, DC_CHAT_ID_ALLDONE_HINT, DC_CHAT_ID_ARCHIVED_LINK, DC_GCL_ADD_ALLDONE_HINT, DC_GCL_ARCHIVED_ONLY, DC_GCL_FOR_FORWARDING, DC_GCL_NO_SPECIALS, }; -use crate::contact::{Contact, ContactId}; +use crate::contact::ContactId; use crate::context::Context; use crate::message::{Message, MessageState, MsgId}; use crate::param::{Param, Params}; -use crate::stock_str; use crate::summary::Summary; use crate::tools::IsNoneOrEmpty; @@ -22,7 +21,8 @@ pub static IS_UNREAD_FILTER: Lazy = /// An object representing a single chatlist in memory. /// -/// Chatlist objects contain chat IDs and, if possible, message IDs belonging to them. +/// Chatlist objects contain IDs of chats and, optionally, their last messages, recently updated +/// come first. /// The chatlist object is not updated; if you want an update, you have to recreate the object. /// /// For a **typical chat overview**, the idea is to get the list of all chats via dc_get_chatlist() @@ -347,9 +347,11 @@ impl Chatlist { Ok(*chat_id) } - /// Get a single message ID of a chatlist. + /// Get the last message ID of a chat with index `index`. /// - /// To get the message object from the message ID, use dc_get_msg(). + /// If you want to obtain the [Message] object further, it's not recommended to use as the + /// message may expire (if it's ephemeral f.e.). Better use [`Self::get_chat_id()`] + + /// [`Message::load_from_db_last_for_chat()`]. pub fn get_msg_id(&self, index: usize) -> Result> { let (_chat_id, msg_id) = self .ids @@ -373,56 +375,28 @@ impl Chatlist { .ids .get(index) .context("chatlist index is out of range")?; - Chatlist::get_summary2(context, *chat_id, *lastmsg_id, chat).await + let lastmsg = if let Some(lastmsg_id) = lastmsg_id { + Some( + Message::load_from_db(context, *lastmsg_id) + .await + .with_context(|| format!("Loading message {lastmsg_id} failed"))?, + ) + } else { + None + }; + chat_id.get_summary(context, chat, lastmsg.as_ref()).await } - /// Returns a summary for a given chatlist item. + /// Returns a summary for a given chat. pub async fn get_summary2( context: &Context, chat_id: ChatId, - lastmsg_id: Option, chat: Option<&Chat>, ) -> Result { - let chat_loaded: Chat; - let chat = if let Some(chat) = chat { - chat - } else { - let chat = Chat::load_from_db(context, chat_id).await?; - chat_loaded = chat; - &chat_loaded - }; - - let (lastmsg, lastcontact) = if let Some(lastmsg_id) = lastmsg_id { - let lastmsg = Message::load_from_db(context, lastmsg_id) - .await - .context("loading message failed")?; - if lastmsg.from_id == ContactId::SELF { - (Some(lastmsg), None) - } else { - match chat.typ { - Chattype::Group | Chattype::Broadcast | Chattype::Mailinglist => { - let lastcontact = Contact::get_by_id(context, lastmsg.from_id) - .await - .context("loading contact failed")?; - (Some(lastmsg), Some(lastcontact)) - } - Chattype::Single => (Some(lastmsg), None), - } - } - } else { - (None, None) - }; - - if chat.id.is_archived_link() { - Ok(Default::default()) - } else if let Some(lastmsg) = lastmsg.filter(|msg| msg.from_id != ContactId::UNDEFINED) { - Ok(Summary::new(context, &lastmsg, chat, lastcontact.as_ref()).await) - } else { - Ok(Summary { - text: stock_str::no_messages(context).await, - ..Default::default() - }) - } + let lastmsg = Message::load_from_db_last_for_chat(context, chat_id) + .await + .context("Loading message failed")?; + chat_id.get_summary(context, chat, lastmsg.as_ref()).await } /// Returns chatlist item position for the given chat ID. @@ -431,6 +405,9 @@ impl Chatlist { } /// An iterator visiting all chatlist items. + /// + /// See [`Self::get_msg_id()`] why it's not recommended to use returned [MsgId]-s to obtain + /// [Message] objects and what to use instead. pub fn iter(&self) -> impl Iterator)> { self.ids.iter() } @@ -448,8 +425,10 @@ pub async fn get_archived_cnt(context: &Context) -> Result { Ok(count) } -/// Gets the last message of a chat, the message that would also be displayed in the ChatList -/// Used for passing to `deltachat::chatlist::Chatlist::get_summary2` +/// Gets the id of the last user-visible message of a chat. +/// +/// See [`Chatlist::get_msg_id()`] why it's not recommended to use returned [MsgId] to obtain the +/// [Message] object and what to use instead. pub async fn get_last_message_for_chat( context: &Context, chat_id: ChatId, @@ -474,7 +453,8 @@ mod tests { add_contact_to_chat, create_group_chat, get_chat_contacts, remove_contact_from_chat, send_text_msg, ProtectionStatus, }; - use crate::message::Viewtype; + use crate::contact::Contact; + use crate::message::{Message, Viewtype}; use crate::receive_imf::receive_imf; use crate::stock_str::StockMessage; use crate::test_utils::TestContext; diff --git a/src/message.rs b/src/message.rs index e31fb554d4..78141628e8 100644 --- a/src/message.rs +++ b/src/message.rs @@ -466,87 +466,9 @@ impl Message { let msg = context .sql .query_row( - concat!( - "SELECT", - " m.id AS id,", - " rfc724_mid AS rfc724mid,", - " m.mime_in_reply_to AS mime_in_reply_to,", - " m.chat_id AS chat_id,", - " m.from_id AS from_id,", - " m.to_id AS to_id,", - " m.timestamp AS timestamp,", - " m.timestamp_sent AS timestamp_sent,", - " m.timestamp_rcvd AS timestamp_rcvd,", - " m.ephemeral_timer AS ephemeral_timer,", - " m.ephemeral_timestamp AS ephemeral_timestamp,", - " m.type AS type,", - " m.state AS state,", - " m.download_state AS download_state,", - " m.error AS error,", - " m.msgrmsg AS msgrmsg,", - " m.mime_modified AS mime_modified,", - " m.txt AS txt,", - " m.subject AS subject,", - " m.param AS param,", - " m.hidden AS hidden,", - " m.location_id AS location,", - " c.blocked AS blocked", - " FROM msgs m LEFT JOIN chats c ON c.id=m.chat_id", - " WHERE m.id=?;" - ), + &Self::select_query_with_filter("WHERE m.id=?"), (id,), - |row| { - let text = match row.get_ref("txt")? { - rusqlite::types::ValueRef::Text(buf) => { - match String::from_utf8(buf.to_vec()) { - Ok(t) => t, - Err(_) => { - warn!( - context, - concat!( - "dc_msg_load_from_db: could not get ", - "text column as non-lossy utf8 id {}" - ), - id - ); - String::from_utf8_lossy(buf).into_owned() - } - } - } - _ => String::new(), - }; - let msg = Message { - id: row.get("id")?, - rfc724_mid: row.get::<_, String>("rfc724mid")?, - in_reply_to: row - .get::<_, Option>("mime_in_reply_to")? - .and_then(|in_reply_to| parse_message_id(&in_reply_to).ok()), - chat_id: row.get("chat_id")?, - from_id: row.get("from_id")?, - to_id: row.get("to_id")?, - timestamp_sort: row.get("timestamp")?, - timestamp_sent: row.get("timestamp_sent")?, - timestamp_rcvd: row.get("timestamp_rcvd")?, - ephemeral_timer: row.get("ephemeral_timer")?, - ephemeral_timestamp: row.get("ephemeral_timestamp")?, - viewtype: row.get("type")?, - state: row.get("state")?, - download_state: row.get("download_state")?, - error: Some(row.get::<_, String>("error")?) - .filter(|error| !error.is_empty()), - is_dc_message: row.get("msgrmsg")?, - mime_modified: row.get("mime_modified")?, - text, - subject: row.get("subject")?, - param: row.get::<_, String>("param")?.parse().unwrap_or_default(), - hidden: row.get("hidden")?, - location_id: row.get("location")?, - chat_blocked: row - .get::<_, Option>("blocked")? - .unwrap_or_default(), - }; - Ok(msg) - }, + |row| Self::from_row(context, row), ) .await .with_context(|| format!("failed to load message {id} from the database"))?; @@ -554,6 +476,107 @@ impl Message { Ok(msg) } + /// Loads the last user-visible message of the chat with given `id` from the database. + pub async fn load_from_db_last_for_chat( + context: &Context, + chat_id: ChatId, + ) -> Result> { + let msg = context + .sql + .query_row_optional( + &Self::select_query_with_filter(concat!( + "WHERE m.chat_id=? ", + "AND (m.hidden=0 OR m.state=?) ", + "ORDER BY m.timestamp DESC, m.id DESC LIMIT 1", + )), + (chat_id, MessageState::OutDraft), + |row| Self::from_row(context, row), + ) + .await + .with_context(|| { + format!("Failed to load last message for chat {chat_id} from the database") + })?; + + Ok(msg) + } + + fn select_query_with_filter(filter: &str) -> String { + "\ + SELECT \ + m.id AS id, \ + rfc724_mid AS rfc724mid, \ + m.mime_in_reply_to AS mime_in_reply_to, \ + m.chat_id AS chat_id, \ + m.from_id AS from_id, \ + m.to_id AS to_id, \ + m.timestamp AS timestamp, \ + m.timestamp_sent AS timestamp_sent, \ + m.timestamp_rcvd AS timestamp_rcvd, \ + m.ephemeral_timer AS ephemeral_timer, \ + m.ephemeral_timestamp AS ephemeral_timestamp, \ + m.type AS type, \ + m.state AS state, \ + m.download_state AS download_state, \ + m.error AS error, \ + m.msgrmsg AS msgrmsg, \ + m.mime_modified AS mime_modified, \ + m.txt AS txt, \ + m.subject AS subject, \ + m.param AS param, \ + m.hidden AS hidden, \ + m.location_id AS location, \ + c.blocked AS blocked \ + FROM msgs m LEFT JOIN chats c ON c.id=m.chat_id " + .to_string() + + filter + } + + fn from_row(context: &Context, row: &rusqlite::Row) -> rusqlite::Result { + let text = match row.get_ref("txt")? { + rusqlite::types::ValueRef::Text(buf) => match String::from_utf8(buf.to_vec()) { + Ok(t) => t, + Err(_) => { + warn!( + context, + "Message::from_row: could not get text column as a non-lossy UTF-8", + ); + String::from_utf8_lossy(buf).into_owned() + } + }, + _ => String::new(), + }; + let msg = Message { + id: row.get("id")?, + rfc724_mid: row.get::<_, String>("rfc724mid")?, + in_reply_to: row + .get::<_, Option>("mime_in_reply_to")? + .and_then(|in_reply_to| parse_message_id(&in_reply_to).ok()), + chat_id: row.get("chat_id")?, + from_id: row.get("from_id")?, + to_id: row.get("to_id")?, + timestamp_sort: row.get("timestamp")?, + timestamp_sent: row.get("timestamp_sent")?, + timestamp_rcvd: row.get("timestamp_rcvd")?, + ephemeral_timer: row.get("ephemeral_timer")?, + ephemeral_timestamp: row.get("ephemeral_timestamp")?, + viewtype: row.get("type")?, + state: row.get("state")?, + download_state: row.get("download_state")?, + error: Some(row.get::<_, String>("error")?).filter(|error| !error.is_empty()), + is_dc_message: row.get("msgrmsg")?, + mime_modified: row.get("mime_modified")?, + text, + subject: row.get("subject")?, + param: row.get::<_, String>("param")?.parse().unwrap_or_default(), + hidden: row.get("hidden")?, + location_id: row.get("location")?, + chat_blocked: row + .get::<_, Option>("blocked")? + .unwrap_or_default(), + }; + Ok(msg) + } + /// Returns the MIME type of an attached file if it exists. /// /// If the MIME type is not known, the function guesses the MIME type diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 4a76db5833..c907cbdaec 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -3381,8 +3381,11 @@ On 2020-10-25, Bob wrote: .unwrap(); let chats = Chatlist::try_load(&t.ctx, 0, None, None).await.unwrap(); - let msg_id = chats.get_msg_id(0).unwrap().unwrap(); - let msg = Message::load_from_db(&t.ctx, msg_id).await.unwrap(); + let chat_id = chats.get_chat_id(0).unwrap(); + let msg = Message::load_from_db_last_for_chat(&t.ctx, chat_id) + .await + .unwrap() + .unwrap(); assert_eq!(msg.text, "subj with important info – body text"); assert_eq!(msg.viewtype, Viewtype::Image); diff --git a/src/peerstate.rs b/src/peerstate.rs index 8bd0c37e8a..e5d857c6c3 100644 --- a/src/peerstate.rs +++ b/src/peerstate.rs @@ -598,9 +598,9 @@ impl Peerstate { .await } }; - for (chat_id, msg_id) in chats.iter() { - let timestamp_sort = if let Some(msg_id) = msg_id { - let lastmsg = Message::load_from_db(context, *msg_id).await?; + for (chat_id, _) in chats.iter() { + let lastmsg = Message::load_from_db_last_for_chat(context, *chat_id).await?; + let timestamp_sort = if let Some(lastmsg) = lastmsg { lastmsg.timestamp_sort } else { context diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index 2c03c1ffbb..8ecdb0e299 100644 --- a/src/receive_imf/tests.rs +++ b/src/receive_imf/tests.rs @@ -320,6 +320,7 @@ async fn test_no_from() { let context = &t; let chats = Chatlist::try_load(&t, 0, None, None).await.unwrap(); + assert!(chats.get_chat_id(0).is_err()); assert!(chats.get_msg_id(0).is_err()); let received = receive_imf( @@ -362,6 +363,7 @@ async fn test_no_message_id_header() { let t = TestContext::new_alice().await; let chats = Chatlist::try_load(&t, 0, None, None).await.unwrap(); + assert!(chats.get_chat_id(0).is_err()); assert!(chats.get_msg_id(0).is_err()); let received = receive_imf( @@ -464,8 +466,9 @@ async fn test_escaped_recipients() { assert_eq!(contact.get_display_name(), "h2"); let chats = Chatlist::try_load(&t, 0, None, None).await.unwrap(); - let msg = Message::load_from_db(&t, chats.get_msg_id(0).unwrap().unwrap()) + let msg = Message::load_from_db_last_for_chat(&t, chats.get_chat_id(0).unwrap()) .await + .unwrap() .unwrap(); assert_eq!(msg.is_dc_message, MessengerMessage::Yes); assert_eq!(msg.text, "hello"); @@ -652,7 +655,7 @@ async fn test_parse_ndn( .unwrap(); let chats = Chatlist::try_load(&t, 0, None, None).await.unwrap(); - let msg_id = chats.get_msg_id(0).unwrap().unwrap(); + let chat_id = chats.get_chat_id(0).unwrap(); // Check that the ndn would be downloaded: let headers = mailparse::parse_mail(raw_ndn).unwrap().headers; @@ -663,7 +666,10 @@ async fn test_parse_ndn( ); receive_imf(&t, raw_ndn, false).await.unwrap(); - let msg = Message::load_from_db(&t, msg_id).await.unwrap(); + let msg = Message::load_from_db_last_for_chat(&t, chat_id) + .await + .unwrap() + .unwrap(); assert_eq!( msg.state, @@ -733,8 +739,11 @@ async fn load_imf_email(context: &Context, imf_raw: &[u8]) -> Message { .unwrap(); receive_imf(context, imf_raw, false).await.unwrap(); let chats = Chatlist::try_load(context, 0, None, None).await.unwrap(); - let msg_id = chats.get_msg_id(0).unwrap().unwrap(); - Message::load_from_db(context, msg_id).await.unwrap() + let chat_id = chats.get_chat_id(0).unwrap(); + Message::load_from_db_last_for_chat(context, chat_id) + .await + .unwrap() + .unwrap() } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/src/summary.rs b/src/summary.rs index dfeab5f59e..bc40464012 100644 --- a/src/summary.rs +++ b/src/summary.rs @@ -7,7 +7,7 @@ use crate::chat::Chat; use crate::constants::Chattype; use crate::contact::{Contact, ContactId}; use crate::context::Context; -use crate::message::{Message, MessageState, Viewtype}; +use crate::message::{Message, MessageState, MsgId, Viewtype}; use crate::mimeparser::SystemMessage; use crate::stock_str; use crate::tools::truncate; @@ -52,6 +52,12 @@ pub struct Summary { /// Message preview image path pub thumbnail_path: Option, + + /// Message viewtype. + pub viewtype: Viewtype, + + /// Message ID. + pub id: Option, } impl Summary { @@ -108,6 +114,8 @@ impl Summary { timestamp: msg.get_timestamp(), state: msg.state, thumbnail_path, + viewtype: msg.viewtype, + id: Some(msg.id), } } diff --git a/src/test_utils.rs b/src/test_utils.rs index 20de434fe3..c3b970085a 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -561,10 +561,11 @@ impl TestContext { // The chatlist describes what you see when you open DC, a list of chats and in each of them // the first words of the last message. To get the last message overall, we look at the chat at the top of the // list, which has the index 0. - let msg_id = chats.get_msg_id(0).unwrap().unwrap(); - Message::load_from_db(&self.ctx, msg_id) + let chat_id = chats.get_chat_id(0).unwrap(); + Message::load_from_db_last_for_chat(&self.ctx, chat_id) .await .expect("failed to load msg") + .unwrap() } /// Returns the [`Contact`] for the other [`TestContext`], creating it if necessary.