diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index 84c2c994aa..7f72e487aa 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -2457,14 +2457,20 @@ impl CommandApi { async fn send_msg(&self, account_id: u32, chat_id: u32, data: MessageData) -> Result { let ctx = self.get_context(account_id).await?; + let reuse_existing_draft = data.reuse_existing_draft; let mut message = data .create_message(&ctx) .await .context("Failed to create message")?; - let msg_id = chat::send_msg(&ctx, ChatId::new(chat_id), &mut message) - .await - .context("Failed to send created message")? - .to_u32(); + let msg_id = chat::send_msg_ex( + &ctx, + ChatId::new(chat_id), + &mut message, + reuse_existing_draft.into(), + ) + .await + .context("Failed to send created message")? + .to_u32(); Ok(msg_id) } diff --git a/deltachat-jsonrpc/src/api/types/message.rs b/deltachat-jsonrpc/src/api/types/message.rs index 40640880a3..3ddb4f2b58 100644 --- a/deltachat-jsonrpc/src/api/types/message.rs +++ b/deltachat-jsonrpc/src/api/types/message.rs @@ -616,6 +616,27 @@ pub struct MessageData { /// Quoted message id. Takes preference over `quoted_text` (see below). pub quoted_message_id: Option, pub quoted_text: Option, + /// Useful for WebXDC app attachments, which can also be opened + /// for draft messages. + /// Setting this to `true` will ensure that the WebXDC status updates + /// of the current draft are preserved when sending the message + /// or updating the draft. + /// + /// `false` by default, for backwards compatibility. + /// However, you probably want to set it to `true` + /// when sending or updating the draft + /// from the main message composer section, + /// and to `false` when sending a message from secondary places, + /// such as a notification "Reply" input. + /// + /// Reusing the draft will also automatically remove the draft + /// when it's sent. + /// + /// Note that sometimes the draft cannot be reused, + /// for example when the WebXDC attachment [`Self::file`] changes. + /// If the draft cannot be reused, it will not get auto-removed. + #[serde(default)] + pub reuse_existing_draft: bool, } impl MessageData { diff --git a/src/chat.rs b/src/chat.rs index 57f69acac9..a050fc1ac5 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -13,6 +13,7 @@ use chrono::TimeZone; use deltachat_contact_tools::{ContactAddress, sanitize_bidi_characters, sanitize_single_line}; use humansize::{BINARY, format_size}; use mail_builder::mime::MimePart; +use rusqlite::OptionalExtension; use serde::{Deserialize, Serialize}; use strum_macros::EnumIter; @@ -738,13 +739,21 @@ SELECT id, rfc724_mid, pre_rfc724_mid, timestamp, ?, 1 FROM msgs WHERE chat_id=? /// Returns ID of the draft message, if there is one. async fn get_draft_msg_id(self, context: &Context) -> Result> { - let msg_id: Option = context + let query_only = true; + context .sql - .query_get_value( + // `call` instead of `transaction_ex` because it's a single query. + .call(query_only, |conn| self.get_draft_msg_id_trans(conn)) + .await + } + fn get_draft_msg_id_trans(self, conn: &rusqlite::Connection) -> Result> { + let msg_id: Option = conn + .query_row( "SELECT id FROM msgs WHERE chat_id=? AND state=?;", (self, MessageState::OutDraft), + |row| row.get(0), ) - .await?; + .optional()?; Ok(msg_id) } @@ -760,14 +769,52 @@ SELECT id, rfc724_mid, pre_rfc724_mid, timestamp, ?, 1 FROM msgs WHERE chat_id=? Ok(count > 0) } + fn can_reuse_draft(context: &Context, old: &Message, new: &Message) -> Result { + ensure!( + old.chat_id.is_unset() || new.chat_id.is_unset() || new.chat_id == old.chat_id, + "messages belong to different chats" + ); + ensure!( + old.get_state() == MessageState::Undefined || old.get_state() == MessageState::OutDraft, + "old message is a real message, not a draft" + ); + + // Reusing the draft is only useful for WebXDC messages, + // but for consistency let's reuse it whenever possible. + + if old.get_viewtype() == Viewtype::Webxdc + && (new.get_viewtype() != Viewtype::Webxdc + || old.get_file(context) != new.get_file(context)) + { + // Old draft's WebXDC attachment got removed or replaced. + return Ok(false); + } + + Ok(true) + } + /// Returns draft message, if there is one. pub async fn get_draft(self, context: &Context) -> Result> { + let query_only = true; + context + .sql + .transaction_ex(query_only, |transaction| { + self.get_draft_trans(context, transaction) + }) + .await + } + /// See [`Self::get_draft`]. + pub fn get_draft_trans( + self, + context: &Context, + conn: &rusqlite::Connection, + ) -> Result> { if self.is_special() { return Ok(None); } - match self.get_draft_msg_id(context).await? { + match self.get_draft_msg_id_trans(conn)? { Some(draft_msg_id) => { - let msg = Message::load_from_db(context, draft_msg_id).await?; + let msg = Message::load_from_db_trans(context, conn, draft_msg_id)?; Ok(Some(msg)) } None => Ok(None), @@ -795,6 +842,9 @@ SELECT id, rfc724_mid, pre_rfc724_mid, timestamp, ?, 1 FROM msgs WHERE chat_id=? /// thus preserving the ID and possible WebXDC status updates /// associated with the draft message. /// + /// If `msg.id` is specified, the [`Self::can_reuse_draft`] check + /// will be skipped. + /// /// Returns `false` if the existing draft is already at the state /// that the caller tried to set it to, so it was unchanged. async fn do_set_draft(self, context: &Context, msg: &mut Message) -> Result { @@ -833,7 +883,20 @@ SELECT id, rfc724_mid, pre_rfc724_mid, timestamp, ?, 1 FROM msgs WHERE chat_id=? let trans_fn = |transaction: &mut rusqlite::Transaction| { // if possible, replace existing draft and keep id - if !msg.id.is_special() && self.has_draft_with_id(transaction, &msg.id)? { + let reuse_existing_id: Option = if !msg.id.is_special() { + if self.has_draft_with_id(transaction, &msg.id)? { + Some(msg.id) + } else { + None + } + } else if let Some(existing) = self.get_draft_trans(context, transaction)? + && Self::can_reuse_draft(context, &existing, msg)? + { + Some(existing.id) + } else { + None + }; + if let Some(reuse_existing_id) = reuse_existing_id { let affected_rows = transaction.execute( "UPDATE msgs SET timestamp=?1,type=?2,txt=?3,txt_normalized=?4,param=?5,mime_in_reply_to=?6 @@ -850,11 +913,11 @@ SELECT id, rfc724_mid, pre_rfc724_mid, timestamp, ?, 1 FROM msgs WHERE chat_id=? normalize_text(&msg.text), msg.param.to_string(), msg.in_reply_to.as_deref().unwrap_or_default(), - msg.id, + reuse_existing_id, ), )?; let changed = affected_rows > 0; - return Ok((msg.id, changed)); + return Ok((reuse_existing_id, changed)); } // Delete existing draft if it exists. @@ -1766,8 +1829,12 @@ impl Chat { /// writes the record to the database. /// /// If `update_existing_draft == `[`UseExistingDraftPolicy::Reuse`], - /// the existing draft with ID == msg.id is reused. - /// If no such draft exists, an error is returned. + /// we will reuse the draft by the specified `msg.id`, + /// or if it can be reused according to [`ChatId::can_reuse_draft`]. + /// If `msg.id` is specified, the [`ChatId::can_reuse_draft`] check + /// will be skipped. + /// If ID was specified but no such draft exists, an error is returned. + /// /// If `update_existing_draft == `[`UseExistingDraftPolicy::DontReuse`], /// a new record is created. async fn prepare_msg_raw( @@ -1779,10 +1846,6 @@ impl Chat { let mut to_id = 0; let mut location_id = 0; - if msg.rfc724_mid.is_empty() { - msg.rfc724_mid = create_outgoing_rfc724_mid(); - } - if self.typ == Chattype::Single { if let Some(id) = context .sql @@ -1816,11 +1879,13 @@ impl Chat { // Set "In-Reply-To:" to identify the message to which the composed message is a reply. // Set "References:" to identify the "thread" of the conversation. // Both according to [RFC 5322 3.6.4, page 25](https://www.rfc-editor.org/rfc/rfc5322#section-3.6.4). - let new_references; + // + // When `None`, we'll use the current message's `rfc724_mid` as the reference. + let new_references_opt: Option; if self.is_self_talk() { // As self-talks are mainly used to transfer data between devices, // we do not set In-Reply-To/References in this case. - new_references = String::new(); + new_references_opt = Some(String::new()); } else if let Some((parent_rfc724_mid, parent_in_reply_to, parent_references)) = // We don't filter `OutPending` and `OutFailed` messages because the new message for // which `parent_query()` is done may assume that it will be received in a context @@ -1868,9 +1933,9 @@ impl Chat { if references_vec.is_empty() { // As a fallback, use our Message-ID, // same as in the case of top-level message. - new_references = msg.rfc724_mid.clone(); + new_references_opt = None; } else { - new_references = references_vec.join(" "); + new_references_opt = Some(references_vec.join(" ")); } } else { // This is a top-level message. @@ -1878,7 +1943,13 @@ impl Chat { // This allows us to identify replies to our message even if // email server such as Outlook changes `Message-ID:` header. // MUAs usually keep the first Message-ID in `References:` header unchanged. - new_references = msg.rfc724_mid.clone(); + new_references_opt = None; + } + fn get_new_references<'a>( + new_references_opt: &'a Option, + rfc724_mid: &'a str, + ) -> &'a str { + new_references_opt.as_deref().unwrap_or(rfc724_mid) } // add independent location to database @@ -1944,11 +2015,41 @@ impl Chat { // add message to the database let trans_fn = |transaction: &mut rusqlite::Transaction| { - if update_existing_draft == UseExistingDraftPolicy::Reuse { - // This check also covers the `msg.id.is_special()` case. + let try_reuse: bool = update_existing_draft == UseExistingDraftPolicy::Reuse; + let reuse_existing: Option<(MsgId, String)> = if !try_reuse { + None + } else if !msg.id.is_special() { + ensure!( + !msg.rfc724_mid.is_empty(), + concat!( + "cannot reuse existing draft: ", + "when `message.id` is set, `message.rfc724_mid` must also be set ", + "(as well as all other necessary properties); " + ) + ); + + Some((msg.id, msg.rfc724_mid.to_owned())) + } else if let Some(existing) = self.id.get_draft_trans(context, transaction)? + && ChatId::can_reuse_draft(context, &existing, msg)? + { + ensure!( + !existing.rfc724_mid.is_empty(), + concat!( + "cannot reuse existing draft: ", + "expected its `rfc724_mid` to be already set in the DB, but it's empty" + ) + ); + + Some((existing.id, existing.rfc724_mid.to_owned())) + } else { + None + }; + if let Some((reuse_existing_id, rfc724_mid)) = reuse_existing { + ensure!(!rfc724_mid.is_empty()); + // Maybe we could try to somehow gracefully recover from this, // but better safe than sorry. - if !self.id.has_draft_with_id(transaction, &msg.id)? { + if !self.id.has_draft_with_id(transaction, &reuse_existing_id)? { bail!( concat!( "wanted to prepare existing draft for sending in chat {0}, ", @@ -1956,7 +2057,7 @@ impl Chat { "(it might have been sent or deleted)" ), self.id, - msg.id + reuse_existing_id ); } @@ -1969,7 +2070,7 @@ impl Chat { ephemeral_timestamp=? WHERE id=?;", params_slice![ - msg.rfc724_mid, + rfc724_mid, msg.chat_id, msg.from_id, to_id, @@ -1982,18 +2083,24 @@ impl Chat { msg.param.to_string(), msg.hidden, msg.in_reply_to.as_deref().unwrap_or_default(), - new_references, + get_new_references(&new_references_opt, &rfc724_mid), new_mime_headers.is_some(), new_mime_headers.unwrap_or_default(), location_id as i32, ephemeral_timer, ephemeral_timestamp, - msg.id + reuse_existing_id ], )?; let inserted = false; - Ok((msg.id, inserted)) + Ok((reuse_existing_id, rfc724_mid, inserted)) } else { + let rfc724_mid = if !msg.rfc724_mid.is_empty() { + &msg.rfc724_mid + } else { + &create_outgoing_rfc724_mid() + }; + transaction.execute( "INSERT INTO msgs ( rfc724_mid, @@ -2018,7 +2125,7 @@ impl Chat { ephemeral_timestamp) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,1,?,?,?);", params_slice![ - msg.rfc724_mid, + rfc724_mid, msg.chat_id, msg.from_id, to_id, @@ -2031,7 +2138,7 @@ impl Chat { msg.param.to_string(), msg.hidden, msg.in_reply_to.as_deref().unwrap_or_default(), - new_references, + get_new_references(&new_references_opt, rfc724_mid), new_mime_headers.is_some(), new_mime_headers.unwrap_or_default(), location_id as i32, @@ -2041,12 +2148,13 @@ impl Chat { )?; let msg_id = MsgId::new(transaction.last_insert_rowid().try_into()?); let inserted = true; - Ok((msg_id, inserted)) + Ok((msg_id, rfc724_mid.to_string(), inserted)) } }; - let (msg_id, inserted) = context.sql.transaction(trans_fn).await?; + let (msg_id, rfc724_mid, inserted) = context.sql.transaction(trans_fn).await?; msg.id = msg_id; + msg.rfc724_mid = rfc724_mid; if inserted { context.new_msgs_notify.notify_one(); @@ -2636,6 +2744,14 @@ pub async fn is_contact_in_chat( Ok(exists) } +fn is_initialized_draft_msg_of_chat(msg: &Message, chat_id: ChatId) -> bool { + if msg.state == MessageState::OutDraft { + !msg.id.is_special() && msg.chat_id == chat_id + } else { + false + } +} + /// Sends a message object to a chat. /// /// Sends the event #DC_EVENT_MSGS_CHANGED on success. @@ -2643,6 +2759,21 @@ pub async fn is_contact_in_chat( /// sending may be delayed eg. due to network problems. However, from your /// view, you're done with the message. Sooner or later it will find its way. pub async fn send_msg(context: &Context, chat_id: ChatId, msg: &mut Message) -> Result { + let update_existing_draft = is_initialized_draft_msg_of_chat(msg, chat_id); + + send_msg_ex(context, chat_id, msg, update_existing_draft.into()).await +} +/// Unlike [`send_msg`] (and [`send_msg_sync`]), +/// this function allows for reusing the draft even if `msg` +/// is not a fully initialized [`Message`], +/// i.e. if [`Message::get_id`], [`Message::get_viewtype`], +/// [`Message::rfc724_mid`] etc. are unset. +pub async fn send_msg_ex( + context: &Context, + chat_id: ChatId, + msg: &mut Message, + update_existing_draft: UseExistingDraftPolicy, +) -> Result { ensure!( !chat_id.is_special(), "chat_id cannot be a special chat: {chat_id}" @@ -2659,7 +2790,10 @@ pub async fn send_msg(context: &Context, chat_id: ChatId, msg: &mut Message) -> msg.text = sanitize_bidi_characters(&msg.text); } - if !prepare_send_msg(context, chat_id, msg).await?.is_empty() { + if !prepare_send_msg(context, chat_id, msg, update_existing_draft) + .await? + .is_empty() + { if !msg.hidden { context.emit_msgs_changed(msg.chat_id, msg.id); } @@ -2679,7 +2813,9 @@ pub async fn send_msg(context: &Context, chat_id: ChatId, msg: &mut Message) -> /// Creates jobs in the `smtp` table, then drectly opens an SMTP connection and sends the /// message. If this fails, the jobs remain in the database for later sending. pub async fn send_msg_sync(context: &Context, chat_id: ChatId, msg: &mut Message) -> Result { - let rowids = prepare_send_msg(context, chat_id, msg).await?; + let update_existing_draft = is_initialized_draft_msg_of_chat(msg, chat_id); + + let rowids = prepare_send_msg(context, chat_id, msg, update_existing_draft.into()).await?; if rowids.is_empty() { return Ok(msg.id); } @@ -2700,6 +2836,7 @@ async fn prepare_send_msg( context: &Context, chat_id: ChatId, msg: &mut Message, + update_existing_draft: UseExistingDraftPolicy, ) -> Result> { let mut chat = Chat::load_from_db(context, chat_id).await?; @@ -2744,17 +2881,9 @@ async fn prepare_send_msg( ); } - // check current MessageState for drafts (to keep msg_id) ... - let update_existing_draft = if msg.state == MessageState::OutDraft { + if msg.state == MessageState::OutDraft { msg.hidden = false; - if !msg.id.is_special() && msg.chat_id == chat_id { - UseExistingDraftPolicy::Reuse - } else { - UseExistingDraftPolicy::DontReuse - } - } else { - UseExistingDraftPolicy::DontReuse - }; + } if msg.state == MessageState::Undefined // Legacy SecureJoin "v*-request" messages are unencrypted. @@ -2790,7 +2919,7 @@ async fn prepare_send_msg( /// /// This is basically a more explicit bool. #[derive(Debug, PartialEq)] -enum UseExistingDraftPolicy { +pub enum UseExistingDraftPolicy { /// Create a brand new draft or message, don't reuse the existing one's ID. DontReuse, /// Resuse the existing draft, so that the new draft or message diff --git a/src/message.rs b/src/message.rs index 550e0ad5e9..266434ee3c 100644 --- a/src/message.rs +++ b/src/message.rs @@ -10,6 +10,7 @@ use deltachat_derive::{FromSql, ToSql}; use humansize::BINARY; use humansize::format_size; use num_traits::FromPrimitive; +use rusqlite::OptionalExtension; use serde::{Deserialize, Serialize}; use tokio::{fs, io}; @@ -493,8 +494,22 @@ impl Message { /// /// Returns an error if the message does not exist. pub async fn load_from_db(context: &Context, id: MsgId) -> Result { - let message = Self::load_from_db_optional(context, id) - .await? + let query_only = true; + context + .sql + // `call` instead of `transaction_ex` because it's a single query. + .call(query_only, |conn| { + Self::load_from_db_trans(context, conn, id) + }) + .await + } + /// See [`Self::load_from_db`]. + pub(crate) fn load_from_db_trans( + context: &Context, + conn: &rusqlite::Connection, + id: MsgId, + ) -> Result { + let message = Self::load_from_db_optional_trans(context, conn, id)? .with_context(|| format!("Message {id} does not exist"))?; Ok(message) } @@ -503,13 +518,27 @@ impl Message { /// /// Returns `None` if the message does not exist. pub async fn load_from_db_optional(context: &Context, id: MsgId) -> Result> { + let query_only = true; + context + .sql + // `call` instead of `transaction_ex` because it's a single query. + .call(query_only, |conn| { + Self::load_from_db_optional_trans(context, conn, id) + }) + .await + } + /// See [`Self::load_from_db_optional`]. + pub(crate) fn load_from_db_optional_trans( + context: &Context, + conn: &rusqlite::Connection, + id: MsgId, + ) -> Result> { ensure!( !id.is_special(), "Can not load special message ID {id} from DB" ); - let mut msg = context - .sql - .query_row_optional( + let mut msg = conn + .query_row( "SELECT m.id AS id, rfc724_mid AS rfc724mid, @@ -603,7 +632,7 @@ impl Message { Ok(msg) }, ) - .await + .optional() .with_context(|| format!("failed to load message {id} from the database"))?; if let Some(msg) = &mut msg {