refactor: Don't update message state to OutMdnRcvd anymore

This state can be computed from the `msgs_mdns` table without significant overhead as we have an
index by msg_id there.

src/message.rs

@@ -81,7 +81,20 @@ impl MsgId {
     pub async fn get_state(self, context: &Context) -> Result<MessageState> {
         let result = context
             .sql
-            .query_get_value("SELECT state FROM msgs WHERE id=?", (self,))
+            .query_row_optional(
+                concat!(
+                    "SELECT m.state, mdns.msg_id",
+                    " FROM msgs m LEFT JOIN msgs_mdns mdns ON mdns.msg_id=m.id",
+                    " WHERE id=?",
+                    " LIMIT 1",
+                ),
+                (self,),
+                |row| {
+                    let state: MessageState = row.get(0)?;
+                    let mdn_msg_id: Option<MsgId> = row.get(1)?;
+                    Ok(state.with_mdns(mdn_msg_id.is_some()))
+                },
+            )
             .await?
             .unwrap_or_default();
         Ok(result)
@@ -519,6 +532,7 @@ impl Message {
                     "    m.ephemeral_timestamp AS ephemeral_timestamp,",
                     "    m.type AS type,",
                     "    m.state AS state,",
+                    "    mdns.msg_id AS mdn_msg_id,",
                     "    m.download_state AS download_state,",
                     "    m.error AS error,",
                     "    m.msgrmsg AS msgrmsg,",
@@ -529,11 +543,16 @@ impl Message {
                     "    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",
+                    " FROM msgs m",
-                    " WHERE m.id=? AND chat_id!=3;"
+                    " LEFT JOIN chats c ON c.id=m.chat_id",
+                    " LEFT JOIN msgs_mdns mdns ON mdns.msg_id=m.id",
+                    " WHERE m.id=? AND chat_id!=3",
+                    " LIMIT 1",
                 ),
                 (id,),
                 |row| {
+                    let state: MessageState = row.get("state")?;
+                    let mdn_msg_id: Option<MsgId> = row.get("mdn_msg_id")?;
                     let text = match row.get_ref("txt")? {
                         rusqlite::types::ValueRef::Text(buf) => {
                             match String::from_utf8(buf.to_vec()) {
@@ -568,7 +587,7 @@ impl Message {
                         ephemeral_timer: row.get("ephemeral_timer")?,
                         ephemeral_timestamp: row.get("ephemeral_timestamp")?,
                         viewtype: row.get("type")?,
-                        state: row.get("state")?,
+                        state: state.with_mdns(mdn_msg_id.is_some()),
                         download_state: row.get("download_state")?,
                         error: Some(row.get::<_, String>("error")?)
                             .filter(|error| !error.is_empty()),
@@ -1353,7 +1372,7 @@ pub enum MessageState {
     OutDelivered = 26,
 
     /// Outgoing message read by the recipient (two checkmarks; this
-    /// requires goodwill on the receiver's side)
+    /// requires goodwill on the receiver's side). Not used in the db for new messages.
     OutMdnRcvd = 28,
 }
 
@@ -1396,6 +1415,14 @@ impl MessageState {
             OutPreparing | OutDraft | OutPending | OutFailed | OutDelivered | OutMdnRcvd
         )
     }
+
+    /// Returns adjusted message state if the message has MDNs.
+    pub(crate) fn with_mdns(self, has_mdns: bool) -> Self {
+        if self == MessageState::OutDelivered && has_mdns {
+            return MessageState::OutMdnRcvd;
+        }
+        self
+    }
 }
 
 /// Returns contacts that sent read receipts and the time of reading.
@@ -1778,6 +1805,10 @@ pub(crate) async fn update_msg_state(
     msg_id: MsgId,
     state: MessageState,
 ) -> Result<()> {
+    ensure!(
+        state != MessageState::OutMdnRcvd,
+        "Update msgs_mdns table instead!"
+    );
     ensure!(state != MessageState::OutFailed, "use set_msg_failed()!");
     let error_subst = match state >= MessageState::OutPending {
         true => ", error=''",
@@ -2565,9 +2596,6 @@ mod tests {
         let payload = alice.pop_sent_msg().await;
         assert_state(&alice, alice_msg.id, MessageState::OutDelivered).await;
 
-        update_msg_state(&alice, alice_msg.id, MessageState::OutMdnRcvd).await?;
-        assert_state(&alice, alice_msg.id, MessageState::OutMdnRcvd).await;
-
         set_msg_failed(&alice, &mut alice_msg, "badly failed").await?;
         assert_state(&alice, alice_msg.id, MessageState::OutFailed).await;
 

src/mimeparser.rs

@@ -28,10 +28,7 @@ use crate::dehtml::dehtml;
 use crate::events::EventType;
 use crate::headerdef::{HeaderDef, HeaderDefMap};
 use crate::key::{self, load_self_secret_keyring, DcKey, Fingerprint, SignedPublicKey};
-use crate::message::{
+use crate::message::{self, get_vcard_summary, set_msg_failed, Message, MsgId, Viewtype};
-    self, get_vcard_summary, set_msg_failed, update_msg_state, Message, MessageState, MsgId,
-    Viewtype,
-};
 use crate::param::{Param, Params};
 use crate::peerstate::Peerstate;
 use crate::simplify::{simplify, SimplifiedText};
@@ -2157,24 +2154,32 @@ async fn handle_mdn(
         return Ok(());
     }
 
-    let Some((msg_id, chat_id, msg_state)) = context
+    let Some((msg_id, chat_id, has_mdns, is_dup)) = context
         .sql
         .query_row_optional(
             concat!(
                 "SELECT",
                 "    m.id AS msg_id,",
                 "    c.id AS chat_id,",
-                "    m.state AS state",
+                "    mdns.contact_id AS mdn_contact",
-                " FROM msgs m LEFT JOIN chats c ON m.chat_id=c.id",
+                " FROM msgs m ",
+                " LEFT JOIN chats c ON m.chat_id=c.id",
+                " LEFT JOIN msgs_mdns mdns ON mdns.msg_id=m.id",
                 " WHERE rfc724_mid=? AND from_id=1",
-                " ORDER BY m.id"
+                " ORDER BY msg_id DESC, mdn_contact=? DESC",
+                " LIMIT 1",
             ),
-            (&rfc724_mid,),
+            (&rfc724_mid, from_id),
             |row| {
                 let msg_id: MsgId = row.get("msg_id")?;
                 let chat_id: ChatId = row.get("chat_id")?;
-                let msg_state: MessageState = row.get("state")?;
+                let mdn_contact: Option<ContactId> = row.get("mdn_contact")?;
-                Ok((msg_id, chat_id, msg_state))
+                Ok((
+                    msg_id,
+                    chat_id,
+                    mdn_contact.is_some(),
+                    mdn_contact == Some(from_id),
+                ))
             },
         )
         .await?
@@ -2186,28 +2191,17 @@ async fn handle_mdn(
         return Ok(());
     };
 
-    if !context
+    if is_dup {
-        .sql
+        return Ok(());
-        .exists(
-            "SELECT COUNT(*) FROM msgs_mdns WHERE msg_id=? AND contact_id=?",
-            (msg_id, from_id),
-        )
-        .await?
-    {
-        context
-            .sql
-            .execute(
-                "INSERT INTO msgs_mdns (msg_id, contact_id, timestamp_sent) VALUES (?, ?, ?)",
-                (msg_id, from_id, timestamp_sent),
-            )
-            .await?;
     }
-
+    context
-    if msg_state == MessageState::OutPreparing
+        .sql
-        || msg_state == MessageState::OutPending
+        .execute(
-        || msg_state == MessageState::OutDelivered
+            "INSERT INTO msgs_mdns (msg_id, contact_id, timestamp_sent) VALUES (?, ?, ?)",
-    {
+            (msg_id, from_id, timestamp_sent),
-        update_msg_state(context, msg_id, MessageState::OutMdnRcvd).await?;
+        )
+        .await?;
+    if !has_mdns {
         context.emit_event(EventType::MsgRead { chat_id, msg_id });
         // note(treefit): only matters if it is the last message in chat (but probably too expensive to check, debounce also solves it)
         chatlist_events::emit_chatlist_item_changed(context, chat_id);
@@ -2315,7 +2309,7 @@ mod tests {
         chat,
         chatlist::Chatlist,
         constants::{Blocked, DC_DESIRED_TEXT_LEN, DC_ELLIPSIS},
-        message::MessengerMessage,
+        message::{MessageState, MessengerMessage},
         receive_imf::receive_imf,
         test_utils::{TestContext, TestContextManager},
         tools::time,